ClangImporter: fix crash when importing type containing bitfields (regression from #6531) #6816
Conversation
Following PR swiftlang#6531 there is a position mismatch in the final loop between the position of the member in the members arrays and the position in the valueParameters array. As a consequence, a structure containing indirect fields before a computed properties (like a bitfield) caused an invalid access in the valueParameters array resulting in a crash of the compiler. This patch maintains a separate position for accessing valueParameters. A non-regression test is also added. Signed-off-by: Florent Bruneau <[email protected]>
|
Just getting this tested. @jrose-apple or @milseman should take a look at this @swift-ci please smoke test. |
|
Admittedly we missed it the first time around too. :-) |
| import IndirectFields | ||
|
|
||
| func build_struct(a: Int32, c: Int32, d: Int32) -> StructWithIndirectField { | ||
| return StructWithIndirectField(__Anonymous_field0: .init(a: a), c: c, d: d) |
There was a problem hiding this comment.
This is making me think we're doing this wrong too; we'd really want something like StructWithIndirectField(a: a, c: c, d: d), right? But that seems much harder (and combinatoric, ultimately).
There was a problem hiding this comment.
Clearly, the best solution would be to have an init() per argument combination, but this can rapidly become a mess. Another approach, would be to simply use anonymous parameters in the initializer for fields corresponding to anonymous struct or union.
struct StructWithIndirectField {
init(_ __Anonymous_field0: __Unnamed_union___Anonymous_field0, c: Int32, d: Int32)
}
StructWithIndirectField(.init(a: a), c: c, d: d)This would also avoid exposing the generated field name.
There was a problem hiding this comment.
That's not bad, but having a mix of named and nameless arguments is weird. I suppose we could try to trust that most C cases like this will put the transparent struct/union at the front or back of the structure?
@milseman, @DougGregor, thoughts?
There was a problem hiding this comment.
Seems like the lesser of evils, unless there's any better ideas here.
Following PR #6531 there is a position mismatch in the final loop between
the position of the member in the members arrays and the position in the
valueParameters array.
As a consequence, a structure containing indirect fields before a computed
properties (like a bitfield) caused an invalid access in the
valueParameters array resulting in a crash of the compiler.
This patch maintains a separate position for accessing valueParameters. A
non-regression test is also added.