qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio

qapi: Test for various name collisions

Expose some weaknesses in the generator: we don't always forbid
the generation of structs that contain multiple members that map
to the same C or QMP name. This has already been marked FIXME in
qapi.py in commit d90675f, but having more tests will make sure
future patches produce desired behavior; and updating existing
patches to better document things doesn't hurt, either. Some of
these collisions are already caught in the old-style parser
checks, but ultimately we want all collisions to be caught in the
new-style QAPISchema*.check() methods.

This patch focuses on C struct members, and does not consider
collisions between commands and events (affecting C function
names), or even collisions between generated C type names with
user type names (for things like automatic FOOList struct
representing array types or FOOKind for an implicit enum).

There are two types of struct collisions we want to catch:
1) Collision between two keys in a JSON object. qapi.py prevents
that within a single struct (see test duplicate-key), but it is
possible to have collisions between a type's members and its
base type's members (existing tests struct-base-clash,
struct-base-clash-deep), and its flat union variant members
(renamed test flat-union-clash-member).
2) Collision between two members of the C struct that is generated
for a given QAPI type:
a) Multiple QAPI names map to the same C name (new test
args-name-clash)
b) A QAPI name maps to a C name that is used for another purpose
(new tests flat-union-clash-branch, struct-base-clash-base,
union-clash-data). We already fixed some such cases in commit
0f61af3e and 1e6c1616, but more remain.
c) Two C names generated for other purposes clash
(updated test alternate-clash, new test union-clash-branches,
union-clash-type, flat-union-clash-type)

Ultimately, if we need to have a flat union where a tag value
clashes with a base member name, we could change the generator to
name the union (using 'foo.u.value' rather than 'foo.value') or
otherwise munge the C name corresponding to tag values. But
unless such a need arises, it will probably be easier to just
forbid these collisions.

Some of these negative tests will be deleted later, and positive
tests added to qapi-schema-test.json in their place, when the
generator code is reworked to avoid particular code generation
collisions in class 2).

[Note that viewing this patch with git rename detection enabled
may see some confusion due to renaming some tests while adding
others, but where the content is similar enough that git picks
the wrong pre- and post-patch files to associate]

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-6-git-send-email-eblake@redhat.com>
[Improve commit message and comments a bit, drop an unrelated test]
Signed-off-by: Markus Armbruster <armbru@redhat.com>

authored by

Eric Blake and committed by
Markus Armbruster
d220fbcd 437db254

+182 -15
+8 -1
tests/Makefile
··· 241 241 qapi-schema += args-member-array-bad.json 242 242 qapi-schema += args-member-array.json 243 243 qapi-schema += args-member-unknown.json 244 + qapi-schema += args-name-clash.json 244 245 qapi-schema += args-union.json 245 246 qapi-schema += args-unknown.json 246 247 qapi-schema += bad-base.json ··· 276 277 qapi-schema += flat-union-bad-discriminator.json 277 278 qapi-schema += flat-union-base-any.json 278 279 qapi-schema += flat-union-base-union.json 279 - qapi-schema += flat-union-branch-clash.json 280 + qapi-schema += flat-union-clash-branch.json 281 + qapi-schema += flat-union-clash-member.json 282 + qapi-schema += flat-union-clash-type.json 280 283 qapi-schema += flat-union-inline.json 281 284 qapi-schema += flat-union-int-branch.json 282 285 qapi-schema += flat-union-invalid-branch-key.json ··· 318 321 qapi-schema += returns-int.json 319 322 qapi-schema += returns-unknown.json 320 323 qapi-schema += returns-whitelist.json 324 + qapi-schema += struct-base-clash-base.json 321 325 qapi-schema += struct-base-clash-deep.json 322 326 qapi-schema += struct-base-clash.json 323 327 qapi-schema += struct-data-invalid.json ··· 331 335 qapi-schema += unicode-str.json 332 336 qapi-schema += union-bad-branch.json 333 337 qapi-schema += union-base-no-discriminator.json 338 + qapi-schema += union-clash-branches.json 339 + qapi-schema += union-clash-data.json 340 + qapi-schema += union-clash-type.json 334 341 qapi-schema += union-invalid-base.json 335 342 qapi-schema += union-max.json 336 343 qapi-schema += union-optional-branch.json
+1 -1
tests/qapi-schema/alternate-clash.err
··· 1 - tests/qapi-schema/alternate-clash.json:2: Alternate 'Alt1' member 'ONE' clashes with 'one' 1 + tests/qapi-schema/alternate-clash.json:7: Alternate 'Alt1' member 'a_b' clashes with 'a-b'
+7 -2
tests/qapi-schema/alternate-clash.json
··· 1 - # we detect C enum collisions in an alternate 1 + # Alternate branch name collision 2 + # Reject an alternate that would result in a collision in generated C 3 + # names (this would try to generate two enum values 'ALT1_KIND_A_B'). 4 + # TODO: In the future, if alternates are simplified to not generate 5 + # the implicit Alt1Kind enum, we would still have a collision with the 6 + # resulting C union trying to have two members named 'a_b'. 2 7 { 'alternate': 'Alt1', 3 - 'data': { 'one': 'str', 'ONE': 'int' } } 8 + 'data': { 'a-b': 'str', 'a_b': 'int' } }
tests/qapi-schema/args-name-clash.err

This is a binary file and will not be displayed.

+1
tests/qapi-schema/args-name-clash.exit
··· 1 + 0
+5
tests/qapi-schema/args-name-clash.json
··· 1 + # C member name collision 2 + # FIXME - This parses, but fails to compile, because the C struct is given 3 + # two 'a_b' members. Either reject this at parse time, or munge the C names 4 + # to avoid the collision. 5 + { 'command': 'oops', 'data': { 'a-b': 'str', 'a_b': 'str' } }
+6
tests/qapi-schema/args-name-clash.out
··· 1 + object :empty 2 + object :obj-oops-arg 3 + member a-b: str optional=False 4 + member a_b: str optional=False 5 + command oops :obj-oops-arg -> None 6 + gen=True success_response=True
+1 -1
tests/qapi-schema/duplicate-key.err
··· 1 - tests/qapi-schema/duplicate-key.json:2:10: Duplicate key "key" 1 + tests/qapi-schema/duplicate-key.json:3:10: Duplicate key "key"
+1
tests/qapi-schema/duplicate-key.json
··· 1 + # QAPI cannot include the same key more than once in any {} 1 2 { 'key': 'value', 2 3 'key': 'value' }
+1 -1
tests/qapi-schema/flat-union-base-union.err
··· 1 - tests/qapi-schema/flat-union-base-union.json:11: Base 'UnionBase' is not a valid struct 1 + tests/qapi-schema/flat-union-base-union.json:14: Base 'UnionBase' is not a valid struct
+4 -1
tests/qapi-schema/flat-union-base-union.json
··· 1 - # we require the base to be a struct 1 + # For now, we require the base to be a struct without variants 2 + # TODO: It would be possible to allow a union as a base, as long as all 3 + # permutations of QMP names exposed by base do not clash with any QMP 4 + # member names added by local variants. 2 5 { 'enum': 'TestEnum', 3 6 'data': [ 'value1', 'value2' ] } 4 7 { 'struct': 'TestTypeA',
-1
tests/qapi-schema/flat-union-branch-clash.err
··· 1 - tests/qapi-schema/flat-union-branch-clash.json:10: Member name 'name' of branch 'value1' clashes with base 'Base'
tests/qapi-schema/flat-union-branch-clash.exit tests/qapi-schema/flat-union-clash-member.exit
+2 -1
tests/qapi-schema/flat-union-branch-clash.json tests/qapi-schema/flat-union-clash-member.json
··· 1 - # we check for no duplicate keys between branches and base 1 + # We check for no duplicate keys between branch members and base 2 + # base's member 'name' clashes with Branch1's 2 3 { 'enum': 'TestEnum', 3 4 'data': [ 'value1', 'value2' ] } 4 5 { 'struct': 'Base',
tests/qapi-schema/flat-union-branch-clash.out tests/qapi-schema/flat-union-clash-member.out
tests/qapi-schema/flat-union-clash-branch.err

This is a binary file and will not be displayed.

+1
tests/qapi-schema/flat-union-clash-branch.exit
··· 1 + 0
+18
tests/qapi-schema/flat-union-clash-branch.json
··· 1 + # Flat union branch name collision 2 + # FIXME: this parses, but then fails to compile due to a duplicate 'c_d' 3 + # (one from the base member, the other from the branch name). We should 4 + # either reject the collision at parse time, or munge the generated branch 5 + # name to allow this to compile. 6 + { 'enum': 'TestEnum', 7 + 'data': [ 'base', 'c-d' ] } 8 + { 'struct': 'Base', 9 + 'data': { 'enum1': 'TestEnum', '*c_d': 'str' } } 10 + { 'struct': 'Branch1', 11 + 'data': { 'string': 'str' } } 12 + { 'struct': 'Branch2', 13 + 'data': { 'value': 'int' } } 14 + { 'union': 'TestUnion', 15 + 'base': 'Base', 16 + 'discriminator': 'enum1', 17 + 'data': { 'base': 'Branch1', 18 + 'c-d': 'Branch2' } }
+14
tests/qapi-schema/flat-union-clash-branch.out
··· 1 + object :empty 2 + object Base 3 + member enum1: TestEnum optional=False 4 + member c_d: str optional=True 5 + object Branch1 6 + member string: str optional=False 7 + object Branch2 8 + member value: int optional=False 9 + enum TestEnum ['base', 'c-d'] 10 + object TestUnion 11 + base Base 12 + tag enum1 13 + case base: Branch1 14 + case c-d: Branch2
+1
tests/qapi-schema/flat-union-clash-member.err
··· 1 + tests/qapi-schema/flat-union-clash-member.json:11: Member name 'name' of branch 'value1' clashes with base 'Base'
+16
tests/qapi-schema/flat-union-clash-type.err
··· 1 + Traceback (most recent call last): 2 + File "tests/qapi-schema/test-qapi.py", line 55, in <module> 3 + schema = QAPISchema(sys.argv[1]) 4 + File "scripts/qapi.py", line 1116, in __init__ 5 + self.check() 6 + File "scripts/qapi.py", line 1299, in check 7 + ent.check(self) 8 + File "scripts/qapi.py", line 962, in check 9 + self.variants.check(schema, members, seen) 10 + File "scripts/qapi.py", line 1024, in check 11 + v.check(schema, self.tag_member.type, vseen) 12 + File "scripts/qapi.py", line 1032, in check 13 + QAPISchemaObjectTypeMember.check(self, schema, [], seen) 14 + File "scripts/qapi.py", line 994, in check 15 + assert self.name not in seen 16 + AssertionError
+1
tests/qapi-schema/flat-union-clash-type.exit
··· 1 + 1
+16
tests/qapi-schema/flat-union-clash-type.json
··· 1 + # Flat union branch 'type' 2 + # FIXME: this triggers an assertion failure. But even with that fixed, 3 + # we would have a clash in generated C, between the member 'type' 4 + # inherited from 'Base' and the branch name 'type' within the 5 + # union. We should either reject this, or munge the generated C to let 6 + # it compile. 7 + { 'enum': 'TestEnum', 8 + 'data': [ 'type' ] } 9 + { 'struct': 'Base', 10 + 'data': { 'type': 'TestEnum' } } 11 + { 'struct': 'Branch1', 12 + 'data': { 'string': 'str' } } 13 + { 'union': 'TestUnion', 14 + 'base': 'Base', 15 + 'discriminator': 'type', 16 + 'data': { 'type': 'Branch1' } }
tests/qapi-schema/flat-union-clash-type.out

This is a binary file and will not be displayed.

+5 -2
tests/qapi-schema/qapi-schema-test.json
··· 32 32 'dict1': 'UserDefTwoDict' } } 33 33 34 34 # for testing unions 35 + # Among other things, test that a name collision between branches does 36 + # not cause any problems (since only one branch can be in use at a time), 37 + # by intentionally using two branches that both have a C member 'a_b' 35 38 { 'struct': 'UserDefA', 36 - 'data': { 'boolean': 'bool' } } 39 + 'data': { 'boolean': 'bool', '*a_b': 'int' } } 37 40 38 41 { 'struct': 'UserDefB', 39 - 'data': { 'intb': 'int' } } 42 + 'data': { 'intb': 'int', '*a-b': 'bool' } } 40 43 41 44 { 'union': 'UserDefFlatUnion', 42 45 'base': 'UserDefUnionBase', # intentional forward reference
+2
tests/qapi-schema/qapi-schema-test.out
··· 71 71 prefix QENUM_TWO 72 72 object UserDefA 73 73 member boolean: bool optional=False 74 + member a_b: int optional=True 74 75 alternate UserDefAlternate 75 76 case uda: UserDefA 76 77 case s: str ··· 78 79 enum UserDefAlternateKind ['uda', 's', 'i'] 79 80 object UserDefB 80 81 member intb: int optional=False 82 + member a-b: bool optional=True 81 83 object UserDefC 82 84 member string1: str optional=False 83 85 member string2: str optional=False
tests/qapi-schema/struct-base-clash-base.err

This is a binary file and will not be displayed.

+1
tests/qapi-schema/struct-base-clash-base.exit
··· 1 + 0
+9
tests/qapi-schema/struct-base-clash-base.json
··· 1 + # Struct member 'base' 2 + # FIXME: this parses, but then fails to compile due to a duplicate 'base' 3 + # (one explicit in QMP, the other used to box the base class members). 4 + # We should either reject the collision at parse time, or change the 5 + # generated struct to allow this to compile. 6 + { 'struct': 'Base', 'data': {} } 7 + { 'struct': 'Sub', 8 + 'base': 'Base', 9 + 'data': { 'base': 'str' } }
+5
tests/qapi-schema/struct-base-clash-base.out
··· 1 + object :empty 2 + object Base 3 + object Sub 4 + base Base 5 + member base: str optional=False
+1 -1
tests/qapi-schema/struct-base-clash-deep.err
··· 1 - tests/qapi-schema/struct-base-clash-deep.json:7: Member name 'name' clashes with base 'Base' 1 + tests/qapi-schema/struct-base-clash-deep.json:10: Member name 'name' clashes with base 'Base'
+4 -1
tests/qapi-schema/struct-base-clash-deep.json
··· 1 - # we check for no duplicate keys with indirect base 1 + # Reject attempts to duplicate QMP members 2 + # Here, 'name' would have to appear twice on the wire, locally and 3 + # indirectly for the grandparent base; the collision doesn't care that 4 + # one instance is optional. 2 5 { 'struct': 'Base', 3 6 'data': { 'name': 'str' } } 4 7 { 'struct': 'Mid',
+1 -1
tests/qapi-schema/struct-base-clash.err
··· 1 - tests/qapi-schema/struct-base-clash.json:4: Member name 'name' clashes with base 'Base' 1 + tests/qapi-schema/struct-base-clash.json:5: Member name 'name' clashes with base 'Base'
+2 -1
tests/qapi-schema/struct-base-clash.json
··· 1 - # we check for no duplicate keys with base 1 + # Reject attempts to duplicate QMP members 2 + # Here, 'name' would have to appear twice on the wire, locally and for base. 2 3 { 'struct': 'Base', 3 4 'data': { 'name': 'str' } } 4 5 { 'struct': 'Sub',
+1
tests/qapi-schema/union-clash-branches.err
··· 1 + tests/qapi-schema/union-clash-branches.json:4: Union 'TestUnion' member 'a_b' clashes with 'a-b'
+1
tests/qapi-schema/union-clash-branches.exit
··· 1 + 1
+5
tests/qapi-schema/union-clash-branches.json
··· 1 + # Union branch name collision 2 + # Reject a union that would result in a collision in generated C names (this 3 + # would try to generate two enum values 'TEST_UNION_KIND_A_B'). 4 + { 'union': 'TestUnion', 5 + 'data': { 'a-b': 'int', 'a_b': 'str' } }
tests/qapi-schema/union-clash-branches.out

This is a binary file and will not be displayed.

tests/qapi-schema/union-clash-data.err

This is a binary file and will not be displayed.

+1
tests/qapi-schema/union-clash-data.exit
··· 1 + 0
+7
tests/qapi-schema/union-clash-data.json
··· 1 + # Union branch 'data' 2 + # FIXME: this parses, but then fails to compile due to a duplicate 'data' 3 + # (one from the branch name, another as a filler to avoid an empty union). 4 + # we should either detect the collision at parse time, or change the 5 + # generated struct to allow this to compile. 6 + { 'union': 'TestUnion', 7 + 'data': { 'data': 'int' } }
+6
tests/qapi-schema/union-clash-data.out
··· 1 + object :empty 2 + object :obj-int-wrapper 3 + member data: int optional=False 4 + object TestUnion 5 + case data: :obj-int-wrapper 6 + enum TestUnionKind ['data']
+16
tests/qapi-schema/union-clash-type.err
··· 1 + Traceback (most recent call last): 2 + File "tests/qapi-schema/test-qapi.py", line 55, in <module> 3 + schema = QAPISchema(sys.argv[1]) 4 + File "scripts/qapi.py", line 1116, in __init__ 5 + self.check() 6 + File "scripts/qapi.py", line 1299, in check 7 + ent.check(self) 8 + File "scripts/qapi.py", line 962, in check 9 + self.variants.check(schema, members, seen) 10 + File "scripts/qapi.py", line 1024, in check 11 + v.check(schema, self.tag_member.type, vseen) 12 + File "scripts/qapi.py", line 1032, in check 13 + QAPISchemaObjectTypeMember.check(self, schema, [], seen) 14 + File "scripts/qapi.py", line 994, in check 15 + assert self.name not in seen 16 + AssertionError
+1
tests/qapi-schema/union-clash-type.exit
··· 1 + 1
+10
tests/qapi-schema/union-clash-type.json
··· 1 + # Union branch 'type' 2 + # FIXME: this triggers an assertion failure. But even with that fixed, 3 + # we would have a clash in generated C, between the simple union's 4 + # implicit tag member 'kind' and the branch name 'kind' within the 5 + # union. We should either reject this, or munge the generated C to let 6 + # it compile. 7 + # TODO: Even when the generated C is switched to use 'type' rather than 8 + # 'kind', to match the QMP spelling, the collision should still be detected. 9 + { 'union': 'TestUnion', 10 + 'data': { 'kind': 'int', 'type': 'str' } }
tests/qapi-schema/union-clash-type.out

This is a binary file and will not be displayed.