Mangle __unaligned in Itanium ABI

[rogfer01]

Summary

__unaligned is not currently mangled in any way in the Itanium ABI. This causes failures when using -fms-extensions and C++ in targets using Itanium ABI.

As suggested by @rsmith the simplest thing to do here is actually mangle the qualifier as a vendor extension.

As an alternative, we could continue not mangling this qualifier. But if I read the code right, in order to get substitutions correctly, we would have to rebuild some types and stripe the __unaligned qualifier prior any mangling and substitution.

This patch also removes the change done in D31976 and updates its test to the new reality.

[rsmith]

lib/AST/ItaniumMangle.cpp

2330–2334

I don't think this is the right place/way to handle this: given

void f(struct X __unaligned *p, struct X *q) {}

it looks like we'll mangle as _Z1fP1XP1X with this patch, which seems wrong: this should presumably instead be _Z1fP1XS0_.

But regardless, I think the right thing to do is to invent a mangling for __unaligned, since we support overloading on it; the most appropriate mangling would be U11__unaligned, per http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling-type. (You'll need to extend mangleQualifiers to emit this and hasMangledSubstitutionQualifiers to ignore it.)

[rogfer01]

lib/AST/ItaniumMangle.cpp

2330–2334

Oh. I see. Thanks for the review.

I'll update this patch and then I'll post another one for the mangling of unaligned.

[rogfer01]

Abandon the old approach which cannot possibly work and just opt to mangle __unaligned.

[aaron.ballman]

test/CodeGenCXX/pr33080.cpp

20

Can we also get a test like struct A * __unaligned * __unaligned *? Also, perhaps a test case involving templates would be good.

[rogfer01]

Thanks @aaron.ballman for the review.

I have extended the test with your suggestions.

[aaron.ballman]

Can you run clang-format over both the test files? Aside from that, looks good to me, but you should wait for @rsmith or @majnemer to sign off before committing.

[rogfer01]

Fix formatting of tests.

Thanks for the review @aaron.ballman !

[rnk]

This was filed as https://bugs.llvm.org/show_bug.cgi?id=33178

Isn't there a space in the mangling for vendor extensions? Can we use that here?

[rsmith]

In D33398#769152, @rnk wrote:

Isn't there a space in the mangling for vendor extensions? Can we use that here?

We are using that here: that's what mangleVendorQualifier does.

lib/AST/ItaniumMangle.cpp

2210

Too much indentation here. Also, the ABI requires the "unordered" vendor qualifiers to be emitted in reverse alphabetical order, so this should be emitted after __weak and __strong but before __autoreleasing.

[rogfer01]

Changelog:

• Fix formatting.
• Emit __unaligned in the right order (as defined by the Itanium ABI) when there are Objective-C++ ARC vendor qualifiers.
• New test for __unaligned and ARC's __weak, __strong and __autorelease

[rogfer01]

lib/AST/ItaniumMangle.cpp

2210

I think you meant after __weak but before __strong and __autoreleasing? Maybe I'm misinterpreting something here.

The current patch emits __weak, then __unaligned and then the remaining ARC ones.

[rsmith]

Looks great, thanks!

lib/AST/ItaniumMangle.cpp

2184

first __weak -> __weak first

2210

The alphabet is hard, apparently :) Yes, thanks!

lib/AST/ItaniumMangle.cpp

void CXXNameMangler::mangleNestedName(const NamedDecl *ND,
 
   Out << 'N';
   if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(ND)) {
     Qualifiers MethodQuals =
         Qualifiers::fromCVRUMask(Method->getTypeQualifiers());
     // We do not consider restrict a distinguishing attribute for overloading
     // purposes so we must not mangle it.
     MethodQuals.removeRestrict();
-    // __unaligned is not currently mangled in any way, so remove it.
-    MethodQuals.removeUnaligned();
     mangleQualifiers(MethodQuals);
     mangleRefQualifier(Method->getRefQualifier());
   }
   
   // Check if we have a template.
   const TemplateArgumentList *TemplateArgs = nullptr;
   if (const TemplateDecl *TD = isTemplate(ND, TemplateArgs)) {
     mangleTemplatePrefix(TD, NoFunction);
void CXXNameMangler::mangleQualifiers(Qualifiers Quals) {
   // The ARC ownership qualifiers start with underscores.
   switch (Quals.getObjCLifetime()) {
   // Objective-C ARC Extension:
   //
   //   <type> ::= U "__strong"
   //   <type> ::= U "__weak"
   //   <type> ::= U "__autoreleasing"
   case Qualifiers::OCL_None:
     break;
     
   case Qualifiers::OCL_Weak:
     mangleVendorQualifier("__weak");
     break;
     
   case Qualifiers::OCL_Strong:
     mangleVendorQualifier("__strong");
     break;
     
   case Qualifiers::OCL_Autoreleasing:
     mangleVendorQualifier("__autoreleasing");
     break;
     
   case Qualifiers::OCL_ExplicitNone:
     // The __unsafe_unretained qualifier is *not* mangled, so that
     // __unsafe_unretained types in ARC produce the same manglings as the
     // equivalent (but, naturally, unqualified) types in non-ARC, providing
     // better ABI compatibility.
     //
     // It's safe to do this because unqualified 'id' won't show up
     // in any type signatures that need to be mangled.
     break;
   }
 
+  if (Quals.hasUnaligned())
+      mangleVendorQualifier("__unaligned");
+
   // <CV-qualifiers> ::= [r] [V] [K]    # restrict (C99), volatile, const
   if (Quals.hasRestrict())
     Out << 'r';
   if (Quals.hasVolatile())
     Out << 'V';
   if (Quals.hasConst())
     Out << 'K';
 }
void CXXNameMangler::mangleType(QualType T) {
   if (quals && isa<ArrayType>(T)) {
     ty = Context.getASTContext().getAsArrayType(T);
     quals = Qualifiers();
 
     // Note that we don't update T: we want to add the
     // substitution at the original type.
   }
 
   if (quals) {
     mangleQualifiers(quals);
     // Recurse:  even if the qualified type isn't yet substitutable,
     // the unqualified type might be.
     mangleType(QualType(ty, 0));
   } else {
     switch (ty->getTypeClass()) {
 #define ABSTRACT_TYPE(CLASS, PARENT)
 #define NON_CANONICAL_TYPE(CLASS, PARENT) \
     case Type::CLASS: \
       llvm_unreachable("can't mangle non-canonical type " #CLASS "Type"); \
       return;
 #define TYPE(CLASS, PARENT) \
bool CXXNameMangler::mangleSubstitution(const NamedDecl *ND) {
   ND = cast<NamedDecl>(ND->getCanonicalDecl());
   return mangleSubstitution(reinterpret_cast<uintptr_t>(ND));
 }
 
 /// Determine whether the given type has any qualifiers that are relevant for
 /// substitutions.
 static bool hasMangledSubstitutionQualifiers(QualType T) {
   Qualifiers Qs = T.getQualifiers();
-  return Qs.getCVRQualifiers() || Qs.hasAddressSpace();
+  return Qs.getCVRQualifiers() || Qs.hasAddressSpace() || Qs.hasUnaligned();
 }
 
 bool CXXNameMangler::mangleSubstitution(QualType T) {
   if (!hasMangledSubstitutionQualifiers(T)) {
     if (const RecordType *RT = T->getAs<RecordType>())
       return mangleSubstitution(RT->getDecl());
   }
 

test/CodeGenCXX/pr33080.cpp

+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm -o- %s | FileCheck %s
+
+void fa(__unaligned struct A*) {}
+// CHECK: define void @_Z2faPU11__unaligned1A(
+
+void ga(struct A*, struct A*) {}
+// CHECK: define void @_Z2gaP1AS0_(
+
+void gb(__unaligned struct A*, struct A*) {}
+// CHECK: define void @_Z2gbPU11__unaligned1APS_(
+
+void gc(struct A*, __unaligned struct A*) {}
+// CHECK: define void @_Z2gcP1APU11__unalignedS_(
+
+void gd(__unaligned struct A*, __unaligned struct A*) {}
+// CHECK: define void @_Z2gdPU11__unaligned1AS1_(
+
+void hb(__unaligned struct A*, __unaligned const struct A*) {}
+// CHECK: define void @_Z2hbPU11__unaligned1APU11__unalignedKS_(
+
+void ja(__unaligned struct A *, __unaligned struct A *__unaligned *, __unaligned struct A *__unaligned *__unaligned *) {}
+// CHECK: define void @_Z2jaPU11__unaligned1APU11__unalignedS1_PU11__unalignedS3_(
+
+void jb(__unaligned struct A *, __unaligned struct A * *, __unaligned struct A *__unaligned *__unaligned *) {}
+// CHECK: @_Z2jbPU11__unaligned1APS1_PU11__unalignedPU11__unalignedS1_(
+
+
+template <typename T, typename Q>
+void ta(T&, Q*) { }
+
+void ia(__unaligned struct A &a) {
+  ta(a, &a);
+}
+
+// CHECK: @_Z2taIU11__unaligned1AS1_EvRT_PT0_(

test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp

-// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm-only %s -verify
-
-struct A
-{
-    int x;
-    void foo() __unaligned;
-    void foo();
-};
-
-void A::foo() __unaligned
-{
-    this->x++;
-}
-
-void A::foo() // expected-error {{definition with same mangled name as another definition}}
-              // expected-note@-6 {{previous definition is here}}
-{
-    this->x++;
-}
-

test/CodeGenCXX/unaligned-member-qualifier.cpp

+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm %s -o- | FileCheck %s
+
+struct A
+{
+    void foo() __unaligned;
+    void foo() const __unaligned;
+    void foo() volatile __unaligned;
+    void foo() const volatile __unaligned;
+};
+
+void A::foo() __unaligned { }
+// CHECK: define void @_ZNU11__unaligned1A3fooEv(
+
+void A::foo() const __unaligned { }
+// CHECK: define void @_ZNU11__unalignedK1A3fooEv(
+
+void A::foo() volatile __unaligned { }
+// CHECK: define void @_ZNU11__unalignedV1A3fooEv(
+
+void A::foo() const volatile __unaligned { }
+// CHECK: define void @_ZNU11__unalignedVK1A3fooEv(