From f75ad4bc1b3c6f1110c3799d951dba02598e6a11 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Mon, 1 Mar 2021 13:43:23 -0800 Subject: [PATCH] [InstrProfiling] Place __llvm_prf_vnodes and __llvm_prf_names in llvm.used on ELF `__llvm_prf_vnodes` and `__llvm_prf_names` are used by runtime but not referenced via relocation in the translation unit. With `-z start-stop-gc` (D96914 https://sourceware.org/bugzilla/show_bug.cgi?id=27451), the linker no longer lets `__start_/__stop_` references retain them. Place `__llvm_prf_vnodes` and `__llvm_prf_names` in `llvm.used` to make them retained by the linker. This patch changes most existing `UsedVars` cases to `CompilerUsedVars` to reflect the ideal state - if the binary format properly supports section based GC (dead stripping), `llvm.compiler.used` should be sufficient. `__llvm_prf_vnodes` and `__llvm_prf_names` are switched to `UsedVars` since we want them to be unconditionally retained by both compiler and linker. Behaviors on other COFF/Mach-O are not affected. Differential Revision: https://reviews.llvm.org/D97649 --- .../Instrumentation/InstrProfiling.h | 1 + .../Instrumentation/InstrProfiling.cpp | 21 +++++++++++++++---- test/Instrumentation/InstrProfiling/icall.ll | 6 ++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/include/llvm/Transforms/Instrumentation/InstrProfiling.h b/include/llvm/Transforms/Instrumentation/InstrProfiling.h index 5242211138f..94b156f3b13 100644 --- a/include/llvm/Transforms/Instrumentation/InstrProfiling.h +++ b/include/llvm/Transforms/Instrumentation/InstrProfiling.h @@ -57,6 +57,7 @@ private: } }; DenseMap ProfileDataMap; + std::vector CompilerUsedVars; std::vector UsedVars; std::vector ReferencedNames; GlobalVariable *NamesVar; diff --git a/lib/Transforms/Instrumentation/InstrProfiling.cpp b/lib/Transforms/Instrumentation/InstrProfiling.cpp index d73bb66ed00..a17d6f52d77 100644 --- a/lib/Transforms/Instrumentation/InstrProfiling.cpp +++ b/lib/Transforms/Instrumentation/InstrProfiling.cpp @@ -539,6 +539,7 @@ bool InstrProfiling::run( NamesVar = nullptr; NamesSize = 0; ProfileDataMap.clear(); + CompilerUsedVars.clear(); UsedVars.clear(); TT = Triple(M.getTargetTriple()); @@ -921,7 +922,7 @@ InstrProfiling::getOrCreateRegionCounters(InstrProfIncrementInst *Inc) { ProfileDataMap[NamePtr] = PD; // Mark the data variable as used so that it isn't stripped out. - UsedVars.push_back(Data); + CompilerUsedVars.push_back(Data); // Now that the linkage set by the FE has been passed to the data and counter // variables, reset Name variable's linkage and visibility to private so that // it can be removed later by the compiler. @@ -976,6 +977,8 @@ void InstrProfiling::emitVNodes() { Constant::getNullValue(VNodesTy), getInstrProfVNodesVarName()); VNodesVar->setSection( getInstrProfSectionName(IPSK_vnodes, TT.getObjectFormat())); + // VNodesVar is used by runtime but not referenced via relocation by other + // sections. Conservatively make it linker retained. UsedVars.push_back(VNodesVar); } @@ -1004,6 +1007,8 @@ void InstrProfiling::emitNameData() { // linker from inserting padding before the start of the names section or // between names entries. NamesVar->setAlignment(Align(1)); + // NamesVar is used by runtime but not referenced via relocation by other + // sections. Conservatively make it linker retained. UsedVars.push_back(NamesVar); for (auto *NamePtr : ReferencedNames) @@ -1031,6 +1036,9 @@ void InstrProfiling::emitRegistration() { getInstrProfRegFuncName(), M); IRBuilder<> IRB(BasicBlock::Create(M->getContext(), "", RegisterF)); + for (Value *Data : CompilerUsedVars) + if (Data != NamesVar && !isa(Data)) + IRB.CreateCall(RuntimeRegisterF, IRB.CreateBitCast(Data, VoidPtrTy)); for (Value *Data : UsedVars) if (Data != NamesVar && !isa(Data)) IRB.CreateCall(RuntimeRegisterF, IRB.CreateBitCast(Data, VoidPtrTy)); @@ -1081,7 +1089,7 @@ bool InstrProfiling::emitRuntimeHook() { IRB.CreateRet(Load); // Mark the user variable as used so that it isn't stripped out. - UsedVars.push_back(User); + CompilerUsedVars.push_back(User); return true; } @@ -1094,9 +1102,14 @@ void InstrProfiling::emitUses() { // or discarded as a unit, so llvm.compiler.used is sufficient. Otherwise, // conservatively make all of them retained by the linker. if (TT.isOSBinFormatELF()) - appendToCompilerUsed(*M, UsedVars); + appendToCompilerUsed(*M, CompilerUsedVars); else - appendToUsed(*M, UsedVars); + appendToUsed(*M, CompilerUsedVars); + + // We do not add proper references from used metadata sections to NamesVar and + // VNodesVar, so we have to be conservative and place them in llvm.used + // regardless of the target, + appendToUsed(*M, UsedVars); } void InstrProfiling::emitInitialization() { diff --git a/test/Instrumentation/InstrProfiling/icall.ll b/test/Instrumentation/InstrProfiling/icall.ll index 311770ae570..bc7d6c90d0b 100644 --- a/test/Instrumentation/InstrProfiling/icall.ll +++ b/test/Instrumentation/InstrProfiling/icall.ll @@ -50,6 +50,12 @@ attributes #0 = { nounwind } ; DYN-NOT: @__profvp_foo ; DYN-NOT: @__llvm_prf_vnodes +;; __llvm_prf_vnodes and __llvm_prf_nm are not referenced by other metadata sections. +;; We have to conservatively place them in llvm.used. +; STATIC: @llvm.used = appending global +; STATIC-SAME: @__llvm_prf_vnodes +; STATIC-SAME: @__llvm_prf_nm + ; STATIC: call void @__llvm_profile_instrument_target(i64 %3, i8* bitcast ({ i64, i64, i64*, i8*, i8*, i32, [2 x i16] }* @__profd_foo to i8*), i32 0) ; STATIC-EXT: call void @__llvm_profile_instrument_target(i64 %3, i8* bitcast ({ i64, i64, i64*, i8*, i8*, i32, [2 x i16] }* @__profd_foo to i8*), i32 zeroext 0) ; STATIC-SEXT: call void @__llvm_profile_instrument_target(i64 %3, i8* bitcast ({ i64, i64, i64*, i8*, i8*, i32, [2 x i16] }* @__profd_foo to i8*), i32 signext 0)