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