Skip to content

Commit 286ab11

Browse files
committed
Reapply "[llvm][CFI] Do not canonicalize COFF functions in a comdat (#139962)"
This reapplies 33684ac with appropriate requires on tests. COFF requires that a function exists with the same name as a comdat. Not having this key function results in `LLVM ERROR: Associative COMDAT symbol '...' does not exist.` CFI by default will attempt to canonicalize a function by appending `.cfi` to its name which allows external references to refer to the new canonical alias, but it does not change the comdat name. We can change the comdat name since symbol and comdat resolution occurs before LTO, so we already know which symbols are prevailing.
1 parent 1fa8394 commit 286ab11

File tree

2 files changed

+51
-1
lines changed

2 files changed

+51
-1
lines changed

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1711,8 +1711,22 @@ void LowerTypeTestsModule::buildBitSetsFromFunctionsNative(
17111711
F->getValueType(), 0, F->getLinkage(), "", CombinedGlobalElemPtr, &M);
17121712
FAlias->setVisibility(F->getVisibility());
17131713
FAlias->takeName(F);
1714-
if (FAlias->hasName())
1714+
if (FAlias->hasName()) {
17151715
F->setName(FAlias->getName() + ".cfi");
1716+
// For COFF we should also rename the comdat if this function also
1717+
// happens to be the key function. Even if the comdat name changes, this
1718+
// should still be fine since comdat and symbol resolution happens
1719+
// before LTO, so all symbols which would prevail have been selected.
1720+
if (F->hasComdat() && ObjectFormat == Triple::COFF &&
1721+
F->getComdat()->getName() == FAlias->getName()) {
1722+
Comdat *OldComdat = F->getComdat();
1723+
Comdat *NewComdat = M.getOrInsertComdat(F->getName());
1724+
for (GlobalObject &GO : M.global_objects()) {
1725+
if (GO.getComdat() == OldComdat)
1726+
GO.setComdat(NewComdat);
1727+
}
1728+
}
1729+
}
17161730
replaceCfiUses(F, FAlias, IsJumpTableCanonical);
17171731
if (!F->hasLocalLinkage())
17181732
F->setVisibility(GlobalVariable::HiddenVisibility);
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
; REQUIRES: x86-registered-target
2+
; RUN: opt -S -passes=lowertypetests %s | FileCheck %s
3+
4+
;; This is a check to assert we don't crash with:
5+
;;
6+
;; LLVM ERROR: Associative COMDAT symbol '...' does not exist.
7+
;;
8+
;; So this just needs to exit normally.
9+
; RUN: opt -S -passes=lowertypetests %s | llc -asm-verbose=false
10+
11+
target datalayout = "e-p:64:64"
12+
target triple = "x86_64-pc-windows-msvc"
13+
14+
@a = global [2 x ptr] [ptr @f1, ptr @f2]
15+
16+
; CHECK: $f1.cfi = comdat any
17+
$f1 = comdat any
18+
19+
; CHECK: @f1.cfi() comdat !type !0
20+
define void @f1() comdat !type !0 {
21+
ret void
22+
}
23+
24+
; CHECK: @f2.cfi() comdat($f1.cfi) !type !0
25+
define void @f2() comdat($f1) !type !0 {
26+
ret void
27+
}
28+
29+
declare i1 @llvm.type.test(ptr %ptr, metadata %bitset) nounwind readnone
30+
31+
define i1 @foo(ptr %p) {
32+
%x = call i1 @llvm.type.test(ptr %p, metadata !"typeid1")
33+
ret i1 %x
34+
}
35+
36+
!0 = !{i32 0, !"typeid1"}

0 commit comments

Comments
 (0)