cmd/cgo: inconsistent type for char arrays in structs #8428
Labels
Milestone
Comments
I think this is related to the heuristic that was used to address issue #2806, but I'm still trying to grok exactly what's going on. For what it's worth, as a workaround, if the structs are defined using proper C99 flexible array members (i.e., "char data[];" instead of "char data[0];"), then cgo is consistently uses [0]_Ctype_char for both structs. The problem seems to be that for "struct { void *next; char flag; char data[0]; }"'s data member, debug/dwarf is returning a dwarf.ArrayType with Count==-1, whereas changing "[0]" to "[]" or removing the "next" field causes debug/dwarf to return one with Count==0 instead. |
Nevermind, the flexible array members "workaround" is actually just a side effect of a bug in debug/dwarf. debug/dwarf keeps a cache of already processed DWARF types, but it then calls zeroArray() to mutate parsed dwarf.Type values based on their presence within a particular struct. However, that mutation might be invalid if the dwarf.Type is shared by other structs that don't meet the same criteria to invoke zeroArray(). E.g., either disabling the type cache or removing any Go references to C.struct_result1_t will cause C.struct_result2_t to be generated with [0]byte again instead of [0]_Ctype_char. |
As a quick update: I've made some progress on this issue and seem to have a fix, but I'm still trying to make sure it doesn't accidentally break any of the cases that led to the current code. Incidentally, I've noticed a few additional bugs related to this that I think are worth noting so I don't forget: 1. zeroArray() is a bit overzealous by recursing into child array types, which results in "int x[][4];" getting translated to "x [0][0]int32" instead of "x [0][4]int32". 2. Currently zeroArray() doesn't take into account that the array element type might already have size 0, so "int x[4][0];" gets translated to "x [0][0]int32". (In practice this probably isn't worth worrying about, but it's also easy to fix.) 3. The handling of arrays with more than two dimensions is wrong; e.g., it translates "int z[1][2][3][4];" into "z [1][4][3][2]int32". |
CL https://golang.org/cl/127980043 mentions this issue. |
|
This issue was closed by revision 078a9cb. Status changed to Fixed. |
CL https://golang.org/cl/135990043 mentions this issue. |
wheatman
pushed a commit
to wheatman/go-akaros
that referenced
this issue
Jun 25, 2018
In cgo, now that recursive calls to typeConv.Type() always work, we can more robustly calculate the array sizes based on the size of our element type. Also, in debug/dwarf, the decision to call zeroType is made based on a type's usage within a particular struct, but dwarf.Type values are cached in typeCache, so the modification might affect uses of the type in other structs. Current compilers don't appear to share DWARF type entries for "[]foo" and "[0]foo", but they also don't consistently share type entries in other cases. Arguably modifying the types is an improvement in some cases, but varying translated types according to compiler whims seems like a bad idea. Lastly, also in debug/dwarf, zeroType only needs to rewrite the top-level dimension, and only if the rest of the array size is non-zero. Fixes golang#8428. LGTM=iant R=iant CC=golang-codereviews https://golang.org/cl/127980043
wheatman
pushed a commit
to wheatman/go-akaros
that referenced
this issue
Jul 9, 2018
In cgo, now that recursive calls to typeConv.Type() always work, we can more robustly calculate the array sizes based on the size of our element type. Also, in debug/dwarf, the decision to call zeroType is made based on a type's usage within a particular struct, but dwarf.Type values are cached in typeCache, so the modification might affect uses of the type in other structs. Current compilers don't appear to share DWARF type entries for "[]foo" and "[0]foo", but they also don't consistently share type entries in other cases. Arguably modifying the types is an improvement in some cases, but varying translated types according to compiler whims seems like a bad idea. Lastly, also in debug/dwarf, zeroType only needs to rewrite the top-level dimension, and only if the rest of the array size is non-zero. Fixes golang#8428. LGTM=iant R=iant CC=golang-codereviews https://golang.org/cl/127980043
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Attachments:
The text was updated successfully, but these errors were encountered: