[SanitizerBinaryMetadata] Use weak __start_/__stop_ instead of dummy empty section

[MaskRay]

Summary

D130887 uses a dummy empty section sanmd_covered (with the SHF_GNU_RETAIN flag on
ELF) to prevent undefined symbol: __start_sanmd_covered if all sanmd_covered
are discarded by ld --gc-sections (in -z start-stop-gc mode).

The dummy sanmd_covered does not have the SHF_LINK_ORDER flag, so mixing it
with SHF_LINK_ORDER sanmd_covered causes an issue to GNU ld<2.36
(https://sourceware.org/bugzilla/show_bug.cgi?id=26256).

Similar to D98903 for SanitizerCoverage, let's make encapsulation symbols
undefined weak instead. This additionally avoids size cost due to the dummy
section and symbol.

[melver]

Thanks - yes this looks better.

clang/test/CodeGen/sanitize-metadata.c

 // RUN: %clang_cc1 -O -fexperimental-sanitize-metadata=atomics -triple x86_64-gnu-linux -x c -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,ATOMICS
 // RUN: %clang_cc1 -O -fexperimental-sanitize-metadata=atomics -triple aarch64-gnu-linux -x c -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,ATOMICS
 
+// CHECK: @__start_sanmd_atomics = extern_weak hidden global ptr
+// CHECK: @__stop_sanmd_atomics = extern_weak hidden global ptr
+// CHECK: @__start_sanmd_covered = extern_weak hidden global ptr
+// CHECK: @__stop_sanmd_covered = extern_weak hidden global ptr
+
 int x, y;
 
 void empty() {
 // CHECK-NOT: define dso_local void @empty() {{.*}} !pcsections
 }
 
 int atomics() {
 // ATOMICS-LABEL: define dso_local i32 @atomics()

llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

private:
   // function with memory operations (atomic or not) requires covered metadata
   // to determine if a memory operation is atomic or not in modules compiled
   // with SanitizerBinaryMetadata.
   bool runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB);
 
   // Get start/end section marker pointer.
   GlobalVariable *getSectionMarker(const Twine &MarkerName, Type *Ty);
 
-  // Create a 0-sized object in a section, so that the section is not discarded
-  // if all inputs have been discarded.
-  void createZeroSizedObjectInSection(Type *Ty, StringRef SectionSuffix);
-
   // Returns the target-dependent section name.
   StringRef getSectionName(StringRef SectionSuffix);
 
   // Returns the section start marker name.
   Twine getSectionStart(StringRef SectionSuffix);
 
   // Returns the section end marker name.
   Twine getSectionEnd(StringRef SectionSuffix);
if (TargetTriple.supportsCOMDAT()) {
       // Use COMDAT to deduplicate constructor/destructor function.
       Ctor->setComdat(Mod.getOrInsertComdat(Ctor->getName()));
       Dtor->setComdat(Mod.getOrInsertComdat(Dtor->getName()));
       CtorData = Ctor;
       DtorData = Dtor;
     }
     appendToGlobalCtors(Mod, Ctor, kCtorDtorPriority, CtorData);
     appendToGlobalDtors(Mod, Dtor, kCtorDtorPriority, DtorData);
-    createZeroSizedObjectInSection(Int8PtrTy, MI->SectionSuffix);
   }
 
   return true;
 }
 
 void SanitizerBinaryMetadata::runOn(Function &F, MetadataInfoSet &MIS) {
   if (F.empty())
     return;
if (!InstMetadata.empty()) {
     I.setMetadata(LLVMContext::MD_pcsections, MDB.createPCSections(Sections));
   }
 
   return RequiresCovered;
 }
 
 GlobalVariable *
 SanitizerBinaryMetadata::getSectionMarker(const Twine &MarkerName, Type *Ty) {
+  // Use ExternalWeak so that if all sections are discarded due to section
+  // garbage collection, the linker will not report undefined symbol errors.
   auto *Marker = new GlobalVariable(Mod, Ty, /*isConstant=*/false,
-                                    GlobalVariable::ExternalLinkage,
+                                    GlobalVariable::ExternalWeakLinkage,
                                     /*Initializer=*/nullptr, MarkerName);
   Marker->setVisibility(GlobalValue::HiddenVisibility);
   return Marker;
 }
 
-void SanitizerBinaryMetadata::createZeroSizedObjectInSection(
-    Type *Ty, StringRef SectionSuffix) {
-  auto *DummyInit = ConstantAggregateZero::get(ArrayType::get(Ty, 0));
-  auto *DummyEntry = new GlobalVariable(Mod, DummyInit->getType(), true,
-                                        GlobalVariable::ExternalLinkage,
-                                        DummyInit, "__dummy_" + SectionSuffix);
-  DummyEntry->setSection(getSectionName(SectionSuffix));
-  DummyEntry->setVisibility(GlobalValue::HiddenVisibility);
-  if (TargetTriple.supportsCOMDAT())
-    DummyEntry->setComdat(Mod.getOrInsertComdat(DummyEntry->getName()));
-  // Make sure the section isn't discarded by gc-sections.
-  appendToUsed(Mod, DummyEntry);
-}
-
 StringRef SanitizerBinaryMetadata::getSectionName(StringRef SectionSuffix) {
   // FIXME: Other TargetTriple (req. string pool)
   return SectionSuffix;
 }
 
 Twine SanitizerBinaryMetadata::getSectionStart(StringRef SectionSuffix) {
   return "__start_" + SectionSuffix;
 }

llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll

 ; RUN: opt < %s -passes='module(sanmd-module)' -sanitizer-metadata-atomics -S | FileCheck %s
 
 ; Check that atomic memory operations receive PC sections metadata.
+
+; CHECK: @__start_sanmd_atomics = extern_weak hidden global ptr
+; CHECK: @__stop_sanmd_atomics = extern_weak hidden global ptr
+; CHECK: @__start_sanmd_covered = extern_weak hidden global ptr
+; CHECK: @__stop_sanmd_covered = extern_weak hidden global ptr
+
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 define i8 @atomic8_load_unordered(ptr %a) nounwind uwtable {
 entry:
   %0 = load atomic i8, ptr %a unordered, align 1
   ret i8 %0
 }
 ; CHECK-LABEL: atomic8_load_unordered{{.*}}!pcsections !0