SanitizerCoverage: Use `createSanitizerCtor` to create ctor and call init

[samsonov]

lib/Transforms/Instrumentation/SanitizerCoverage.cpp

163 ↗

(On Diff #23072)

Isn't this function supposed to be created in createSanitizerCtor?

[ismailp]

lib/Transforms/Instrumentation/SanitizerCoverage.cpp

163 ↗

(On Diff #23072)

Good catch, thanks! I will include it in the next revision. It needs a test to check init function is called from module ctor, as well as renaming createSanitizerCtor below.

[ismailp]

• createSanitizerCtor renamed to createSanitizerCtorAndInitFunctions to match with changes in D8777
• SanCovModuleInit was initialized with checkInterfaceFunction by mistake; this is already done by createSanitizerCtorAndInitFunctions

Turns out, there is already a test checking module ctor, and call to init function, in test/Instrumentation/SanitizerCoverage/coverage.ll.

[samsonov]

LGTM

lib/Transforms/Instrumentation/SanitizerCoverage.cpp

271 ↗

(On Diff #25227)

No need to initialize this variable, as it should be initialized by subsequent call.

272 ↗

(On Diff #25227)

Note that SanCovModuleInit is not used anywhere, maybe you can also turn it into a local variable, and delete the class member?

[samsonov]

lib/Transforms/Instrumentation/SanitizerCoverage.cpp

272 ↗

(On Diff #25227)

(or just use std::ignore)

[ismailp]

Thank you for your time, and feedback.

It seems like we can remove SanCovModuleInit.

llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

private:
   void SetNoSanitizeMetadata(Instruction *I);
   void InjectCoverageAtBlock(Function &F, BasicBlock &BB, bool UseCalls);
   unsigned NumberOfInstrumentedBlocks() {
     return SanCovFunction->getNumUses() + SanCovWithCheckFunction->getNumUses();
   }
   Function *SanCovFunction;
   Function *SanCovWithCheckFunction;
   Function *SanCovIndirCallFunction;
-  Function *SanCovModuleInit;
   Function *SanCovTraceEnter, *SanCovTraceBB;
   Function *SanCovTraceCmpFunction;
   InlineAsm *EmptyAsm;
   Type *IntptrTy, *Int64Ty;
   LLVMContext *C;
   const DataLayout *DL;
 
   GlobalVariable *GuardArray;
bool SanitizerCoverageModule::runOnModule(Module &M) {
   DL = &M.getDataLayout();
   IntptrTy = Type::getIntNTy(*C, DL->getPointerSizeInBits());
   Type *VoidTy = Type::getVoidTy(*C);
   IRBuilder<> IRB(*C);
   Type *Int8PtrTy = PointerType::getUnqual(IRB.getInt8Ty());
   Type *Int32PtrTy = PointerType::getUnqual(IRB.getInt32Ty());
   Int64Ty = IRB.getInt64Ty();
 
-  Function *CtorFunc =
-      Function::Create(FunctionType::get(VoidTy, false),
-                       GlobalValue::InternalLinkage, kSanCovModuleCtorName, &M);
-  ReturnInst::Create(*C, BasicBlock::Create(*C, "", CtorFunc));
-  appendToGlobalCtors(M, CtorFunc, kSanCtorAndDtorPriority);
-
   SanCovFunction = checkSanitizerInterfaceFunction(
       M.getOrInsertFunction(kSanCovName, VoidTy, Int32PtrTy, nullptr));
   SanCovWithCheckFunction = checkSanitizerInterfaceFunction(
       M.getOrInsertFunction(kSanCovWithCheckName, VoidTy, Int32PtrTy, nullptr));
   SanCovIndirCallFunction =
       checkSanitizerInterfaceFunction(M.getOrInsertFunction(
           kSanCovIndirCallName, VoidTy, IntptrTy, IntptrTy, nullptr));
   SanCovTraceCmpFunction =
       checkSanitizerInterfaceFunction(M.getOrInsertFunction(
           kSanCovTraceCmp, VoidTy, Int64Ty, Int64Ty, Int64Ty, nullptr));
 
-  SanCovModuleInit = checkSanitizerInterfaceFunction(M.getOrInsertFunction(
-      kSanCovModuleInitName, VoidTy, Int32PtrTy, IntptrTy,
-      Int8PtrTy, Int8PtrTy, nullptr));
-  SanCovModuleInit->setLinkage(Function::ExternalLinkage);
   // We insert an empty inline asm after cov callbacks to avoid callback merge.
   EmptyAsm = InlineAsm::get(FunctionType::get(IRB.getVoidTy(), false),
                             StringRef(""), StringRef(""),
                             /*hasSideEffects=*/true);
 
   if (Options.TraceBB) {
     SanCovTraceEnter = checkSanitizerInterfaceFunction(
         M.getOrInsertFunction(kSanCovTraceEnter, VoidTy, Int32PtrTy, nullptr));
bool SanitizerCoverageModule::runOnModule(Module &M) {
 
   // Create variable for module (compilation unit) name
   Constant *ModNameStrConst =
       ConstantDataArray::getString(M.getContext(), M.getName(), true);
   GlobalVariable *ModuleName =
       new GlobalVariable(M, ModNameStrConst->getType(), true,
                          GlobalValue::PrivateLinkage, ModNameStrConst);
 
-  // Call __sanitizer_cov_module_init
-  IRB.SetInsertPoint(CtorFunc->getEntryBlock().getTerminator());
-  IRB.CreateCall4(
-      SanCovModuleInit, IRB.CreatePointerCast(RealGuardArray, Int32PtrTy),
+  ArrayRef<Value *> InitArgs = {
+      IRB.CreatePointerCast(RealGuardArray, Int32PtrTy),
       ConstantInt::get(IntptrTy, N),
       Options.Use8bitCounters
           ? IRB.CreatePointerCast(RealEightBitCounterArray, Int8PtrTy)
           : Constant::getNullValue(Int8PtrTy),
-      IRB.CreatePointerCast(ModuleName, Int8PtrTy));
+      IRB.CreatePointerCast(ModuleName, Int8PtrTy)};
+
+  Function *CtorFunc;
+  std::tie(CtorFunc, std::ignore) = createSanitizerCtorAndInitFunctions(
+      M, kSanCovModuleCtorName, kSanCovModuleInitName,
+      {Int32PtrTy, IntptrTy, Int8PtrTy, Int8PtrTy}, InitArgs);
+
+  appendToGlobalCtors(M, CtorFunc, kSanCtorAndDtorPriority);
+
   return true;
 }
 
 bool SanitizerCoverageModule::runOnFunction(Function &F) {
   if (F.empty()) return false;
   if (F.getName().find(".module_ctor") != std::string::npos)
     return false;  // Should not instrument sanitizer init functions.
   if (Options.CoverageType >= SanitizerCoverageOptions::SCK_Edge)