Bug in C++ ABI mangling ?

Clang Frontend
1

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 ?

Greetings

Dmitri Rubinstein

2

[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]

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