[llvm] 27830bc - [asan] Avoid putting globals in a comdat section when targetting elf.
Pierre Gousseau via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 24 04:07:30 PST 2021
Author: Pierre Gousseau
Date: 2021-02-24T12:01:56Z
New Revision: 27830bc2b1b876b1948bcd8ae7ee62819be69758
URL: https://github.com/llvm/llvm-project/commit/27830bc2b1b876b1948bcd8ae7ee62819be69758
DIFF: https://github.com/llvm/llvm-project/commit/27830bc2b1b876b1948bcd8ae7ee62819be69758.diff
LOG: [asan] Avoid putting globals in a comdat section when targetting elf.
Putting globals in a comdat for dead-stripping changes the semantic and
can potentially cause 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 for the odr indicator symbols.
This fixes PR 47925
Added:
llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
Modified:
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
llvm/test/Instrumentation/AddressSanitizer/instrument-section-invalid-c-ident.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index bb7fc123ff5a..dcfff19de013 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2125,6 +2125,12 @@ void ModuleAddressSanitizer::InstrumentGlobalsELF(
const std::string &UniqueModuleId) {
assert(ExtendedGlobals.size() == MetadataInitializers.size());
+ // Putting globals in a comdat changes the semantic and potentially cause
+ // 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;
+
SmallVector<GlobalValue *, 16> MetadataGlobals(ExtendedGlobals.size());
for (size_t i = 0; i < ExtendedGlobals.size(); i++) {
GlobalVariable *G = ExtendedGlobals[i];
@@ -2134,7 +2140,8 @@ void ModuleAddressSanitizer::InstrumentGlobalsELF(
Metadata->setMetadata(LLVMContext::MD_associated, MD);
MetadataGlobals[i] = Metadata;
- SetComdatForGlobalMetadata(G, Metadata, UniqueModuleId);
+ if (UseComdatForGlobalsGC)
+ SetComdatForGlobalMetadata(G, Metadata, UniqueModuleId);
}
// Update llvm.compiler.used, adding the new metadata globals. This is
diff --git a/llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll b/llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
index 220ab5af8bc0..53bcd468c811 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
@@ -19,8 +19,8 @@ target triple = "x86_64-unknown-linux-gnu"
; Check that globals were instrumented:
-; CHECK: @global = global { i32, [60 x i8] } zeroinitializer, comdat, align 32
-; CHECK: @.str = internal constant { [14 x i8], [50 x i8] } { [14 x i8] c"Hello, world!\00", [50 x i8] zeroinitializer }, comdat($".str${{[01-9a-f]+}}"), align 32
+; CHECK: @global = global { i32, [60 x i8] } zeroinitializer, align 32
+; CHECK: @.str = internal constant { [14 x i8], [50 x i8] } { [14 x i8] c"Hello, world!\00", [50 x i8] zeroinitializer }, align 32
; Check emitted location descriptions:
; CHECK: [[VARNAME:@___asan_gen_.[0-9]+]] = private unnamed_addr constant [7 x i8] c"global\00", align 1
diff --git a/llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll b/llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
new file mode 100644
index 000000000000..e7df431ca24a
--- /dev/null
+++ b/llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
@@ -0,0 +1,100 @@
+; Check that we disable the use of comdat sections for the garbage collection
+; of globals.
+; This is to avoid false negative ODR violations detection at link time.
+; 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: opt < %s -asan -asan-module -enable-new-pm=0 -asan-globals-live-support=1 -S | FileCheck %s --check-prefixes=CHECK,NOCOMDAT
+; RUN: opt < %s -passes='asan-pipeline' -asan-globals-live-support=1 -S | FileCheck %s --check-prefixes=CHECK,NOCOMDAT
+; Check that enabling odr indicators enables comdat for globals.
+; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -asan-globals-live-support=1 -asan-use-odr-indicator=1 -S | FileCheck %s --check-prefixes=CHECK,COMDAT
+; RUN: opt < %s -passes='asan-pipeline' -asan-globals-live-support=1 -asan-use-odr-indicator=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"
+
+; Globals:
+ at global = global i32 0, align 4
+ at dyn_init_global = global i32 0, align 4
+ at blacklisted_global = global i32 0, align 4
+ at _ZZ4funcvE10static_var = internal global i32 0, align 4
+ at .str = private unnamed_addr constant [14 x i8] c"Hello, world!\00", align 1
+ at llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_asan_globals.cpp, i8* null }]
+
+; Check that globals were instrumented:
+
+; COMDAT: $global = comdat any
+; COMDAT: $dyn_init_global = comdat any
+; COMDAT: $"_ZZ4funcvE10static_var${{[01-9a-f]+}}" = comdat any
+; COMDAT: $".str${{[01-9a-f]+}}" = comdat any
+
+; NOCOMDAT-NOT: $global = comdat any
+; NOCOMDAT-NOT: $dyn_init_global = comdat any
+; NOCOMDAT-NOT: $"_ZZ4funcvE10static_var${{[01-9a-f]+}}" = comdat any
+; NOCOMDAT-NOT: $".str${{[01-9a-f]+}}" = comdat any
+
+; COMDAT: @global = global { i32, [60 x i8] } zeroinitializer, comdat, align 32
+; COMDAT: @dyn_init_global = global { i32, [60 x i8] } zeroinitializer, comdat, align 32
+; COMDAT: @_ZZ4funcvE10static_var = internal global { i32, [60 x i8] } zeroinitializer, comdat($"_ZZ4funcvE10static_var${{[01-9a-f]+}}"), align 32
+; COMDAT: @.str = internal constant { [14 x i8], [50 x i8] } { [14 x i8] c"Hello, world!\00", [50 x i8] zeroinitializer }, comdat($".str${{[01-9a-f]+}}"), align 32
+
+; NOCOMDAT: @global = global { i32, [60 x i8] } zeroinitializer, align 32
+; NOCOMDAT: @dyn_init_global = global { i32, [60 x i8] } zeroinitializer, align 32
+; NOCOMDAT: @_ZZ4funcvE10static_var = internal global { i32, [60 x i8] } zeroinitializer, align 32
+; NOCOMDAT: @.str = internal constant { [14 x i8], [50 x i8] } { [14 x i8] c"Hello, world!\00", [50 x i8] zeroinitializer }, align 32
+
+; Check emitted location descriptions:
+; CHECK: [[VARNAME:@___asan_gen_.[0-9]+]] = private unnamed_addr constant [7 x i8] c"global\00", align 1
+; CHECK: [[FILENAME:@___asan_gen_.[0-9]+]] = private unnamed_addr constant [22 x i8] c"/tmp/asan-globals.cpp\00", align 1
+; CHECK: [[LOCDESCR:@___asan_gen_.[0-9]+]] = private unnamed_addr constant { [22 x i8]*, i32, i32 } { [22 x i8]* [[FILENAME]], i32 5, i32 5 }
+; COMDAT: @__asan_global_global = {{.*}}i64 ptrtoint ({ i32, [60 x i8] }* @global to i64){{.*}} section "asan_globals"{{.*}}, !associated
+; COMDAT: @__asan_global_.str = {{.*}}i64 ptrtoint ({ [14 x i8], [50 x i8] }* @{{.str|1}} to i64){{.*}} section "asan_globals"{{.*}}, !associated
+
+; The metadata has to be inserted to llvm.compiler.used to avoid being stripped
+; during LTO.
+; CHECK: @llvm.compiler.used {{.*}} @__asan_global_global {{.*}} section "llvm.metadata"
+
+; Check that location descriptors and global names were passed into __asan_register_globals:
+; CHECK: call void @__asan_register_elf_globals(i64 ptrtoint (i64* @___asan_globals_registered to i64), i64 ptrtoint (i64* @__start_asan_globals to i64), i64 ptrtoint (i64* @__stop_asan_globals to i64))
+
+; Function Attrs: nounwind sanitize_address
+define internal void @__cxx_global_var_init() #0 section ".text.startup" {
+entry:
+ %0 = load i32, i32* @global, align 4
+ store i32 %0, i32* @dyn_init_global, align 4
+ ret void
+}
+
+; Function Attrs: nounwind sanitize_address
+define void @_Z4funcv() #1 {
+entry:
+ %literal = alloca i8*, align 8
+ store i8* getelementptr inbounds ([14 x i8], [14 x i8]* @.str, i32 0, i32 0), i8** %literal, align 8
+ ret void
+}
+
+; Function Attrs: nounwind sanitize_address
+define internal void @_GLOBAL__sub_I_asan_globals.cpp() #0 section ".text.startup" {
+entry:
+ call void @__cxx_global_var_init()
+ ret void
+}
+
+attributes #0 = { nounwind sanitize_address }
+attributes #1 = { nounwind sanitize_address "less-precise-fpmad"="false" "frame-pointer"="none" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-realign-stack" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.asan.globals = !{!0, !1, !2, !3, !4}
+!llvm.ident = !{!5}
+
+!0 = !{i32* @global, !6, !"global", i1 false, i1 false}
+!1 = !{i32* @dyn_init_global, !7, !"dyn_init_global", i1 true, i1 false}
+!2 = !{i32* @blacklisted_global, null, null, i1 false, i1 true}
+!3 = !{i32* @_ZZ4funcvE10static_var, !8, !"static_var", i1 false, i1 false}
+!4 = !{[14 x i8]* @.str, !9, !"<string literal>", i1 false, i1 false}
+
+!5 = !{!"clang version 3.5.0 (211282)"}
+
+!6 = !{!"/tmp/asan-globals.cpp", i32 5, i32 5}
+!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}
diff --git a/llvm/test/Instrumentation/AddressSanitizer/instrument-section-invalid-c-ident.ll b/llvm/test/Instrumentation/AddressSanitizer/instrument-section-invalid-c-ident.ll
index a320f797a713..ca4468ad05dc 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/instrument-section-invalid-c-ident.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/instrument-section-invalid-c-ident.ll
@@ -11,7 +11,7 @@ module asm ".hidden invalid$c$name"
@data2 = dso_local global i32 2, align 4
@__invalid$c$name_sym_data1 = internal constant i8* bitcast (i32* @data1 to i8*), section "invalid$c$name", align 8
@__invalid$c$name_sym_data2 = internal constant i8* bitcast (i32* @data2 to i8*), section "invalid$c$name", align 8
-; CHECK: @"__invalid$c$name_sym_data1" = internal constant{{.*}}, section "invalid$c$name", comdat
-; CHECK-NEXT: @"__invalid$c$name_sym_data2" = internal constant{{.*}}, section "invalid$c$name", comdat
+; CHECK: @"__invalid$c$name_sym_data1" = internal constant{{.*}}, section "invalid$c$name"
+; CHECK-NEXT: @"__invalid$c$name_sym_data2" = internal constant{{.*}}, section "invalid$c$name"
; CHECK: @"__asan_global___invalid$c$name_sym_data1"
; CHECK-NEXT: @"__asan_global___invalid$c$name_sym_data2"
More information about the llvm-commits
mailing list