[SanitizerCoverage] Clarify llvm.used/llvm.compiler.used and partially fix unmatched metadata sections on Windows

[MaskRay]

Summary

__sancov_pcs parallels the other metadata section(s). While some optimizers
(e.g. GlobalDCE) respect linker semantics for comdat and retain or discard the
sections as a unit, some (e.g. GlobalOpt/ConstantMerge) do not. So we have to
conservatively retain all unconditionally in the compiler.

When a comdat is used, the COFF/ELF linkers' GC semantics ensure the
associated parallel array elements are retained or discarded together,
so llvm.compiler.used is sufficient.

Otherwise (MachO (see rL311955/rL311959), COFF special case where comdat is not
used), we have to use llvm.used to conservatively make all sections retain by
the linker. This will fix the Windows problem once internal linkage GlobalObject's in llvm.used are retained via /INCLUDE:.

[morehouse]

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

684

Don't we still need compiler used? https://llvm.org/docs/LangRef.html#associated-metadata

Also even without this change they are only marked "used" on Mac currently, IIRC due to GC issues with their linker.

696

What happened to the associated metadata that's added here at ToT?

[MaskRay]

Fix a bug on Windows

[MaskRay]

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

684

Yes, I realized that llvm.compiler.used is needed to prevent agggressive optimizations from GlobalOpt/ConstantMerge.

I clarified why llvm.used is used - and use it on a case which may be an existing bug on Wondows.

696

I think !associated cannot be used when inlining is possible. See D97430.

This means even interposable linkages (which are normally not inlinable) can be inlined with LTO, so !associated cannot be used at all.

[morehouse]

LGTM

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

bool ModuleSanitizerCoverage::instrumentModule(
   }
   if (Ctor && Options.PCTable) {
     auto SecStartEnd = CreateSecStartEnd(M, SanCovPCsSectionName, IntptrPtrTy);
     FunctionCallee InitFunction = declareSanitizerInitFunction(
         M, SanCovPCsInitName, {IntptrPtrTy, IntptrPtrTy});
     IRBuilder<> IRBCtor(Ctor->getEntryBlock().getTerminator());
     IRBCtor.CreateCall(InitFunction, {SecStartEnd.first, SecStartEnd.second});
   }
-  // We don't reference these arrays directly in any of our runtime functions,
-  // so we need to prevent them from being dead stripped.
-  if (TargetTriple.isOSBinFormatMachO())
-    appendToUsed(M, GlobalsToAppendToUsed);
   appendToCompilerUsed(M, GlobalsToAppendToCompilerUsed);
   return true;
 }
 
 // True if block has successors and it dominates all of them.
 static bool isFullDominator(const BasicBlock *BB, const DominatorTree *DT) {
   if (succ_empty(BB))
     return false;
auto Array = new GlobalVariable(
       Constant::getNullValue(ArrayTy), "__sancov_gen_");
 
   if (TargetTriple.supportsCOMDAT() &&
       (TargetTriple.isOSBinFormatELF() || !F.isInterposable()))
     if (auto Comdat = getOrCreateFunctionComdat(F, TargetTriple))
       Array->setComdat(Comdat);
   Array->setSection(getSectionName(Section));
   Array->setAlignment(Align(DL->getTypeStoreSize(Ty).getFixedSize()));
-  GlobalsToAppendToUsed.push_back(Array);
+
+  // sancov_pcs parallels the other metadata section(s). Optimizers (e.g.
+  // GlobalOpt/ConstantMerge) may not discard sancov_pcs and the other
+  // section(s) as a unit, so we conservatively retain all unconditionally in
+  // the compiler.
+  //
+  // With comdat (COFF/ELF), the linker can guarantee the associated sections
+  // will be retained or discarded as a unit, so llvm.compiler.used is
+  // sufficient. Otherwise, conservatively make all of them retained by the
+  // linker.
+  if (Array->hasComdat())
     GlobalsToAppendToCompilerUsed.push_back(Array);
+  else
+    GlobalsToAppendToUsed.push_back(Array);
 
   return Array;
 }
 
 GlobalVariable *
 ModuleSanitizerCoverage::CreatePCArray(Function &F,
                                        ArrayRef<BasicBlock *> AllBlocks) {
   size_t N = AllBlocks.size();
   assert(N);

llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll

-; RUN: opt < %s -sancov -sanitizer-coverage-level=4 -sanitizer-coverage-trace-pc-guard -mtriple=x86_64 -S -enable-new-pm=0 | FileCheck %s --check-prefixes=CHECK,ELF
-; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=4 -sanitizer-coverage-trace-pc-guard -mtriple=x86_64 -S | FileCheck %s --check-prefixes=CHECK,ELF
+; RUN: opt < %s -sancov -sanitizer-coverage-level=4 -sanitizer-coverage-trace-pc-guard -mtriple=x86_64 -S -enable-new-pm=0 | FileCheck %s --check-prefixes=CHECK,COMDAT,ELF
+; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=4 -sanitizer-coverage-trace-pc-guard -mtriple=x86_64 -S | FileCheck %s --check-prefixes=CHECK,COMDAT,ELF
 
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=4 -sanitizer-coverage-trace-pc-guard -mtriple=aarch64-apple-darwin -S -enable-new-pm=0 | FileCheck %s --check-prefixes=CHECK,MACHO
 ; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=4 -sanitizer-coverage-trace-pc-guard -mtriple=aarch64-apple-darwin -S | FileCheck %s --check-prefixes=CHECK,MACHO
 
-; ELF:        $foo = comdat noduplicates
-; ELF:        $CallViaVptr = comdat noduplicates
+; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=4 -sanitizer-coverage-trace-pc-guard -mtriple=x86_64-windows -S | FileCheck %s --check-prefixes=CHECK,COMDAT,WIN
+
+; COMDAT:     $foo = comdat noduplicates
+; COMDAT:     $CallViaVptr = comdat noduplicates
+
 ; ELF:        @__sancov_gen_ = private global [3 x i32] zeroinitializer, section "__sancov_guards", comdat($foo), align 4{{$}}
 ; ELF-NEXT:   @__sancov_gen_.1 = private global [1 x i32] zeroinitializer, section "__sancov_guards", comdat($CallViaVptr), align 4{{$}}
 
 ; MACHO:      @__sancov_gen_ = private global [3 x i32] zeroinitializer, section "__DATA,__sancov_guards", align 4{{$}}
 ; MACHO-NEXT: @__sancov_gen_.1 = private global [1 x i32] zeroinitializer, section "__DATA,__sancov_guards", align 4{{$}}
 
+; WIN:        @__sancov_gen_ = private global [3 x i32] zeroinitializer, section ".SCOV$GM", comdat($foo), align 4{{$}}
+; WIN-NEXT:   @__sancov_gen_.1 = private global [1 x i32] zeroinitializer, section ".SCOV$GM", comdat($CallViaVptr), align 4{{$}}
+
 ; ELF-NOT:    @llvm.used =
+; ELF:        @llvm.compiler.used = appending global [2 x i8*] [i8* bitcast ([3 x i32]* @__sancov_gen_ to i8*), i8* bitcast ([1 x i32]* @__sancov_gen_.1 to i8*)], section "llvm.metadata"
 ; MACHO:      @llvm.used = appending global [2 x i8*] [i8* bitcast ([3 x i32]* @__sancov_gen_ to i8*), i8* bitcast ([1 x i32]* @__sancov_gen_.1 to i8*)], section "llvm.metadata"
-; CHECK:      @llvm.compiler.used = appending global [2 x i8*] [i8* bitcast ([3 x i32]* @__sancov_gen_ to i8*), i8* bitcast ([1 x i32]* @__sancov_gen_.1 to i8*)], section "llvm.metadata"
+; MACHO-NOT:  @llvm.compiler.used =
+; WIN:        @llvm.used = appending global [1 x i8*] [i8* bitcast (void ()* @sancov.module_ctor_trace_pc_guard to i8*)], section "llvm.metadata"
+; WIN-NEXT:   @llvm.compiler.used = appending global [2 x i8*] [i8* bitcast ([3 x i32]* @__sancov_gen_ to i8*), i8* bitcast ([1 x i32]* @__sancov_gen_.1 to i8*)], section "llvm.metadata"
 
 ; CHECK-LABEL: define void @foo
 ; CHECK:         call void @__sanitizer_cov_trace_pc
 ; CHECK:         ret void
 
 define void @foo(i32* %a) sanitize_address {
 entry:
   %tobool = icmp eq i32* %a, null