[cfe-dev] Bug in C++ ABI mangling ?

[David Blaikie]

[Dmitri: sorry for sending this as a private reply instead of a
reply-all initially - resending so the list can see my
response/correct me if I'm wrong, etc]

On Fri, Dec 2, 2011 at 3:33 PM, Dmitri Rubinstein
<dmitri.rubinstein at googlemail.com> wrote:
> While experimenting with the address_space attribute in clang 2.9 I got
> a following error message:
>
> clang:
> /home/rubinste/proj_de3/llvm-svn/tools/clang/lib/AST/ItaniumMangle.cpp:2585:
> void<unnamed>::CXXNameMangler::addSubstitution(uintptr_t): Assertion
> `!Substitutions.count(Ptr) && "Substitution already exists!"' failed.
>
> You can reproduce it with this C++ code snippet:
>
> struct OpaqueType;
> typedef OpaqueType __attribute__((address_space(100))) * OpaqueTypePtr;
>
> static inline void check(OpaqueTypePtr)
> {
> }
>
> After some debugging I detected a problem: addSubstitution only checks
> that qualified type does not have CVR qualifier, but there is no check
> for address space qualifier. After adding the check for address space
> qualifier problem disappeared:
>
> void CXXNameMangler::addSubstitution(QualType T) {
>   if (!T.getCVRQualifiers() && !T.getQualifiers().hasAddressSpace()) {
>     if (const RecordType *RT = T->getAs<RecordType>()) {
>       addSubstitution(RT->getDecl());
>       return;
>     }
>   }
>
>   uintptr_t TypePtr = reinterpret_cast<uintptr_t>(T.getAsOpaquePtr());
>   addSubstitution(TypePtr);
> }
>
> The problem also disappear when I put a check function into extern "C" {
>  ... }, thus deactivating C++ name mangling.
>
> Is this a bug, or is the address_space attribute generally not allowed
> in C++ code ?

Judging by the myriad of test cases we have checked in that use
address_space in .cpp files, I'd say it's safe to say this is allowed
& what you've found is a bug (& what you've suggested as the fix is
probably the right fix, too). Sending the patch you described (as an
actual patch file) to cfe-commits would be great. Please include a
test case. ( test/CodeGenCXX/mangle-address-space.cpp looks like it
might be the right place, if this is specific to name mangling)

- David