From eecb22d8e1fe5ac6cc35ace63ae517c33ff40d85 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Sun, 4 Dec 2022 15:06:34 -0800 Subject: [PATCH] [SanitizerBinaryMetadata] Use weak __start_/__stop_ instead of dummy empty section 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[1]. This additionally avoids size cost due to the dummy section and symbol. [1]: https://maskray.me/blog/2021-01-31-metadata-sections-comdat-and-shf-link-order Reviewed By: melver Differential Revision: https://reviews.llvm.org/D139276 --- clang/test/CodeGen/sanitize-metadata.c | 5 +++++ .../Instrumentation/SanitizerBinaryMetadata.cpp | 23 +++------------------- .../SanitizerBinaryMetadata/atomics.ll | 6 ++++++ 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/clang/test/CodeGen/sanitize-metadata.c b/clang/test/CodeGen/sanitize-metadata.c index 58d47ff..3ab0fbc 100644 --- a/clang/test/CodeGen/sanitize-metadata.c +++ b/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() { diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp index 58c29d8..169e876 100644 --- a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp +++ b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp @@ -147,10 +147,6 @@ private: // 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 @@ bool SanitizerBinaryMetadata::run() { } appendToGlobalCtors(Mod, Ctor, kCtorDtorPriority, CtorData); appendToGlobalDtors(Mod, Dtor, kCtorDtorPriority, DtorData); - createZeroSizedObjectInSection(Int8PtrTy, MI->SectionSuffix); } return true; @@ -285,27 +280,15 @@ bool SanitizerBinaryMetadata::runOn(Instruction &I, MetadataInfoSet &MIS, 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; diff --git a/llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll b/llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll index 06893b2..01e504d 100644 --- a/llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll +++ b/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 { -- 2.7.4