LTO: Redesign the CFI !aliases metadata.

With the current aliases metadata we lose information about which groups
of aliases survive symbol resolution. This causes various problems such
as #150075 where symbol resolution breaks the link between alias groups.

In this redesign of the aliases metadata, we stop representing the
individual aliases in !aliases. Instead, the individual aliases are
represented in !cfi.functions in the same way as functions, and the
alias groups (i.e. groups of symbols with the same address) are stored
in !aliases. At symbol resolution time, we filter out all non-prevailing
members of !aliases; the resulting set is used by LowerTypeTests to
recreate the aliases.

With this change it is now possible for a jump table entry to refer
to an alias in one of the ThinLTO object files (e.g. if a function is
non-prevailing but its alias is prevailing), so instead of deleting them,
rename them with the ".cfi" suffix.

Fixes #150070.

Fixes #150075.

Reviewers: teresajohnson, vitalybuka

Reviewed By: vitalybuka

Pull Request: https://github.com/llvm/llvm-project/pull/150690
This commit is contained in:
Peter Collingbourne 2025-07-30 14:04:11 -07:00 committed by GitHub
parent 62187a60e6
commit ff38981a58
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 148 additions and 100 deletions

View File

@ -546,10 +546,12 @@ private:
// resolutions used by a single input module. Functions return ranges refering
// to the resolutions for the remaining modules in the InputFile.
Expected<ArrayRef<SymbolResolution>>
addModule(InputFile &Input, unsigned ModI, ArrayRef<SymbolResolution> Res);
addModule(InputFile &Input, ArrayRef<SymbolResolution> InputRes,
unsigned ModI, ArrayRef<SymbolResolution> Res);
Expected<std::pair<RegularLTOState::AddedModule, ArrayRef<SymbolResolution>>>
addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
addRegularLTO(InputFile &Input, ArrayRef<SymbolResolution> InputRes,
BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
ArrayRef<SymbolResolution> Res);
Error linkRegularLTO(RegularLTOState::AddedModule Mod,
bool LivenessFromIndex);

View File

@ -743,8 +743,9 @@ Error LTO::add(std::unique_ptr<InputFile> Input,
Conf.VisibilityScheme = Config::ELF;
}
ArrayRef<SymbolResolution> InputRes = Res;
for (unsigned I = 0; I != Input->Mods.size(); ++I) {
if (auto Err = addModule(*Input, I, Res).moveInto(Res))
if (auto Err = addModule(*Input, InputRes, I, Res).moveInto(Res))
return Err;
}
@ -753,8 +754,8 @@ Error LTO::add(std::unique_ptr<InputFile> Input,
}
Expected<ArrayRef<SymbolResolution>>
LTO::addModule(InputFile &Input, unsigned ModI,
ArrayRef<SymbolResolution> Res) {
LTO::addModule(InputFile &Input, ArrayRef<SymbolResolution> InputRes,
unsigned ModI, ArrayRef<SymbolResolution> Res) {
Expected<BitcodeLTOInfo> LTOInfo = Input.Mods[ModI].getLTOInfo();
if (!LTOInfo)
return LTOInfo.takeError();
@ -791,7 +792,7 @@ LTO::addModule(InputFile &Input, unsigned ModI,
return addThinLTO(BM, ModSyms, Res);
RegularLTO.EmptyCombinedModule = false;
auto ModOrErr = addRegularLTO(BM, ModSyms, Res);
auto ModOrErr = addRegularLTO(Input, InputRes, BM, ModSyms, Res);
if (!ModOrErr)
return ModOrErr.takeError();
Res = ModOrErr->second;
@ -846,7 +847,8 @@ handleNonPrevailingComdat(GlobalValue &GV,
// linkRegularLTO.
Expected<
std::pair<LTO::RegularLTOState::AddedModule, ArrayRef<SymbolResolution>>>
LTO::addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
LTO::addRegularLTO(InputFile &Input, ArrayRef<SymbolResolution> InputRes,
BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
ArrayRef<SymbolResolution> Res) {
RegularLTOState::AddedModule Mod;
Expected<std::unique_ptr<Module>> MOrErr =
@ -860,13 +862,34 @@ LTO::addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
if (Error Err = M.materializeMetadata())
return std::move(Err);
// If cfi.functions is present and we are in regular LTO mode, LowerTypeTests
// will rename local functions in the merged module as "<function name>.1".
// This causes linking errors, since other parts of the module expect the
// original function name.
if (LTOMode == LTOK_UnifiedRegular)
if (LTOMode == LTOK_UnifiedRegular) {
// cfi.functions metadata is intended to be used with ThinLTO and may
// trigger invalid IR transformations if they are present when doing regular
// LTO, so delete it.
if (NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions"))
M.eraseNamedMetadata(CfiFunctionsMD);
} else if (NamedMDNode *AliasesMD = M.getNamedMetadata("aliases")) {
// Delete aliases entries for non-prevailing symbols on the ThinLTO side of
// this input file.
DenseSet<StringRef> Prevailing;
for (auto [I, R] : zip(Input.symbols(), Res))
if (R.Prevailing && !I.getIRName().empty())
Prevailing.insert(I.getIRName());
std::vector<MDNode *> AliasGroups;
for (MDNode *AliasGroup : AliasesMD->operands()) {
std::vector<Metadata *> Aliases;
for (Metadata *Alias : AliasGroup->operands()) {
if (isa<MDString>(Alias) &&
Prevailing.count(cast<MDString>(Alias)->getString()))
Aliases.push_back(Alias);
}
if (Aliases.size() > 1)
AliasGroups.push_back(MDTuple::get(RegularLTO.Ctx, Aliases));
}
AliasesMD->clearOperands();
for (MDNode *G : AliasGroups)
AliasesMD->addOperand(G);
}
UpgradeDebugInfo(M);

View File

@ -502,8 +502,7 @@ class LowerTypeTestsModule {
uint8_t *exportTypeId(StringRef TypeId, const TypeIdLowering &TIL);
TypeIdLowering importTypeId(StringRef TypeId);
void importTypeTest(CallInst *CI);
void importFunction(Function *F, bool isJumpTableCanonical,
std::vector<GlobalAlias *> &AliasesToErase);
void importFunction(Function *F, bool isJumpTableCanonical);
BitSetInfo
buildBitSet(Metadata *TypeId,
@ -1103,9 +1102,8 @@ void LowerTypeTestsModule::maybeReplaceComdat(Function *F,
// ThinLTO backend: the function F has a jump table entry; update this module
// accordingly. isJumpTableCanonical describes the type of the jump table entry.
void LowerTypeTestsModule::importFunction(
Function *F, bool isJumpTableCanonical,
std::vector<GlobalAlias *> &AliasesToErase) {
void LowerTypeTestsModule::importFunction(Function *F,
bool isJumpTableCanonical) {
assert(F->getType()->getAddressSpace() == 0);
GlobalValue::VisibilityTypes Visibility = F->getVisibility();
@ -1135,23 +1133,23 @@ void LowerTypeTestsModule::importFunction(
} else {
F->setName(Name + ".cfi");
maybeReplaceComdat(F, Name);
F->setLinkage(GlobalValue::ExternalLinkage);
FDecl = Function::Create(F->getFunctionType(), GlobalValue::ExternalLinkage,
F->getAddressSpace(), Name, &M);
FDecl->setVisibility(Visibility);
Visibility = GlobalValue::HiddenVisibility;
// Delete aliases pointing to this function, they'll be re-created in the
// merged output. Don't do it yet though because ScopedSaveAliaseesAndUsed
// will want to reset the aliasees first.
// Update aliases pointing to this function to also include the ".cfi" suffix,
// We expect the jump table entry to either point to the real function or an
// alias. Redirect all other users to the jump table entry.
for (auto &U : F->uses()) {
if (auto *A = dyn_cast<GlobalAlias>(U.getUser())) {
std::string AliasName = A->getName().str() + ".cfi";
Function *AliasDecl = Function::Create(
F->getFunctionType(), GlobalValue::ExternalLinkage,
F->getAddressSpace(), "", &M);
AliasDecl->takeName(A);
A->replaceAllUsesWith(AliasDecl);
AliasesToErase.push_back(A);
A->setName(AliasName);
}
}
}
@ -2077,16 +2075,13 @@ bool LowerTypeTestsModule::lower() {
Decls.push_back(&F);
}
std::vector<GlobalAlias *> AliasesToErase;
{
ScopedSaveAliaseesAndUsed S(M);
for (auto *F : Defs)
importFunction(F, /*isJumpTableCanonical*/ true, AliasesToErase);
importFunction(F, /*isJumpTableCanonical*/ true);
for (auto *F : Decls)
importFunction(F, /*isJumpTableCanonical*/ false, AliasesToErase);
importFunction(F, /*isJumpTableCanonical*/ false);
}
for (GlobalAlias *GA : AliasesToErase)
GA->eraseFromParent();
return true;
}
@ -2137,6 +2132,18 @@ bool LowerTypeTestsModule::lower() {
if (auto Alias = dyn_cast<AliasSummary>(RefGVS.get()))
AddressTaken.insert(Alias->getAliaseeGUID());
}
auto IsAddressTaken = [&](GlobalValue::GUID GUID) {
if (AddressTaken.count(GUID))
return true;
auto VI = ExportSummary->getValueInfo(GUID);
if (!VI)
return false;
for (auto &I : VI.getSummaryList())
if (auto Alias = dyn_cast<AliasSummary>(I.get()))
if (AddressTaken.count(Alias->getAliaseeGUID()))
return true;
return false;
};
for (auto *FuncMD : CfiFunctionsMD->operands()) {
assert(FuncMD->getNumOperands() >= 2);
StringRef FunctionName =
@ -2153,7 +2160,7 @@ bool LowerTypeTestsModule::lower() {
// have no live references (and are not exported with cross-DSO CFI.)
if (!ExportSummary->isGUIDLive(GUID))
continue;
if (!AddressTaken.count(GUID)) {
if (!IsAddressTaken(GUID)) {
if (!CrossDsoCfi || Linkage != CFL_Definition)
continue;
@ -2227,6 +2234,43 @@ bool LowerTypeTestsModule::lower() {
}
}
struct AliasToCreate {
Function *Alias;
std::string TargetName;
};
std::vector<AliasToCreate> AliasesToCreate;
// Parse alias data to replace stand-in function declarations for aliases
// with an alias to the intended target.
if (ExportSummary) {
if (NamedMDNode *AliasesMD = M.getNamedMetadata("aliases")) {
for (auto *AliasMD : AliasesMD->operands()) {
SmallVector<Function *> Aliases;
for (Metadata *MD : AliasMD->operands()) {
auto *MDS = dyn_cast<MDString>(MD);
if (!MDS)
continue;
StringRef AliasName = MDS->getString();
if (!ExportedFunctions.count(AliasName))
continue;
auto *AliasF = M.getFunction(AliasName);
if (AliasF)
Aliases.push_back(AliasF);
}
if (Aliases.empty())
continue;
for (unsigned I = 1; I != Aliases.size(); ++I) {
auto *AliasF = Aliases[I];
ExportedFunctions.erase(AliasF->getName());
AliasesToCreate.push_back(
{AliasF, std::string(Aliases[0]->getName())});
}
}
}
}
DenseMap<GlobalObject *, GlobalTypeMember *> GlobalTypeMembers;
for (GlobalObject &GO : M.global_objects()) {
if (isa<GlobalVariable>(GO) && GO.isDeclarationForLinker())
@ -2414,47 +2458,16 @@ bool LowerTypeTestsModule::lower() {
allocateByteArrays();
// Parse alias data to replace stand-in function declarations for aliases
// with an alias to the intended target.
if (ExportSummary) {
if (NamedMDNode *AliasesMD = M.getNamedMetadata("aliases")) {
for (auto *AliasMD : AliasesMD->operands()) {
assert(AliasMD->getNumOperands() >= 4);
StringRef AliasName =
cast<MDString>(AliasMD->getOperand(0))->getString();
StringRef Aliasee = cast<MDString>(AliasMD->getOperand(1))->getString();
if (auto It = ExportedFunctions.find(Aliasee);
It == ExportedFunctions.end() ||
It->second.Linkage != CFL_Definition || !M.getNamedAlias(Aliasee))
continue;
GlobalValue::VisibilityTypes Visibility =
static_cast<GlobalValue::VisibilityTypes>(
cast<ConstantAsMetadata>(AliasMD->getOperand(2))
->getValue()
->getUniqueInteger()
.getZExtValue());
bool Weak =
static_cast<bool>(cast<ConstantAsMetadata>(AliasMD->getOperand(3))
->getValue()
->getUniqueInteger()
.getZExtValue());
auto *Alias = GlobalAlias::create("", M.getNamedAlias(Aliasee));
Alias->setVisibility(Visibility);
if (Weak)
Alias->setLinkage(GlobalValue::WeakAnyLinkage);
if (auto *F = M.getFunction(AliasName)) {
Alias->takeName(F);
F->replaceAllUsesWith(Alias);
F->eraseFromParent();
} else {
Alias->setName(AliasName);
}
}
}
for (auto A : AliasesToCreate) {
auto *Target = M.getNamedValue(A.TargetName);
if (!isa<GlobalAlias>(Target))
continue;
auto *AliasGA = GlobalAlias::create("", Target);
AliasGA->setVisibility(A.Alias->getVisibility());
AliasGA->setLinkage(A.Alias->getLinkage());
AliasGA->takeName(A.Alias);
A.Alias->replaceAllUsesWith(AliasGA);
A.Alias->eraseFromParent();
}
// Emit .symver directives for exported functions, if they exist.

View File

@ -384,6 +384,10 @@ void splitAndWriteThinLTOBitcode(
for (auto &F : M)
if ((!F.hasLocalLinkage() || F.hasAddressTaken()) && HasTypeMetadata(&F))
CfiFunctions.insert(&F);
for (auto &A : M.aliases())
if (auto *F = dyn_cast<Function>(A.getAliasee()))
if (HasTypeMetadata(F))
CfiFunctions.insert(&A);
// Remove all globals with type metadata, globals with comdats that live in
// MergedM, and aliases pointing to such globals from the thin LTO module.
@ -403,12 +407,12 @@ void splitAndWriteThinLTOBitcode(
auto &Ctx = MergedM->getContext();
SmallVector<MDNode *, 8> CfiFunctionMDs;
for (auto *V : CfiFunctions) {
Function &F = *cast<Function>(V);
Function &F = *cast<Function>(V->getAliaseeObject());
SmallVector<MDNode *, 2> Types;
F.getMetadata(LLVMContext::MD_type, Types);
SmallVector<Metadata *, 4> Elts;
Elts.push_back(MDString::get(Ctx, F.getName()));
Elts.push_back(MDString::get(Ctx, V->getName()));
CfiFunctionLinkage Linkage;
if (lowertypetests::isJumpTableCanonical(&F))
Linkage = CFL_Definition;
@ -428,29 +432,24 @@ void splitAndWriteThinLTOBitcode(
NMD->addOperand(MD);
}
SmallVector<MDNode *, 8> FunctionAliases;
MapVector<Function *, std::vector<GlobalAlias *>> FunctionAliases;
for (auto &A : M.aliases()) {
if (!isa<Function>(A.getAliasee()))
continue;
auto *F = cast<Function>(A.getAliasee());
Metadata *Elts[] = {
MDString::get(Ctx, A.getName()),
MDString::get(Ctx, F->getName()),
ConstantAsMetadata::get(
ConstantInt::get(Type::getInt8Ty(Ctx), A.getVisibility())),
ConstantAsMetadata::get(
ConstantInt::get(Type::getInt8Ty(Ctx), A.isWeakForLinker())),
};
FunctionAliases.push_back(MDTuple::get(Ctx, Elts));
FunctionAliases[F].push_back(&A);
}
if (!FunctionAliases.empty()) {
NamedMDNode *NMD = MergedM->getOrInsertNamedMetadata("aliases");
for (auto *MD : FunctionAliases)
NMD->addOperand(MD);
for (auto &Alias : FunctionAliases) {
SmallVector<Metadata *> Elts;
Elts.push_back(MDString::get(Ctx, Alias.first->getName()));
for (auto *A : Alias.second)
Elts.push_back(MDString::get(Ctx, A->getName()));
NMD->addOperand(MDTuple::get(Ctx, Elts));
}
}
SmallVector<MDNode *, 8> Symvers;

View File

@ -19,4 +19,12 @@ GlobalValueMap:
15859245615183425489: # guid("internal")
- Linkage: 7 # internal
Live: true
1062103744896965210: # guid("alias1")
- Linkage: 4 # weak
Live: true
Aliasee: 16594175687743574550 # guid("external_addrtaken")
2510616090736846890: # guid("alias2")
- Linkage: 0 # weak
Live: true
Aliasee: 16594175687743574550 # guid("external_addrtaken")
...

View File

@ -1,21 +1,19 @@
; RUN: opt -S %s -passes=lowertypetests -lowertypetests-summary-action=export -lowertypetests-read-summary=%S/Inputs/exported-funcs.yaml | FileCheck %s
;
; CHECK: @alias1 = weak alias [8 x i8], ptr @external_addrtaken
; CHECK: @alias2 = hidden alias [8 x i8], ptr @external_addrtaken
; CHECK: @alias1 = alias [8 x i8], ptr @external_addrtaken
; CHECK: @alias2 = alias [8 x i8], ptr @external_addrtaken
; CHECK-NOT: @alias3 = alias
; CHECK-NOT: @not_present
target triple = "x86_64-unknown-linux"
!cfi.functions = !{!0, !2, !3}
!aliases = !{!4, !5, !6}
!cfi.functions = !{!0, !2, !3, !4}
!aliases = !{!5, !6}
!0 = !{!"external_addrtaken", i8 0, !1}
!1 = !{i64 0, !"typeid1"}
!2 = !{!"alias1", i8 1, !1}
; alias2 not included here, this could happen if the only reference to alias2
; is in a module compiled without cfi-icall
!3 = !{!"alias3", i8 1, !1}
!4 = !{!"alias1", !"external_addrtaken", i8 0, i8 1}
!5 = !{!"alias2", !"external_addrtaken", i8 1, i8 0}
!6 = !{!"alias3", !"not_present", i8 0, i8 0}
!2 = !{!"alias1", i8 0, !1}
!3 = !{!"alias2", i8 0, !1}
!4 = !{!"alias3", i8 0, !1}
!5 = !{!"external_addrtaken", !"alias1", !"alias2"}
!6 = !{!"not_present", !"alias3"}

View File

@ -7,11 +7,16 @@ define hidden void @Func() !type !0 {
ret void
}
; CHECK1: !aliases = !{![[A1:[0-9]+]], ![[A2:[0-9]+]], ![[A3:[0-9]+]]}
; CHECK1: !cfi.functions = !{![[F1:[0-9]+]], ![[F2:[0-9]+]], ![[F3:[0-9]+]], ![[F4:[0-9]+]]}
; CHECK1: !aliases = !{![[A:[0-9]+]]}
; CHECK1: ![[A1]] = !{!"Alias", !"Func", i8 1, i8 0}
; CHECK1: ![[A2]] = !{!"Hidden_Alias", !"Func", i8 1, i8 0}
; CHECK1: ![[A3]] = !{!"Weak_Alias", !"Func", i8 0, i8 1}
; CHECK1: ![[F1]] = !{!"Func", i8 0, ![[T:[0-9]+]]}
; CHECK1: ![[T]] = !{i64 0, !"_ZTSFvvE"}
; CHECK1: ![[F2]] = !{!"Alias", i8 0, ![[T]]}
; CHECK1: ![[F3]] = !{!"Hidden_Alias", i8 0, ![[T]]}
; CHECK1: ![[F4]] = !{!"Weak_Alias", i8 0, ![[T]]}
;
; CHECK1: ![[A]] = !{!"Func", !"Alias", !"Hidden_Alias", !"Weak_Alias"}
@Alias = hidden alias void (), ptr @Func
@Hidden_Alias = hidden alias void (), ptr @Func
@Weak_Alias = weak alias void (), ptr @Func