[asan] Don't check ODR violations for particular types of globals

[vitalybuka]

Summary

private and internal: should not trigger ODR at all.
unnamed_addr: current ODR checking approach fail and rereport false violation if
a linker merges such globals
linkonce_odr, weak_odr: could cause similar problems and they are already not
instrumented for ELF.

[eugenis]

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

2193

did you mean "from linker optimizations"?

[vitalybuka]

typo

[eugenis]

LGTM

clang/test/CodeGen/asan-static-odr.cpp

+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-linux %s | FileCheck %s --check-prefixes=CHECK,ALIAS1
+
+// No alias on Windows but indicators should work.
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-windows-msvc %s | FileCheck %s --check-prefixes=CHECK,ALIAS0
+
+static int global;
+
+int main() {
+  return global;
+}
+
+// CHECK-NOT: __odr_asan_gen
+// CHECK-NOT: private alias
+// CHECK: [[VAR:@.*global.*]] ={{.*}} global { i32, [60 x i8] } zeroinitializer, align 32
+// CHECK: @0 = internal global {{.*}} [[VAR]] to i64), {{.*}}, i64 -1 }]
+// CHECK: call void @__asan_register_globals(i64 ptrtoint ([1 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)
+// CHECK: call void @__asan_unregister_globals(i64 ptrtoint ([1 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)

compiler-rt/lib/asan/asan_globals.cc

 
 static bool IsAddressNearGlobal(uptr addr, const __asan_global &g) {
   if (addr <= g.beg - kMinimalDistanceFromAnotherGlobal) return false;
   if (addr >= g.beg + g.size_with_redzone) return false;
   return true;
 }
 
 static void ReportGlobal(const Global &g, const char *prefix) {
-  Report("%s Global[%p]: beg=%p size=%zu/%zu name=%s module=%s dyn_init=%zu\n",
-         prefix, &g, (void *)g.beg, g.size, g.size_with_redzone, g.name,
-         g.module_name, g.has_dynamic_init);
+  Report(
+      "%s Global[%p]: beg=%p size=%zu/%zu name=%s module=%s dyn_init=%zu "
+      "odr_indicator=%p\n",
+      g.module_name, g.has_dynamic_init, (void *)g.odr_indicator);
   if (g.location) {
     Report("  location (%p): name=%s[%p], %d %d\n", g.location,
            g.location->filename, g.location->filename, g.location->line_no,
            g.location->column_no);
   }
 }
 
 static u32 FindRegistrationSite(const Global *g) {
enum GlobalSymbolState {
   UNREGISTERED = 0,
   REGISTERED = 1
 };
 
 // Check ODR violation for given global G via special ODR indicator. We use
 // this method in case compiler instruments global variables through their
 // local aliases.
 static void CheckODRViolationViaIndicator(const Global *g) {
+  // Instrumentation requests to skip ODR check.
+  if (g->odr_indicator == UINTPTR_MAX)
+    return;
   u8 *odr_indicator = reinterpret_cast<u8 *>(g->odr_indicator);
   if (*odr_indicator == UNREGISTERED) {
     *odr_indicator = REGISTERED;
     return;
   }
   // If *odr_indicator is DEFINED, some module have already registered
   // externally visible symbol with the same name. This is an ODR violation.
   for (ListOfGlobals *l = list_of_all_globals; l; l = l->next) {
static void UnregisterGlobal(const Global *g) {
   CHECK(AddrIsAlignedByGranularity(g->size_with_redzone));
   if (CanPoisonMemory())
     PoisonShadowForGlobal(g, 0);
   // We unpoison the shadow memory for the global but we do not remove it from
   // the list because that would require O(n^2) time with the current list
   // implementation. It might not be worth doing anyway.
 
   // Release ODR indicator.
-  if (UseODRIndicator(g)) {
+  if (UseODRIndicator(g) && g->odr_indicator != UINTPTR_MAX) {
     u8 *odr_indicator = reinterpret_cast<u8 *>(g->odr_indicator);
     *odr_indicator = UNREGISTERED;
   }
 }
 
 void StopInitOrderChecking() {
   BlockingMutexLock lock(&mu_for_globals);
   if (!flags()->check_initialization_order || !dynamic_init_globals)

compiler-rt/test/asan/TestCases/Linux/odr_indicators.cc

+// RUN: %clangxx_asan -fPIC %s -o %t
+// RUN: %env_asan_opts=report_globals=2 %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK,INDICATOR0
+
+// RUN: %clangxx_asan -fsanitize-address-use-odr-indicator -fPIC %s -o %t
+// RUN: %env_asan_opts=report_globals=2 %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK,INDICATOR1
+
+#include <stdio.h>
+
+int test_global_1;
+// INDICATOR0-DAG: Added Global{{.*}} name=test_global_1{{.*}} odr_indicator={{0x0+$}}
+// INDICATOR1-DAG: Added Global{{.*}} name=test_global_1{{.*}} odr_indicator={{0x0*[^0]+.*$}}
+
+static int test_global_2;
+// CHECK-DAG: Added Global{{.*}} name=test_global_2{{.*}} odr_indicator={{0xf+$}}
+
+namespace {
+static int test_global_3;
+// CHECK-DAG: Added Global{{.*}} name={{.*}}::test_global_3{{.*}} odr_indicator={{0xf+$}}
+} // namespace
+
+int main() {
+  const char f[] = "%d %d %d\n";
+  // CHECK-DAG: Added Global{{.*}} name=__const.main.f{{.*}} odr_indicator={{0xf+$}}
+  printf(f, test_global_1, test_global_2, test_global_3);
+  return 0;
+}

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

bool CanUsePrivateAliases =
         TargetTriple.isOSBinFormatWasm();
     if (CanUsePrivateAliases && UsePrivateAlias) {
       // Create local alias for NewGlobal to avoid crash on ODR between
       // instrumented and non-instrumented libraries.
       InstrumentedGlobal =
           GlobalAlias::create(GlobalValue::PrivateLinkage, "", NewGlobal);
     }
 
-    if (UseOdrIndicator) {
+    // ODR check is not useful for the following, but we see false reports are
+    // linker optimizations.
+    if (NewGlobal->hasLocalLinkage() || NewGlobal->hasGlobalUnnamedAddr() ||
+        NewGlobal->hasLinkOnceODRLinkage() || NewGlobal->hasWeakODRLinkage()) {
+      ODRIndicator = ConstantExpr::getIntToPtr(ConstantInt::get(IntptrTy, -1),
+                                               IRB.getInt8PtrTy());
+    } else if (UseOdrIndicator) {
       // With local aliases, we need to provide another externally visible
       // symbol __odr_asan_XXX to detect ODR violation.
       auto *ODRIndicatorSym =
           new GlobalVariable(M, IRB.getInt8Ty(), false, Linkage,
                              Constant::getNullValue(IRB.getInt8Ty()),
                              kODRGenPrefix + NameForGlobal, nullptr,
                              NewGlobal->getThreadLocalMode());
 

llvm/test/Instrumentation/AddressSanitizer/do-not-touch-odr-global.ll

 ; This test checks that we instrument regular globals, but do not touch
 ; the linkonce_odr ones.
 ; RUN: opt < %s -asan -asan-module -S | FileCheck %s
 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"
 ; no action should be taken for these globals
 @global_noinst = linkonce_odr constant [2 x i8] [i8 1, i8 2]
+@global_weak_noinst = weak_odr constant [2 x i8] [i8 1, i8 2]
 @global_inst = private constant [2 x i8] [i8 1, i8 2]
 ; CHECK-NOT: {{asan_gen.*global_noinst}}
+; CHECK-NOT: {{asan_gen.*global_weak_noinst}}
 ; CHECK: {{asan_gen.*global_inst}}
 ; CHECK: @asan.module_ctor

llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll

 ; CHECK: @global = global { i32, [60 x i8] } zeroinitializer, comdat, align 32
 ; CHECK: @.str = internal unnamed_addr 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 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 }
 ; CHECK: @__asan_global_global = {{.*}}i64 ptrtoint ({ i32, [60 x i8] }* @global to i64){{.*}} section "asan_globals"{{.*}}, !associated
-; CHECK: @__asan_global_.str = {{.*}}i64 ptrtoint ({ [14 x i8], [50 x i8] }* @.str to i64){{.*}} section "asan_globals"{{.*}}, !associated
+; CHECK: @__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))
 

llvm/test/Instrumentation/AddressSanitizer/local_alias.ll

 ; RUN: opt < %s -asan -asan-module -S | FileCheck %s --check-prefixes=CHECK-NOALIAS,CHECK-NOINDICATOR
 ; RUN: opt < %s -asan -asan-module -asan-use-private-alias=1 -S | FileCheck %s --check-prefixes=CHECK-ALIAS,CHECK-NOINDICATOR
 ; RUN: opt < %s -asan -asan-module -asan-use-odr-indicator=1 -S | FileCheck %s --check-prefixes=CHECK-INDICATOR,CHECK-NOALIAS
 ; RUN: opt < %s -asan -asan-module -asan-use-private-alias=1 -asan-use-odr-indicator=1 -S | FileCheck %s --check-prefixes=CHECK-ALIAS,CHECK-INDICATOR
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@a = internal global [2 x i32] zeroinitializer, align 4
+@a = dso_local global [2 x i32] zeroinitializer, align 4
+@b = private global [2 x i32] zeroinitializer, align 4
+@c = internal global [2 x i32] zeroinitializer, align 4
+@d = unnamed_addr global [2 x i32] zeroinitializer, align 4
 
 ; Check that we generate internal alias and odr indicator symbols for global to be protected.
 ; CHECK-NOINDICATOR-NOT: __odr_asan_gen_a
 ; CHECK-NOALIAS-NOT: private alias
-; CHECK-INDICATOR: @__odr_asan_gen_a = internal global i8 0, align 1
+; CHECK-INDICATOR: @__odr_asan_gen_a = global i8 0, align 1
 ; CHECK-ALIAS: @0 = private alias { [2 x i32], [56 x i8] }, { [2 x i32], [56 x i8] }* @a
 
+; CHECK-ALIAS: @1 = private alias { [2 x i32], [56 x i8] }, { [2 x i32], [56 x i8] }* @b
+; CHECK-ALIAS: @2 = private alias { [2 x i32], [56 x i8] }, { [2 x i32], [56 x i8] }* @c
+; CHECK-ALIAS: @3 = private alias { [2 x i32], [56 x i8] }, { [2 x i32], [56 x i8] }* @d
+
 ; Function Attrs: nounwind sanitize_address uwtable
 define i32 @foo(i32 %M) #0 {
 entry:
   %M.addr = alloca i32, align 4
   store i32 %M, i32* %M.addr, align 4
   store volatile i32 6, i32* getelementptr inbounds ([2 x i32], [2 x i32]* @a, i64 2, i64 0), align 4
   %0 = load i32, i32* %M.addr, align 4
   %idxprom = sext i32 %0 to i64
   %arrayidx = getelementptr inbounds [2 x i32], [2 x i32]* @a, i64 0, i64 %idxprom
   %1 = load volatile i32, i32* %arrayidx, align 4
   ret i32 %1
 }

llvm/test/Instrumentation/AddressSanitizer/odr-check-ignore.ll

+; RUN: opt < %s -asan -asan-module -S | FileCheck %s 
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@a = global [2 x i32] zeroinitializer, align 4
+@b = private global [2 x i32] zeroinitializer, align 4
+@c = internal global [2 x i32] zeroinitializer, align 4
+@d = unnamed_addr global [2 x i32] zeroinitializer, align 4
+
+; CHECK: @__asan_global_a = private global { i64, i64, i64, i64, i64, i64, i64, i64 } { i64 ptrtoint ({ [2 x i32], [56 x i8] }* @a to i64), i64 8, i64 64, i64 ptrtoint ([2 x i8]* @___asan_gen_.1 to i64), i64 ptrtoint ([8 x i8]* @___asan_gen_ to i64), i64 0, i64 0, i64 0 }
+
+; CHECK: @__asan_global_b = private global { i64, i64, i64, i64, i64, i64, i64, i64 } { i64 ptrtoint ({ [2 x i32], [56 x i8] }* @b to i64), i64 8, i64 64, i64 ptrtoint ([2 x i8]* @___asan_gen_.2 to i64), i64 ptrtoint ([8 x i8]* @___asan_gen_ to i64), i64 0, i64 0, i64 -1 }
+
+; CHECK: @__asan_global_c = private global { i64, i64, i64, i64, i64, i64, i64, i64 } { i64 ptrtoint ({ [2 x i32], [56 x i8] }* @c to i64), i64 8, i64 64, i64 ptrtoint ([2 x i8]* @___asan_gen_.3 to i64), i64 ptrtoint ([8 x i8]* @___asan_gen_ to i64), i64 0, i64 0, i64 -1 }
+
+; CHECK: @__asan_global_d = private global { i64, i64, i64, i64, i64, i64, i64, i64 } { i64 ptrtoint ({ [2 x i32], [56 x i8] }* @d to i64), i64 8, i64 64, i64 ptrtoint ([2 x i8]* @___asan_gen_.4 to i64), i64 ptrtoint ([8 x i8]* @___asan_gen_ to i64), i64 0, i64 0, i64 -1 }

llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll

 ; CHECK-SAME: linkonce_odr dso_local unnamed_addr constant { [5 x i8], [59 x i8] }
 ; CHECK-SAME: { [5 x i8] c"asdf\00", [59 x i8] zeroinitializer }, comdat, align 32
 
 ; CHECK: @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
 ; CHECK-SAME: private global { i64, i64, i64, i64, i64, i64, i64, i64 }
 ; CHECK-SAME: { i64 ptrtoint ({ [5 x i8], [59 x i8] }* @"??_C@_04JIHMPGLA@asdf?$AA@" to i64),
 ; CHECK-SAME:   i64 5, i64 64, i64 ptrtoint ([17 x i8]* @___asan_gen_.1 to i64),
 ; CHECK-SAME:   i64 ptrtoint ([8 x i8]* @___asan_gen_ to i64), i64 0,
-; CHECK-SAME:   i64 ptrtoint ({ [6 x i8]*, i32, i32 }* @___asan_gen_.3 to i64), i64 0 },
+; CHECK-SAME:   i64 ptrtoint ({ [6 x i8]*, i32, i32 }* @___asan_gen_.3 to i64), i64 -1 },
 ; CHECK-SAME:   section ".ASAN$GL", comdat($"??_C@_04JIHMPGLA@asdf?$AA@"), align 64
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
 target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-windows-msvc19.13.26131"
 
 $"??_C@_04JIHMPGLA@asdf?$AA@" = comdat any