[PGO] Don't value-instrument llvm.global_ctors and llvm.global_dtors functions

[xur]

Summary

The runtime of value profiling (__llvm_profile_instrument_target) has dynamic memory allocation (i.e. malloc). Malloc call can be overwritten by other memory allocator through llvm.global_ctors functions. Value-instrumenting these functions can result in a deadlock.

This patch disables the value-instrumentation (for indirect-call) of functions referenced in llvm.global.ctors and llvm.global_dtors.

[davidxl]

This does not work well for O0 compilation. __cxx_global_var_init also needs to be skipped, but it is not directly referenced by llvm.global_ctors. Probably just skip functions in startup section.

@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_t.cc, i8* null }]

Function Attrs: uwtable
define internal void @__cxx_global_var_init() #0 section ".text.startup" {

call void @_ZN1AC1Ev(%struct.A* @a)
ret void

}

; Function Attrs: uwtable
define internal void @_GLOBAL__sub_I_t.cc() #0 section ".text.startup" {

call void @__cxx_global_var_init()
ret void

}

[silvas]

In D19855#420069, @davidxl wrote:

This does not work well for O0 compilation. __cxx_global_var_init also needs to be skipped, but it is not directly referenced by llvm.global_ctors. Probably just skip functions in startup section.

I don't think that is a good solution. The name of that section is not always applied and varies from platform to platform even when it is applied (see getStaticInitSectionSpecifier in clang and its overrides; duplicating that information in this pass is undesirable).

Maybe we could do a CG walk from the llvm.global_[cd]tors functions or something? A naive walk is probably not sufficient (would find too many things). But maybe functions used transitively by llvm.global_[cd]tors and that is the only use?

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

 // used in populating profile counters.
 // The MST implementation is in Class CFGMST (CFGMST.h).
 //
 //===----------------------------------------------------------------------===//
 
 #include "CFGMST.h"
 #include "IndirectCallSiteVisitor.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/CFG.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/IRBuilder.h"
struct BBInfo {
   BBInfo(unsigned IX) : Group(this), Index(IX), Rank(0) {}
 
   // Return the information string of this object.
   const std::string infoString() const {
     return (Twine("Index=") + Twine(Index)).str();
   }
 };
 
+// The helper class checks if a particular function is a global_ctor or
+// global_dtor.
+class ModuleCtorDtorFuncs {
+public:
+  ModuleCtorDtorFuncs(const Module &M) : M(M) {}
+  // Return true if F is a function referenced in llvm.global_ctors
+  // or llvm.global_dtors.
+  bool isCtorDtor(Function *F) const { return CtorDtorFuncs.count(F) == 1; }
+  // Collect the functions referenced in llvm.global_ctors and
+  // llvm.global_dtors and put them in a set.
+  void collect() {
+    collectCtorDtorFuncs("llvm.global_ctors");
+    collectCtorDtorFuncs("llvm.global_dtors");
+  }
+
+private:
+  const Module &M;
+  void collectCtorDtorFuncs(const char *GVName);
+  SmallPtrSet<Function *, 4> CtorDtorFuncs;
+};
+
+// GVName can be either "llvm.global_ctors" and "llvm.global_dtors".
+// This function Parses the array in these globals, extracts the function decls,
+// and puts them to a set.
+void ModuleCtorDtorFuncs::collectCtorDtorFuncs(const char *GVName) {
+  GlobalVariable *GV = M.getGlobalVariable(GVName);
+  if (!GV)
+    return;
+  ConstantArray *InitList = cast<ConstantArray>(GV->getInitializer());
+  if (!InitList)
+    return;
+  for (Value *OI : InitList->operands()) {
+    ConstantStruct *CS = dyn_cast<ConstantStruct>(OI);
+    if (!CS)
+      continue;
+    // Found a null terminator, skip the rest.
+    if (CS->getOperand(1)->isNullValue())
+      break;
+    Function *F = dyn_cast<Function>(CS->getOperand(1));
+    if (F)
+      CtorDtorFuncs.insert(F);
+  }
+}
+
 // This class implements the CFG edges. Note the CFG can be a multi-graph.
 template <class Edge, class BBInfo> class FuncPGOInstrumentation {
 private:
   Function &F;
   void computeCFGHash();
 
 public:
   std::string FuncName;
BasicBlock *FuncPGOInstrumentation<Edge, BBInfo>::getInstrBB(Edge *E) {
   E->Removed = true;
   return InstrBB;
 }
 
 // Visit all edge and instrument the edges not in MST, and do value profiling.
 // Critical edges will be split.
 static void instrumentOneFunc(Function &F, Module *M,
                               BranchProbabilityInfo *BPI,
-                              BlockFrequencyInfo *BFI) {
+                              BlockFrequencyInfo *BFI,
+                              const ModuleCtorDtorFuncs &CtorDtorFuncs) {
   unsigned NumCounters = 0;
   FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(F, true, BPI, BFI);
   for (auto &E : FuncInfo.MST.AllEdges) {
     if (!E->InMST && !E->Removed)
       NumCounters++;
   }
 
   uint32_t I = 0;
Builder.CreateCall(
         Intrinsic::getDeclaration(M, Intrinsic::instrprof_increment),
         {llvm::ConstantExpr::getBitCast(FuncInfo.FuncNameVar, I8PtrTy),
          Builder.getInt64(FuncInfo.FunctionHash), Builder.getInt32(NumCounters),
          Builder.getInt32(I++)});
   }
 
   if (DisableValueProfiling)
     return;
+  if (CtorDtorFuncs.isCtorDtor(&F))
+    return;
 
   unsigned NumIndirectCallSites = 0;
   for (auto &I : findIndirectCallSites(F)) {
     CallSite CS(I);
     Value *Callee = CS.getCalledValue();
     DEBUG(dbgs() << "Instrument one indirect call: CallSite Index = "
                  << NumIndirectCallSites << "\n");
     IRBuilder<> Builder(I);
if (E->Removed)
       continue;
     Total += E->CountValue;
   }
   return Total;
 }
 
 class PGOUseFunc {
 public:
-  PGOUseFunc(Function &Func, Module *Modu, BranchProbabilityInfo *BPI = nullptr,
+  PGOUseFunc(Function &Func, Module *Modu,
+             const ModuleCtorDtorFuncs &CtorDtorFuncs,
+             BranchProbabilityInfo *BPI = nullptr,
              BlockFrequencyInfo *BFI = nullptr)
-      : F(Func), M(Modu), FuncInfo(Func, false, BPI, BFI),
-        FreqAttr(FFA_Normal) {}
+      : F(Func), M(Modu), CtorDtorFuncs(CtorDtorFuncs),
+        FuncInfo(Func, false, BPI, BFI), FreqAttr(FFA_Normal) {}
 
   // Read counts for the instrumented BB from profile.
   bool readCounters(IndexedInstrProfReader *PGOReader);
 
   // Populate the counts for all BBs.
   void populateCounters();
 
   // Set the branch weights based on the count values.
   void setBranchWeights();
 
   // Annotate the indirect call sites.
   void annotateIndirectCallSites();
 
   // The hotness of the function from the profile count.
   enum FuncFreqAttr { FFA_Normal, FFA_Cold, FFA_Hot };
 
   // Return the function hotness from the profile.
   FuncFreqAttr getFuncFreqAttr() const { return FreqAttr; }
 
 private:
   Function &F;
   Module *M;
+  const ModuleCtorDtorFuncs &CtorDtorFuncs;
   // This member stores the shared information with class PGOGenFunc.
   FuncPGOInstrumentation<PGOUseEdge, UseBBInfo> FuncInfo;
 
   // Return the auxiliary BB information.
   UseBBInfo &getBBInfo(const BasicBlock *BB) const {
     return FuncInfo.getBBInfo(BB);
   }
 
DEBUG(dbgs() << "Weight is: ";
           dbgs() << "\n";);
   }
 }
 
 // Traverse all the indirect callsites and annotate the instructions.
 void PGOUseFunc::annotateIndirectCallSites() {
   if (DisableValueProfiling)
     return;
-
+  if (CtorDtorFuncs.isCtorDtor(&F))
+    return;
   // Create the PGOFuncName meta data.
   createPGOFuncNameMetadata(F, FuncInfo.FuncName);
 
   unsigned IndirectCallSiteIndex = 0;
   auto IndirectCallSites = findIndirectCallSites(F);
   unsigned NumValueSites =
       ProfileRecord.getNumValueSites(IPVK_IndirectCallTarget);
   if (NumValueSites != IndirectCallSites.size()) {
else
     IRLevelVersionVariable->setComdat(M.getOrInsertComdat(
         StringRef(INSTR_PROF_QUOTE(IR_LEVEL_PROF_VERSION_VAR))));
 }
 
 static bool InstrumentAllFunctions(
     Module &M, function_ref<BranchProbabilityInfo &(Function &)> LookupBPI,
     function_ref<BlockFrequencyInfo &(Function &)> LookupBFI) {
   createIRLevelProfileFlagVariable(M);
+  ModuleCtorDtorFuncs CtorDtorFuncs(M);
+  if (!DisableValueProfiling)
+    CtorDtorFuncs.collect();
   for (auto &F : M) {
     if (F.isDeclaration())
       continue;
     auto &BPI = LookupBPI(F);
     auto &BFI = LookupBFI(F);
-    instrumentOneFunc(F, &M, &BPI, &BFI);
+    instrumentOneFunc(F, &M, &BPI, &BFI, CtorDtorFuncs);
   }
   return true;
 }
 
 bool PGOInstrumentationGen::runOnModule(Module &M) {
   if (skipModule(M))
     return false;
 
bool PGOInstrumentationUse::runOnModule(Module &M) {
   }
   // TODO: might need to change the warning once the clang option is finalized.
   if (!PGOReader->isIRLevelProfile()) {
     Ctx.diagnose(DiagnosticInfoPGOProfile(
         ProfileFileName.data(), "Not an IR level instrumentation profile"));
     return false;
   }
 
+  ModuleCtorDtorFuncs CtorDtorFuncs(M);
+  if (!DisableValueProfiling)
+    CtorDtorFuncs.collect();
   std::vector<Function *> HotFunctions;
   std::vector<Function *> ColdFunctions;
   for (auto &F : M) {
     if (F.isDeclaration())
       continue;
     BranchProbabilityInfo *BPI =
         &(getAnalysis<BranchProbabilityInfoWrapperPass>(F).getBPI());
     BlockFrequencyInfo *BFI =
         &(getAnalysis<BlockFrequencyInfoWrapperPass>(F).getBFI());
-    PGOUseFunc Func(F, &M, BPI, BFI);
+    PGOUseFunc Func(F, &M, CtorDtorFuncs, BPI, BFI);
     setPGOCountOnFunc(Func, PGOReader.get());
     PGOUseFunc::FuncFreqAttr FreqAttr = Func.getFuncFreqAttr();
     if (FreqAttr == PGOUseFunc::FFA_Cold)
       ColdFunctions.push_back(&F);
     else if (FreqAttr == PGOUseFunc::FFA_Hot)
       HotFunctions.push_back(&F);
   }
 

test/Transforms/PGOProfile/global_ctor.ll

+; RUN: opt < %s -pgo-instr-gen -S | FileCheck %s --check-prefix=GEN
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = common global i32 (...)* null, align 8
+@bar = common global i32 (...)* null, align 8
+
+define void @c_global_ctor() {
+entry:
+; GEN: call void @llvm.instrprof.increment
+; GEN-NOT: call void @llvm.instrprof.value.profile
+  %tmp = load i32 (...)*, i32 (...)** @foo, align 8
+  %call = call i32 (...) %tmp()
+  ret void
+}
+
+define void @d_global_ctor() {
+entry:
+; GEN: call void @llvm.instrprof.increment
+; GEN-NOT: call void @llvm.instrprof.value.profile
+  %tmp = load i32 (...)*, i32 (...)** @bar, align 8
+  %call = call i32 (...) %tmp()
+  ret void
+}
+
[e-mail]_ctors = appending global [2 x { i32, void ()*, i8* } ] [
+  { i32, void ()*, i8* } { i32 65535, void ()* @c_global_ctor, i8* null },
+  { i32, void ()*, i8* } { i32 65535, void ()* @d_global_ctor, i8* null }
+]