[CFI] Propagate GUIDs correctly after PR #184065#200542
Draft
mtrofin wants to merge 10 commits into
Draft
Conversation
llvmorg-github-actions
Bot
added
LTO
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesLowerTypeTests and the ModuleSummaryIndex mapped CFI function declarations and definitions based purely on the string name. If an internal function was promoted and renamed during ThinLTO, its name-derived GUID would change, leading to mismatches, resulting in runtime traps. This patch saves the GUIDs in the CFI tables. LowerTypeTests doesn't need to perform name-based lookups, relying on this GUID instead. Patch is 22.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/200542.diff 16 Files Affected:
diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index ef037a25fac2e..81859745386e0 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -638,6 +638,10 @@ class GlobalValue : public Constant {
/// that might pre-date the storage of GUIDs in metadata.
GUID getGUIDOrFallback() const;
+ /// same as getGUIDOrFallback, but for fallback we dropLLVMManglingEscape the
+ /// name first.
+ GUID getGUIDOrFallbackDropEscape() const;
+
/// @name Materialization
/// Materialization is used to construct functions only as they're needed.
/// This
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index ed39eb99a9196..512ffde9aab04 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1355,8 +1355,12 @@ class CfiFunctionIndex {
CfiFunctionIndex() = default;
template <typename It> CfiFunctionIndex(It B, It E) {
- for (; B != E; ++B)
- emplace(*B);
+ for (; B != E; ++B) {
+ StringRef S(*B);
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(S));
+ Index[GUID].emplace(S);
+ }
}
std::vector<StringRef> symbols() const {
@@ -1381,21 +1385,9 @@ class CfiFunctionIndex {
return make_range(I->second.begin(), I->second.end());
}
- template <typename... Args> void emplace(Args &&...A) {
- StringRef S(std::forward<Args>(A)...);
- GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
- GlobalValue::dropLLVMManglingEscape(S));
- Index[GUID].emplace(S);
- }
+ void emplace(GlobalValue::GUID GUID, StringRef S) { Index[GUID].emplace(S); }
- size_t count(StringRef S) const {
- GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
- GlobalValue::dropLLVMManglingEscape(S));
- auto I = Index.find(GUID);
- if (I == Index.end())
- return 0;
- return I->second.count(S);
- }
+ size_t count(GlobalValue::GUID GUID) const { return Index.count(GUID); }
bool empty() const { return Index.empty(); }
};
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index f4324aa37f2e8..99922579c4e5e 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -8189,17 +8189,23 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
case bitc::FS_CFI_FUNCTION_DEFS: {
auto &CfiFunctionDefs = TheIndex.cfiFunctionDefs();
- for (unsigned I = 0; I != Record.size(); I += 2)
- CfiFunctionDefs.emplace(Strtab.data() + Record[I],
- static_cast<size_t>(Record[I + 1]));
+ for (unsigned I = 0; I != Record.size(); I += 3) {
+ uint64_t GUID = Record[I];
+ StringRef Name(Strtab.data() + Record[I + 1],
+ static_cast<size_t>(Record[I + 2]));
+ CfiFunctionDefs.emplace(GUID, Name);
+ }
break;
}
case bitc::FS_CFI_FUNCTION_DECLS: {
auto &CfiFunctionDecls = TheIndex.cfiFunctionDecls();
- for (unsigned I = 0; I != Record.size(); I += 2)
- CfiFunctionDecls.emplace(Strtab.data() + Record[I],
- static_cast<size_t>(Record[I + 1]));
+ for (unsigned I = 0; I != Record.size(); I += 3) {
+ uint64_t GUID = Record[I];
+ StringRef Name(Strtab.data() + Record[I + 1],
+ static_cast<size_t>(Record[I + 2]));
+ CfiFunctionDecls.emplace(GUID, Name);
+ }
break;
}
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 21cb0d2a28e36..3788365366426 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -5360,21 +5360,30 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
getReferencedTypeIds(FS, ReferencedTypeIds);
}
- SmallVector<StringRef, 4> Functions;
+ struct CfiFunction {
+ GlobalValue::GUID GUID = 0;
+ StringRef Name;
+ bool operator<(const CfiFunction &RHS) const {
+ return Name < RHS.Name;
+ }
+ };
+ std::vector<CfiFunction> Functions;
auto EmitCfiFunctions = [&](const CfiFunctionIndex &CfiIndex,
bitc::GlobalValueSummarySymtabCodes Code) {
if (CfiIndex.empty())
return;
for (GlobalValue::GUID GUID : DefOrUseGUIDs) {
- auto Defs = CfiIndex.forGuid(GUID);
- llvm::append_range(Functions, Defs);
+ for (StringRef Name : CfiIndex.forGuid(GUID)) {
+ Functions.push_back({GUID, Name});
+ }
}
if (Functions.empty())
return;
llvm::sort(Functions);
- for (const auto &S : Functions) {
- NameVals.push_back(StrtabBuilder.add(S));
- NameVals.push_back(S.size());
+ for (const auto &F : Functions) {
+ NameVals.push_back(F.GUID);
+ NameVals.push_back(StrtabBuilder.add(F.Name));
+ NameVals.push_back(F.Name.size());
}
Stream.EmitRecord(Code, NameVals);
NameVals.clear();
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 832eed31fa40a..0ab265c4e756f 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -106,6 +106,13 @@ GlobalValue::GUID GlobalValue::getGUIDOrFallback() const {
return getGUIDAssumingExternalLinkage(getGlobalIdentifier());
}
+GlobalValue::GUID GlobalValue::getGUIDOrFallbackDropEscape() const {
+ auto GuidIfPresent = getGUIDIfAssigned();
+ return GuidIfPresent ? *GuidIfPresent
+ : GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(getName()));
+}
+
std::optional<GlobalValue::GUID> GlobalValue::getGUIDIfAssigned() const {
// First check the metadata.
auto *MD = getGUIDMetadata();
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index a57e0c59726a3..1e0f6b28647c0 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -1788,10 +1788,11 @@ void LowerTypeTestsModule::buildBitSetsFromFunctionsNative(
}
if (IsExported) {
+ GlobalValue::GUID GUID = F->getGUIDOrFallbackDropEscape();
if (IsJumpTableCanonical)
- ExportSummary->cfiFunctionDefs().emplace(F->getName());
+ ExportSummary->cfiFunctionDefs().emplace(GUID, F->getName());
else
- ExportSummary->cfiFunctionDecls().emplace(F->getName());
+ ExportSummary->cfiFunctionDecls().emplace(GUID, F->getName());
}
if (!IsJumpTableCanonical) {
@@ -2123,9 +2124,10 @@ bool LowerTypeTestsModule::lower() {
// have the same name, but it's not the one we are looking for.
if (F.hasLocalLinkage())
continue;
- if (ImportSummary->cfiFunctionDefs().count(F.getName()))
+ auto GUID = F.getGUIDOrFallback();
+ if (ImportSummary->cfiFunctionDefs().count(GUID))
Defs.push_back(&F);
- else if (ImportSummary->cfiFunctionDecls().count(F.getName()))
+ else if (ImportSummary->cfiFunctionDecls().count(GUID))
Decls.push_back(&F);
}
@@ -2169,7 +2171,7 @@ bool LowerTypeTestsModule::lower() {
struct ExportedFunctionInfo {
CfiFunctionLinkage Linkage;
- MDNode *FuncMD; // {name, linkage, type[, type...]}
+ MDNode *FuncMD; // {name, linkage, stable_GUID, type[, type...]}
};
MapVector<StringRef, ExportedFunctionInfo> ExportedFunctions;
if (ExportSummary) {
@@ -2207,9 +2209,18 @@ bool LowerTypeTestsModule::lower() {
->getValue()
->getUniqueInteger()
.getZExtValue());
- const GlobalValue::GUID GUID =
- GlobalValue::getGUIDAssumingExternalLinkage(
- GlobalValue::dropLLVMManglingEscape(FunctionName));
+ // Use the stable GUID stored in !cfi.functions (element 2) if
+ // present. Promoted internal functions have a stable GUID in their
+ // !guid metadata that differs from
+ // getGUIDAssumingExternalLinkage(promoted_name), and the combined
+ // index stores entries under the stable GUID.
+ assert(FuncMD->getNumOperands() >= 3 &&
+ isa<ConstantAsMetadata>(FuncMD->getOperand(2)) &&
+ "GUID metadata missing");
+ GlobalValue::GUID GUID = cast<ConstantAsMetadata>(FuncMD->getOperand(2))
+ ->getValue()
+ ->getUniqueInteger()
+ .getZExtValue();
// Do not emit jumptable entries for functions that are not-live and
// have no live references (and are not exported with cross-DSO CFI.)
if (!ExportSummary->isGUIDLive(GUID))
@@ -2248,11 +2259,15 @@ bool LowerTypeTestsModule::lower() {
F = nullptr;
}
- if (!F)
+ if (!F) {
F = Function::Create(
FunctionType::get(Type::getVoidTy(M.getContext()), false),
GlobalVariable::ExternalLinkage,
M.getDataLayout().getProgramAddressSpace(), FunctionName, &M);
+ F->setMetadata(
+ LLVMContext::MD_unique_id,
+ MDTuple::get(M.getContext(), {FuncMD->getOperand(2).get()}));
+ }
// If the function is available_externally, remove its definition so
// that it is handled the same way as a declaration. Later we will try
@@ -2280,7 +2295,12 @@ bool LowerTypeTestsModule::lower() {
F->setLinkage(GlobalValue::ExternalWeakLinkage);
F->eraseMetadata(LLVMContext::MD_type);
- for (unsigned I = 2; I < FuncMD->getNumOperands(); ++I)
+ // Type metadata starts at operand 3 (operand 2 is the stable GUID).
+ assert(FuncMD->getNumOperands() >= 3 &&
+ isa<ConstantAsMetadata>(FuncMD->getOperand(2)) &&
+ "GUID metadata missing");
+ unsigned TypesStart = 3;
+ for (unsigned I = TypesStart; I < FuncMD->getNumOperands(); ++I)
F->addMetadata(LLVMContext::MD_type,
*cast<MDNode>(FuncMD->getOperand(I).get()));
}
diff --git a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
index 8ceefae72d9bf..a95e4b1d47cfb 100644
--- a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -438,6 +438,10 @@ void splitAndWriteThinLTOBitcode(
Linkage = CFL_Declaration;
Elts.push_back(ConstantAsMetadata::get(
llvm::ConstantInt::get(Type::getInt8Ty(Ctx), Linkage)));
+ // Store the function's stable GUID so that LowerTypeTests can find the
+ // corresponding entry in the combined index even after promotion renaming.
+ Elts.push_back(ConstantAsMetadata::get(llvm::ConstantInt::get(
+ Type::getInt64Ty(Ctx), V->getGUIDOrFallbackDropEscape())));
append_range(Elts, Types);
CfiFunctionMDs.push_back(MDTuple::get(Ctx, Elts));
}
diff --git a/llvm/test/ThinLTO/X86/cfi-icall-only-defuse.ll b/llvm/test/ThinLTO/X86/cfi-icall-only-defuse.ll
index 41fb17d574985..a1467920ebf24 100644
--- a/llvm/test/ThinLTO/X86/cfi-icall-only-defuse.ll
+++ b/llvm/test/ThinLTO/X86/cfi-icall-only-defuse.ll
@@ -40,7 +40,7 @@ define i8 @f(i1 %i, ptr %p) {
!0 = !{i64 0, !"t1"}
; FOOBAZ: <GLOBALVAL_SUMMARY_BLOCK
-; FOOBAZ: <CFI_FUNCTION_DEFS op0=0 op1=3 op2=3 op3=3 op4=6 op5=3/>
+; FOOBAZ: <CFI_FUNCTION_DEFS op0=-2012135647395072713 op1=0 op2=3 op3=7546896869197086323 op4=3 op5=3 op6=6699318081062747564 op7=6 op8=3/>
; FOOBAZ: <TYPE_ID op0=9 op1=2 op2=4 op3=7 op4=0 op5=0 op6=0 op7=0/>
; FOOBAZ: </GLOBALVAL_SUMMARY_BLOCK>
; FOOBAZ: <STRTAB_BLOCK
@@ -48,7 +48,7 @@ define i8 @f(i1 %i, ptr %p) {
; FOOBAZ-NEXT: </STRTAB_BLOCK>
; BARQUX: <GLOBALVAL_SUMMARY_BLOCK
-; BARQUX: <CFI_FUNCTION_DEFS op0=0 op1=3 op2=3 op3=3 op4=6 op5=3/>
+; BARQUX: <CFI_FUNCTION_DEFS op0=-2012135647395072713 op1=0 op2=3 op3=7546896869197086323 op4=3 op5=3 op6=-2941065755689329704 op7=6 op8=3/>
; BARQUX: <TYPE_ID op0=9 op1=2 op2=4 op3=7 op4=0 op5=0 op6=0 op7=0/>
; BARQUX: </GLOBALVAL_SUMMARY_BLOCK>
; BARQUX: <STRTAB_BLOCK
diff --git a/llvm/test/ThinLTO/X86/cfi-icall-thinlto.ll b/llvm/test/ThinLTO/X86/cfi-icall-thinlto.ll
new file mode 100644
index 0000000000000..8bf3d0b74ce4f
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/cfi-icall-thinlto.ll
@@ -0,0 +1,106 @@
+; REQUIRES: x86-registered-target
+
+; RUN: rm -rf %t.dir && split-file %s %t.dir
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit %t.dir/lib.ll -o %t.dir/lib.bc
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit %t.dir/main.ll -o %t.dir/main.bc
+; RUN: llvm-lto2 run -save-temps %t.dir/lib.bc %t.dir/main.bc -o %t.dir/summary \
+; RUN: -r=%t.dir/lib.bc,_Z11public_funcv,plx \
+; RUN: -r=%t.dir/lib.bc,syscall, \
+; RUN: -r=%t.dir/lib.bc,_ZN12_GLOBAL__N_113internal_funcEv.35df87b54cddf81e734207bfc5eea57a,pl \
+; RUN: -r=%t.dir/main.bc,main,plx \
+; RUN: -r=%t.dir/main.bc,_Z11public_funcv,l
+; RUN: llvm-dis %t.dir/summary.2.4.opt.bc -o - | FileCheck %s
+
+; RUN: llvm-lto2 run -thinlto-distributed-indexes %t.dir/lib.bc %t.dir/main.bc \
+; RUN: -o %t.dir/distidx \
+; RUN: -r=%t.dir/lib.bc,_Z11public_funcv,plx \
+; RUN: -r=%t.dir/lib.bc,syscall, \
+; RUN: -r=%t.dir/lib.bc,_ZN12_GLOBAL__N_113internal_funcEv.35df87b54cddf81e734207bfc5eea57a,pl \
+; RUN: -r=%t.dir/main.bc,main,plx \
+; RUN: -r=%t.dir/main.bc,_Z11public_funcv,l
+; RUN: llvm-bcanalyzer -dump %t.dir/lib.bc.thinlto.bc | FileCheck %s --check-prefix=DIST
+
+; Verify that the type test is NOT lowered to an unconditional trap,
+; and that the initialization path (which calls syscall) is preserved.
+; CHECK: define hidden noundef i32 @main()
+; CHECK: %[[LOAD:.*]] = load i1, ptr @_ZN12_GLOBAL__N_18lazy_valE.1.llvm.{{.*}}
+; CHECK-NEXT: br i1 %[[LOAD]], label %{{.*}}, label %[[LABEL:.*]]
+; CHECK: [[LABEL]]:
+; CHECK: call i64 (i64, ...) @syscall(i64 noundef 186)
+; CHECK-NOT: call void @llvm.ubsantrap
+
+; Verify that the distributed index records CFI_FUNCTION_DEFS with the
+; 3-field format [GUID, strtab_offset, name_length]. The promoted internal
+; function is listed with its stable pre-promotion GUID.
+; DIST: <CFI_FUNCTION_DEFS op0={{[-0-9]+}} op1=0 op2=67/>
+; DIST: <STRTAB_BLOCK
+; DIST-NEXT: <BLOB abbrevid=4/> blob data = '_ZN12_GLOBAL__N_113internal_funcEv.35df87b54cddf81e734207bfc5eea57a_ZTSFjvE'
+
+;--- lib.ll
+target triple = "x86_64-unknown-linux-gnu"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+
+@_ZN12_GLOBAL__N_18lazy_valE.1 = internal unnamed_addr global i1 false, align 4
+
+define hidden void @_Z11public_funcv() local_unnamed_addr #0 !type !10 !type !11 {
+ %1 = load i1, ptr @_ZN12_GLOBAL__N_18lazy_valE.1, align 1
+ %2 = zext i1 %1 to i8
+ %3 = trunc nuw i8 %2 to i1
+ br i1 %3, label %9, label %4
+
+4: ; preds = %0
+ %5 = tail call i1 @llvm.type.test(ptr nonnull @_ZN12_GLOBAL__N_113internal_funcEv, metadata !"_ZTSFjvE")
+ br i1 %5, label %7, label %6
+6: ; preds = %4
+ tail call void @llvm.ubsantrap(i8 2) #5
+ unreachable
+
+7: ; preds = %4
+ %8 = tail call i64 (i64, ...) @syscall(i64 noundef 186)
+ store i1 true, ptr @_ZN12_GLOBAL__N_18lazy_valE.1, align 1
+ br label %9
+
+9: ; preds = %0, %7
+ ret void
+}
+
+define internal noundef i32 @_ZN12_GLOBAL__N_113internal_funcEv() #1 !type !15 !type !16 {
+ %1 = tail call i64 (i64, ...) @syscall(i64 noundef 186)
+ %2 = trunc i64 %1 to i32
+ ret i32 %2
+}
+
+declare i1 @llvm.type.test(ptr, metadata) #2
+declare void @llvm.ubsantrap(i8 immarg) #3
+declare i64 @syscall(i64 noundef, ...) #4
+
+attributes #0 = { alwaysinline mustprogress nounwind uwtable "target-cpu"="x86-64" }
+attributes #1 = { mustprogress nounwind uwtable "target-cpu"="x86-64" }
+attributes #2 = { mustprogress nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+attributes #3 = { cold noreturn nounwind memory(inaccessiblemem: write) }
+attributes #4 = { nounwind "target-cpu"="x86-64" }
+attributes #5 = { nomerge noreturn nounwind }
+
+!10 = !{i64 0, !"_ZTSFvvE"}
+!11 = !{i64 0, !"_ZTSFvvE.generalized"}
+!15 = !{i64 0, !"_ZTSFjvE"}
+!16 = !{i64 0, !"_ZTSFjvE.generalized"}
+
+;--- main.ll
+target triple = "x86_64-unknown-linux-gnu"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+
+define hidden noundef i32 @main() local_unnamed_addr #0 !type !8 !type !9 {
+ tail call void @_Z11public_funcv()
+ ret i32 0
+}
+
+declare !type !11 !type !12 dso_local void @_Z11public_funcv() local_unnamed_addr #1
+
+attributes #0 = { mustprogress norecurse uwtable "target-cpu"="x86-64" }
+attributes #1 = { "target-cpu"="x86-64" }
+
+!8 = !{i64 0, !"_ZTSFivE"}
+!9 = !{i64 0, !"_ZTSFivE.generalized"}
+!11 = !{i64 0, !"_ZTSFvvE"}
+!12 = !{i64 0, !"_ZTSFvvE.generalized"}
diff --git a/llvm/test/ThinLTO/X86/cfi-icall.ll b/llvm/test/ThinLTO/X86/cfi-icall.ll
index 76847e8300434..15f519ecf523a 100644
--- a/llvm/test/ThinLTO/X86/cfi-icall.ll
+++ b/llvm/test/ThinLTO/X86/cfi-icall.ll
@@ -27,8 +27,8 @@ declare i1 @llvm.type.test(ptr %ptr, metadata %type) nounwind readnone
!0 = !{i64 0, !"typeid1"}
; COMBINED: <GLOBALVAL_SUMMARY_BLOCK
-; COMBINED: <CFI_FUNCTION_DEFS op0=0 op1=3/>
-; COMBINED: <CFI_FUNCTION_DECLS op0=3 op1=3/>
+; COMBINED: <CFI_FUNCTION_DEFS op0={{-?[0-9]+}} op1=0 op2=3/>
+; COMBINED: <CFI_FUNCTION_DECLS op0={{-?[0-9]+}} op1=3 op2=3/>
; COMBINED: <TYPE_ID op0=6 op1=7 op2=4 op3=7 op4=0 op5=0 op6=0 op7=0/>
; COMBINED: </GLOBALVAL_SUMMARY_BLOCK>
diff --git a/llvm/test/Transforms/LowerTypeTests/export-alias.ll b/llvm/test/Transforms/LowerTypeTests/export-alias.ll
index 25d34833c82c3..ac9dd09a4cbd7 100644
--- a/llvm/test/Transforms/LowerTypeTests/export-alias.ll
+++ b/llvm/test/Transforms/LowerTypeTests/export-alias.ll
@@ -10,10 +10,10 @@ target triple = "x86_64-unknown-linux"
!cfi.functions = !{!0, !2, !3, !4}
!aliases = !{!5, !6}
-!0 = !{!"external_addrtaken", i8 0, !1}
+!0 = !{!"external_addrtaken", i8 0, i64 16594175687743574550, !1}
!1 = !{i64 0, !"typeid1"}
-!2 = !{!"alias1", i8 0, !1}
-!3 = !{!"alias2", i8 0, !1}
-!4 = !{!"alias3", i8 0, !1}
+!2 = !{!"alias1", i8 0, i64 1062103744896965210, !1}
+!3 = !{!"alias2", i8 0, i64 2510616090736846890, !1}
+!4 = !{!"alias3", i8 0, i64 9766217518394409673, !1}
!5 = !{!"external_addrtaken", !"alias1", !"alias2"}
!6 = !{!"not_present", !"alias3"}
diff --git a/llvm/test/Transforms/LowerTypeTests/export-cross-dso-cfi.ll b/llvm/test/Transforms/LowerTypeTests/export-cross-dso-cfi.ll
index 68fff336e4138..dbc1e22fae029 100644
--- a/llvm/test/Transforms/LowerTypeTests/export-cross-dso-cfi.ll
+++ b/llvm/test/Transforms/LowerTypeTests/export-cross-dso-cfi.ll
@@ -31,9 +31,9 @@ define internal void @regularlto_internal() !type !1 !type !2 {
!cfi.functions = !{!0, !3, !4}
!llvm.module.flags = !{!5}
-!0 = !{!"external", i8 0, !1, !2}
+!0 = !{!"external", i8 0, i64 5224464028922159466, !1, !2}
!1 = !{i64 0, !"typeid1"}
!2 = !{i64 0, i64 1234}
-!3 = !{!"external2", i8 1, !1, !2}
-!4 = !{!"internal", i8 0, !1, !2}
+!3 = !{!"external2", i8 1, i64 16430208882958242304, !1, !2}
+!4 = !{!"internal", i8 0, i64 15859245615183425489, !1, !2}
!5 = !{i32 4, !"Cross-DSO CFI", i32 1}
diff --git a/llvm/test/Transforms/LowerTypeTests/export-icall.ll b/llvm/test/Transforms/LowerTypeTests/export-icall.ll
index f8adb2d69910f..5830bcb97989c 100644
--- a/llvm/test/Transforms/LowerTypeTests/export-icall.ll
+++ b/llvm/test/Transforms/LowerTypeTests/export-icall.ll
@@ -19,19 +19,19 @@ define void @f3(i32 %x) !type !8 {
!cfi.functions = !{!0, !1, !3, !9, !10, !4, !5, !6}
; declaration of @h with a different type is ignored
-!0 = !{!"h", i8 1, !7}
+!0 = !{!"h", i8 1, i64 8124147457056772133, !7}
; extern_weak declaration of @h with a different type is ignored as well
-!1 = !{!"h", i8 2, !8}
+!1 = !{!"h", i8 2, i64 8124147457056772133, !8}
!2 = !{i64 0, !"typeid1"}
; definitions of @f and @f2 replace types on the IR declarations above
...
[truncated]
|
|
@llvm/pr-subscribers-lto Author: Mircea Trofin (mtrofin) ChangesLowerTypeTests and the ModuleSummaryIndex mapped CFI function declarations and definitions based purely on the string name. If an internal function was promoted and renamed during ThinLTO, its name-derived GUID would change, leading to mismatches, resulting in runtime traps. This patch saves the GUIDs in the CFI tables. LowerTypeTests doesn't need to perform name-based lookups, relying on this GUID instead. Patch is 22.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/200542.diff 16 Files Affected:
diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index ef037a25fac2e..81859745386e0 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -638,6 +638,10 @@ class GlobalValue : public Constant {
/// that might pre-date the storage of GUIDs in metadata.
GUID getGUIDOrFallback() const;
+ /// same as getGUIDOrFallback, but for fallback we dropLLVMManglingEscape the
+ /// name first.
+ GUID getGUIDOrFallbackDropEscape() const;
+
/// @name Materialization
/// Materialization is used to construct functions only as they're needed.
/// This
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index ed39eb99a9196..512ffde9aab04 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1355,8 +1355,12 @@ class CfiFunctionIndex {
CfiFunctionIndex() = default;
template <typename It> CfiFunctionIndex(It B, It E) {
- for (; B != E; ++B)
- emplace(*B);
+ for (; B != E; ++B) {
+ StringRef S(*B);
+ GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(S));
+ Index[GUID].emplace(S);
+ }
}
std::vector<StringRef> symbols() const {
@@ -1381,21 +1385,9 @@ class CfiFunctionIndex {
return make_range(I->second.begin(), I->second.end());
}
- template <typename... Args> void emplace(Args &&...A) {
- StringRef S(std::forward<Args>(A)...);
- GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
- GlobalValue::dropLLVMManglingEscape(S));
- Index[GUID].emplace(S);
- }
+ void emplace(GlobalValue::GUID GUID, StringRef S) { Index[GUID].emplace(S); }
- size_t count(StringRef S) const {
- GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
- GlobalValue::dropLLVMManglingEscape(S));
- auto I = Index.find(GUID);
- if (I == Index.end())
- return 0;
- return I->second.count(S);
- }
+ size_t count(GlobalValue::GUID GUID) const { return Index.count(GUID); }
bool empty() const { return Index.empty(); }
};
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index f4324aa37f2e8..99922579c4e5e 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -8189,17 +8189,23 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
case bitc::FS_CFI_FUNCTION_DEFS: {
auto &CfiFunctionDefs = TheIndex.cfiFunctionDefs();
- for (unsigned I = 0; I != Record.size(); I += 2)
- CfiFunctionDefs.emplace(Strtab.data() + Record[I],
- static_cast<size_t>(Record[I + 1]));
+ for (unsigned I = 0; I != Record.size(); I += 3) {
+ uint64_t GUID = Record[I];
+ StringRef Name(Strtab.data() + Record[I + 1],
+ static_cast<size_t>(Record[I + 2]));
+ CfiFunctionDefs.emplace(GUID, Name);
+ }
break;
}
case bitc::FS_CFI_FUNCTION_DECLS: {
auto &CfiFunctionDecls = TheIndex.cfiFunctionDecls();
- for (unsigned I = 0; I != Record.size(); I += 2)
- CfiFunctionDecls.emplace(Strtab.data() + Record[I],
- static_cast<size_t>(Record[I + 1]));
+ for (unsigned I = 0; I != Record.size(); I += 3) {
+ uint64_t GUID = Record[I];
+ StringRef Name(Strtab.data() + Record[I + 1],
+ static_cast<size_t>(Record[I + 2]));
+ CfiFunctionDecls.emplace(GUID, Name);
+ }
break;
}
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 21cb0d2a28e36..3788365366426 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -5360,21 +5360,30 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
getReferencedTypeIds(FS, ReferencedTypeIds);
}
- SmallVector<StringRef, 4> Functions;
+ struct CfiFunction {
+ GlobalValue::GUID GUID = 0;
+ StringRef Name;
+ bool operator<(const CfiFunction &RHS) const {
+ return Name < RHS.Name;
+ }
+ };
+ std::vector<CfiFunction> Functions;
auto EmitCfiFunctions = [&](const CfiFunctionIndex &CfiIndex,
bitc::GlobalValueSummarySymtabCodes Code) {
if (CfiIndex.empty())
return;
for (GlobalValue::GUID GUID : DefOrUseGUIDs) {
- auto Defs = CfiIndex.forGuid(GUID);
- llvm::append_range(Functions, Defs);
+ for (StringRef Name : CfiIndex.forGuid(GUID)) {
+ Functions.push_back({GUID, Name});
+ }
}
if (Functions.empty())
return;
llvm::sort(Functions);
- for (const auto &S : Functions) {
- NameVals.push_back(StrtabBuilder.add(S));
- NameVals.push_back(S.size());
+ for (const auto &F : Functions) {
+ NameVals.push_back(F.GUID);
+ NameVals.push_back(StrtabBuilder.add(F.Name));
+ NameVals.push_back(F.Name.size());
}
Stream.EmitRecord(Code, NameVals);
NameVals.clear();
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 832eed31fa40a..0ab265c4e756f 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -106,6 +106,13 @@ GlobalValue::GUID GlobalValue::getGUIDOrFallback() const {
return getGUIDAssumingExternalLinkage(getGlobalIdentifier());
}
+GlobalValue::GUID GlobalValue::getGUIDOrFallbackDropEscape() const {
+ auto GuidIfPresent = getGUIDIfAssigned();
+ return GuidIfPresent ? *GuidIfPresent
+ : GlobalValue::getGUIDAssumingExternalLinkage(
+ GlobalValue::dropLLVMManglingEscape(getName()));
+}
+
std::optional<GlobalValue::GUID> GlobalValue::getGUIDIfAssigned() const {
// First check the metadata.
auto *MD = getGUIDMetadata();
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index a57e0c59726a3..1e0f6b28647c0 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -1788,10 +1788,11 @@ void LowerTypeTestsModule::buildBitSetsFromFunctionsNative(
}
if (IsExported) {
+ GlobalValue::GUID GUID = F->getGUIDOrFallbackDropEscape();
if (IsJumpTableCanonical)
- ExportSummary->cfiFunctionDefs().emplace(F->getName());
+ ExportSummary->cfiFunctionDefs().emplace(GUID, F->getName());
else
- ExportSummary->cfiFunctionDecls().emplace(F->getName());
+ ExportSummary->cfiFunctionDecls().emplace(GUID, F->getName());
}
if (!IsJumpTableCanonical) {
@@ -2123,9 +2124,10 @@ bool LowerTypeTestsModule::lower() {
// have the same name, but it's not the one we are looking for.
if (F.hasLocalLinkage())
continue;
- if (ImportSummary->cfiFunctionDefs().count(F.getName()))
+ auto GUID = F.getGUIDOrFallback();
+ if (ImportSummary->cfiFunctionDefs().count(GUID))
Defs.push_back(&F);
- else if (ImportSummary->cfiFunctionDecls().count(F.getName()))
+ else if (ImportSummary->cfiFunctionDecls().count(GUID))
Decls.push_back(&F);
}
@@ -2169,7 +2171,7 @@ bool LowerTypeTestsModule::lower() {
struct ExportedFunctionInfo {
CfiFunctionLinkage Linkage;
- MDNode *FuncMD; // {name, linkage, type[, type...]}
+ MDNode *FuncMD; // {name, linkage, stable_GUID, type[, type...]}
};
MapVector<StringRef, ExportedFunctionInfo> ExportedFunctions;
if (ExportSummary) {
@@ -2207,9 +2209,18 @@ bool LowerTypeTestsModule::lower() {
->getValue()
->getUniqueInteger()
.getZExtValue());
- const GlobalValue::GUID GUID =
- GlobalValue::getGUIDAssumingExternalLinkage(
- GlobalValue::dropLLVMManglingEscape(FunctionName));
+ // Use the stable GUID stored in !cfi.functions (element 2) if
+ // present. Promoted internal functions have a stable GUID in their
+ // !guid metadata that differs from
+ // getGUIDAssumingExternalLinkage(promoted_name), and the combined
+ // index stores entries under the stable GUID.
+ assert(FuncMD->getNumOperands() >= 3 &&
+ isa<ConstantAsMetadata>(FuncMD->getOperand(2)) &&
+ "GUID metadata missing");
+ GlobalValue::GUID GUID = cast<ConstantAsMetadata>(FuncMD->getOperand(2))
+ ->getValue()
+ ->getUniqueInteger()
+ .getZExtValue();
// Do not emit jumptable entries for functions that are not-live and
// have no live references (and are not exported with cross-DSO CFI.)
if (!ExportSummary->isGUIDLive(GUID))
@@ -2248,11 +2259,15 @@ bool LowerTypeTestsModule::lower() {
F = nullptr;
}
- if (!F)
+ if (!F) {
F = Function::Create(
FunctionType::get(Type::getVoidTy(M.getContext()), false),
GlobalVariable::ExternalLinkage,
M.getDataLayout().getProgramAddressSpace(), FunctionName, &M);
+ F->setMetadata(
+ LLVMContext::MD_unique_id,
+ MDTuple::get(M.getContext(), {FuncMD->getOperand(2).get()}));
+ }
// If the function is available_externally, remove its definition so
// that it is handled the same way as a declaration. Later we will try
@@ -2280,7 +2295,12 @@ bool LowerTypeTestsModule::lower() {
F->setLinkage(GlobalValue::ExternalWeakLinkage);
F->eraseMetadata(LLVMContext::MD_type);
- for (unsigned I = 2; I < FuncMD->getNumOperands(); ++I)
+ // Type metadata starts at operand 3 (operand 2 is the stable GUID).
+ assert(FuncMD->getNumOperands() >= 3 &&
+ isa<ConstantAsMetadata>(FuncMD->getOperand(2)) &&
+ "GUID metadata missing");
+ unsigned TypesStart = 3;
+ for (unsigned I = TypesStart; I < FuncMD->getNumOperands(); ++I)
F->addMetadata(LLVMContext::MD_type,
*cast<MDNode>(FuncMD->getOperand(I).get()));
}
diff --git a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
index 8ceefae72d9bf..a95e4b1d47cfb 100644
--- a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -438,6 +438,10 @@ void splitAndWriteThinLTOBitcode(
Linkage = CFL_Declaration;
Elts.push_back(ConstantAsMetadata::get(
llvm::ConstantInt::get(Type::getInt8Ty(Ctx), Linkage)));
+ // Store the function's stable GUID so that LowerTypeTests can find the
+ // corresponding entry in the combined index even after promotion renaming.
+ Elts.push_back(ConstantAsMetadata::get(llvm::ConstantInt::get(
+ Type::getInt64Ty(Ctx), V->getGUIDOrFallbackDropEscape())));
append_range(Elts, Types);
CfiFunctionMDs.push_back(MDTuple::get(Ctx, Elts));
}
diff --git a/llvm/test/ThinLTO/X86/cfi-icall-only-defuse.ll b/llvm/test/ThinLTO/X86/cfi-icall-only-defuse.ll
index 41fb17d574985..a1467920ebf24 100644
--- a/llvm/test/ThinLTO/X86/cfi-icall-only-defuse.ll
+++ b/llvm/test/ThinLTO/X86/cfi-icall-only-defuse.ll
@@ -40,7 +40,7 @@ define i8 @f(i1 %i, ptr %p) {
!0 = !{i64 0, !"t1"}
; FOOBAZ: <GLOBALVAL_SUMMARY_BLOCK
-; FOOBAZ: <CFI_FUNCTION_DEFS op0=0 op1=3 op2=3 op3=3 op4=6 op5=3/>
+; FOOBAZ: <CFI_FUNCTION_DEFS op0=-2012135647395072713 op1=0 op2=3 op3=7546896869197086323 op4=3 op5=3 op6=6699318081062747564 op7=6 op8=3/>
; FOOBAZ: <TYPE_ID op0=9 op1=2 op2=4 op3=7 op4=0 op5=0 op6=0 op7=0/>
; FOOBAZ: </GLOBALVAL_SUMMARY_BLOCK>
; FOOBAZ: <STRTAB_BLOCK
@@ -48,7 +48,7 @@ define i8 @f(i1 %i, ptr %p) {
; FOOBAZ-NEXT: </STRTAB_BLOCK>
; BARQUX: <GLOBALVAL_SUMMARY_BLOCK
-; BARQUX: <CFI_FUNCTION_DEFS op0=0 op1=3 op2=3 op3=3 op4=6 op5=3/>
+; BARQUX: <CFI_FUNCTION_DEFS op0=-2012135647395072713 op1=0 op2=3 op3=7546896869197086323 op4=3 op5=3 op6=-2941065755689329704 op7=6 op8=3/>
; BARQUX: <TYPE_ID op0=9 op1=2 op2=4 op3=7 op4=0 op5=0 op6=0 op7=0/>
; BARQUX: </GLOBALVAL_SUMMARY_BLOCK>
; BARQUX: <STRTAB_BLOCK
diff --git a/llvm/test/ThinLTO/X86/cfi-icall-thinlto.ll b/llvm/test/ThinLTO/X86/cfi-icall-thinlto.ll
new file mode 100644
index 0000000000000..8bf3d0b74ce4f
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/cfi-icall-thinlto.ll
@@ -0,0 +1,106 @@
+; REQUIRES: x86-registered-target
+
+; RUN: rm -rf %t.dir && split-file %s %t.dir
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit %t.dir/lib.ll -o %t.dir/lib.bc
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit %t.dir/main.ll -o %t.dir/main.bc
+; RUN: llvm-lto2 run -save-temps %t.dir/lib.bc %t.dir/main.bc -o %t.dir/summary \
+; RUN: -r=%t.dir/lib.bc,_Z11public_funcv,plx \
+; RUN: -r=%t.dir/lib.bc,syscall, \
+; RUN: -r=%t.dir/lib.bc,_ZN12_GLOBAL__N_113internal_funcEv.35df87b54cddf81e734207bfc5eea57a,pl \
+; RUN: -r=%t.dir/main.bc,main,plx \
+; RUN: -r=%t.dir/main.bc,_Z11public_funcv,l
+; RUN: llvm-dis %t.dir/summary.2.4.opt.bc -o - | FileCheck %s
+
+; RUN: llvm-lto2 run -thinlto-distributed-indexes %t.dir/lib.bc %t.dir/main.bc \
+; RUN: -o %t.dir/distidx \
+; RUN: -r=%t.dir/lib.bc,_Z11public_funcv,plx \
+; RUN: -r=%t.dir/lib.bc,syscall, \
+; RUN: -r=%t.dir/lib.bc,_ZN12_GLOBAL__N_113internal_funcEv.35df87b54cddf81e734207bfc5eea57a,pl \
+; RUN: -r=%t.dir/main.bc,main,plx \
+; RUN: -r=%t.dir/main.bc,_Z11public_funcv,l
+; RUN: llvm-bcanalyzer -dump %t.dir/lib.bc.thinlto.bc | FileCheck %s --check-prefix=DIST
+
+; Verify that the type test is NOT lowered to an unconditional trap,
+; and that the initialization path (which calls syscall) is preserved.
+; CHECK: define hidden noundef i32 @main()
+; CHECK: %[[LOAD:.*]] = load i1, ptr @_ZN12_GLOBAL__N_18lazy_valE.1.llvm.{{.*}}
+; CHECK-NEXT: br i1 %[[LOAD]], label %{{.*}}, label %[[LABEL:.*]]
+; CHECK: [[LABEL]]:
+; CHECK: call i64 (i64, ...) @syscall(i64 noundef 186)
+; CHECK-NOT: call void @llvm.ubsantrap
+
+; Verify that the distributed index records CFI_FUNCTION_DEFS with the
+; 3-field format [GUID, strtab_offset, name_length]. The promoted internal
+; function is listed with its stable pre-promotion GUID.
+; DIST: <CFI_FUNCTION_DEFS op0={{[-0-9]+}} op1=0 op2=67/>
+; DIST: <STRTAB_BLOCK
+; DIST-NEXT: <BLOB abbrevid=4/> blob data = '_ZN12_GLOBAL__N_113internal_funcEv.35df87b54cddf81e734207bfc5eea57a_ZTSFjvE'
+
+;--- lib.ll
+target triple = "x86_64-unknown-linux-gnu"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+
+@_ZN12_GLOBAL__N_18lazy_valE.1 = internal unnamed_addr global i1 false, align 4
+
+define hidden void @_Z11public_funcv() local_unnamed_addr #0 !type !10 !type !11 {
+ %1 = load i1, ptr @_ZN12_GLOBAL__N_18lazy_valE.1, align 1
+ %2 = zext i1 %1 to i8
+ %3 = trunc nuw i8 %2 to i1
+ br i1 %3, label %9, label %4
+
+4: ; preds = %0
+ %5 = tail call i1 @llvm.type.test(ptr nonnull @_ZN12_GLOBAL__N_113internal_funcEv, metadata !"_ZTSFjvE")
+ br i1 %5, label %7, label %6
+6: ; preds = %4
+ tail call void @llvm.ubsantrap(i8 2) #5
+ unreachable
+
+7: ; preds = %4
+ %8 = tail call i64 (i64, ...) @syscall(i64 noundef 186)
+ store i1 true, ptr @_ZN12_GLOBAL__N_18lazy_valE.1, align 1
+ br label %9
+
+9: ; preds = %0, %7
+ ret void
+}
+
+define internal noundef i32 @_ZN12_GLOBAL__N_113internal_funcEv() #1 !type !15 !type !16 {
+ %1 = tail call i64 (i64, ...) @syscall(i64 noundef 186)
+ %2 = trunc i64 %1 to i32
+ ret i32 %2
+}
+
+declare i1 @llvm.type.test(ptr, metadata) #2
+declare void @llvm.ubsantrap(i8 immarg) #3
+declare i64 @syscall(i64 noundef, ...) #4
+
+attributes #0 = { alwaysinline mustprogress nounwind uwtable "target-cpu"="x86-64" }
+attributes #1 = { mustprogress nounwind uwtable "target-cpu"="x86-64" }
+attributes #2 = { mustprogress nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+attributes #3 = { cold noreturn nounwind memory(inaccessiblemem: write) }
+attributes #4 = { nounwind "target-cpu"="x86-64" }
+attributes #5 = { nomerge noreturn nounwind }
+
+!10 = !{i64 0, !"_ZTSFvvE"}
+!11 = !{i64 0, !"_ZTSFvvE.generalized"}
+!15 = !{i64 0, !"_ZTSFjvE"}
+!16 = !{i64 0, !"_ZTSFjvE.generalized"}
+
+;--- main.ll
+target triple = "x86_64-unknown-linux-gnu"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+
+define hidden noundef i32 @main() local_unnamed_addr #0 !type !8 !type !9 {
+ tail call void @_Z11public_funcv()
+ ret i32 0
+}
+
+declare !type !11 !type !12 dso_local void @_Z11public_funcv() local_unnamed_addr #1
+
+attributes #0 = { mustprogress norecurse uwtable "target-cpu"="x86-64" }
+attributes #1 = { "target-cpu"="x86-64" }
+
+!8 = !{i64 0, !"_ZTSFivE"}
+!9 = !{i64 0, !"_ZTSFivE.generalized"}
+!11 = !{i64 0, !"_ZTSFvvE"}
+!12 = !{i64 0, !"_ZTSFvvE.generalized"}
diff --git a/llvm/test/ThinLTO/X86/cfi-icall.ll b/llvm/test/ThinLTO/X86/cfi-icall.ll
index 76847e8300434..15f519ecf523a 100644
--- a/llvm/test/ThinLTO/X86/cfi-icall.ll
+++ b/llvm/test/ThinLTO/X86/cfi-icall.ll
@@ -27,8 +27,8 @@ declare i1 @llvm.type.test(ptr %ptr, metadata %type) nounwind readnone
!0 = !{i64 0, !"typeid1"}
; COMBINED: <GLOBALVAL_SUMMARY_BLOCK
-; COMBINED: <CFI_FUNCTION_DEFS op0=0 op1=3/>
-; COMBINED: <CFI_FUNCTION_DECLS op0=3 op1=3/>
+; COMBINED: <CFI_FUNCTION_DEFS op0={{-?[0-9]+}} op1=0 op2=3/>
+; COMBINED: <CFI_FUNCTION_DECLS op0={{-?[0-9]+}} op1=3 op2=3/>
; COMBINED: <TYPE_ID op0=6 op1=7 op2=4 op3=7 op4=0 op5=0 op6=0 op7=0/>
; COMBINED: </GLOBALVAL_SUMMARY_BLOCK>
diff --git a/llvm/test/Transforms/LowerTypeTests/export-alias.ll b/llvm/test/Transforms/LowerTypeTests/export-alias.ll
index 25d34833c82c3..ac9dd09a4cbd7 100644
--- a/llvm/test/Transforms/LowerTypeTests/export-alias.ll
+++ b/llvm/test/Transforms/LowerTypeTests/export-alias.ll
@@ -10,10 +10,10 @@ target triple = "x86_64-unknown-linux"
!cfi.functions = !{!0, !2, !3, !4}
!aliases = !{!5, !6}
-!0 = !{!"external_addrtaken", i8 0, !1}
+!0 = !{!"external_addrtaken", i8 0, i64 16594175687743574550, !1}
!1 = !{i64 0, !"typeid1"}
-!2 = !{!"alias1", i8 0, !1}
-!3 = !{!"alias2", i8 0, !1}
-!4 = !{!"alias3", i8 0, !1}
+!2 = !{!"alias1", i8 0, i64 1062103744896965210, !1}
+!3 = !{!"alias2", i8 0, i64 2510616090736846890, !1}
+!4 = !{!"alias3", i8 0, i64 9766217518394409673, !1}
!5 = !{!"external_addrtaken", !"alias1", !"alias2"}
!6 = !{!"not_present", !"alias3"}
diff --git a/llvm/test/Transforms/LowerTypeTests/export-cross-dso-cfi.ll b/llvm/test/Transforms/LowerTypeTests/export-cross-dso-cfi.ll
index 68fff336e4138..dbc1e22fae029 100644
--- a/llvm/test/Transforms/LowerTypeTests/export-cross-dso-cfi.ll
+++ b/llvm/test/Transforms/LowerTypeTests/export-cross-dso-cfi.ll
@@ -31,9 +31,9 @@ define internal void @regularlto_internal() !type !1 !type !2 {
!cfi.functions = !{!0, !3, !4}
!llvm.module.flags = !{!5}
-!0 = !{!"external", i8 0, !1, !2}
+!0 = !{!"external", i8 0, i64 5224464028922159466, !1, !2}
!1 = !{i64 0, !"typeid1"}
!2 = !{i64 0, i64 1234}
-!3 = !{!"external2", i8 1, !1, !2}
-!4 = !{!"internal", i8 0, !1, !2}
+!3 = !{!"external2", i8 1, i64 16430208882958242304, !1, !2}
+!4 = !{!"internal", i8 0, i64 15859245615183425489, !1, !2}
!5 = !{i32 4, !"Cross-DSO CFI", i32 1}
diff --git a/llvm/test/Transforms/LowerTypeTests/export-icall.ll b/llvm/test/Transforms/LowerTypeTests/export-icall.ll
index f8adb2d69910f..5830bcb97989c 100644
--- a/llvm/test/Transforms/LowerTypeTests/export-icall.ll
+++ b/llvm/test/Transforms/LowerTypeTests/export-icall.ll
@@ -19,19 +19,19 @@ define void @f3(i32 %x) !type !8 {
!cfi.functions = !{!0, !1, !3, !9, !10, !4, !5, !6}
; declaration of @h with a different type is ignored
-!0 = !{!"h", i8 1, !7}
+!0 = !{!"h", i8 1, i64 8124147457056772133, !7}
; extern_weak declaration of @h with a different type is ignored as well
-!1 = !{!"h", i8 2, !8}
+!1 = !{!"h", i8 2, i64 8124147457056772133, !8}
!2 = !{i64 0, !"typeid1"}
; definitions of @f and @f2 replace types on the IR declarations above
...
[truncated]
|
mtrofin
marked this pull request as draft
May 30, 2026 04:02
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
LowerTypeTests and the ModuleSummaryIndex mapped CFI function declarations and definitions based purely on the string name. If an internal function was promoted and renamed during ThinLTO, its name-derived GUID would change, leading to mismatches, resulting in runtime traps. This patch saves the GUIDs in the CFI tables. LowerTypeTests doesn't need to perform name-based lookups, relying on this GUID instead.
mtrofin
marked this pull request as ready for review
June 1, 2026 15:12
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
pcc
requested changes
Jun 1, 2026
pcc
left a comment
pcc
left a comment
Contributor
There was a problem hiding this comment.
LowerTypeTests doesn't need to perform name-based lookups, relying on this GUID instead.
LowerTypeTests deliberately uses the name for lookups. "GUID" is a misleading name because they aren't actually unique due to hash collisions. In case of a hash collision, LowerTypeTests needs to find the correct information for the type.
Member
Author
Sorry, I was referring to the change at |
Member
Author
Wait, that's also using GUID underneath. |
Contributor
We started using the GUID to optimize name lookups in #130382. The final lookup has always been based on the name. Using a different GUID for the initial lookup will likely break some invariants that |
Member
Author
I see - so in this case it's basically an implementation detail, and, while hash collisions may still happen, placing this guid in the global namespace used by thinlto is undesirable, if I understand it correctly. |
Member
Author
(looking more) it seems, however, that there is a current assumption about the GUID being the same as the one used by ThinLTO: see |
Contributor
|
@pcc or @vitalybuka I think it would be helpful to have some comments on the definition of the CfiFunctionIndex and its members - could someone add that? Does the GUID there need to match the GUID used to index into the main ModuleSummaryIndex? |
mtrofin
marked this pull request as draft
June 2, 2026 21:52
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LowerTypeTests and the ModuleSummaryIndex mapped CFI function declarations and definitions based on the string name, including to constructing GUIDs to query the ExportSummary. After PR #184065, we use bitcode-embedded information (metadata or tables) to reliably capture the GUID of a function, and name-based calculations are an implementation detail. PR #184065 did copy along the GUID during module splitting, but didn't also capture that in the cfi.functions metadata, meaning that LowerTypeTests would still try to use the old, name-based mechanism, which would result in a GUID that couldn't be found in the summary.
This patch saves the GUIDs in the CFI tables. LowerTypeTests doesn't need to perform name-based lookups, relying on this GUID instead.
The patch also contains the necessary autoupgrade change.