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

[samsonov]

lib/Transforms/Instrumentation/SanitizerCoverage.cpp

197–198

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

[ismailp]

lib/Transforms/Instrumentation/SanitizerCoverage.cpp

197–198

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

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

272

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

(or just use std::ignore)

[ismailp]

Thank you for your time, and feedback.

It seems like we can remove SanCovModuleInit.

lib/Transforms/Instrumentation/SanitizerCoverage.cpp

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));
     SanCovTraceBB = checkSanitizerInterfaceFunction(
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 = nullptr;
+  std::tie(CtorFunc, SanCovModuleInit) = 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)