[cfe-commits] Fwd: Re: [cfe-dev] Bug in C++ ABI mangling ?

[Dmitri Rubinstein]

As proposed by David (see below) in attachment is a patch with a fix and 
test case for C++ ABI bug with the address_space qualifier.

I created the patch on the git trunk of clang.

Dmitri Rubinstein

-------- Original-Nachricht --------
Betreff: Re: [cfe-dev] Bug in C++ ABI mangling ?
Datum: Fri, 2 Dec 2011 15:45:37 -0800
Von: David Blaikie <dblaikie at gmail.com>
An: Dmitri Rubinstein <dmitri.rubinstein at googlemail.com>
CC: cfe-dev at cs.uiuc.edu

[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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-C-ABI-mangling-of-the-address_space-attribute-qu.patch
Type: text/x-patch
Size: 2092 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111203/e3177a4d/attachment.bin>