New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang] crash in ItaniumMangler when using GNU anonymous union+struct as a non-type template parameter #54588
Comments
@llvm/issue-subscribers-clang-codegen |
The problem ends up being creating the initializer for the 'struct' field inside the union. The 'braced-expression' initialization here: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.braced-expression I see that GCC has this same problem, so I suspect this might be an issue with the Itanium ABI. Note that if I change the struct to have a field-name (FOO), we get: I'll post an issue on the Itanium ABI github to see if they can guide the right solution here. |
We probably ought to just ignore anonymous members for the purposes of this mangling. You can't have a conflict in the enclosing scope between member names of different anonymous aggregates. |
Sorry, I'm not getting what you mean? Are you saying if the 'union' has an unnamed-initialized-field, it should decompose the 'inner' type? So you'd expect it to be something more like: This I think ends up being pretty awkward/weird (though perhaps implementable?) when you have a number of initialization at diferent levels. So you'd have something like:
with: |
Since this is primarily an Itanium ABI issue, let's focus the conversation there. |
Discussion on the Itanium ABI gave me sufficient info to implement: https://reviews.llvm.org/D122820 |
As reported in #54588 and discussed in itanium-cxx-abi/cxx-abi#139 We are supposed to do a DFS, pre-order, decl-order search for a name for the union in this case. Prevoiusly we crashed because the IdentiferInfo pointer was nullptr, so this makes sure we have a name in the cases described by the ABI. I added an llvm-unreachable to cover an unexpected case at the end of the new function with information/reference to the ABI in case we come up with some way to get back to here. Differential Revision: https://reviews.llvm.org/D122820
Fixed here: https://reviews.llvm.org/rG4cf98f973a13c5049322abff43f0dff3c214311b Thank you very much for your help/patience on this one @rjmccall ! |
As reported in llvm/llvm-project#54588 and discussed in itanium-cxx-abi/cxx-abi#139 We are supposed to do a DFS, pre-order, decl-order search for a name for the union in this case. Prevoiusly we crashed because the IdentiferInfo pointer was nullptr, so this makes sure we have a name in the cases described by the ABI. I added an llvm-unreachable to cover an unexpected case at the end of the new function with information/reference to the ABI in case we come up with some way to get back to here. Differential Revision: https://reviews.llvm.org/D122820
When using a non-zero-initialized class that contains a nested anonymous union+struct (GNU extension) as a C++17/20 auto/non-type template parameter, this crashes the Itanium mangler. I haven't tested this on the Microsoft mangler, but it's probably a good idea to test this as well.
Reduced repro code (build with -std=gnu++20):
Location where this crashes (5630 evaluates to false, leading to the call of FD->getIdentifier() which returns nullptr for the field):
llvm-project/clang/lib/AST/ItaniumMangle.cpp
Line 5615 in 3251ba2
Tested with clang 14.0.0, 13.0.1, Apple clang 13.1 and clang trunk on godbolt.
godbolt:
https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,selection:(endColumn:1,endLineNumber:19,positionColumn:1,positionLineNumber:19,selectionStartColumn:1,selectionStartLineNumber:19,startColumn:1,startLineNumber:19),source:'%0Atemplate+%3Ctypename+T%3E%0Aclass+wrapper+%7B%0Apublic:%0A%09union+%7B%0A%09%09struct+%7B%0A%09%09%09T+val%3B%0A%09%09%7D%3B%0A%09%7D%3B%0A%7D%3B%0A%0Atemplate+%3Cauto+tparam%3E+int+dummy()+%7B+return+0%3B+%7D%0A%0Aint+main()+%7B%0A%09dummy%3Cwrapper%3Cint%3E+%7B%7D%3E()%3B+//+succeeds+due+to+%22isZeroInitialized()%22%0A%09dummy%3Cwrapper%3Cint%3E+%7B+42+%7D%3E()%3B+//+fails/crashes+to+due+%22FD-%3EgetIdentifier()%22+returning+nullptr%0A%09return+0%3B%0A%7D%0A'),l:'5',n:'0',o:'C%2B%2B+source+%231',t:'0')),k:33.333333333333336,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:compiler,i:(compiler:clang_assertions_trunk,filters:(b:'0',binary:'1',commentOnly:'0',demangle:'0',directives:'0',execute:'1',intel:'0',libraryCode:'0',trim:'1'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,libs:!(),options:'-std%3Dgnu%2B%2B20',selection:(endColumn:45,endLineNumber:3,positionColumn:45,positionLineNumber:3,selectionStartColumn:45,selectionStartLineNumber:3,startColumn:45,startLineNumber:3),source:1,tree:'1'),l:'5',n:'0',o:'x86-64+clang+(assertions+trunk)+(C%2B%2B,+Editor+%231,+Compiler+%231)',t:'0')),k:33.333333333333336,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:output,i:(compiler:1,editor:1,fontScale:14,fontUsePx:'0',tree:'1',wrap:'1'),l:'5',n:'0',o:'Output+of+x86-64+clang+(assertions+trunk)+(Compiler+%231)',t:'0')),k:33.33333333333333,l:'4',n:'0',o:'',s:0,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4
stack trace on Apple clang 13.1:
The text was updated successfully, but these errors were encountered: