[SanitizerCoverage][NFC] Use appendToUsed instead of include

[metzman]

Summary

Use appendToUsed instead of include to ensure that
SanitizerCoverage's constructors are not stripped.

Also, use isOSBinFormatCOFF() to determine if target
binary format is COFF.

[metzman]

• fix comment

[metzman]

Peter could you please take a look.
This change uses appendToUsed instead of /include, for the constructors,. as you suggested.

[pcc]

Doesn't this need a test case update? (or a new test case, if this code wasn't being covered before)

[metzman]

In D56369#1348358, @pcc wrote:

Doesn't this need a test case update? (or a new test case, if this code wasn't being covered before)

There is a(n indirect) test for this that is general enough that it doesn't need an update.
In libFuzzer there is a test to ensure fuzz targets can be compiled with the "/OPT:REF" linker option.
If "/include" isn't emitted during IR generation this test fails, because the constructors get stripped.
The test still passes though, because as you correctly pointed out in your email to me, appendToUsed causes /include to be emitted.

If you would like I can try to add a more direct test to LLVM to ensure that /include is emitted during IR generation.

[metzman]

Ping

[pcc]

Can you add the test that /include is being emitted?

[pcc]

Or just test that the ctor appears in llvm.used

[metzman]

• Add test

[metzman]

• fix comment

[metzman]

In D56369#1351477, @pcc wrote:

Or just test that the ctor appears in llvm.used

Done.

[pcc]

LGTM

[sidneym]

Unrelated to this change, I made an error in my commit message for: r351356
referring to this review instead of https://reviews.llvm.org/D56726. Sorry for any confusion this may cause.

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

GlobalVariable *SecStart =
                          getSectionStart(Section));
   SecStart->setVisibility(GlobalValue::HiddenVisibility);
   GlobalVariable *SecEnd =
       new GlobalVariable(M, Ty, false, GlobalVariable::ExternalLinkage,
                          nullptr, getSectionEnd(Section));
   SecEnd->setVisibility(GlobalValue::HiddenVisibility);
   IRBuilder<> IRB(M.getContext());
   Value *SecEndPtr = IRB.CreatePointerCast(SecEnd, Ty);
-  if (TargetTriple.getObjectFormat() != Triple::COFF)
+  if (!TargetTriple.isOSBinFormatCOFF())
     return std::make_pair(IRB.CreatePointerCast(SecStart, Ty), SecEndPtr);
 
   // Account for the fact that on windows-msvc __start_* symbols actually
   // point to a uint64_t before the start of the array.
   auto SecStartI8Ptr = IRB.CreatePointerCast(SecStart, Int8PtrTy);
   auto GEP = IRB.CreateGEP(SecStartI8Ptr,
                            ConstantInt::get(IntptrTy, sizeof(uint64_t)));
   return std::make_pair(IRB.CreatePointerCast(GEP, Ty), SecEndPtr);
Function *SanitizerCoverageModule::CreateInitCallsForSections(
   if (TargetTriple.supportsCOMDAT()) {
     // Use comdat to dedup CtorFunc.
     CtorFunc->setComdat(M.getOrInsertComdat(SanCovModuleCtorName));
     appendToGlobalCtors(M, CtorFunc, SanCtorAndDtorPriority, CtorFunc);
   } else {
     appendToGlobalCtors(M, CtorFunc, SanCtorAndDtorPriority);
   }
 
-  if (TargetTriple.getObjectFormat() == Triple::COFF) {
+  if (TargetTriple.isOSBinFormatCOFF()) {
     // In COFF files, if the contructors are set as COMDAT (they are because
     // COFF supports COMDAT) and the linker flag /OPT:REF (strip unreferenced
     // functions and data) is used, the constructors get stripped. To prevent
-    // this, give the constructors weak ODR linkage and tell the linker to
-    // always include the sancov constructor. This way the linker can
-    // deduplicate the constructors but always leave one copy.
+    // this, give the constructors weak ODR linkage and ensure the linker knows
+    // to include the sancov constructor. This way the linker can deduplicate
+    // the constructors but always leave one copy.
     CtorFunc->setLinkage(GlobalValue::WeakODRLinkage);
-    SmallString<20> PartialIncDirective("/include:");
-    // Get constructor's mangled name in order to support i386.
-    SmallString<40> MangledName;
-    Mangler().getNameWithPrefix(MangledName, CtorFunc, true);
-    Twine IncDirective = PartialIncDirective + MangledName;
-    Metadata *Args[1] = {MDString::get(*C, IncDirective.str())};
-    MDNode *MetadataNode = MDNode::get(*C, Args);
-    NamedMDNode *NamedMetadata =
-        M.getOrInsertNamedMetadata("llvm.linker.options");
-    NamedMetadata->addOperand(MetadataNode);
+    appendToUsed(M, CtorFunc);
   }
   return CtorFunc;
 }
 
 bool SanitizerCoverageModule::runOnModule(Module &M) {
   if (Options.CoverageType == SanitizerCoverageOptions::SCK_None)
     return false;
   C = &(M.getContext());
if (Options.StackDepth && IsEntryBB && !IsLeafFunc) {
     auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
     SetNoSanitizeMetadata(LowestStack);
     SetNoSanitizeMetadata(Store);
   }
 }
 
 std::string
 SanitizerCoverageModule::getSectionName(const std::string &Section) const {
-  if (TargetTriple.getObjectFormat() == Triple::COFF) {
+  if (TargetTriple.isOSBinFormatCOFF()) {
     if (Section == SanCovCountersSectionName)
       return ".SCOV$CM";
     if (Section == SanCovPCsSectionName)
       return ".SCOVP$M";
     return ".SCOV$GM"; // For SanCovGuardsSectionName.
   }
   if (TargetTriple.isOSBinFormatMachO())
     return "__DATA,__" + Section;

llvm/trunk/test/Instrumentation/SanitizerCoverage/coff-used-ctor.ll

+; Checks that sancov.module_ctor is marked used.
+; RUN: opt < %s -sancov -sanitizer-coverage-level=1 -sanitizer-coverage-inline-8bit-counters=1 -sanitizer-coverage-pc-table=1 -S | FileCheck %s
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.14.26433"
+
+define void @foo() {
+entry:
+  ret void
+}
+
+; CHECK: @llvm.used = appending global {{.*}} @sancov.module_ctor
 No newline at end of file