Revert #67745 "[asan] Ensure __asan_register_elf_globals is called in COMDAT asan.module_ctor (#67745)" This reverts commit 1a4b9b6f73391d94f4f96cc964cbf89cfdd59b5b. When getUniqueModuleId(&M) is empty, we may add comdat to internal constants like $.str, causing spurious `error: relocation refers to a symbol in a discarded section` lld errors. GitOrigin-RevId: 5d87665de956eaa237def9c7a35c449e71629302
diff --git a/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/lib/Transforms/Instrumentation/AddressSanitizer.cpp index 6ec592f..bde5fba 100644 --- a/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -819,7 +819,7 @@ private: void initializeCallbacks(Module &M); - void InstrumentGlobals(IRBuilder<> &IRB, Module &M, bool *CtorComdat); + bool InstrumentGlobals(IRBuilder<> &IRB, Module &M, bool *CtorComdat); void InstrumentGlobalsCOFF(IRBuilder<> &IRB, Module &M, ArrayRef<GlobalVariable *> ExtendedGlobals, ArrayRef<Constant *> MetadataInitializers); @@ -2237,7 +2237,7 @@ // We also need to unregister globals at the end, e.g., when a shared library // gets closed. - if (DestructorKind != AsanDtorKind::None && !MetadataGlobals.empty()) { + if (DestructorKind != AsanDtorKind::None) { IRBuilder<> IrbDtor(CreateAsanModuleDtor(M)); IrbDtor.CreateCall(AsanUnregisterElfGlobals, {IRB.CreatePointerCast(RegisteredFlag, IntptrTy), @@ -2343,8 +2343,10 @@ // redzones and inserts this function into llvm.global_ctors. // Sets *CtorComdat to true if the global registration code emitted into the // asan constructor is comdat-compatible. -void ModuleAddressSanitizer::InstrumentGlobals(IRBuilder<> &IRB, Module &M, +bool ModuleAddressSanitizer::InstrumentGlobals(IRBuilder<> &IRB, Module &M, bool *CtorComdat) { + *CtorComdat = false; + // Build set of globals that are aliased by some GA, where // getExcludedAliasedGlobal(GA) returns the relevant GlobalVariable. SmallPtrSet<const GlobalVariable *, 16> AliasedGlobalExclusions; @@ -2362,6 +2364,11 @@ } size_t n = GlobalsToChange.size(); + if (n == 0) { + *CtorComdat = true; + return false; + } + auto &DL = M.getDataLayout(); // A global is described by a structure @@ -2384,11 +2391,8 @@ // We shouldn't merge same module names, as this string serves as unique // module ID in runtime. - GlobalVariable *ModuleName = - n != 0 - ? createPrivateGlobalForString(M, M.getModuleIdentifier(), - /*AllowMerging*/ false, kAsanGenPrefix) - : nullptr; + GlobalVariable *ModuleName = createPrivateGlobalForString( + M, M.getModuleIdentifier(), /*AllowMerging*/ false, kAsanGenPrefix); for (size_t i = 0; i < n; i++) { GlobalVariable *G = GlobalsToChange[i]; @@ -2513,27 +2517,19 @@ } appendToCompilerUsed(M, ArrayRef<GlobalValue *>(GlobalsToAddToUsedList)); - if (UseGlobalsGC && TargetTriple.isOSBinFormatELF()) { - // Use COMDAT and register globals even if n == 0 to ensure that (a) the - // linkage unit will only have one module constructor, and (b) the register - // function will be called. The module destructor is not created when n == - // 0. + std::string ELFUniqueModuleId = + (UseGlobalsGC && TargetTriple.isOSBinFormatELF()) ? getUniqueModuleId(&M) + : ""; + + if (!ELFUniqueModuleId.empty()) { + InstrumentGlobalsELF(IRB, M, NewGlobals, Initializers, ELFUniqueModuleId); *CtorComdat = true; - InstrumentGlobalsELF(IRB, M, NewGlobals, Initializers, - getUniqueModuleId(&M)); - } else if (n == 0) { - // When UseGlobalsGC is false, COMDAT can still be used if n == 0, because - // all compile units will have identical module constructor/destructor. - *CtorComdat = TargetTriple.isOSBinFormatELF(); + } else if (UseGlobalsGC && TargetTriple.isOSBinFormatCOFF()) { + InstrumentGlobalsCOFF(IRB, M, NewGlobals, Initializers); + } else if (UseGlobalsGC && ShouldUseMachOGlobalsSection()) { + InstrumentGlobalsMachO(IRB, M, NewGlobals, Initializers); } else { - *CtorComdat = false; - if (UseGlobalsGC && TargetTriple.isOSBinFormatCOFF()) { - InstrumentGlobalsCOFF(IRB, M, NewGlobals, Initializers); - } else if (UseGlobalsGC && ShouldUseMachOGlobalsSection()) { - InstrumentGlobalsMachO(IRB, M, NewGlobals, Initializers); - } else { - InstrumentGlobalsWithMetadataArray(IRB, M, NewGlobals, Initializers); - } + InstrumentGlobalsWithMetadataArray(IRB, M, NewGlobals, Initializers); } // Create calls for poisoning before initializers run and unpoisoning after. @@ -2541,6 +2537,7 @@ createInitializerPoisonCalls(M, ModuleName); LLVM_DEBUG(dbgs() << M); + return true; } uint64_t
diff --git a/test/Instrumentation/AddressSanitizer/basic.ll b/test/Instrumentation/AddressSanitizer/basic.ll index f9c5b14..068d6d1 100644 --- a/test/Instrumentation/AddressSanitizer/basic.ll +++ b/test/Instrumentation/AddressSanitizer/basic.ll
@@ -210,10 +210,8 @@ ;; 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 internal void @asan.module_ctor() #[[#ATTR:]] {{(comdat )?}}{ ; CHECK: call void @__asan_init() -;; __asan_register_elf_globals is called even if this module does not contain global variables. -; CHECK: call void @__asan_register_elf_globals(i64 ptrtoint (ptr @___asan_globals_registered to i64), i64 ptrtoint (ptr @__start_asan_globals to i64), i64 ptrtoint (ptr @__stop_asan_globals to i64)) ; CHECK: attributes #[[#ATTR]] = { nounwind }
diff --git a/test/Instrumentation/AddressSanitizer/global_metadata_array.ll b/test/Instrumentation/AddressSanitizer/global_metadata_array.ll index 1617bf9..0e4d1f1 100644 --- a/test/Instrumentation/AddressSanitizer/global_metadata_array.ll +++ b/test/Instrumentation/AddressSanitizer/global_metadata_array.ll
@@ -1,12 +1,8 @@ -; RUN: rm -rf %t && split-file %s %t && cd %t -; RUN: opt < a.ll -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-unknown-linux-gnu -S | FileCheck --check-prefix=CHECK %s -; RUN: opt < a.ll -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-apple-macosx10.11.0 -S | FileCheck --check-prefix=CHECK %s -; RUN: opt < a.ll -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-pc-windows-msvc19.0.24215 -S | FileCheck --check-prefix=CHECK %s -; RUN: opt < a.ll -passes=asan -asan-globals-live-support=0 -asan-mapping-scale=5 -mtriple=x86_64-unknown-linux-gnu -S | FileCheck --check-prefixes=CHECK,CHECK-S5 %s +; RUN: opt < %s -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-unknown-linux-gnu -S | FileCheck --check-prefix=CHECK %s +; RUN: opt < %s -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-apple-macosx10.11.0 -S | FileCheck --check-prefix=CHECK %s +; RUN: opt < %s -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-pc-windows-msvc19.0.24215 -S | FileCheck --check-prefix=CHECK %s +; RUN: opt < %s -passes=asan -asan-globals-live-support=0 -asan-mapping-scale=5 -mtriple=x86_64-unknown-linux-gnu -S | FileCheck --check-prefixes=CHECK,CHECK-S5 %s -; RUN: opt < empty.ll -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-unknown-linux-gnu -S | FileCheck --check-prefix=ELF-NOGC %s - -;--- a.ll target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" ; Globals: @@ -63,10 +59,3 @@ !7 = !{!"/tmp/asan-globals.cpp", i32 7, i32 5} !8 = !{!"/tmp/asan-globals.cpp", i32 12, i32 14} !9 = !{!"/tmp/asan-globals.cpp", i32 14, i32 25} - -;; In the presence of instrumented global variables, asan.module_ctor do not use comdat. -; CHECK: define internal void @asan.module_ctor() #[[#ATTR:]] { - -; ELF-NOGC: define internal void @asan.module_ctor() #[[#ATTR:]] comdat { - -;--- empty.ll