Skip to content

Commit ff38981

Browse files
pccpcc
authored
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: #150690
1 parent 62187a6 commit ff38981

File tree

7 files changed

+148
-100
lines changed

7 files changed

+148
-100
lines changed

‎llvm/include/llvm/LTO/LTO.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -546,10 +546,12 @@ class LTO {
546546
// resolutions used by a single input module. Functions return ranges refering
547547
// to the resolutions for the remaining modules in the InputFile.
548548
Expected<ArrayRef<SymbolResolution>>
549-
addModule(InputFile &Input, unsigned ModI, ArrayRef<SymbolResolution> Res);
549+
addModule(InputFile &Input, ArrayRef<SymbolResolution> InputRes,
550+
unsigned ModI, ArrayRef<SymbolResolution> Res);
550551

551552
Expected<std::pair<RegularLTOState::AddedModule, ArrayRef<SymbolResolution>>>
552-
addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
553+
addRegularLTO(InputFile &Input, ArrayRef<SymbolResolution> InputRes,
554+
BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
553555
ArrayRef<SymbolResolution> Res);
554556
Error linkRegularLTO(RegularLTOState::AddedModule Mod,
555557
bool LivenessFromIndex);

‎llvm/lib/LTO/LTO.cpp‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -743,8 +743,9 @@ Error LTO::add(std::unique_ptr<InputFile> Input,
743743
Conf.VisibilityScheme = Config::ELF;
744744
}
745745

746+
ArrayRef<SymbolResolution> InputRes = Res;
746747
for (unsigned I = 0; I != Input->Mods.size(); ++I) {
747-
if (auto Err = addModule(*Input, I, Res).moveInto(Res))
748+
if (auto Err = addModule(*Input, InputRes, I, Res).moveInto(Res))
748749
return Err;
749750
}
750751

@@ -753,8 +754,8 @@ Error LTO::add(std::unique_ptr<InputFile> Input,
753754
}
754755

755756
Expected<ArrayRef<SymbolResolution>>
756-
LTO::addModule(InputFile &Input, unsigned ModI,
757-
ArrayRef<SymbolResolution> Res) {
757+
LTO::addModule(InputFile &Input, ArrayRef<SymbolResolution> InputRes,
758+
unsigned ModI, ArrayRef<SymbolResolution> Res) {
758759
Expected<BitcodeLTOInfo> LTOInfo = Input.Mods[ModI].getLTOInfo();
759760
if (!LTOInfo)
760761
return LTOInfo.takeError();
@@ -791,7 +792,7 @@ LTO::addModule(InputFile &Input, unsigned ModI,
791792
return addThinLTO(BM, ModSyms, Res);
792793

793794
RegularLTO.EmptyCombinedModule = false;
794-
auto ModOrErr = addRegularLTO(BM, ModSyms, Res);
795+
auto ModOrErr = addRegularLTO(Input, InputRes, BM, ModSyms, Res);
795796
if (!ModOrErr)
796797
return ModOrErr.takeError();
797798
Res = ModOrErr->second;
@@ -846,7 +847,8 @@ handleNonPrevailingComdat(GlobalValue &GV,
846847
// linkRegularLTO.
847848
Expected<
848849
std::pair<LTO::RegularLTOState::AddedModule, ArrayRef<SymbolResolution>>>
849-
LTO::addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
850+
LTO::addRegularLTO(InputFile &Input, ArrayRef<SymbolResolution> InputRes,
851+
BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
850852
ArrayRef<SymbolResolution> Res) {
851853
RegularLTOState::AddedModule Mod;
852854
Expected<std::unique_ptr<Module>> MOrErr =
@@ -860,13 +862,34 @@ LTO::addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
860862
if (Error Err = M.materializeMetadata())
861863
return std::move(Err);
862864

863-
// If cfi.functions is present and we are in regular LTO mode, LowerTypeTests
864-
// will rename local functions in the merged module as "<function name>.1".
865-
// This causes linking errors, since other parts of the module expect the
866-
// original function name.
867-
if (LTOMode == LTOK_UnifiedRegular)
865+
if (LTOMode == LTOK_UnifiedRegular) {
866+
// cfi.functions metadata is intended to be used with ThinLTO and may
867+
// trigger invalid IR transformations if they are present when doing regular
868+
// LTO, so delete it.
868869
if (NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions"))
869870
M.eraseNamedMetadata(CfiFunctionsMD);
871+
} else if (NamedMDNode *AliasesMD = M.getNamedMetadata("aliases")) {
872+
// Delete aliases entries for non-prevailing symbols on the ThinLTO side of
873+
// this input file.
874+
DenseSet<StringRef> Prevailing;
875+
for (auto [I, R] : zip(Input.symbols(), Res))
876+
if (R.Prevailing && !I.getIRName().empty())
877+
Prevailing.insert(I.getIRName());
878+
std::vector<MDNode *> AliasGroups;
879+
for (MDNode *AliasGroup : AliasesMD->operands()) {
880+
std::vector<Metadata *> Aliases;
881+
for (Metadata *Alias : AliasGroup->operands()) {
882+
if (isa<MDString>(Alias) &&
883+
Prevailing.count(cast<MDString>(Alias)->getString()))
884+
Aliases.push_back(Alias);
885+
}
886+
if (Aliases.size() > 1)
887+
AliasGroups.push_back(MDTuple::get(RegularLTO.Ctx, Aliases));
888+
}
889+
AliasesMD->clearOperands();
890+
for (MDNode *G : AliasGroups)
891+
AliasesMD->addOperand(G);
892+
}
870893

871894
UpgradeDebugInfo(M);
872895

‎llvm/lib/Transforms/IPO/LowerTypeTests.cpp‎

Lines changed: 70 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -502,8 +502,7 @@ class LowerTypeTestsModule {
502502
uint8_t *exportTypeId(StringRef TypeId, const TypeIdLowering &TIL);
503503
TypeIdLowering importTypeId(StringRef TypeId);
504504
void importTypeTest(CallInst *CI);
505-
void importFunction(Function *F, bool isJumpTableCanonical,
506-
std::vector<GlobalAlias *> &AliasesToErase);
505+
void importFunction(Function *F, bool isJumpTableCanonical);
507506

508507
BitSetInfo
509508
buildBitSet(Metadata *TypeId,
@@ -1103,9 +1102,8 @@ void LowerTypeTestsModule::maybeReplaceComdat(Function *F,
11031102

11041103
// ThinLTO backend: the function F has a jump table entry; update this module
11051104
// accordingly. isJumpTableCanonical describes the type of the jump table entry.
1106-
void LowerTypeTestsModule::importFunction(
1107-
Function *F, bool isJumpTableCanonical,
1108-
std::vector<GlobalAlias *> &AliasesToErase) {
1105+
void LowerTypeTestsModule::importFunction(Function *F,
1106+
bool isJumpTableCanonical) {
11091107
assert(F->getType()->getAddressSpace() == 0);
11101108

11111109
GlobalValue::VisibilityTypes Visibility = F->getVisibility();
@@ -1135,23 +1133,23 @@ void LowerTypeTestsModule::importFunction(
11351133
} else {
11361134
F->setName(Name + ".cfi");
11371135
maybeReplaceComdat(F, Name);
1138-
F->setLinkage(GlobalValue::ExternalLinkage);
11391136
FDecl = Function::Create(F->getFunctionType(), GlobalValue::ExternalLinkage,
11401137
F->getAddressSpace(), Name, &M);
11411138
FDecl->setVisibility(Visibility);
11421139
Visibility = GlobalValue::HiddenVisibility;
11431140

1144-
// Delete aliases pointing to this function, they'll be re-created in the
1145-
// merged output. Don't do it yet though because ScopedSaveAliaseesAndUsed
1146-
// will want to reset the aliasees first.
1141+
// Update aliases pointing to this function to also include the ".cfi" suffix,
1142+
// We expect the jump table entry to either point to the real function or an
1143+
// alias. Redirect all other users to the jump table entry.
11471144
for (auto &U : F->uses()) {
11481145
if (auto *A = dyn_cast<GlobalAlias>(U.getUser())) {
1146+
std::string AliasName = A->getName().str() + ".cfi";
11491147
Function *AliasDecl = Function::Create(
11501148
F->getFunctionType(), GlobalValue::ExternalLinkage,
11511149
F->getAddressSpace(), "", &M);
11521150
AliasDecl->takeName(A);
11531151
A->replaceAllUsesWith(AliasDecl);
1154-
AliasesToErase.push_back(A);
1152+
A->setName(AliasName);
11551153
}
11561154
}
11571155
}
@@ -2077,16 +2075,13 @@ bool LowerTypeTestsModule::lower() {
20772075
Decls.push_back(&F);
20782076
}
20792077

2080-
std::vector<GlobalAlias *> AliasesToErase;
20812078
{
20822079
ScopedSaveAliaseesAndUsed S(M);
20832080
for (auto *F : Defs)
2084-
importFunction(F, /*isJumpTableCanonical*/ true, AliasesToErase);
2081+
importFunction(F, /*isJumpTableCanonical*/ true);
20852082
for (auto *F : Decls)
2086-
importFunction(F, /*isJumpTableCanonical*/ false, AliasesToErase);
2083+
importFunction(F, /*isJumpTableCanonical*/ false);
20872084
}
2088-
for (GlobalAlias *GA : AliasesToErase)
2089-
GA->eraseFromParent();
20902085

20912086
return true;
20922087
}
@@ -2137,6 +2132,18 @@ bool LowerTypeTestsModule::lower() {
21372132
if (auto Alias = dyn_cast<AliasSummary>(RefGVS.get()))
21382133
AddressTaken.insert(Alias->getAliaseeGUID());
21392134
}
2135+
auto IsAddressTaken = [&](GlobalValue::GUID GUID) {
2136+
if (AddressTaken.count(GUID))
2137+
return true;
2138+
auto VI = ExportSummary->getValueInfo(GUID);
2139+
if (!VI)
2140+
return false;
2141+
for (auto &I : VI.getSummaryList())
2142+
if (auto Alias = dyn_cast<AliasSummary>(I.get()))
2143+
if (AddressTaken.count(Alias->getAliaseeGUID()))
2144+
return true;
2145+
return false;
2146+
};
21402147
for (auto *FuncMD : CfiFunctionsMD->operands()) {
21412148
assert(FuncMD->getNumOperands() >= 2);
21422149
StringRef FunctionName =
@@ -2153,7 +2160,7 @@ bool LowerTypeTestsModule::lower() {
21532160
// have no live references (and are not exported with cross-DSO CFI.)
21542161
if (!ExportSummary->isGUIDLive(GUID))
21552162
continue;
2156-
if (!AddressTaken.count(GUID)) {
2163+
if (!IsAddressTaken(GUID)) {
21572164
if (!CrossDsoCfi || Linkage != CFL_Definition)
21582165
continue;
21592166

@@ -2227,6 +2234,43 @@ bool LowerTypeTestsModule::lower() {
22272234
}
22282235
}
22292236

2237+
struct AliasToCreate {
2238+
Function *Alias;
2239+
std::string TargetName;
2240+
};
2241+
std::vector<AliasToCreate> AliasesToCreate;
2242+
2243+
// Parse alias data to replace stand-in function declarations for aliases
2244+
// with an alias to the intended target.
2245+
if (ExportSummary) {
2246+
if (NamedMDNode *AliasesMD = M.getNamedMetadata("aliases")) {
2247+
for (auto *AliasMD : AliasesMD->operands()) {
2248+
SmallVector<Function *> Aliases;
2249+
for (Metadata *MD : AliasMD->operands()) {
2250+
auto *MDS = dyn_cast<MDString>(MD);
2251+
if (!MDS)
2252+
continue;
2253+
StringRef AliasName = MDS->getString();
2254+
if (!ExportedFunctions.count(AliasName))
2255+
continue;
2256+
auto *AliasF = M.getFunction(AliasName);
2257+
if (AliasF)
2258+
Aliases.push_back(AliasF);
2259+
}
2260+
2261+
if (Aliases.empty())
2262+
continue;
2263+
2264+
for (unsigned I = 1; I != Aliases.size(); ++I) {
2265+
auto *AliasF = Aliases[I];
2266+
ExportedFunctions.erase(AliasF->getName());
2267+
AliasesToCreate.push_back(
2268+
{AliasF, std::string(Aliases[0]->getName())});
2269+
}
2270+
}
2271+
}
2272+
}
2273+
22302274
DenseMap<GlobalObject *, GlobalTypeMember *> GlobalTypeMembers;
22312275
for (GlobalObject &GO : M.global_objects()) {
22322276
if (isa<GlobalVariable>(GO) && GO.isDeclarationForLinker())
@@ -2414,47 +2458,16 @@ bool LowerTypeTestsModule::lower() {
24142458

24152459
allocateByteArrays();
24162460

2417-
// Parse alias data to replace stand-in function declarations for aliases
2418-
// with an alias to the intended target.
2419-
if (ExportSummary) {
2420-
if (NamedMDNode *AliasesMD = M.getNamedMetadata("aliases")) {
2421-
for (auto *AliasMD : AliasesMD->operands()) {
2422-
assert(AliasMD->getNumOperands() >= 4);
2423-
StringRef AliasName =
2424-
cast<MDString>(AliasMD->getOperand(0))->getString();
2425-
StringRef Aliasee = cast<MDString>(AliasMD->getOperand(1))->getString();
2426-
2427-
if (auto It = ExportedFunctions.find(Aliasee);
2428-
It == ExportedFunctions.end() ||
2429-
It->second.Linkage != CFL_Definition || !M.getNamedAlias(Aliasee))
2430-
continue;
2431-
2432-
GlobalValue::VisibilityTypes Visibility =
2433-
static_cast<GlobalValue::VisibilityTypes>(
2434-
cast<ConstantAsMetadata>(AliasMD->getOperand(2))
2435-
->getValue()
2436-
->getUniqueInteger()
2437-
.getZExtValue());
2438-
bool Weak =
2439-
static_cast<bool>(cast<ConstantAsMetadata>(AliasMD->getOperand(3))
2440-
->getValue()
2441-
->getUniqueInteger()
2442-
.getZExtValue());
2443-
2444-
auto *Alias = GlobalAlias::create("", M.getNamedAlias(Aliasee));
2445-
Alias->setVisibility(Visibility);
2446-
if (Weak)
2447-
Alias->setLinkage(GlobalValue::WeakAnyLinkage);
2448-
2449-
if (auto *F = M.getFunction(AliasName)) {
2450-
Alias->takeName(F);
2451-
F->replaceAllUsesWith(Alias);
2452-
F->eraseFromParent();
2453-
} else {
2454-
Alias->setName(AliasName);
2455-
}
2456-
}
2457-
}
2461+
for (auto A : AliasesToCreate) {
2462+
auto *Target = M.getNamedValue(A.TargetName);
2463+
if (!isa<GlobalAlias>(Target))
2464+
continue;
2465+
auto *AliasGA = GlobalAlias::create("", Target);
2466+
AliasGA->setVisibility(A.Alias->getVisibility());
2467+
AliasGA->setLinkage(A.Alias->getLinkage());
2468+
AliasGA->takeName(A.Alias);
2469+
A.Alias->replaceAllUsesWith(AliasGA);
2470+
A.Alias->eraseFromParent();
24582471
}
24592472

24602473
// Emit .symver directives for exported functions, if they exist.

‎llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,10 @@ void splitAndWriteThinLTOBitcode(
384384
for (auto &F : M)
385385
if ((!F.hasLocalLinkage() || F.hasAddressTaken()) && HasTypeMetadata(&F))
386386
CfiFunctions.insert(&F);
387+
for (auto &A : M.aliases())
388+
if (auto *F = dyn_cast<Function>(A.getAliasee()))
389+
if (HasTypeMetadata(F))
390+
CfiFunctions.insert(&A);
387391

388392
// Remove all globals with type metadata, globals with comdats that live in
389393
// MergedM, and aliases pointing to such globals from the thin LTO module.
@@ -403,12 +407,12 @@ void splitAndWriteThinLTOBitcode(
403407
auto &Ctx = MergedM->getContext();
404408
SmallVector<MDNode *, 8> CfiFunctionMDs;
405409
for (auto *V : CfiFunctions) {
406-
Function &F = *cast<Function>(V);
410+
Function &F = *cast<Function>(V->getAliaseeObject());
407411
SmallVector<MDNode *, 2> Types;
408412
F.getMetadata(LLVMContext::MD_type, Types);
409413

410414
SmallVector<Metadata *, 4> Elts;
411-
Elts.push_back(MDString::get(Ctx, F.getName()));
415+
Elts.push_back(MDString::get(Ctx, V->getName()));
412416
CfiFunctionLinkage Linkage;
413417
if (lowertypetests::isJumpTableCanonical(&F))
414418
Linkage = CFL_Definition;
@@ -428,29 +432,24 @@ void splitAndWriteThinLTOBitcode(
428432
NMD->addOperand(MD);
429433
}
430434

431-
SmallVector<MDNode *, 8> FunctionAliases;
435+
MapVector<Function *, std::vector<GlobalAlias *>> FunctionAliases;
432436
for (auto &A : M.aliases()) {
433437
if (!isa<Function>(A.getAliasee()))
434438
continue;
435439

436440
auto *F = cast<Function>(A.getAliasee());
437-
438-
Metadata *Elts[] = {
439-
MDString::get(Ctx, A.getName()),
440-
MDString::get(Ctx, F->getName()),
441-
ConstantAsMetadata::get(
442-
ConstantInt::get(Type::getInt8Ty(Ctx), A.getVisibility())),
443-
ConstantAsMetadata::get(
444-
ConstantInt::get(Type::getInt8Ty(Ctx), A.isWeakForLinker())),
445-
};
446-
447-
FunctionAliases.push_back(MDTuple::get(Ctx, Elts));
441+
FunctionAliases[F].push_back(&A);
448442
}
449443

450444
if (!FunctionAliases.empty()) {
451445
NamedMDNode *NMD = MergedM->getOrInsertNamedMetadata("aliases");
452-
for (auto *MD : FunctionAliases)
453-
NMD->addOperand(MD);
446+
for (auto &Alias : FunctionAliases) {
447+
SmallVector<Metadata *> Elts;
448+
Elts.push_back(MDString::get(Ctx, Alias.first->getName()));
449+
for (auto *A : Alias.second)
450+
Elts.push_back(MDString::get(Ctx, A->getName()));
451+
NMD->addOperand(MDTuple::get(Ctx, Elts));
452+
}
454453
}
455454

456455
SmallVector<MDNode *, 8> Symvers;

‎llvm/test/Transforms/LowerTypeTests/Inputs/exported-funcs.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,12 @@ GlobalValueMap:
1919
15859245615183425489: # guid("internal")
2020
- Linkage: 7 # internal
2121
Live: true
22+
1062103744896965210: # guid("alias1")
23+
- Linkage: 4 # weak
24+
Live: true
25+
Aliasee: 16594175687743574550 # guid("external_addrtaken")
26+
2510616090736846890: # guid("alias2")
27+
- Linkage: 0 # weak
28+
Live: true
29+
Aliasee: 16594175687743574550 # guid("external_addrtaken")
2230
...

0 commit comments

Comments
 (0)