[clang] [MS-ABI] create unique vftable name for vftable defined with internal alias. (PR #71748)

via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 8 16:02:38 PST 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-codegen

Author: None (jyu2-git)

<details>
<summary>Changes</summary>

We got a error:
LLVM ERROR: Associative COMDAT symbol '??_7?$T@<!-- -->V<lambda_0>@@@@<!-- -->6B@' is not a key for its COMDAT
Current we create internal alias for vftable when lambd is used.
For the test, IR generate:
 $"??_7?$T@<!-- -->V<lambda_0>@@$0A@@@<!-- -->6b@" = comdat any

@<!-- -->0 = private unnamed_addr constant { [2 x ptr] } { [2 x ptr] [ptr @"??_R4?$T@<!-- -->V<lambda_0>@@$0A@@@<!-- -->6b@", ptr @"?c@<!-- -->b@@<!-- -->UEAAXXZ"] }, comdat($"??_7?$T@<!-- -->V<lambda_0>@@$0A@@@<!-- -->6b@")

@"??_7?$T@<!-- -->V<lambda_0>@@$0A@@@<!-- -->6b@" = internal unnamed_addr alias ptr, getelementptr inbounds ({ [2 x ptr] }, ptr @<!-- -->0, i32 0, i32 0, i32 1)

According LLVM language reference manual section on COMDATs:
There are some restrictions on the properties of the global object. It,
or an alias to it, must have the same name as the COMDAT group when
targeting COFF. The contents and size of this object may be used during
link-time to determine which COMDAT groups get selected depending on the
selection kind. Because the name of the object must match the name of the
COMDAT group, the linkage of the global object must not be local; local
symbols can get renamed if a collision occurs in the symbol table.

So one way to fix this is to generate unqiue virtual function table name, to aviod local symbol renaming.
The fix is to generate unqiue vftable name for every TU where encoded file name in addition.

    after the change:
    <lambda_A7884B04_0>
    before:
    <lambda_0>


---
Full diff: https://github.com/llvm/llvm-project/pull/71748.diff


6 Files Affected:

- (modified) clang/include/clang/AST/Mangle.h (+2-1) 
- (modified) clang/lib/AST/MicrosoftMangle.cpp (+14-2) 
- (modified) clang/lib/CodeGen/CGCall.cpp (+3) 
- (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+10-5) 
- (modified) clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp (+23) 
- (added) clang/test/CodeGenCXX/ms-local-vft-alias-comdat.cpp (+31) 


``````````diff
diff --git a/clang/include/clang/AST/Mangle.h b/clang/include/clang/AST/Mangle.h
index e586b0cec43dfd6..420ba52f255eb2c 100644
--- a/clang/include/clang/AST/Mangle.h
+++ b/clang/include/clang/AST/Mangle.h
@@ -239,7 +239,8 @@ class MicrosoftMangleContext : public MangleContext {
   /// them correctly.
   virtual void mangleCXXVFTable(const CXXRecordDecl *Derived,
                                 ArrayRef<const CXXRecordDecl *> BasePath,
-                                raw_ostream &Out) = 0;
+                                raw_ostream &Out,
+                                bool IsLocalVFTAliasReq = false) = 0;
 
   /// Mangle vbtable symbols.  Only a subset of the bases along the path
   /// to the vbtable are included in the name.  It's up to the caller to pick
diff --git a/clang/lib/AST/MicrosoftMangle.cpp b/clang/lib/AST/MicrosoftMangle.cpp
index 50ab6ea59be9d03..a84959b3680fdde 100644
--- a/clang/lib/AST/MicrosoftMangle.cpp
+++ b/clang/lib/AST/MicrosoftMangle.cpp
@@ -148,6 +148,7 @@ class MicrosoftMangleContextImpl : public MicrosoftMangleContext {
   llvm::DenseMap<GlobalDecl, unsigned> SEHFilterIds;
   llvm::DenseMap<GlobalDecl, unsigned> SEHFinallyIds;
   SmallString<16> AnonymousNamespaceHash;
+  bool IsLocalVFTAliasReq = false;
 
 public:
   MicrosoftMangleContextImpl(ASTContext &Context, DiagnosticsEngine &Diags,
@@ -165,7 +166,8 @@ class MicrosoftMangleContextImpl : public MicrosoftMangleContext {
                           raw_ostream &) override;
   void mangleCXXVFTable(const CXXRecordDecl *Derived,
                         ArrayRef<const CXXRecordDecl *> BasePath,
-                        raw_ostream &Out) override;
+                        raw_ostream &Out,
+                        bool IsLocalVFTAliaesReq = false) override;
   void mangleCXXVBTable(const CXXRecordDecl *Derived,
                         ArrayRef<const CXXRecordDecl *> BasePath,
                         raw_ostream &Out) override;
@@ -211,6 +213,10 @@ class MicrosoftMangleContextImpl : public MicrosoftMangleContext {
   void mangleSEHFinallyBlock(GlobalDecl EnclosingDecl,
                              raw_ostream &Out) override;
   void mangleStringLiteral(const StringLiteral *SL, raw_ostream &Out) override;
+  void setIsLocalVFTAliasReq(bool isLocalVFTAliasReq) {
+    IsLocalVFTAliasReq = isLocalVFTAliasReq;
+  }
+  bool getIsLocalVFTAliasReq() { return IsLocalVFTAliasReq; }
   bool getNextDiscriminator(const NamedDecl *ND, unsigned &disc) {
     const DeclContext *DC = getEffectiveDeclContext(ND);
     if (!DC->isFunctionOrMethod())
@@ -1106,6 +1112,10 @@ void MicrosoftCXXNameMangler::mangleUnqualifiedName(GlobalDecl GD,
       if (const CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(TD)) {
         if (Record->isLambda()) {
           llvm::SmallString<10> Name("<lambda_");
+          if (Context.getIsLocalVFTAliasReq()) {
+            Name += Context.getAnonymousNamespaceHash();
+            Name += "_";
+          }
 
           Decl *LambdaContextDecl = Record->getLambdaContextDecl();
           unsigned LambdaManglingNumber = Record->getLambdaManglingNumber();
@@ -3634,11 +3644,12 @@ void MicrosoftMangleContextImpl::mangleCXXDtorThunk(
 
 void MicrosoftMangleContextImpl::mangleCXXVFTable(
     const CXXRecordDecl *Derived, ArrayRef<const CXXRecordDecl *> BasePath,
-    raw_ostream &Out) {
+    raw_ostream &Out, bool IsLocalVFTAliasReq) {
   // <mangled-name> ::= ?_7 <class-name> <storage-class>
   //                    <cvr-qualifiers> [<name>] @
   // NOTE: <cvr-qualifiers> here is always 'B' (const). <storage-class>
   // is always '6' for vftables.
+  setIsLocalVFTAliasReq(IsLocalVFTAliasReq);
   msvc_hashing_ostream MHO(Out);
   MicrosoftCXXNameMangler Mangler(*this, MHO);
   if (Derived->hasAttr<DLLImportAttr>())
@@ -3650,6 +3661,7 @@ void MicrosoftMangleContextImpl::mangleCXXVFTable(
   for (const CXXRecordDecl *RD : BasePath)
     Mangler.mangleName(RD);
   Mangler.getStream() << '@';
+  setIsLocalVFTAliasReq(false);
 }
 
 void MicrosoftMangleContextImpl::mangleCXXVBTable(
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index e7951b3a3f4d855..bc367b89992bbd9 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -3507,6 +3507,9 @@ static llvm::StoreInst *findDominatingStoreToReturnValue(CodeGenFunction &CGF) {
       return nullptr;
     // These aren't actually possible for non-coerced returns, and we
     // only care about non-coerced returns on this code path.
+    // All memory instructions inside __try block are volatile.
+    if (CGF.currentFunctionUsesSEHTry() && SI->isVolatile())
+      return SI;
     assert(!SI->isAtomic() && !SI->isVolatile());
     return SI;
   };
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index cc7bc3b73589877..6218afebe6ff65e 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1778,9 +1778,11 @@ llvm::Value *MicrosoftCXXABI::getVTableAddressPointInStructor(
 
 static void mangleVFTableName(MicrosoftMangleContext &MangleContext,
                               const CXXRecordDecl *RD, const VPtrInfo &VFPtr,
-                              SmallString<256> &Name) {
+                              SmallString<256> &Name,
+                              bool IsLocalVFTAliasReq = false) {
   llvm::raw_svector_ostream Out(Name);
-  MangleContext.mangleCXXVFTable(RD, VFPtr.MangledPath, Out);
+  MangleContext.mangleCXXVFTable(RD, VFPtr.MangledPath, Out,
+                                 IsLocalVFTAliasReq);
 }
 
 llvm::Constant *
@@ -1844,9 +1846,6 @@ llvm::GlobalVariable *MicrosoftCXXABI::getAddrOfVTable(const CXXRecordDecl *RD,
   }
   const std::unique_ptr<VPtrInfo> &VFPtr = *VFPtrI;
 
-  SmallString<256> VFTableName;
-  mangleVFTableName(getMangleContext(), RD, *VFPtr, VFTableName);
-
   // Classes marked __declspec(dllimport) need vftables generated on the
   // import-side in order to support features like constexpr.  No other
   // translation unit relies on the emission of the local vftable, translation
@@ -1863,6 +1862,12 @@ llvm::GlobalVariable *MicrosoftCXXABI::getAddrOfVTable(const CXXRecordDecl *RD,
   bool VTableAliasIsRequred =
       !VFTableComesFromAnotherTU && getContext().getLangOpts().RTTIData;
 
+  bool IsLocalVFTAliasReq =
+      llvm::GlobalValue::isLocalLinkage(VFTableLinkage) && VTableAliasIsRequred;
+
+  SmallString<256> VFTableName;
+  mangleVFTableName(getMangleContext(), RD, *VFPtr, VFTableName,
+                    IsLocalVFTAliasReq);
   if (llvm::GlobalValue *VFTable =
           CGM.getModule().getNamedGlobal(VFTableName)) {
     VFTablesMap[ID] = VFTable;
diff --git a/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp b/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
index fab567e763df443..ce2a9528e1908a9 100644
--- a/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
+++ b/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
@@ -67,3 +67,26 @@ void foo()
     }
   }
 }
+
+// CHECK-LABEL:@"?bar@@YAHXZ"()
+// CHECK: invoke.cont:
+// CHECK: invoke void @llvm.seh.try.begin()
+// CHECK: invoke.cont1:
+// CHECK: store volatile i32 1, ptr %cleanup.dest.slot
+// CHECK: invoke void @llvm.seh.try.end()
+// CHECK: invoke.cont2:
+// CHECK: call void @"?fin$0 at 0@bar@@"
+// CHECK: %cleanup.dest3 = load i32, ptr %cleanup.dest.slot
+// CHECK: return:
+// CHECK: ret i32 11
+int bar()
+{
+  int x;
+  __try {
+    return 11;
+  } __finally {
+    if (_abnormal_termination()) {
+      x = 9;
+    }
+  }
+}
diff --git a/clang/test/CodeGenCXX/ms-local-vft-alias-comdat.cpp b/clang/test/CodeGenCXX/ms-local-vft-alias-comdat.cpp
new file mode 100644
index 000000000000000..675f91384b70fda
--- /dev/null
+++ b/clang/test/CodeGenCXX/ms-local-vft-alias-comdat.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fcxx-exceptions -triple=x86_64-windows-msvc  \
+// RUN:  -Wmicrosoft-template -fms-compatibility -emit-llvm %s -o - \
+// RUN:  | FileCheck %s
+
+template <typename a> struct T {
+  virtual void c();
+  T(a h) {}
+};
+struct m {
+  template <typename j> void ab(j ac) {
+    using ad = T<j>;
+    ad j(ac);
+  }
+};
+template <typename ae> struct n {
+  template <typename j> n(j ac) { q.ab(ac); }
+  ae q;
+};
+class s : n<m> {
+  using ag = n<m>;
+public:
+  template <typename j> s(j ac) : ag(ac) {}
+};
+struct ah {
+  ah(s);
+} a([]{});
+
+//CHECK: "??_7?$T at V
+//CHECK-SAME: <lambda_[[UNIQ:.*]]_0>
+//CHECK-NOT: <lambda_0>
+//CHECK-SAME: @@@@6B@"

``````````

</details>


https://github.com/llvm/llvm-project/pull/71748


More information about the cfe-commits mailing list