[PATCH] D97430: [SanitizerCoverage] Drop !associated on metadata sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: morehouse, rnk, vitalybuka.
Herald added a subscriber: hiraditya.
MaskRay requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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 sh_link 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 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 let's use the
same mechanism as non-interposable functions for simplicity.  We need to use the
`noduplicates` variant (section group flag 0) to alter section resolution.  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 intend to change the behavior of COFF.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97430

Files:
  llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
  llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol-nocomdat.ll


Index: llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol-nocomdat.ll
===================================================================
--- llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol-nocomdat.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol-nocomdat.ll
@@ -31,15 +31,17 @@
   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:      @__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 {
+; COFF: define linkonce void @LinkOnce() {
+; ELF: define weak void @Weak() comdat {
+; 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 {
Index: llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
===================================================================
--- llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
@@ -2,7 +2,7 @@
 ; 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: @__sancov_gen_ = private global [1 x i1] zeroinitializer, section "__sancov_bools", comdat($foo), align 1, !associated !0
+; CHECK: @__sancov_gen_ = private global [1 x i1] zeroinitializer, section "__sancov_bools", comdat($foo), align 1{{$}}
 
 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"
 target triple = "x86_64-unknown-linux-gnu"
@@ -23,5 +23,4 @@
 }
 ; CHECK: call void @__sanitizer_cov_bool_flag_init(i1* @__start___sancov_bools, i1* @__stop___sancov_bools)
 
-; CHECK: !0 = !{void ()* @foo}
 ; CHECK: ![[#EMPTY]] = !{}
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -676,7 +676,8 @@
       *CurModule, ArrayTy, false, GlobalVariable::PrivateLinkage,
       Constant::getNullValue(ArrayTy), "__sancov_gen_");
 
-  if (TargetTriple.supportsCOMDAT() && !F.isInterposable())
+  if (TargetTriple.supportsCOMDAT() &&
+      (TargetTriple.isOSBinFormatELF() || !F.isInterposable()))
     if (auto Comdat =
             GetOrCreateFunctionComdat(F, TargetTriple, CurModuleUniqueId))
       Array->setComdat(Comdat);
@@ -684,8 +685,6 @@
   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;
 }
Index: llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
+++ llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
@@ -94,7 +94,7 @@
   // 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 (T.isOSBinFormatELF() || (T.isOSBinFormatCOFF() && !F.isWeakForLinker()))
     C->setSelectionKind(Comdat::NoDuplicates);
   F.setComdat(C);
   return C;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97430.326255.patch
Type: text/x-patch
Size: 4928 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210225/c1682bec/attachment.bin>