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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: melver, dvyukov.
Herald added subscribers: Enna1, StephenFan, hiraditya.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

D130887 <https://reviews.llvm.org/D130887> uses a dummy empty section `sanmd_covered` (with the SHF_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 <https://reviews.llvm.org/D98903> for SanitizerCoverage, let's make encapsulation symbols
undefined weak instead. This additionally avoids size cost due to the dummy
section and symbol.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139276

Files:
  clang/test/CodeGen/sanitize-metadata.c
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
  llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll


Index: llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll
===================================================================
--- llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll
+++ llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll
@@ -1,6 +1,12 @@
 ; 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 {
Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -147,10 +147,6 @@
   // 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);
 
@@ -213,7 +209,6 @@
     }
     appendToGlobalCtors(Mod, Ctor, kCtorDtorPriority, CtorData);
     appendToGlobalDtors(Mod, Dtor, kCtorDtorPriority, DtorData);
-    createZeroSizedObjectInSection(Int8PtrTy, MI->SectionSuffix);
   }
 
   return true;
@@ -285,27 +280,15 @@
 
 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;
Index: clang/test/CodeGen/sanitize-metadata.c
===================================================================
--- clang/test/CodeGen/sanitize-metadata.c
+++ clang/test/CodeGen/sanitize-metadata.c
@@ -1,6 +1,11 @@
 // 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() {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139276.479931.patch
Type: text/x-patch
Size: 4280 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221204/ca736593/attachment.bin>