melver created this revision.
melver added reviewers: MaskRay, vitalybuka, fmayer, pcc.
Herald added subscribers: luke, Enna1, kosarev, frasercrmck, kerbowa, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, hiraditya, jvesely.
Herald added a project: All.
melver requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, pcwang-thead.
Herald added projects: clang, LLVM.

Currently structors of various sanitizers have internal linkage, even
with a COMDAT set. This means that semantics is not the same with LTO,
because LTO uses linkage to deduplicate and not the COMDAT. The result
is that a target built with LTO currently has numerous duplicate
redundant constructors and destructors generated by the sanitizers.

This can be fixed by marking the structor as having external linkage.
However, care must be taken to also set hidden visibility, otherwise a
DSO may not call its own structor but another, which is usually wrong.

To allow the caller full flexibility over precise linkage and
visibility, we can't just set it for the caller.

Add an assertion that checks the various rules.

Along the way, we now have to fix up ASan, HWASan, and SanCov to emit
the right linkage.

See also: https://maskray.me/blog/2021-07-25-comdat-and-section-group#grp_comdat


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143634

Files:
  clang/test/CodeGen/asan-destructor-kind.cpp
  clang/test/CodeGen/asan-no-globals-no-comdat.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/lib/Transforms/Utils/ModuleUtils.cpp
  llvm/test/CodeGen/AArch64/pacbti-llvm-generated-funcs-1.ll
  llvm/test/DebugInfo/AArch64/asan-stack-vars.mir
  llvm/test/DebugInfo/COFF/asan-module-ctor.ll
  llvm/test/DebugInfo/COFF/asan-module-without-functions.ll
  llvm/test/DebugInfo/Generic/incorrect-variable-debugloc.ll
  llvm/test/DebugInfo/X86/dbg_value_direct.ll
  
llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_do_not_instrument_lds.ll
  llvm/test/Instrumentation/AddressSanitizer/basic.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_global.ll
  llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
  llvm/test/Instrumentation/AddressSanitizer/kcfi.ll
  llvm/test/Instrumentation/AddressSanitizer/module-flags.ll
  llvm/test/Instrumentation/AddressSanitizer/no-global-ctors.ll
  llvm/test/Instrumentation/AddressSanitizer/no-globals.ll
  llvm/test/Instrumentation/AddressSanitizer/no_global_dtors.ll
  llvm/test/Instrumentation/AddressSanitizer/program-addrspace.ll
  llvm/test/Instrumentation/AddressSanitizer/version-mismatch-check.ll
  llvm/test/Instrumentation/HWAddressSanitizer/RISCV/basic.ll
  llvm/test/Instrumentation/HWAddressSanitizer/RISCV/with-calls.ll
  llvm/test/Instrumentation/HWAddressSanitizer/basic.ll
  llvm/test/Instrumentation/HWAddressSanitizer/with-calls.ll
  llvm/test/Instrumentation/SanitizerCoverage/pc-table.ll
  
llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll
  llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-bool-flag.ll
  llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll
===================================================================
--- llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard.ll
@@ -73,7 +73,7 @@
   ret void
 }
 
-; ELF-LABEL: define internal void @sancov.module_ctor_trace_pc_guard() #2 comdat {
+; ELF-LABEL: define hidden void @sancov.module_ctor_trace_pc_guard() #2 comdat {
 ; MACHO-LABEL: define internal void @sancov.module_ctor_trace_pc_guard() #2 {
 
 ; CHECK: attributes #2 = { nounwind }
Index: llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-bool-flag.ll
===================================================================
--- llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-bool-flag.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-bool-flag.ll
@@ -2,9 +2,11 @@
 
 ; Module ctors should have stable names across modules, not something like
 ; @sancov.module_ctor.3 that may cause duplicate ctors after linked together.
+; They should also have external linkage, so that LTO doesn't end up producing
+; duplicate ctors either.
 
-; CHECK: define internal void @sancov.module_ctor_trace_pc_guard() #[[#]] comdat {
-; CHECK: define internal void @sancov.module_ctor_bool_flag() #[[#]] comdat {
+; CHECK: define hidden void @sancov.module_ctor_trace_pc_guard() #[[#]] comdat {
+; CHECK: define hidden void @sancov.module_ctor_bool_flag() #[[#]] comdat {
 
 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"
Index: llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll
===================================================================
--- llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll
@@ -3,8 +3,8 @@
 ; Module ctors should have stable names across modules, not something like
 ; @sancov.module_ctor.3 that may cause duplicate ctors after linked together.
 
-; CHECK: define internal void @sancov.module_ctor_trace_pc_guard() #[[#]] comdat {
-; CHECK: define internal void @sancov.module_ctor_8bit_counters() #[[#]] comdat {
+; CHECK: define hidden void @sancov.module_ctor_trace_pc_guard() #[[#]] comdat {
+; CHECK: define hidden void @sancov.module_ctor_8bit_counters() #[[#]] comdat {
 
 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"
Index: llvm/test/Instrumentation/SanitizerCoverage/pc-table.ll
===================================================================
--- llvm/test/Instrumentation/SanitizerCoverage/pc-table.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/pc-table.ll
@@ -21,6 +21,6 @@
 ; CHECK: private constant [6 x ptr] [ptr @foo, ptr inttoptr (i64 1 to ptr), ptr blockaddress(@foo, %entry.if.end_crit_edge), ptr null, ptr blockaddress(@foo, %if.then), ptr null], section "__sancov_pcs", comdat($foo), align 8
 ; CHECK:      @__start___sancov_pcs = extern_weak hidden global i64
 ; CHECK-NEXT: @__stop___sancov_pcs = extern_weak hidden global i64
-; CHECK: define internal void @sancov.module_ctor
+; CHECK: define hidden void @sancov.module_ctor
 ; CHECK: call void @__sanitizer_cov
 ; CHECK: call void @__sanitizer_cov_pcs_init
Index: llvm/test/Instrumentation/HWAddressSanitizer/with-calls.ll
===================================================================
--- llvm/test/Instrumentation/HWAddressSanitizer/with-calls.ll
+++ llvm/test/Instrumentation/HWAddressSanitizer/with-calls.ll
@@ -197,7 +197,7 @@
 
 ; CHECK: declare void @__hwasan_init()
 
-; CHECK:      define internal void @hwasan.module_ctor() #[[#]] comdat {
+; CHECK:      define hidden void @hwasan.module_ctor() #[[#]] comdat {
 ; CHECK-NEXT:   call void @__hwasan_init()
 ; CHECK-NEXT:   ret void
 ; CHECK-NEXT: }
Index: llvm/test/Instrumentation/HWAddressSanitizer/basic.ll
===================================================================
--- llvm/test/Instrumentation/HWAddressSanitizer/basic.ll
+++ llvm/test/Instrumentation/HWAddressSanitizer/basic.ll
@@ -393,7 +393,7 @@
 
 ; CHECK: declare void @__hwasan_init()
 
-; CHECK:      define internal void @hwasan.module_ctor() #[[#ATTR:]] comdat {
+; CHECK:      define hidden void @hwasan.module_ctor() #[[#ATTR:]] comdat {
 ; CHECK-NEXT:   call void @__hwasan_init()
 ; CHECK-NEXT:   ret void
 ; CHECK-NEXT: }
Index: llvm/test/Instrumentation/HWAddressSanitizer/RISCV/with-calls.ll
===================================================================
--- llvm/test/Instrumentation/HWAddressSanitizer/RISCV/with-calls.ll
+++ llvm/test/Instrumentation/HWAddressSanitizer/RISCV/with-calls.ll
@@ -197,7 +197,7 @@
 
 ; CHECK: declare void @__hwasan_init()
 
-; CHECK:      define internal void @hwasan.module_ctor() #[[#]] comdat {
+; CHECK:      define hidden void @hwasan.module_ctor() #[[#]] comdat {
 ; CHECK-NEXT:   call void @__hwasan_init()
 ; CHECK-NEXT:   ret void
 ; CHECK-NEXT: }
Index: llvm/test/Instrumentation/HWAddressSanitizer/RISCV/basic.ll
===================================================================
--- llvm/test/Instrumentation/HWAddressSanitizer/RISCV/basic.ll
+++ llvm/test/Instrumentation/HWAddressSanitizer/RISCV/basic.ll
@@ -393,7 +393,7 @@
 
 ; CHECK: declare void @__hwasan_init()
 
-; CHECK:      define internal void @hwasan.module_ctor() #[[#ATTR:]] comdat {
+; CHECK:      define hidden void @hwasan.module_ctor() #[[#ATTR:]] comdat {
 ; CHECK-NEXT:   call void @__hwasan_init()
 ; CHECK-NEXT:   ret void
 ; CHECK-NEXT: }
Index: llvm/test/Instrumentation/AddressSanitizer/version-mismatch-check.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/version-mismatch-check.ll
+++ llvm/test/Instrumentation/AddressSanitizer/version-mismatch-check.ll
@@ -7,6 +7,6 @@
 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"
 
-; CHECK-LABEL: define internal void @asan.module_ctor()
+; CHECK-LABEL: define hidden void @asan.module_ctor()
 ; CHECK:         call void @__asan_version_mismatch_check_
 ; NOGUARD-NOT:   call void @__asan_version_mismatch_check_
Index: llvm/test/Instrumentation/AddressSanitizer/program-addrspace.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/program-addrspace.ll
+++ llvm/test/Instrumentation/AddressSanitizer/program-addrspace.ll
@@ -8,7 +8,7 @@
 ; CHECK: @llvm.used = appending global [1 x ptr] [ptr addrspacecast (ptr addrspace(1) @asan.module_ctor to ptr)], section "llvm.metadata"
 ; CHECK: @llvm.global_ctors = appending global [1 x { i32, ptr addrspace(1), ptr }] [{ i32, ptr addrspace(1), ptr } { i32 1, ptr addrspace(1) @asan.module_ctor, ptr addrspacecast (ptr addrspace(1) @asan.module_ctor to ptr) }]
 
-; CHECK: define internal void @asan.module_ctor() addrspace(1) #0 comdat {
+; CHECK: define hidden void @asan.module_ctor() addrspace(1) #0 comdat {
 
 target datalayout = "P1"
 
Index: llvm/test/Instrumentation/AddressSanitizer/no_global_dtors.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/no_global_dtors.ll
+++ llvm/test/Instrumentation/AddressSanitizer/no_global_dtors.ll
@@ -9,7 +9,7 @@
 ; RUN:   -asan-destructor-kind=none -S | \
 ; RUN:   FileCheck %s
 ; CHECK-NOT: llvm.global_dtor{{.+}}asan.module_dtor
-; CHECK-NOT: define internal void @asan.module_dtor
+; CHECK-NOT: define{{.*}}void @asan.module_dtor
 
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx11.0.0"
Index: llvm/test/Instrumentation/AddressSanitizer/no-globals.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/no-globals.ll
+++ llvm/test/Instrumentation/AddressSanitizer/no-globals.ll
@@ -8,5 +8,5 @@
 ; CHECK-NOT: @llvm.global_dtors
 ; CHECK: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 1, ptr @asan.module_ctor, ptr @asan.module_ctor }]
 ; CHECK-NOT: @llvm.global_dtors
-; CHECK: define internal void @asan.module_ctor() #[[#]] comdat
+; CHECK: define hidden void @asan.module_ctor() #[[#]] comdat
 ; CHECK-NOT: @llvm.global_dtors
Index: llvm/test/Instrumentation/AddressSanitizer/no-global-ctors.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/no-global-ctors.ll
+++ llvm/test/Instrumentation/AddressSanitizer/no-global-ctors.ll
@@ -9,7 +9,7 @@
 ; RUN:   -asan-constructor-kind=none -S | \
 ; RUN:   FileCheck %s
 ; CHECK-NOT: llvm.global_ctor{{.+}}asan.module_ctor
-; CHECK-NOT: define internal void @asan.module_ctor
+; CHECK-NOT: define{{.*}}void @asan.module_ctor
 
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx11.0.0"
Index: llvm/test/Instrumentation/AddressSanitizer/module-flags.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/module-flags.ll
+++ llvm/test/Instrumentation/AddressSanitizer/module-flags.ll
@@ -19,6 +19,6 @@
 !1 = !{i32 7, !"frame-pointer", i32 2}
 
 ;; Set the uwtable attribute on ctor/dtor.
-; CHECK: define internal void @asan.module_ctor() #[[#ATTR:]]
-; CHECK: define internal void @asan.module_dtor() #[[#ATTR]]
+; CHECK: define hidden void @asan.module_ctor() #[[#ATTR:]]
+; CHECK: define hidden void @asan.module_dtor() #[[#ATTR]]
 ; CHECK: attributes #[[#ATTR]] = { nounwind uwtable "frame-pointer"="all" }
Index: llvm/test/Instrumentation/AddressSanitizer/kcfi.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/kcfi.ll
+++ llvm/test/Instrumentation/AddressSanitizer/kcfi.ll
@@ -4,7 +4,7 @@
 
 ; CHECK: @llvm.global_ctors = {{.*}}{ i32 1, ptr @asan.module_ctor, ptr @asan.module_ctor }
 
-; CHECK: define internal void @asan.module_ctor()
+; CHECK: define hidden void @asan.module_ctor()
 ; CHECK-SAME: !kcfi_type
 
 !llvm.module.flags = !{!0}
Index: llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
+++ llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
@@ -4,7 +4,7 @@
 
 ; CHECK: @llvm.global_ctors = {{.*}}{ i32 1, ptr @asan.module_ctor, ptr @asan.module_ctor }
 
-; CHECK: define internal void @asan.module_ctor()
+; CHECK: define hidden void @asan.module_ctor()
 ; CHECK-SAME: #[[#ATTR:]]
 ; CHECK-SAME: !kcfi_type
 
Index: llvm/test/Instrumentation/AddressSanitizer/instrument_global.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/instrument_global.ll
+++ llvm/test/Instrumentation/AddressSanitizer/instrument_global.ll
@@ -69,12 +69,12 @@
 ; CHECK: ret i32
 }
 
-; CHECK-LABEL: define internal void @asan.module_ctor
+; CHECK-LABEL: define hidden void @asan.module_ctor
 ; CHECK-NOT: ret
 ; CHECK: call void @__asan_register_elf_globals
 ; CHECK: ret
 
-; CHECK-LABEL: define internal void @asan.module_dtor
+; CHECK-LABEL: define hidden void @asan.module_dtor
 ; CHECK-NOT: ret
 ; CHECK: call void @__asan_unregister_elf_globals
 ; CHECK: ret
Index: llvm/test/Instrumentation/AddressSanitizer/basic.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/basic.ll
+++ llvm/test/Instrumentation/AddressSanitizer/basic.ll
@@ -189,7 +189,7 @@
 
 ;; ctor/dtor have the nounwind attribute. See uwtable.ll, they additionally have
 ;; the uwtable attribute with the module flag "uwtable".
-; CHECK: define internal void @asan.module_ctor() #[[#ATTR:]] {{(comdat )?}}{
+; CHECK: define hidden void @asan.module_ctor() #[[#ATTR:]] {{(comdat )?}}{
 ; CHECK: call void @__asan_init()
 
 ; CHECK: attributes #[[#ATTR]] = { nounwind }
Index: llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_do_not_instrument_lds.ll
===================================================================
--- llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_do_not_instrument_lds.ll
+++ llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_do_not_instrument_lds.ll
@@ -24,4 +24,4 @@
   ret void
 }
 
-; CHECK-LABEL: define internal void @asan.module_ctor()
+; CHECK-LABEL: define hidden void @asan.module_ctor()
Index: llvm/test/DebugInfo/X86/dbg_value_direct.ll
===================================================================
--- llvm/test/DebugInfo/X86/dbg_value_direct.ll
+++ llvm/test/DebugInfo/X86/dbg_value_direct.ll
@@ -89,7 +89,7 @@
 
 declare void @_ZN1AC1Ev(ptr) #2
 
-define internal void @asan.module_ctor()  "stack-protector-buffer-size"="1" {
+define hidden void @asan.module_ctor()  "stack-protector-buffer-size"="1" {
   call void @__asan_init_v3()
   %1 = load volatile i64, ptr @__asan_mapping_offset
   %2 = load volatile i64, ptr @__asan_mapping_scale
Index: llvm/test/DebugInfo/Generic/incorrect-variable-debugloc.ll
===================================================================
--- llvm/test/DebugInfo/Generic/incorrect-variable-debugloc.ll
+++ llvm/test/DebugInfo/Generic/incorrect-variable-debugloc.ll
@@ -198,7 +198,7 @@
 ; Function Attrs: nounwind readnone
 declare void @llvm.dbg.value(metadata, metadata, metadata) #3
 
-define internal void @asan.module_ctor() {
+define hidden void @asan.module_ctor() {
   tail call void @__asan_init_v3()
   ret void
 }
Index: llvm/test/DebugInfo/COFF/asan-module-without-functions.ll
===================================================================
--- llvm/test/DebugInfo/COFF/asan-module-without-functions.ll
+++ llvm/test/DebugInfo/COFF/asan-module-without-functions.ll
@@ -20,7 +20,7 @@
 @0 = internal global [1 x { i32, i32, i32, i32, i32, i32 }] [{ i32, i32, i32, i32, i32, i32 } { i32 ptrtoint (ptr @c to i32), i32 1, i32 64, i32 ptrtoint (ptr @___asan_gen_1 to i32), i32 ptrtoint (ptr @___asan_gen_ to i32), i32 0 }]
 @llvm.global_dtors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 1, ptr @asan.module_dtor, ptr null }]
 
-define internal void @asan.module_ctor() {
+define hidden void @asan.module_ctor() {
   call void @__asan_init_v3()
   call void @__asan_register_globals(i32 ptrtoint (ptr @0 to i32), i32 1)
   ret void
@@ -36,7 +36,7 @@
 
 declare void @__asan_unregister_globals(i32, i32)
 
-define internal void @asan.module_dtor() {
+define hidden void @asan.module_dtor() {
   call void @__asan_unregister_globals(i32 ptrtoint (ptr @0 to i32), i32 1)
   ret void
 }
Index: llvm/test/DebugInfo/COFF/asan-module-ctor.ll
===================================================================
--- llvm/test/DebugInfo/COFF/asan-module-ctor.ll
+++ llvm/test/DebugInfo/COFF/asan-module-ctor.ll
@@ -29,7 +29,7 @@
   ret i32 0, !dbg !10
 }
 
-define internal void @asan.module_ctor() {
+define hidden void @asan.module_ctor() {
   call void @__asan_init_v3()
   ret void
 }
Index: llvm/test/DebugInfo/AArch64/asan-stack-vars.mir
===================================================================
--- llvm/test/DebugInfo/AArch64/asan-stack-vars.mir
+++ llvm/test/DebugInfo/AArch64/asan-stack-vars.mir
@@ -252,7 +252,7 @@
 
   declare i8* @objc_autoreleaseReturnValue(i8* returned)
 
-  define internal void @asan.module_ctor() {
+  define hidden void @asan.module_ctor() {
     call void @__asan_init()
     call void @__asan_version_mismatch_check_v8()
     ret void
Index: llvm/test/CodeGen/AArch64/pacbti-llvm-generated-funcs-1.ll
===================================================================
--- llvm/test/CodeGen/AArch64/pacbti-llvm-generated-funcs-1.ll
+++ llvm/test/CodeGen/AArch64/pacbti-llvm-generated-funcs-1.ll
@@ -14,7 +14,7 @@
 declare void @__asan_init()
 declare void @__asan_version_mismatch_check_v8()
 
-define internal void @asan.module_ctor() {
+define hidden void @asan.module_ctor() {
   call void @__asan_init()
   call void @__asan_version_mismatch_check_v8()
   ret void
Index: llvm/lib/Transforms/Utils/ModuleUtils.cpp
===================================================================
--- llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -19,16 +19,17 @@
 #include "llvm/IR/Module.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/xxhash.h"
+#include "llvm/TargetParser/Triple.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "moduleutils"
 
-static void appendToGlobalArray(StringRef ArrayName, Module &M, Function *F,
-                                int Priority, Constant *Data) {
+static void appendToGlobalStructors(StringRef ArrayName, Module &M, Function *F,
+                                    int Priority, Constant *Data) {
   IRBuilder<> IRB(M.getContext());
   FunctionType *FnTy = FunctionType::get(IRB.getVoidTy(), false);
 
-  // Get the current set of static global constructors and add the new ctor
+  // Get the current set of static global structors and add the new ctor
   // to the list.
   SmallVector<Constant *, 16> CurrentCtors;
   StructType *EltTy = StructType::get(
@@ -36,25 +37,57 @@
       IRB.getInt8PtrTy());
 
   if (GlobalVariable *GVCtor = M.getNamedGlobal(ArrayName)) {
-    if (Constant *Init = GVCtor->getInitializer()) {
-      unsigned n = Init->getNumOperands();
-      CurrentCtors.reserve(n + 1);
-      for (unsigned i = 0; i != n; ++i)
-        CurrentCtors.push_back(cast<Constant>(Init->getOperand(i)));
+    if (const Constant *Init = GVCtor->getInitializer()) {
+      CurrentCtors.reserve(Init->getNumOperands() + 1);
+      for (Value *Val : Init->operands())
+        CurrentCtors.push_back(cast<Constant>(Val));
     }
     GVCtor->eraseFromParent();
   }
 
-  // Build a 3 field global_ctor entry.  We don't take a comdat key.
-  Constant *CSVals[3];
-  CSVals[0] = IRB.getInt32(Priority);
-  CSVals[1] = F;
-  CSVals[2] = Data ? ConstantExpr::getPointerCast(Data, IRB.getInt8PtrTy())
-                   : Constant::getNullValue(IRB.getInt8PtrTy());
-  Constant *RuntimeCtorInit =
-      ConstantStruct::get(EltTy, ArrayRef(CSVals, EltTy->getNumElements()));
+  Constant *LinkedTo;
+  if (Data) {
+    // Structor will only run if Data is not discarded.
+    LinkedTo = ConstantExpr::getPointerCast(Data, IRB.getInt8PtrTy());
+
+    // Verify that on targets that support COMDAT and where Data has a COMDAT,
+    // Data has non-internal linkage, otherwise we're in for surprises with LTO:
+    // when multiple TUs are merged, LTO would use linkage to deduplicate. If
+    // linkage is set to internal linkage, we would end up with multiple
+    // identical symbols in the final binary (which without LTO, the linker
+    // would take care of due to the COMDAT, irrespective of linkage). Also
+    // verify that if linkage is external, visibility is hidden, so that DSOs
+    // don't call any other structor.
+    //
+    // We verify this here, rather than in the Verifier, since none of this is
+    // invalid, but for all users of appendToGlobalStructors() this should hold.
+    assert([&] {
+      Triple T(M.getTargetTriple());
+      if (!T.supportsCOMDAT())
+        return true;
+      if (const auto *Key = dyn_cast<GlobalValue>(Data)) {
+        if (!Key->hasComdat())
+          return true; // Nothing to check.
+        if (Key->getLinkage() == GlobalValue::ExternalLinkage) {
+          // Even with external linkage, a DSO should _not_ call any other
+          // structor but its own (which it has either way).
+          return Key->getVisibility() != GlobalValue::DefaultVisibility;
+        }
+        // Assert not internal linkage.
+        return Key->getLinkage() != GlobalValue::InternalLinkage &&
+               Key->getLinkage() != GlobalValue::PrivateLinkage;
+      }
+      return true;
+    }() && "GlobalValue used as COMDAT key for structor has incorrect linkage");
+  } else {
+    LinkedTo = Constant::getNullValue(IRB.getInt8PtrTy());
+  }
 
-  CurrentCtors.push_back(RuntimeCtorInit);
+  // Build a 3 field global_{c,d}tor entry.
+  const std::array<Constant *, 3> CSVals = {IRB.getInt32(Priority), F,
+                                            LinkedTo};
+  assert(CSVals.size() == EltTy->getNumElements());
+  CurrentCtors.push_back(ConstantStruct::get(EltTy, CSVals));
 
   // Create a new initializer.
   ArrayType *AT = ArrayType::get(EltTy, CurrentCtors.size());
@@ -67,11 +100,11 @@
 }
 
 void llvm::appendToGlobalCtors(Module &M, Function *F, int Priority, Constant *Data) {
-  appendToGlobalArray("llvm.global_ctors", M, F, Priority, Data);
+  appendToGlobalStructors("llvm.global_ctors", M, F, Priority, Data);
 }
 
 void llvm::appendToGlobalDtors(Module &M, Function *F, int Priority, Constant *Data) {
-  appendToGlobalArray("llvm.global_dtors", M, F, Priority, Data);
+  appendToGlobalStructors("llvm.global_dtors", M, F, Priority, Data);
 }
 
 static void collectUsedGlobals(GlobalVariable *GV,
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -359,6 +359,8 @@
   if (TargetTriple.supportsCOMDAT()) {
     // Use comdat to dedup CtorFunc.
     CtorFunc->setComdat(M.getOrInsertComdat(CtorName));
+    CtorFunc->setLinkage(GlobalValue::ExternalLinkage);
+    CtorFunc->setVisibility(GlobalValue::HiddenVisibility);
     appendToGlobalCtors(M, CtorFunc, SanCtorAndDtorPriority, CtorFunc);
   } else {
     appendToGlobalCtors(M, CtorFunc, SanCtorAndDtorPriority);
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -457,6 +457,8 @@
           [&](Function *Ctor, FunctionCallee) {
             Comdat *CtorComdat = M.getOrInsertComdat(kHwasanModuleCtorName);
             Ctor->setComdat(CtorComdat);
+            Ctor->setLinkage(GlobalValue::ExternalLinkage);
+            Ctor->setVisibility(GlobalValue::HiddenVisibility);
             appendToGlobalCtors(M, Ctor, 0, Ctor);
           });
 
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2495,10 +2495,14 @@
   if (UseCtorComdat && TargetTriple.isOSBinFormatELF() && CtorComdat) {
     if (AsanCtorFunction) {
       AsanCtorFunction->setComdat(M.getOrInsertComdat(kAsanModuleCtorName));
+      AsanCtorFunction->setLinkage(GlobalValue::ExternalLinkage);
+      AsanCtorFunction->setVisibility(GlobalValue::HiddenVisibility);
       appendToGlobalCtors(M, AsanCtorFunction, Priority, AsanCtorFunction);
     }
     if (AsanDtorFunction) {
       AsanDtorFunction->setComdat(M.getOrInsertComdat(kAsanModuleDtorName));
+      AsanDtorFunction->setLinkage(GlobalValue::ExternalLinkage);
+      AsanDtorFunction->setVisibility(GlobalValue::HiddenVisibility);
       appendToGlobalDtors(M, AsanDtorFunction, Priority, AsanDtorFunction);
     }
   } else {
Index: clang/test/CodeGen/asan-no-globals-no-comdat.cpp
===================================================================
--- clang/test/CodeGen/asan-no-globals-no-comdat.cpp
+++ clang/test/CodeGen/asan-no-globals-no-comdat.cpp
@@ -5,5 +5,5 @@
 // RUN: %clang_cc1 -fsanitize=address -fsanitize-address-globals-dead-stripping -fno-integrated-as -emit-llvm -o - -triple x86_64-linux %s | FileCheck %s --check-prefix=WITHOUT-GC
 // RUN: %clang_cc1 -fsanitize=address -fdata-sections -emit-llvm -o - -triple x86_64-linux %s | FileCheck %s --check-prefix=WITHOUT-GC
 
-// WITH-GC: define internal void @asan.module_ctor() #[[#]] comdat {
+// WITH-GC: define hidden void @asan.module_ctor() #[[#]] comdat {
 // WITHOUT-GC: define internal void @asan.module_ctor() #[[#]] {
Index: clang/test/CodeGen/asan-destructor-kind.cpp
===================================================================
--- clang/test/CodeGen/asan-destructor-kind.cpp
+++ clang/test/CodeGen/asan-destructor-kind.cpp
@@ -31,4 +31,4 @@
 }
 
 // CHECK-NONE-DTOR-NOT: llvm.global_dtor{{.+}}asan.module_dtor
-// CHECK-NONE-DTOR-NOT: define internal void @asan.module_dtor
+// CHECK-NONE-DTOR-NOT: define{{.*}}void @asan.module_dtor
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D143634: [Modu... Marco Elver via Phabricator via cfe-commits

Reply via email to