[compiler-rt] [asan] Speed up ASan ODR indicator-based checking (PR #100923)

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 1 09:58:22 PDT 2024


https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/100923

>From 4dc35066e2333cbaff1e5af445e1e38641e264f8 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 1 Aug 2024 09:49:00 -0700
Subject: [PATCH 1/8] Revert "[NFC][asan][odr] Use IntrusiveList for a
 ListOfGlobals"

This reverts commit 2a5f7e58d71c394b241fdd1d905041ad0537acab.
---
 compiler-rt/lib/asan/asan_globals.cpp | 51 ++++++++++++---------------
 1 file changed, 22 insertions(+), 29 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index ef886e3d484ed..d413b1ebc9fc0 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -21,7 +21,6 @@
 #include "asan_suppressions.h"
 #include "asan_thread.h"
 #include "sanitizer_common/sanitizer_common.h"
-#include "sanitizer_common/sanitizer_list.h"
 #include "sanitizer_common/sanitizer_mutex.h"
 #include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
@@ -31,14 +30,13 @@ namespace __asan {
 
 typedef __asan_global Global;
 
-struct GlobalListNode {
-  const Global *g = nullptr;
-  GlobalListNode *next = nullptr;
+struct ListOfGlobals {
+  const Global *g;
+  ListOfGlobals *next;
 };
-typedef IntrusiveList<GlobalListNode> ListOfGlobals;
 
 static Mutex mu_for_globals;
-static ListOfGlobals list_of_all_globals;
+static ListOfGlobals *list_of_all_globals;
 
 static const int kDynamicInitGlobalsInitialCapacity = 512;
 struct DynInitGlobal {
@@ -75,10 +73,6 @@ ALWAYS_INLINE void PoisonRedZones(const Global &g) {
 
 const uptr kMinimalDistanceFromAnotherGlobal = 64;
 
-static void AddGlobalToList(ListOfGlobals &list, const Global *g) {
-  list.push_front(new (GetGlobalLowLevelAllocator()) GlobalListNode{g});
-}
-
 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;
@@ -120,8 +114,8 @@ int GetGlobalsForAddress(uptr addr, Global *globals, u32 *reg_sites,
   if (!flags()->report_globals) return 0;
   Lock lock(&mu_for_globals);
   int res = 0;
-  for (const auto &l : list_of_all_globals) {
-    const Global &g = *l.g;
+  for (ListOfGlobals *l = list_of_all_globals; l; l = l->next) {
+    const Global &g = *l->g;
     if (flags()->report_globals >= 2)
       ReportGlobal(g, "Search");
     if (IsAddressNearGlobal(addr, g)) {
@@ -155,13 +149,12 @@ static void CheckODRViolationViaIndicator(const Global *g) {
   }
   // If *odr_indicator is DEFINED, some module have already registered
   // externally visible symbol with the same name. This is an ODR violation.
-  for (const auto &l : list_of_all_globals) {
-    if (g->odr_indicator == l.g->odr_indicator &&
-        (flags()->detect_odr_violation >= 2 || g->size != l.g->size) &&
-        !IsODRViolationSuppressed(g->name)) {
-      ReportODRViolation(g, FindRegistrationSite(g), l.g,
-                         FindRegistrationSite(l.g));
-    }
+  for (ListOfGlobals *l = list_of_all_globals; l; l = l->next) {
+    if (g->odr_indicator == l->g->odr_indicator &&
+        (flags()->detect_odr_violation >= 2 || g->size != l->g->size) &&
+        !IsODRViolationSuppressed(g->name))
+      ReportODRViolation(g, FindRegistrationSite(g),
+                         l->g, FindRegistrationSite(l->g));
   }
 }
 
@@ -172,13 +165,12 @@ static void CheckODRViolationViaPoisoning(const Global *g) {
   if (__asan_region_is_poisoned(g->beg, g->size_with_redzone)) {
     // This check may not be enough: if the first global is much larger
     // the entire redzone of the second global may be within the first global.
-    for (const auto &l : list_of_all_globals) {
-      if (g->beg == l.g->beg &&
-          (flags()->detect_odr_violation >= 2 || g->size != l.g->size) &&
-          !IsODRViolationSuppressed(g->name)) {
-        ReportODRViolation(g, FindRegistrationSite(g), l.g,
-                           FindRegistrationSite(l.g));
-      }
+    for (ListOfGlobals *l = list_of_all_globals; l; l = l->next) {
+      if (g->beg == l->g->beg &&
+          (flags()->detect_odr_violation >= 2 || g->size != l->g->size) &&
+          !IsODRViolationSuppressed(g->name))
+        ReportODRViolation(g, FindRegistrationSite(g),
+                           l->g, FindRegistrationSite(l->g));
     }
   }
 }
@@ -233,9 +225,10 @@ static void RegisterGlobal(const Global *g) {
   }
   if (CanPoisonMemory())
     PoisonRedZones(*g);
-
-  AddGlobalToList(list_of_all_globals, g);
-
+  ListOfGlobals *l = new (GetGlobalLowLevelAllocator()) ListOfGlobals;
+  l->g = g;
+  l->next = list_of_all_globals;
+  list_of_all_globals = l;
   if (g->has_dynamic_init) {
     if (!dynamic_init_globals) {
       dynamic_init_globals = new (GetGlobalLowLevelAllocator()) VectorOfGlobals;

>From 34108264658ec44f702826fc7c1a3c21dc935f7c Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Sun, 28 Jul 2024 02:23:43 -0700
Subject: [PATCH 2/8] [asan] Speed up ASan ODR indicator-based checking

When ASan checks for ODR violation on a global it loops over a linked list of all globals to find
those with the matching value of an indicator. For larger binaries with a ton of globals this gets
extremely expensive: O(N^2) iterations + potentially poor memory locality from the linked list.

This patch adds an indicator indexed (multi-)map of globals to speed up the search. With a bit of
extra memory this massively speeds up indicator based ODR checks.

Poisoning based ODR checking remains on the slow path.
---
 compiler-rt/lib/asan/asan_globals.cpp | 43 +++++++++++++++++++++------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index d413b1ebc9fc0..26518c8ce4b98 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -21,6 +21,7 @@
 #include "asan_suppressions.h"
 #include "asan_thread.h"
 #include "sanitizer_common/sanitizer_common.h"
+#include "sanitizer_common/sanitizer_dense_map.h"
 #include "sanitizer_common/sanitizer_mutex.h"
 #include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
@@ -35,8 +36,11 @@ struct ListOfGlobals {
   ListOfGlobals *next;
 };
 
+typedef DenseMap<uptr, ListOfGlobals *> MapOfGlobals;
+
 static Mutex mu_for_globals;
 static ListOfGlobals *list_of_all_globals;
+static MapOfGlobals map_of_globals_by_indicator;
 
 static const int kDynamicInitGlobalsInitialCapacity = 512;
 struct DynInitGlobal {
@@ -73,6 +77,18 @@ ALWAYS_INLINE void PoisonRedZones(const Global &g) {
 
 const uptr kMinimalDistanceFromAnotherGlobal = 64;
 
+static void AddGlobalToList(ListOfGlobals *&list, const Global *g) {
+  ListOfGlobals *l = new (GetGlobalLowLevelAllocator()) ListOfGlobals;
+  l->g = g;
+  l->next = list;
+  list = l;
+}
+
+static void AddGlobalToMap(MapOfGlobals &map, const Global *g) {
+  ListOfGlobals *&in_map = map[g->odr_indicator];
+  AddGlobalToList(in_map, g);
+}
+
 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;
@@ -147,14 +163,20 @@ static void CheckODRViolationViaIndicator(const Global *g) {
     *odr_indicator = REGISTERED;
     return;
   }
+  // Fetch globals with the same ODR indicator
+  auto *relevant_globals_lookup =
+      map_of_globals_by_indicator.find(g->odr_indicator);
+  if (!relevant_globals_lookup) {
+    return;
+  }
+  ListOfGlobals *relevant_globals = relevant_globals_lookup->second;
   // 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) {
-    if (g->odr_indicator == l->g->odr_indicator &&
-        (flags()->detect_odr_violation >= 2 || g->size != l->g->size) &&
+  for (ListOfGlobals *l = relevant_globals; l; l = l->next) {
+    if ((flags()->detect_odr_violation >= 2 || g->size != l->g->size) &&
         !IsODRViolationSuppressed(g->name))
-      ReportODRViolation(g, FindRegistrationSite(g),
-                         l->g, FindRegistrationSite(l->g));
+      ReportODRViolation(g, FindRegistrationSite(g), l->g,
+                         FindRegistrationSite(l->g));
   }
 }
 
@@ -225,10 +247,13 @@ static void RegisterGlobal(const Global *g) {
   }
   if (CanPoisonMemory())
     PoisonRedZones(*g);
-  ListOfGlobals *l = new (GetGlobalLowLevelAllocator()) ListOfGlobals;
-  l->g = g;
-  l->next = list_of_all_globals;
-  list_of_all_globals = l;
+
+  AddGlobalToList(list_of_all_globals, g);
+
+  if (UseODRIndicator(g) && g->odr_indicator != UINTPTR_MAX) {
+    AddGlobalToMap(map_of_globals_by_indicator, g);
+  }
+
   if (g->has_dynamic_init) {
     if (!dynamic_init_globals) {
       dynamic_init_globals = new (GetGlobalLowLevelAllocator()) VectorOfGlobals;

>From 3f0cf9489c957157ea7e15ccaac307e1b910fd43 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 30 Jul 2024 02:57:45 -0700
Subject: [PATCH 3/8] amend! [asan] Speed up ASan ODR indicator-based checking

[asan] Speed up ASan ODR indicator-based checking

When ASan checks for a potential ODR violation on a global it loops over
a linked list of all globals to find those with the matching value of an
indicator. With the default setting 'detect_odr_violation=1', ASan
doesn't report violations on same-size globals but it still has to
traverse the list.

For larger binaries with a ton of shared libs and globals (and a
non-trivial volume of same-sized duplicates) this gets extremely
expensive.

This patch adds an indicator indexed (multi-)map of globals to speed up
the search.
---
 compiler-rt/lib/asan/asan_globals.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index 26518c8ce4b98..cc887bdbec347 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -163,12 +163,13 @@ static void CheckODRViolationViaIndicator(const Global *g) {
     *odr_indicator = REGISTERED;
     return;
   }
-  // Fetch globals with the same ODR indicator
+
+  // Fetch globals with the same ODR indicator.
   auto *relevant_globals_lookup =
       map_of_globals_by_indicator.find(g->odr_indicator);
-  if (!relevant_globals_lookup) {
+  if (!relevant_globals_lookup)
     return;
-  }
+
   ListOfGlobals *relevant_globals = relevant_globals_lookup->second;
   // If *odr_indicator is DEFINED, some module have already registered
   // externally visible symbol with the same name. This is an ODR violation.

>From 56c3d62db15c5f790d53258b85a72a3a94696178 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Tue, 30 Jul 2024 04:30:38 -0700
Subject: [PATCH 4/8] fixup! amend! [asan] Speed up ASan ODR indicator-based
 checking

---
 compiler-rt/lib/asan/asan_globals.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index cc887bdbec347..53923d7466f99 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -251,9 +251,8 @@ static void RegisterGlobal(const Global *g) {
 
   AddGlobalToList(list_of_all_globals, g);
 
-  if (UseODRIndicator(g) && g->odr_indicator != UINTPTR_MAX) {
+  if (UseODRIndicator(g) && g->odr_indicator != UINTPTR_MAX)
     AddGlobalToMap(map_of_globals_by_indicator, g);
-  }
 
   if (g->has_dynamic_init) {
     if (!dynamic_init_globals) {

>From b14651d1221f64c610d2251ae16cf33c9489e515 Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Wed, 31 Jul 2024 12:40:18 -0700
Subject: [PATCH 5/8] fixup! fixup! amend! [asan] Speed up ASan ODR
 indicator-based checking

---
 compiler-rt/lib/asan/asan_globals.cpp | 40 ++++++++++-----------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index 53923d7466f99..3f3a47fd25068 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -84,11 +84,6 @@ static void AddGlobalToList(ListOfGlobals *&list, const Global *g) {
   list = l;
 }
 
-static void AddGlobalToMap(MapOfGlobals &map, const Global *g) {
-  ListOfGlobals *&in_map = map[g->odr_indicator];
-  AddGlobalToList(in_map, g);
-}
-
 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;
@@ -158,27 +153,25 @@ static void CheckODRViolationViaIndicator(const Global *g) {
   // Instrumentation requests to skip ODR check.
   if (g->odr_indicator == UINTPTR_MAX)
     return;
+
+  ListOfGlobals *&relevant_globals =
+      map_of_globals_by_indicator[g->odr_indicator];
+
   u8 *odr_indicator = reinterpret_cast<u8 *>(g->odr_indicator);
-  if (*odr_indicator == UNREGISTERED) {
+  if (*odr_indicator == REGISTERED) {
+    // If *odr_indicator is REGISTERED, some module have already registered
+    // externally visible symbol with the same name. This is an ODR violation.
+    for (ListOfGlobals *l = relevant_globals; l; l = l->next) {
+      if ((flags()->detect_odr_violation >= 2 || g->size != l->g->size) &&
+          !IsODRViolationSuppressed(g->name))
+        ReportODRViolation(g, FindRegistrationSite(g), l->g,
+                           FindRegistrationSite(l->g));
+    }
+  } else {  // UNREGISTERED
     *odr_indicator = REGISTERED;
-    return;
   }
 
-  // Fetch globals with the same ODR indicator.
-  auto *relevant_globals_lookup =
-      map_of_globals_by_indicator.find(g->odr_indicator);
-  if (!relevant_globals_lookup)
-    return;
-
-  ListOfGlobals *relevant_globals = relevant_globals_lookup->second;
-  // 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 = relevant_globals; l; l = l->next) {
-    if ((flags()->detect_odr_violation >= 2 || g->size != l->g->size) &&
-        !IsODRViolationSuppressed(g->name))
-      ReportODRViolation(g, FindRegistrationSite(g), l->g,
-                         FindRegistrationSite(l->g));
-  }
+  AddGlobalToList(relevant_globals, g);
 }
 
 // Check ODR violation for given global G by checking if it's already poisoned.
@@ -251,9 +244,6 @@ static void RegisterGlobal(const Global *g) {
 
   AddGlobalToList(list_of_all_globals, g);
 
-  if (UseODRIndicator(g) && g->odr_indicator != UINTPTR_MAX)
-    AddGlobalToMap(map_of_globals_by_indicator, g);
-
   if (g->has_dynamic_init) {
     if (!dynamic_init_globals) {
       dynamic_init_globals = new (GetGlobalLowLevelAllocator()) VectorOfGlobals;

>From 61870e27720d4f6d17b9f3e4b30c3f32918d9d9b Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Wed, 31 Jul 2024 15:18:26 -0700
Subject: [PATCH 6/8] [asan] Add a test that captures the ODR checker behavior
 when globals get unregistered

The test demonstrates a FN due to indicator value being shared among all globals with the same
indicator address.

Moving dlopen/dlclose around and/or adding more shared libs into the mix can show more edge cases
but this is out of the scope of this commit.
---
 .../Linux/odr_indicator_unregister.cpp        | 76 +++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 compiler-rt/test/asan/TestCases/Linux/odr_indicator_unregister.cpp

diff --git a/compiler-rt/test/asan/TestCases/Linux/odr_indicator_unregister.cpp b/compiler-rt/test/asan/TestCases/Linux/odr_indicator_unregister.cpp
new file mode 100644
index 0000000000000..091ecfae5e5ca
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/Linux/odr_indicator_unregister.cpp
@@ -0,0 +1,76 @@
+// Test that captures the current behavior of indicator-based ASan ODR checker
+// when globals get unregistered.
+//
+// RUN: %clangxx_asan -g -O0 -DSHARED_LIB -DSIZE=1 %s -fPIC -shared -o %t-so-1.so
+// RUN: %clangxx_asan -g -O0 -DSHARED_LIB -DSIZE=2 %s -fPIC -shared -o %t-so-2.so
+// RUN: %clangxx_asan -g -O0 %s %libdl -Wl,--export-dynamic -o %t
+// RUN: %env_asan_opts=report_globals=2:detect_odr_violation=1 %run %t 2>&1 | FileCheck %s
+
+#include <cstdlib>
+#include <dlfcn.h>
+#include <stdio.h>
+#include <string>
+
+#ifdef SHARED_LIB
+namespace foo {
+char G[SIZE];
+}
+#else // SHARED_LIB
+void *dlopen_or_die(std::string &path) {
+  void *handle = dlopen(path.c_str(), RTLD_NOW);
+  if (handle) {
+    printf("Successfully called dlopen() on %s\n", path.c_str());
+  } else {
+    printf("Error in dlopen(): %s\n", dlerror());
+    std::exit(1);
+  }
+
+  return handle;
+}
+
+void dlclose_or_die(void *handle, std::string &path) {
+  if (!dlclose(handle)) {
+    printf("Successfully called dlclose() on %s\n", path.c_str());
+  } else {
+    printf("Error in dlclose(): %s\n", dlerror());
+    std::exit(1);
+  }
+}
+
+namespace foo {
+char G[1];
+}
+
+// main has its own version of foo::G
+// CHECK: Added Global[[MAIN_G:[^\s]+]] size=1/32 name=foo::G {{.*}}
+int main(int argc, char *argv[]) {
+  std::string base_path = std::string(argv[0]);
+
+  std::string path1 = base_path + "-so-1.so";
+  // dlopen() brings another foo::G but it matches MAIN_G in size so it's not a
+  // violation
+  //
+  //
+  // CHECK: Added Global[[SO1_G:[^\s]+]] size=1/32 name=foo::G {{.*}}
+  // CHECK-NOT: ERROR: AddressSanitizer: odr-violation
+  void *handle1 = dlopen_or_die(path1);
+  // CHECK: Removed Global[[SO1_G]] size=1/32 name=foo::G {{.*}}
+  dlclose_or_die(handle1, path1);
+
+  // At this point the indicator for foo::G is switched to UNREGISTERED for
+  // **both** MAIN_G and SO1_G because the indicator value is shared.
+
+  std::string path2 = base_path + "-so-2.so";
+  // CHECK: Added Global[[SO2_G:[^\s]+]] size=2/32 name=foo::G {{.*}}
+  //
+  // This brings another foo::G but now different in size from MAIN_G. We
+  // should've reported a violation, but we actually don't because of what's
+  // described on line60
+  //
+  // CHECK-NOT: ERROR: AddressSanitizer: odr-violation
+  void *handle2 = dlopen_or_die(path2);
+  // CHECK: Removed Global[[MAIN_G]] size=1/32 name=foo::G {{.*}}
+  // CHECK: Removed Global[[SO2_G]] size=2/32 name=foo::G {{.*}}
+}
+
+#endif // SHARED_LIB

>From bb7bd6ecbe1f779ed2285d806e6725270ec9e90b Mon Sep 17 00:00:00 2001
From: Artem Pianykh <arr at fb.com>
Date: Thu, 1 Aug 2024 02:59:36 -0700
Subject: [PATCH 7/8] [asan][odr] Use IntrusiveList for a ListOfGlobals

---
 compiler-rt/lib/asan/asan_globals.cpp | 41 ++++++++++++++-------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index 3f3a47fd25068..98cbb269bbb02 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -22,6 +22,7 @@
 #include "asan_thread.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_dense_map.h"
+#include "sanitizer_common/sanitizer_list.h"
 #include "sanitizer_common/sanitizer_mutex.h"
 #include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
@@ -31,15 +32,16 @@ namespace __asan {
 
 typedef __asan_global Global;
 
-struct ListOfGlobals {
+struct GlobalListNode {
   const Global *g;
-  ListOfGlobals *next;
+  GlobalListNode *next;
 };
 
-typedef DenseMap<uptr, ListOfGlobals *> MapOfGlobals;
+typedef IntrusiveList<GlobalListNode> ListOfGlobals;
+typedef DenseMap<uptr, ListOfGlobals> MapOfGlobals;
 
 static Mutex mu_for_globals;
-static ListOfGlobals *list_of_all_globals;
+static ListOfGlobals list_of_all_globals;
 static MapOfGlobals map_of_globals_by_indicator;
 
 static const int kDynamicInitGlobalsInitialCapacity = 512;
@@ -77,11 +79,10 @@ ALWAYS_INLINE void PoisonRedZones(const Global &g) {
 
 const uptr kMinimalDistanceFromAnotherGlobal = 64;
 
-static void AddGlobalToList(ListOfGlobals *&list, const Global *g) {
-  ListOfGlobals *l = new (GetGlobalLowLevelAllocator()) ListOfGlobals;
+static void AddGlobalToList(ListOfGlobals &list, const Global *g) {
+  GlobalListNode *l = new (GetGlobalLowLevelAllocator()) GlobalListNode;
   l->g = g;
-  l->next = list;
-  list = l;
+  list.push_front(l);
 }
 
 static bool IsAddressNearGlobal(uptr addr, const __asan_global &g) {
@@ -125,8 +126,8 @@ int GetGlobalsForAddress(uptr addr, Global *globals, u32 *reg_sites,
   if (!flags()->report_globals) return 0;
   Lock lock(&mu_for_globals);
   int res = 0;
-  for (ListOfGlobals *l = list_of_all_globals; l; l = l->next) {
-    const Global &g = *l->g;
+  for (const auto &l : list_of_all_globals) {
+    const Global &g = *l.g;
     if (flags()->report_globals >= 2)
       ReportGlobal(g, "Search");
     if (IsAddressNearGlobal(addr, g)) {
@@ -154,18 +155,18 @@ static void CheckODRViolationViaIndicator(const Global *g) {
   if (g->odr_indicator == UINTPTR_MAX)
     return;
 
-  ListOfGlobals *&relevant_globals =
+  ListOfGlobals &relevant_globals =
       map_of_globals_by_indicator[g->odr_indicator];
 
   u8 *odr_indicator = reinterpret_cast<u8 *>(g->odr_indicator);
   if (*odr_indicator == REGISTERED) {
     // If *odr_indicator is REGISTERED, some module have already registered
     // externally visible symbol with the same name. This is an ODR violation.
-    for (ListOfGlobals *l = relevant_globals; l; l = l->next) {
-      if ((flags()->detect_odr_violation >= 2 || g->size != l->g->size) &&
+    for (const auto &l : relevant_globals) {
+      if ((flags()->detect_odr_violation >= 2 || g->size != l.g->size) &&
           !IsODRViolationSuppressed(g->name))
-        ReportODRViolation(g, FindRegistrationSite(g), l->g,
-                           FindRegistrationSite(l->g));
+        ReportODRViolation(g, FindRegistrationSite(g), l.g,
+                           FindRegistrationSite(l.g));
     }
   } else {  // UNREGISTERED
     *odr_indicator = REGISTERED;
@@ -181,12 +182,12 @@ static void CheckODRViolationViaPoisoning(const Global *g) {
   if (__asan_region_is_poisoned(g->beg, g->size_with_redzone)) {
     // This check may not be enough: if the first global is much larger
     // the entire redzone of the second global may be within the first global.
-    for (ListOfGlobals *l = list_of_all_globals; l; l = l->next) {
-      if (g->beg == l->g->beg &&
-          (flags()->detect_odr_violation >= 2 || g->size != l->g->size) &&
+    for (const auto &l : list_of_all_globals) {
+      if (g->beg == l.g->beg &&
+          (flags()->detect_odr_violation >= 2 || g->size != l.g->size) &&
           !IsODRViolationSuppressed(g->name))
-        ReportODRViolation(g, FindRegistrationSite(g),
-                           l->g, FindRegistrationSite(l->g));
+        ReportODRViolation(g, FindRegistrationSite(g), l.g,
+                           FindRegistrationSite(l.g));
     }
   }
 }

>From c5598faf7b524396131927fc1f4d775c3539c789 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 1 Aug 2024 09:56:07 -0700
Subject: [PATCH 8/8] rebase

---
 compiler-rt/lib/asan/asan_globals.cpp | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index 98cbb269bbb02..0445a1d44f682 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -33,10 +33,9 @@ namespace __asan {
 typedef __asan_global Global;
 
 struct GlobalListNode {
-  const Global *g;
-  GlobalListNode *next;
+  const Global *g = nullptr;
+  GlobalListNode *next = nullptr;
 };
-
 typedef IntrusiveList<GlobalListNode> ListOfGlobals;
 typedef DenseMap<uptr, ListOfGlobals> MapOfGlobals;
 
@@ -80,9 +79,7 @@ ALWAYS_INLINE void PoisonRedZones(const Global &g) {
 const uptr kMinimalDistanceFromAnotherGlobal = 64;
 
 static void AddGlobalToList(ListOfGlobals &list, const Global *g) {
-  GlobalListNode *l = new (GetGlobalLowLevelAllocator()) GlobalListNode;
-  l->g = g;
-  list.push_front(l);
+  list.push_front(new (GetGlobalLowLevelAllocator()) GlobalListNode{g});
 }
 
 static bool IsAddressNearGlobal(uptr addr, const __asan_global &g) {
@@ -185,9 +182,10 @@ static void CheckODRViolationViaPoisoning(const Global *g) {
     for (const auto &l : list_of_all_globals) {
       if (g->beg == l.g->beg &&
           (flags()->detect_odr_violation >= 2 || g->size != l.g->size) &&
-          !IsODRViolationSuppressed(g->name))
+          !IsODRViolationSuppressed(g->name)) {
         ReportODRViolation(g, FindRegistrationSite(g), l.g,
                            FindRegistrationSite(l.g));
+      }
     }
   }
 }



More information about the llvm-commits mailing list