[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:11 PST 2023
https://github.com/jyu2-git created https://github.com/llvm/llvm-project/pull/71748
We got a error:
LLVM ERROR: Associative COMDAT symbol '??_7?$T at 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 at V<lambda_0>@@$0A@@@6b@" = comdat any
@0 = private unnamed_addr constant { [2 x ptr] } { [2 x ptr] [ptr @"??_R4?$T at V<lambda_0>@@$0A@@@6b@", ptr @"?c at b@@UEAAXXZ"] }, comdat($"??_7?$T at V<lambda_0>@@$0A@@@6b@")
@"??_7?$T at 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>
>From 3313aca0622da3882a9e5bf304b89f28fecce7fe Mon Sep 17 00:00:00 2001
From: Jennifer Yu <jennifer.yu at intel.com>
Date: Mon, 6 Nov 2023 20:51:39 -0800
Subject: [PATCH 1/2] [SEH] Fix assertin when return scalar value from __try
block.
Current compler assert with `!SI->isAtomic() && !SI->isVolatile()' failed
This due to following rule:
First, no exception can move in or out of _try region., i.e., no "potential
faulty instruction can be moved across _try boundary.
Second, the order of exceptions for instructions 'directly' under a _try
must be preserved (not applied to those in callees).
Finally, global states (local/global/heap variables) that can be read
outside of _try region must be updated in memory (not just in register)
before the subsequent exception occurs.
All memory instructions inside a _try are considered as 'volatile' to assure
2nd and 3rd rules for C-code above. This is a little sub-optimized. But it's
acceptable as the amount of code directly under _try is very small.
However during findDominatingStoreToReturnValue: those are not allowed.
To fix just skip the assertion when current function has seh try.
---
clang/lib/CodeGen/CGCall.cpp | 3 +++
.../CodeGen/windows-seh-EHa-TryInFinally.cpp | 23 +++++++++++++++++++
2 files changed, 26 insertions(+)
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/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;
+ }
+ }
+}
>From 52981cb76b510593c7b20bafc0d65a756b1ccd32 Mon Sep 17 00:00:00 2001
From: Jennifer Yu <jennifer.yu at intel.com>
Date: Tue, 7 Nov 2023 10:05:41 -0800
Subject: [PATCH 2/2] [MSABI] create unique vftable name for vftable defined
with internal alias.
We got a error:
LLVM ERROR: Associative COMDAT symbol '??_7?$T at 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 at V<lambda_0>@@$0A@@@6b@" = comdat any
@0 = private unnamed_addr constant { [2 x ptr] } { [2 x ptr] [ptr @"??_R4?$T at V<lambda_0>@@$0A@@@6b@", ptr @"?c at b@@UEAAXXZ"] }, comdat($"??_7?$T at V<lambda_0>@@$0A@@@6b@")
@"??_7?$T at 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.
My 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>
---
clang/include/clang/AST/Mangle.h | 3 +-
clang/lib/AST/MicrosoftMangle.cpp | 16 ++++++++--
clang/lib/CodeGen/MicrosoftCXXABI.cpp | 15 ++++++---
.../CodeGenCXX/ms-local-vft-alias-comdat.cpp | 31 +++++++++++++++++++
4 files changed, 57 insertions(+), 8 deletions(-)
create mode 100644 clang/test/CodeGenCXX/ms-local-vft-alias-comdat.cpp
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/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/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@"
More information about the cfe-commits
mailing list