[llvm] 6420d33 - Revert "[asan] Ensure __asan_register_elf_globals is called in COMDAT asan.module_ctor (#67745)"

Krasimir Georgiev via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 2 04:48:09 PDT 2023


Author: Krasimir Georgiev
Date: 2023-10-02T11:43:39Z
New Revision: 6420d3301cd4f0793adcf11f59e8398db73737d8

URL: https://github.com/llvm/llvm-project/commit/6420d3301cd4f0793adcf11f59e8398db73737d8
DIFF: https://github.com/llvm/llvm-project/commit/6420d3301cd4f0793adcf11f59e8398db73737d8.diff

LOG: Revert "[asan] Ensure __asan_register_elf_globals is called in COMDAT asan.module_ctor (#67745)"

This reverts commit 16eed8c906875e748c3cb610f3dc4b875f3882aa.

Causes some failures internally, will share privately with the author.

Added: 
    

Modified: 
    llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
    llvm/test/Instrumentation/AddressSanitizer/basic.ll
    llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll
    llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index e80ee1953de6b21..9e9b192d442ce5e 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -819,11 +819,11 @@ class ModuleAddressSanitizer {
 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);
-  void instrumentGlobalsELF(IRBuilder<> &IRB, Module &M,
+  void InstrumentGlobalsELF(IRBuilder<> &IRB, Module &M,
                             ArrayRef<GlobalVariable *> ExtendedGlobals,
                             ArrayRef<Constant *> MetadataInitializers,
                             const std::string &UniqueModuleId);
@@ -2177,7 +2177,7 @@ void ModuleAddressSanitizer::InstrumentGlobalsCOFF(
     appendToCompilerUsed(M, MetadataGlobals);
 }
 
-void ModuleAddressSanitizer::instrumentGlobalsELF(
+void ModuleAddressSanitizer::InstrumentGlobalsELF(
     IRBuilder<> &IRB, Module &M, ArrayRef<GlobalVariable *> ExtendedGlobals,
     ArrayRef<Constant *> MetadataInitializers,
     const std::string &UniqueModuleId) {
@@ -2187,7 +2187,7 @@ void ModuleAddressSanitizer::instrumentGlobalsELF(
   // false negative odr violations at link time. If odr indicators are used, we
   // keep the comdat sections, as link time odr violations will be dectected on
   // the odr indicator symbols.
-  bool UseComdatForGlobalsGC = UseOdrIndicator && !UniqueModuleId.empty();
+  bool UseComdatForGlobalsGC = UseOdrIndicator;
 
   SmallVector<GlobalValue *, 16> MetadataGlobals(ExtendedGlobals.size());
   for (size_t i = 0; i < ExtendedGlobals.size(); i++) {
@@ -2237,7 +2237,7 @@ void ModuleAddressSanitizer::instrumentGlobalsELF(
 
   // 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 @@ void ModuleAddressSanitizer::InstrumentGlobalsWithMetadataArray(
 // 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 @@ void ModuleAddressSanitizer::instrumentGlobals(IRBuilder<> &IRB, Module &M,
   }
 
   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 @@ void ModuleAddressSanitizer::instrumentGlobals(IRBuilder<> &IRB, Module &M,
 
   // 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 @@ void ModuleAddressSanitizer::instrumentGlobals(IRBuilder<> &IRB, Module &M,
   }
   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 @@ void ModuleAddressSanitizer::instrumentGlobals(IRBuilder<> &IRB, Module &M,
     createInitializerPoisonCalls(M, ModuleName);
 
   LLVM_DEBUG(dbgs() << M);
+  return true;
 }
 
 uint64_t
@@ -2604,10 +2601,10 @@ bool ModuleAddressSanitizer::instrumentModule(Module &M) {
     assert(AsanCtorFunction || ConstructorKind == AsanCtorKind::None);
     if (AsanCtorFunction) {
       IRBuilder<> IRB(AsanCtorFunction->getEntryBlock().getTerminator());
-      instrumentGlobals(IRB, M, &CtorComdat);
+      InstrumentGlobals(IRB, M, &CtorComdat);
     } else {
       IRBuilder<> IRB(*C);
-      instrumentGlobals(IRB, M, &CtorComdat);
+      InstrumentGlobals(IRB, M, &CtorComdat);
     }
   }
 

diff  --git a/llvm/test/Instrumentation/AddressSanitizer/basic.ll b/llvm/test/Instrumentation/AddressSanitizer/basic.ll
index 2064db5a0918fc4..068d6d18cd45eba 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/basic.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/basic.ll
@@ -210,10 +210,8 @@ define void @test_swifterror_3() sanitize_address {
 
 ;; 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 instrumented 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/llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll b/llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll
index 1617bf9b67aa7c5..0e4d1f168d0163f 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll
+++ b/llvm/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 @@ attributes #1 = { nounwind sanitize_address "less-precise-fpmad"="false" "frame-
 !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

diff  --git a/llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll b/llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
index 699b8287d358ad6..47bb1f102e2fc25 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
@@ -4,14 +4,10 @@
 ; We keep using comdats for garbage collection if odr indicators are
 ; enabled as indicator symbols will cause link time odr violations.
 ; This is to fix PR 47925.
-; RUN: rm -rf %t && split-file %s %t && cd %t
-; RUN: opt < a.ll -passes=asan -asan-globals-live-support=1 -asan-use-odr-indicator=0 -S | FileCheck %s --check-prefixes=CHECK,NOCOMDAT
+;
+; RUN: opt < %s -passes=asan -asan-globals-live-support=1 -asan-use-odr-indicator=0 -S | FileCheck %s --check-prefixes=CHECK,NOCOMDAT
 ; Check that enabling odr indicators enables comdat for globals.
-; RUN: opt < a.ll -passes=asan -asan-globals-live-support=1 -S | FileCheck %s --check-prefixes=CHECK,COMDAT
-
-; RUN: opt < no_module_id.ll -passes=asan -S | FileCheck %s --check-prefix=NOMODULEID
-
-;--- a.ll
+; RUN: opt < %s -passes=asan -asan-globals-live-support=1 -S | FileCheck %s --check-prefixes=CHECK,COMDAT
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -91,43 +87,3 @@ attributes #1 = { nounwind sanitize_address "less-precise-fpmad"="false" "frame-
 !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}
-
-;--- no_module_id.ll
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-; NOMODULEID:      $asan.module_ctor = comdat any
-; NOMODULEID:      $asan.module_dtor = comdat any
-
-;; Don't place the instrumented globals in a comdat when the unique module ID is empty.
-; NOMODULEID:      @.str = internal constant { [4 x i8], [28 x i8] } { [4 x i8] c"str\00", [28 x i8] zeroinitializer }, align 32
-; NOMODULEID:      @_ZL3buf = internal global { [4 x i8], [28 x i8] } zeroinitializer, align 32
-; NOMODULEID:      @__asan_global_.str = private global {{.*}}, section "asan_globals", !associated !0
-; NOMODULEID:      @__asan_global__ZL3buf = private global {{.*}}, section "asan_globals", !associated !1
-; NOMODULEID:      @llvm.compiler.used = appending global [4 x ptr] [ptr @.str, ptr @_ZL3buf, ptr @__asan_global_.str, ptr @__asan_global__ZL3buf]
-
-; NOMODULEID:      define internal void @asan.module_ctor() #[[#]] comdat {
-; NOMODULEID-NEXT:   call void @__asan_init()
-; NOMODULEID-NEXT:   call void @__asan_version_mismatch_check_v8()
-; NOMODULEID-NEXT:   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))
-; NOMODULEID-NEXT:   ret void
-; NOMODULEID-NEXT: }
-
-; NOMODULEID:      !0 = !{ptr @.str}
-; NOMODULEID:      !1 = !{ptr @_ZL3buf}
-
- at .str = private unnamed_addr constant [4 x i8] c"str\00", align 1
- at _ZL3buf = internal unnamed_addr global [4 x i8] zeroinitializer, align 1
- at llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__sub_I_a.cc, ptr null }]
-
-declare void @ext(ptr noundef)
-
-; Function Attrs: uwtable
-define internal void @_GLOBAL__sub_I_a.cc() #2 section ".text.startup" {
-entry:
-  %0 = load i8, ptr @_ZL3buf, align 1
-  %inc = add i8 %0, 1
-  store i8 %inc, ptr @_ZL3buf, align 1
-  tail call void @ext(ptr noundef nonnull @.str)
-  ret void
-}


        


More information about the llvm-commits mailing list