[llvm] 4d63892 - [SanitizerCoverage] Drop !associated on metadata sections
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 25 11:59:29 PST 2021
Author: Fangrui Song
Date: 2021-02-25T11:59:23-08:00
New Revision: 4d63892acb17ba8ee5b146e83d38f244d8d92222
URL: https://github.com/llvm/llvm-project/commit/4d63892acb17ba8ee5b146e83d38f244d8d92222
DIFF: https://github.com/llvm/llvm-project/commit/4d63892acb17ba8ee5b146e83d38f244d8d92222.diff
LOG: [SanitizerCoverage] Drop !associated on metadata sections
In SanitizerCoverage, the metadata sections (`__sancov_guards`,
`__sancov_cntrs`, `__sancov_bools`) are referenced by functions. After
inlining, such a `__sancov_*` section can be referenced by more than one
functions, but its sh_link still refers to the original function's section.
(Note: a SHF_LINK_ORDER section referenced by a section other than its linked-to
section violates the invariant.)
If the original function's section is discarded (e.g. LTO internalization +
`ld.lld --gc-sections`), ld.lld may report a `sh_link points to discarded section` error.
This above reasoning means that `!associated` is not appropriate to be called by
an inlinable function. Non-interposable functions are inline candidates, so we
have to drop `!associated`. A `__sancov_pcs` is not referenced by other sections
but is expected to parallel a metadata section, so we have to make sure the two
sections are retained or discarded at the same time. A section group does the
trick. (Note: we have a module ctor, so `getUniqueModuleId` guarantees to
return a non-empty string, and `GetOrCreateFunctionComdat` guarantees to return
non-null.)
For interposable functions, we could keep using `!associated`, but
LTO can change the linkage to `internal` and allow such functions to be inlinable,
so we have to drop `!associated`, too. To not interfere with section
group resolution, we need to use the `noduplicates` variant (section group flag 0).
(This allows us to get rid of the ModuleID parameter.)
In -fno-pie and -fpie code (mostly dso_local), instrumented interposable
functions have WeakAny/LinkOnceAny linkages, which are rare. So the
section group header overload should be low.
This patch does not change the object file output for COFF (where `!associated` is ignored).
Reviewed By: morehouse, rnk, vitalybuka
Differential Revision: https://reviews.llvm.org/D97430
Added:
llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol.ll
Modified:
llvm/include/llvm/Transforms/Instrumentation.h
llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll
Removed:
llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol-nocomdat.ll
################################################################################
diff --git a/llvm/include/llvm/Transforms/Instrumentation.h b/llvm/include/llvm/Transforms/Instrumentation.h
index c960d5b0ab50..03108bacb0da 100644
--- a/llvm/include/llvm/Transforms/Instrumentation.h
+++ b/llvm/include/llvm/Transforms/Instrumentation.h
@@ -46,8 +46,7 @@ GlobalVariable *createPrivateGlobalForString(Module &M, StringRef Str,
// Returns F.getComdat() if it exists.
// Otherwise creates a new comdat, sets F's comdat, and returns it.
// Returns nullptr on failure.
-Comdat *GetOrCreateFunctionComdat(Function &F, Triple &T,
- const std::string &ModuleId);
+Comdat *getOrCreateFunctionComdat(Function &F, Triple &T);
// Insert GCOV profiling instrumentation
struct GCOVOptions {
diff --git a/llvm/lib/Transforms/IPO/SampleProfileProbe.cpp b/llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
index a885c3ee4ded..d878d44bf8e3 100644
--- a/llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
@@ -364,9 +364,8 @@ void SampleProfileProber::instrumentOneFunc(Function &F, TargetMachine *TM) {
if (!F.isDeclarationForLinker()) {
if (TM) {
auto Triple = TM->getTargetTriple();
- if (Triple.supportsCOMDAT() && TM->getFunctionSections()) {
- GetOrCreateFunctionComdat(F, Triple, CurModuleUniqueId);
- }
+ if (Triple.supportsCOMDAT() && TM->getFunctionSections())
+ getOrCreateFunctionComdat(F, Triple);
}
}
}
diff --git a/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp b/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
index cfdf3cad97f7..1fe1e3d1abee 100644
--- a/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
@@ -73,28 +73,16 @@ GlobalVariable *llvm::createPrivateGlobalForString(Module &M, StringRef Str,
return GV;
}
-Comdat *llvm::GetOrCreateFunctionComdat(Function &F, Triple &T,
- const std::string &ModuleId) {
+Comdat *llvm::getOrCreateFunctionComdat(Function &F, Triple &T) {
if (auto Comdat = F.getComdat()) return Comdat;
assert(F.hasName());
Module *M = F.getParent();
- std::string Name = std::string(F.getName());
-
- // Make a unique comdat name for internal linkage things on ELF. On COFF, the
- // name of the comdat group identifies the leader symbol of the comdat group.
- // The linkage of the leader symbol is considered during comdat resolution,
- // and internal symbols with the same name from
diff erent objects will not be
- // merged.
- if (T.isOSBinFormatELF() && F.hasLocalLinkage()) {
- if (ModuleId.empty())
- return nullptr;
- Name += ModuleId;
- }
// Make a new comdat for the function. Use the "no duplicates" selection kind
- // for non-weak symbols if the object file format supports it.
- Comdat *C = M->getOrInsertComdat(Name);
- if (T.isOSBinFormatCOFF() && !F.isWeakForLinker())
+ // if the object file format supports it. For COFF we restrict it to non-weak
+ // symbols.
+ Comdat *C = M->getOrInsertComdat(F.getName());
+ if (T.isOSBinFormatELF() || (T.isOSBinFormatCOFF() && !F.isWeakForLinker()))
C->setSelectionKind(Comdat::NoDuplicates);
F.setComdat(C);
return C;
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
index b4b34450ecf2..69512fd92943 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -676,16 +676,14 @@ GlobalVariable *ModuleSanitizerCoverage::CreateFunctionLocalArrayInSection(
*CurModule, ArrayTy, false, GlobalVariable::PrivateLinkage,
Constant::getNullValue(ArrayTy), "__sancov_gen_");
- if (TargetTriple.supportsCOMDAT() && !F.isInterposable())
- if (auto Comdat =
- GetOrCreateFunctionComdat(F, TargetTriple, CurModuleUniqueId))
+ 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);
GlobalsToAppendToCompilerUsed.push_back(Array);
- MDNode *MD = MDNode::get(F.getContext(), ValueAsMetadata::get(&F));
- Array->addMetadata(LLVMContext::MD_associated, *MD);
return Array;
}
diff --git a/llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll b/llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
index 87739db4b072..e711d96a5a43 100644
--- a/llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
+++ b/llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
@@ -2,8 +2,8 @@
; RUN: opt < %s -sancov -sanitizer-coverage-level=1 -sanitizer-coverage-inline-bool-flag=1 -S -enable-new-pm=0 | FileCheck %s
; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=1 -sanitizer-coverage-inline-bool-flag=1 -S | FileCheck %s
-; CHECK: $foo = comdat any
-; CHECK: @__sancov_gen_ = private global [1 x i1] zeroinitializer, section "__sancov_bools", comdat($foo), align 1, !associated !0
+; CHECK: $foo = comdat noduplicates
+; CHECK: @__sancov_gen_ = private global [1 x i1] zeroinitializer, section "__sancov_bools", comdat($foo), align 1{{$}}
; CHECK-NOT: @llvm.used =
; CHECK: @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast ([1 x i1]* @__sancov_gen_ to i8*)], section "llvm.metadata"
@@ -26,5 +26,4 @@ entry:
}
; CHECK: call void @__sanitizer_cov_bool_flag_init(i1* @__start___sancov_bools, i1* @__stop___sancov_bools)
-; CHECK: !0 = !{void ()* @foo}
; CHECK: ![[#EMPTY]] = !{}
diff --git a/llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol-nocomdat.ll b/llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol.ll
similarity index 64%
rename from llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol-nocomdat.ll
rename to llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol.ll
index 6740b45c5ceb..ba528873d281 100644
--- a/llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol-nocomdat.ll
+++ b/llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol.ll
@@ -1,8 +1,8 @@
; Test that interposable symbols do not get put in comdats.
-; RUN: opt < %s -sancov -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard -mtriple x86_64-linux-gnu -S -enable-new-pm=0 | FileCheck %s
-; RUN: opt < %s -sancov -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard -mtriple x86_64-windows-msvc -S -enable-new-pm=0 | FileCheck %s
-; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard -mtriple x86_64-linux-gnu -S | FileCheck %s
-; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard -mtriple x86_64-windows-msvc -S | FileCheck %s
+; RUN: opt < %s -sancov -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard -mtriple x86_64-linux-gnu -S -enable-new-pm=0 | FileCheck %s --check-prefixes=CHECK,ELF
+; RUN: opt < %s -sancov -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard -mtriple x86_64-windows-msvc -S -enable-new-pm=0 | FileCheck %s --check-prefixes=CHECK,COFF
+; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard -mtriple x86_64-linux-gnu -S | FileCheck %s --check-prefixes=CHECK,ELF
+; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard -mtriple x86_64-windows-msvc -S | FileCheck %s --check-prefixes=CHECK,COFF
define void @Vanilla() {
entry:
@@ -31,15 +31,20 @@ entry:
ret void
}
-; CHECK: @__sancov_gen_ = private global [1 x i32] zeroinitializer, section {{.*}}, comdat($Vanilla), align 4, !associated
-; CHECK-NEXT: @__sancov_gen_.1 = private global [1 x i32] zeroinitializer, section {{.*}}, align 4, !associated
-; CHECK-NEXT: @__sancov_gen_.2 = private global [1 x i32] zeroinitializer, section {{.*}}, align 4, !associated
-; CHECK-NEXT: @__sancov_gen_.3 = private global [1 x i32] zeroinitializer, section {{.*}}, comdat($LinkOnceOdr), align 4, !associated
-; CHECK-NEXT: @__sancov_gen_.4 = private global [1 x i32] zeroinitializer, section {{.*}}, comdat($WeakOdr), align 4, !associated
+; CHECK: $Vanilla = comdat noduplicates
+; ELF: $LinkOnceOdr = comdat noduplicates
+; COFF: $LinkOnceOdr = comdat any
+; CHECK: @__sancov_gen_ = private global [1 x i32] zeroinitializer, section {{.*}}, comdat($Vanilla), align 4{{$}}
+; CHECK-NEXT: @__sancov_gen_.1 = private global [1 x i32] zeroinitializer, section {{.*}}, align 4{{$}}
+; CHECK-NEXT: @__sancov_gen_.2 = private global [1 x i32] zeroinitializer, section {{.*}}, align 4{{$}}
+; CHECK-NEXT: @__sancov_gen_.3 = private global [1 x i32] zeroinitializer, section {{.*}}, comdat($LinkOnceOdr), align 4{{$}}
+; CHECK-NEXT: @__sancov_gen_.4 = private global [1 x i32] zeroinitializer, section {{.*}}, comdat($WeakOdr), align 4{{$}}
; CHECK: define void @Vanilla() comdat {
-; CHECK: define linkonce void @LinkOnce() {
-; CHECK: define weak void @Weak() {
+; ELF: define linkonce void @LinkOnce() comdat {
+; ELF: define weak void @Weak() comdat {
+; COFF: define linkonce void @LinkOnce() {
+; COFF: define weak void @Weak() {
; CHECK: declare extern_weak void @ExternWeak()
; CHECK: define linkonce_odr void @LinkOnceOdr() comdat {
; CHECK: define weak_odr void @WeakOdr() comdat {
diff --git a/llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll b/llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll
index ffaef122c13f..a3b3aa834c77 100644
--- a/llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll
+++ b/llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll
@@ -4,13 +4,13 @@
; 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 any
-; ELF: $CallViaVptr = comdat any
-; ELF: @__sancov_gen_ = private global [3 x i32] zeroinitializer, section "__sancov_guards", comdat($foo), align 4, !associated !0
-; ELF-NEXT: @__sancov_gen_.1 = private global [1 x i32] zeroinitializer, section "__sancov_guards", comdat($CallViaVptr), align 4, !associated !1
+; ELF: $foo = comdat noduplicates
+; ELF: $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, !associated !0
-; MACHO-NEXT: @__sancov_gen_.1 = private global [1 x i32] zeroinitializer, section "__DATA,__sancov_guards", align 4, !associated !1
+; 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{{$}}
; ELF-NOT: @llvm.used =
; 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"
More information about the llvm-commits
mailing list