Skip to content

Commit 6420d33

Browse files
krasimirggkrasimirgg
committed
Revert "[asan] Ensure __asan_register_elf_globals is called in COMDAT asan.module_ctor (#67745)"
This reverts commit 16eed8c. Causes some failures internally, will share privately with the author.
1 parent 6741dd0 commit 6420d33

File tree

4 files changed

+37
-97
lines changed

4 files changed

+37
-97
lines changed

‎llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp‎

Lines changed: 29 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -819,11 +819,11 @@ class ModuleAddressSanitizer {
819819
private:
820820
void initializeCallbacks(Module &M);
821821

822-
void instrumentGlobals(IRBuilder<> &IRB, Module &M, bool *CtorComdat);
822+
bool InstrumentGlobals(IRBuilder<> &IRB, Module &M, bool *CtorComdat);
823823
void InstrumentGlobalsCOFF(IRBuilder<> &IRB, Module &M,
824824
ArrayRef<GlobalVariable *> ExtendedGlobals,
825825
ArrayRef<Constant *> MetadataInitializers);
826-
void instrumentGlobalsELF(IRBuilder<> &IRB, Module &M,
826+
void InstrumentGlobalsELF(IRBuilder<> &IRB, Module &M,
827827
ArrayRef<GlobalVariable *> ExtendedGlobals,
828828
ArrayRef<Constant *> MetadataInitializers,
829829
const std::string &UniqueModuleId);
@@ -2177,7 +2177,7 @@ void ModuleAddressSanitizer::InstrumentGlobalsCOFF(
21772177
appendToCompilerUsed(M, MetadataGlobals);
21782178
}
21792179

2180-
void ModuleAddressSanitizer::instrumentGlobalsELF(
2180+
void ModuleAddressSanitizer::InstrumentGlobalsELF(
21812181
IRBuilder<> &IRB, Module &M, ArrayRef<GlobalVariable *> ExtendedGlobals,
21822182
ArrayRef<Constant *> MetadataInitializers,
21832183
const std::string &UniqueModuleId) {
@@ -2187,7 +2187,7 @@ void ModuleAddressSanitizer::instrumentGlobalsELF(
21872187
// false negative odr violations at link time. If odr indicators are used, we
21882188
// keep the comdat sections, as link time odr violations will be dectected on
21892189
// the odr indicator symbols.
2190-
bool UseComdatForGlobalsGC = UseOdrIndicator && !UniqueModuleId.empty();
2190+
bool UseComdatForGlobalsGC = UseOdrIndicator;
21912191

21922192
SmallVector<GlobalValue *, 16> MetadataGlobals(ExtendedGlobals.size());
21932193
for (size_t i = 0; i < ExtendedGlobals.size(); i++) {
@@ -2237,7 +2237,7 @@ void ModuleAddressSanitizer::instrumentGlobalsELF(
22372237

22382238
// We also need to unregister globals at the end, e.g., when a shared library
22392239
// gets closed.
2240-
if (DestructorKind != AsanDtorKind::None && !MetadataGlobals.empty()) {
2240+
if (DestructorKind != AsanDtorKind::None) {
22412241
IRBuilder<> IrbDtor(CreateAsanModuleDtor(M));
22422242
IrbDtor.CreateCall(AsanUnregisterElfGlobals,
22432243
{IRB.CreatePointerCast(RegisteredFlag, IntptrTy),
@@ -2343,8 +2343,10 @@ void ModuleAddressSanitizer::InstrumentGlobalsWithMetadataArray(
23432343
// redzones and inserts this function into llvm.global_ctors.
23442344
// Sets *CtorComdat to true if the global registration code emitted into the
23452345
// asan constructor is comdat-compatible.
2346-
void ModuleAddressSanitizer::instrumentGlobals(IRBuilder<> &IRB, Module &M,
2346+
bool ModuleAddressSanitizer::InstrumentGlobals(IRBuilder<> &IRB, Module &M,
23472347
bool *CtorComdat) {
2348+
*CtorComdat = false;
2349+
23482350
// Build set of globals that are aliased by some GA, where
23492351
// getExcludedAliasedGlobal(GA) returns the relevant GlobalVariable.
23502352
SmallPtrSet<const GlobalVariable *, 16> AliasedGlobalExclusions;
@@ -2362,6 +2364,11 @@ void ModuleAddressSanitizer::instrumentGlobals(IRBuilder<> &IRB, Module &M,
23622364
}
23632365

23642366
size_t n = GlobalsToChange.size();
2367+
if (n == 0) {
2368+
*CtorComdat = true;
2369+
return false;
2370+
}
2371+
23652372
auto &DL = M.getDataLayout();
23662373

23672374
// A global is described by a structure
@@ -2384,11 +2391,8 @@ void ModuleAddressSanitizer::instrumentGlobals(IRBuilder<> &IRB, Module &M,
23842391

23852392
// We shouldn't merge same module names, as this string serves as unique
23862393
// module ID in runtime.
2387-
GlobalVariable *ModuleName =
2388-
n != 0
2389-
? createPrivateGlobalForString(M, M.getModuleIdentifier(),
2390-
/*AllowMerging*/ false, kAsanGenPrefix)
2391-
: nullptr;
2394+
GlobalVariable *ModuleName = createPrivateGlobalForString(
2395+
M, M.getModuleIdentifier(), /*AllowMerging*/ false, kAsanGenPrefix);
23922396

23932397
for (size_t i = 0; i < n; i++) {
23942398
GlobalVariable *G = GlobalsToChange[i];
@@ -2513,34 +2517,27 @@ void ModuleAddressSanitizer::instrumentGlobals(IRBuilder<> &IRB, Module &M,
25132517
}
25142518
appendToCompilerUsed(M, ArrayRef<GlobalValue *>(GlobalsToAddToUsedList));
25152519

2516-
if (UseGlobalsGC && TargetTriple.isOSBinFormatELF()) {
2517-
// Use COMDAT and register globals even if n == 0 to ensure that (a) the
2518-
// linkage unit will only have one module constructor, and (b) the register
2519-
// function will be called. The module destructor is not created when n ==
2520-
// 0.
2520+
std::string ELFUniqueModuleId =
2521+
(UseGlobalsGC && TargetTriple.isOSBinFormatELF()) ? getUniqueModuleId(&M)
2522+
: "";
2523+
2524+
if (!ELFUniqueModuleId.empty()) {
2525+
InstrumentGlobalsELF(IRB, M, NewGlobals, Initializers, ELFUniqueModuleId);
25212526
*CtorComdat = true;
2522-
instrumentGlobalsELF(IRB, M, NewGlobals, Initializers,
2523-
getUniqueModuleId(&M));
2524-
} else if (n == 0) {
2525-
// When UseGlobalsGC is false, COMDAT can still be used if n == 0, because
2526-
// all compile units will have identical module constructor/destructor.
2527-
*CtorComdat = TargetTriple.isOSBinFormatELF();
2527+
} else if (UseGlobalsGC && TargetTriple.isOSBinFormatCOFF()) {
2528+
InstrumentGlobalsCOFF(IRB, M, NewGlobals, Initializers);
2529+
} else if (UseGlobalsGC && ShouldUseMachOGlobalsSection()) {
2530+
InstrumentGlobalsMachO(IRB, M, NewGlobals, Initializers);
25282531
} else {
2529-
*CtorComdat = false;
2530-
if (UseGlobalsGC && TargetTriple.isOSBinFormatCOFF()) {
2531-
InstrumentGlobalsCOFF(IRB, M, NewGlobals, Initializers);
2532-
} else if (UseGlobalsGC && ShouldUseMachOGlobalsSection()) {
2533-
InstrumentGlobalsMachO(IRB, M, NewGlobals, Initializers);
2534-
} else {
2535-
InstrumentGlobalsWithMetadataArray(IRB, M, NewGlobals, Initializers);
2536-
}
2532+
InstrumentGlobalsWithMetadataArray(IRB, M, NewGlobals, Initializers);
25372533
}
25382534

25392535
// Create calls for poisoning before initializers run and unpoisoning after.
25402536
if (HasDynamicallyInitializedGlobals)
25412537
createInitializerPoisonCalls(M, ModuleName);
25422538

25432539
LLVM_DEBUG(dbgs() << M);
2540+
return true;
25442541
}
25452542

25462543
uint64_t
@@ -2604,10 +2601,10 @@ bool ModuleAddressSanitizer::instrumentModule(Module &M) {
26042601
assert(AsanCtorFunction || ConstructorKind == AsanCtorKind::None);
26052602
if (AsanCtorFunction) {
26062603
IRBuilder<> IRB(AsanCtorFunction->getEntryBlock().getTerminator());
2607-
instrumentGlobals(IRB, M, &CtorComdat);
2604+
InstrumentGlobals(IRB, M, &CtorComdat);
26082605
} else {
26092606
IRBuilder<> IRB(*C);
2610-
instrumentGlobals(IRB, M, &CtorComdat);
2607+
InstrumentGlobals(IRB, M, &CtorComdat);
26112608
}
26122609
}
26132610

‎llvm/test/Instrumentation/AddressSanitizer/basic.ll‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,8 @@ define void @test_swifterror_3() sanitize_address {
210210

211211
;; ctor/dtor have the nounwind attribute. See uwtable.ll, they additionally have
212212
;; the uwtable attribute with the module flag "uwtable".
213-
; CHECK: define internal void @asan.module_ctor() #[[#ATTR:]] comdat {
213+
; CHECK: define internal void @asan.module_ctor() #[[#ATTR:]] {{(comdat )?}}{
214214
; CHECK: call void @__asan_init()
215-
;; __asan_register_elf_globals is called even if this module does not contain instrumented global variables.
216-
; 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))
217215

218216
; CHECK: attributes #[[#ATTR]] = { nounwind }
219217

‎llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
1-
; RUN: rm -rf %t && split-file %s %t && cd %t
2-
; RUN: opt < a.ll -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-unknown-linux-gnu -S | FileCheck --check-prefix=CHECK %s
3-
; RUN: opt < a.ll -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-apple-macosx10.11.0 -S | FileCheck --check-prefix=CHECK %s
4-
; 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
5-
; 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
1+
; RUN: opt < %s -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-unknown-linux-gnu -S | FileCheck --check-prefix=CHECK %s
2+
; RUN: opt < %s -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-apple-macosx10.11.0 -S | FileCheck --check-prefix=CHECK %s
3+
; RUN: opt < %s -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-pc-windows-msvc19.0.24215 -S | FileCheck --check-prefix=CHECK %s
4+
; 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
65

7-
; RUN: opt < empty.ll -passes=asan -asan-globals-live-support=0 -mtriple=x86_64-unknown-linux-gnu -S | FileCheck --check-prefix=ELF-NOGC %s
8-
9-
;--- a.ll
106
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
117

128
; Globals:
@@ -63,10 +59,3 @@ attributes #1 = { nounwind sanitize_address "less-precise-fpmad"="false" "frame-
6359
!7 = !{!"/tmp/asan-globals.cpp", i32 7, i32 5}
6460
!8 = !{!"/tmp/asan-globals.cpp", i32 12, i32 14}
6561
!9 = !{!"/tmp/asan-globals.cpp", i32 14, i32 25}
66-
67-
;; In the presence of instrumented global variables, asan.module_ctor do not use comdat.
68-
; CHECK: define internal void @asan.module_ctor() #[[#ATTR:]] {
69-
70-
; ELF-NOGC: define internal void @asan.module_ctor() #[[#ATTR:]] comdat {
71-
72-
;--- empty.ll

‎llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll‎

Lines changed: 3 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,10 @@
44
; We keep using comdats for garbage collection if odr indicators are
55
; enabled as indicator symbols will cause link time odr violations.
66
; This is to fix PR 47925.
7-
; RUN: rm -rf %t && split-file %s %t && cd %t
8-
; RUN: opt < a.ll -passes=asan -asan-globals-live-support=1 -asan-use-odr-indicator=0 -S | FileCheck %s --check-prefixes=CHECK,NOCOMDAT
7+
;
8+
; RUN: opt < %s -passes=asan -asan-globals-live-support=1 -asan-use-odr-indicator=0 -S | FileCheck %s --check-prefixes=CHECK,NOCOMDAT
99
; Check that enabling odr indicators enables comdat for globals.
10-
; RUN: opt < a.ll -passes=asan -asan-globals-live-support=1 -S | FileCheck %s --check-prefixes=CHECK,COMDAT
11-
12-
; RUN: opt < no_module_id.ll -passes=asan -S | FileCheck %s --check-prefix=NOMODULEID
13-
14-
;--- a.ll
10+
; RUN: opt < %s -passes=asan -asan-globals-live-support=1 -S | FileCheck %s --check-prefixes=CHECK,COMDAT
1511
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
1612
target triple = "x86_64-unknown-linux-gnu"
1713

@@ -91,43 +87,3 @@ attributes #1 = { nounwind sanitize_address "less-precise-fpmad"="false" "frame-
9187
!7 = !{!"/tmp/asan-globals.cpp", i32 7, i32 5}
9288
!8 = !{!"/tmp/asan-globals.cpp", i32 12, i32 14}
9389
!9 = !{!"/tmp/asan-globals.cpp", i32 14, i32 25}
94-
95-
;--- no_module_id.ll
96-
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
97-
target triple = "x86_64-unknown-linux-gnu"
98-
99-
; NOMODULEID: $asan.module_ctor = comdat any
100-
; NOMODULEID: $asan.module_dtor = comdat any
101-
102-
;; Don't place the instrumented globals in a comdat when the unique module ID is empty.
103-
; NOMODULEID: @.str = internal constant { [4 x i8], [28 x i8] } { [4 x i8] c"str\00", [28 x i8] zeroinitializer }, align 32
104-
; NOMODULEID: @_ZL3buf = internal global { [4 x i8], [28 x i8] } zeroinitializer, align 32
105-
; NOMODULEID: @__asan_global_.str = private global {{.*}}, section "asan_globals", !associated !0
106-
; NOMODULEID: @__asan_global__ZL3buf = private global {{.*}}, section "asan_globals", !associated !1
107-
; NOMODULEID: @llvm.compiler.used = appending global [4 x ptr] [ptr @.str, ptr @_ZL3buf, ptr @__asan_global_.str, ptr @__asan_global__ZL3buf]
108-
109-
; NOMODULEID: define internal void @asan.module_ctor() #[[#]] comdat {
110-
; NOMODULEID-NEXT: call void @__asan_init()
111-
; NOMODULEID-NEXT: call void @__asan_version_mismatch_check_v8()
112-
; 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))
113-
; NOMODULEID-NEXT: ret void
114-
; NOMODULEID-NEXT: }
115-
116-
; NOMODULEID: !0 = !{ptr @.str}
117-
; NOMODULEID: !1 = !{ptr @_ZL3buf}
118-
119-
@.str = private unnamed_addr constant [4 x i8] c"str\00", align 1
120-
@_ZL3buf = internal unnamed_addr global [4 x i8] zeroinitializer, align 1
121-
@llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__sub_I_a.cc, ptr null }]
122-
123-
declare void @ext(ptr noundef)
124-
125-
; Function Attrs: uwtable
126-
define internal void @_GLOBAL__sub_I_a.cc() #2 section ".text.startup" {
127-
entry:
128-
%0 = load i8, ptr @_ZL3buf, align 1
129-
%inc = add i8 %0, 1
130-
store i8 %inc, ptr @_ZL3buf, align 1
131-
tail call void @ext(ptr noundef nonnull @.str)
132-
ret void
133-
}

0 commit comments

Comments
 (0)