[compiler-rt] [NFC][asan] Track current dynamic init module (PR #101597)

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 3 10:57:40 PDT 2024


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

>From bb3e582a331065076f17f94fbef8fa932b49c237 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 1 Aug 2024 18:30:44 -0700
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20change?=
 =?UTF-8?q?s=20to=20main=20this=20commit=20is=20based=20on?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.4

[skip ci]
---
 compiler-rt/lib/asan/asan_globals.cpp         | 131 +++++++++++-------
 .../Helpers/initialization-nobug-extra.cpp    |   6 +-
 .../asan/TestCases/initialization-nobug.cpp   |  19 +--
 3 files changed, 93 insertions(+), 63 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index e2ff0a51acf77..c7dcc47460e83 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -27,6 +27,7 @@
 #include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
 #include "sanitizer_common/sanitizer_symbolizer.h"
+#include "sanitizer_common/sanitizer_thread_safety.h"
 
 namespace __asan {
 
@@ -39,16 +40,13 @@ struct GlobalListNode {
 typedef IntrusiveList<GlobalListNode> ListOfGlobals;
 
 static Mutex mu_for_globals;
-static ListOfGlobals list_of_all_globals;
+static ListOfGlobals list_of_all_globals SANITIZER_GUARDED_BY(mu_for_globals);
 
-static const int kDynamicInitGlobalsInitialCapacity = 512;
 struct DynInitGlobal {
-  Global g;
-  bool initialized;
+  Global g = {};
+  bool initialized = false;
+  DynInitGlobal *next = nullptr;
 };
-typedef InternalMmapVector<DynInitGlobal> VectorOfGlobals;
-// Lazy-initialized and never deleted.
-static VectorOfGlobals *dynamic_init_globals;
 
 // We want to remember where a certain range of globals was registered.
 struct GlobalRegistrationSite {
@@ -58,7 +56,8 @@ struct GlobalRegistrationSite {
 typedef InternalMmapVector<GlobalRegistrationSite> GlobalRegistrationSiteVector;
 static GlobalRegistrationSiteVector *global_registration_site_vector;
 
-static ListOfGlobals &GlobalsByIndicator(uptr odr_indicator) {
+static ListOfGlobals &GlobalsByIndicator(uptr odr_indicator)
+    SANITIZER_REQUIRES(mu_for_globals) {
   using MapOfGlobals = DenseMap<uptr, ListOfGlobals>;
 
   static MapOfGlobals *globals_by_indicator = nullptr;
@@ -71,6 +70,22 @@ static ListOfGlobals &GlobalsByIndicator(uptr odr_indicator) {
   return (*globals_by_indicator)[odr_indicator];
 }
 
+using DynInitGLobalsByModule =
+    DenseMap<const char *, IntrusiveList<DynInitGlobal>>;
+
+// TODO: Add a NoDestroy helper, this patter is very common in sanitizers.
+static DynInitGLobalsByModule &DynInitGlobals()
+    SANITIZER_REQUIRES(mu_for_globals) {
+  static DynInitGLobalsByModule *globals_by_module = nullptr;
+  if (!globals_by_module) {
+    alignas(alignof(DynInitGLobalsByModule)) static char
+        placeholder[sizeof(DynInitGLobalsByModule)];
+    globals_by_module = new (placeholder) DynInitGLobalsByModule();
+  }
+
+  return *globals_by_module;
+}
+
 ALWAYS_INLINE void PoisonShadowForGlobal(const Global *g, u8 value) {
   FastPoisonShadow(g->beg, g->size_with_redzone, value);
 }
@@ -158,7 +173,8 @@ enum GlobalSymbolState {
 // 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) {
+static void CheckODRViolationViaIndicator(const Global *g)
+    SANITIZER_REQUIRES(mu_for_globals) {
   // Instrumentation requests to skip ODR check.
   if (g->odr_indicator == UINTPTR_MAX)
     return;
@@ -185,7 +201,8 @@ static void CheckODRViolationViaIndicator(const Global *g) {
 // Check ODR violation for given global G by checking if it's already poisoned.
 // We use this method in case compiler doesn't use private aliases for global
 // variables.
-static void CheckODRViolationViaPoisoning(const Global *g) {
+static void CheckODRViolationViaPoisoning(const Global *g)
+    SANITIZER_REQUIRES(mu_for_globals) {
   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.
@@ -223,7 +240,7 @@ static inline bool UseODRIndicator(const Global *g) {
 // Register a global variable.
 // This function may be called more than once for every global
 // so we store the globals in a map.
-static void RegisterGlobal(const Global *g) {
+static void RegisterGlobal(const Global *g) SANITIZER_REQUIRES(mu_for_globals) {
   CHECK(AsanInited());
   if (flags()->report_globals >= 2)
     ReportGlobal(*g, "Added");
@@ -254,16 +271,13 @@ static void RegisterGlobal(const Global *g) {
   AddGlobalToList(list_of_all_globals, g);
 
   if (g->has_dynamic_init) {
-    if (!dynamic_init_globals) {
-      dynamic_init_globals = new (GetGlobalLowLevelAllocator()) VectorOfGlobals;
-      dynamic_init_globals->reserve(kDynamicInitGlobalsInitialCapacity);
-    }
-    DynInitGlobal dyn_global = { *g, false };
-    dynamic_init_globals->push_back(dyn_global);
+    DynInitGlobals()[g->module_name].push_back(
+        new (GetGlobalLowLevelAllocator()) DynInitGlobal{*g, false});
   }
 }
 
-static void UnregisterGlobal(const Global *g) {
+static void UnregisterGlobal(const Global *g)
+    SANITIZER_REQUIRES(mu_for_globals) {
   CHECK(AsanInited());
   if (flags()->report_globals >= 2)
     ReportGlobal(*g, "Removed");
@@ -284,21 +298,42 @@ static void UnregisterGlobal(const Global *g) {
   }
 }
 
-void StopInitOrderChecking() {
-  Lock lock(&mu_for_globals);
-  if (!flags()->check_initialization_order || !dynamic_init_globals)
-    return;
-  flags()->check_initialization_order = false;
-  for (uptr i = 0, n = dynamic_init_globals->size(); i < n; ++i) {
-    DynInitGlobal &dyn_g = (*dynamic_init_globals)[i];
+static void UnpoisonDynamicGlobals(IntrusiveList<DynInitGlobal> &dyn_globals,
+                                   bool mark_initialized) {
+  for (auto &dyn_g : dyn_globals) {
     const Global *g = &dyn_g.g;
+    if (dyn_g.initialized)
+      continue;
     // Unpoison the whole global.
     PoisonShadowForGlobal(g, 0);
     // Poison redzones back.
     PoisonRedZones(*g);
+    if (mark_initialized)
+      dyn_g.initialized = true;
+  }
+}
+
+static void PoisonDynamicGlobals(
+    const IntrusiveList<DynInitGlobal> &dyn_globals) {
+  for (auto &dyn_g : dyn_globals) {
+    const Global *g = &dyn_g.g;
+    if (dyn_g.initialized)
+      continue;
+    PoisonShadowForGlobal(g, kAsanInitializationOrderMagic);
   }
 }
 
+void StopInitOrderChecking() {
+  if (!flags()->check_initialization_order)
+    return;
+  Lock lock(&mu_for_globals);
+  flags()->check_initialization_order = false;
+  DynInitGlobals().forEach([&](auto &kv) {
+    UnpoisonDynamicGlobals(kv.second, /*mark_initialized=*/false);
+    return true;
+  });
+}
+
 static bool IsASCII(unsigned char c) { return /*0x00 <= c &&*/ c <= 0x7F; }
 
 const char *MaybeDemangleGlobalName(const char *name) {
@@ -451,9 +486,7 @@ void __asan_unregister_globals(__asan_global *globals, uptr n) {
 // poisons all global variables not defined in this TU, so that a dynamic
 // initializer can only touch global variables in the same TU.
 void __asan_before_dynamic_init(const char *module_name) {
-  if (!flags()->check_initialization_order ||
-      !CanPoisonMemory() ||
-      !dynamic_init_globals)
+  if (!flags()->check_initialization_order || !CanPoisonMemory())
     return;
   bool strict_init_order = flags()->strict_init_order;
   CHECK(module_name);
@@ -461,37 +494,31 @@ void __asan_before_dynamic_init(const char *module_name) {
   Lock lock(&mu_for_globals);
   if (flags()->report_globals >= 3)
     Printf("DynInitPoison module: %s\n", module_name);
-  for (uptr i = 0, n = dynamic_init_globals->size(); i < n; ++i) {
-    DynInitGlobal &dyn_g = (*dynamic_init_globals)[i];
-    const Global *g = &dyn_g.g;
-    if (dyn_g.initialized)
-      continue;
-    if (g->module_name != module_name)
-      PoisonShadowForGlobal(g, kAsanInitializationOrderMagic);
-    else if (!strict_init_order)
-      dyn_g.initialized = true;
-  }
+
+  DynInitGlobals().forEach([&](auto &kv) {
+    if (kv.first != module_name) {
+      PoisonDynamicGlobals(kv.second);
+    } else {
+      UnpoisonDynamicGlobals(kv.second,
+                             /*mark_initialized=*/!strict_init_order);
+    }
+    return true;
+  });
 }
 
 // This method runs immediately after dynamic initialization in each TU, when
 // all dynamically initialized globals except for those defined in the current
 // TU are poisoned.  It simply unpoisons all dynamically initialized globals.
 void __asan_after_dynamic_init() {
-  if (!flags()->check_initialization_order ||
-      !CanPoisonMemory() ||
-      !dynamic_init_globals)
+  if (!flags()->check_initialization_order || !CanPoisonMemory())
     return;
   CHECK(AsanInited());
   Lock lock(&mu_for_globals);
-  // FIXME: Optionally report that we're unpoisoning globals from a module.
-  for (uptr i = 0, n = dynamic_init_globals->size(); i < n; ++i) {
-    DynInitGlobal &dyn_g = (*dynamic_init_globals)[i];
-    const Global *g = &dyn_g.g;
-    if (!dyn_g.initialized) {
-      // Unpoison the whole global.
-      PoisonShadowForGlobal(g, 0);
-      // Poison redzones back.
-      PoisonRedZones(*g);
-    }
-  }
+  if (flags()->report_globals >= 3)
+    Printf("DynInitUnpoison\n");
+
+  DynInitGlobals().forEach([&](auto &kv) {
+    UnpoisonDynamicGlobals(kv.second, /*mark_initialized=*/false);
+    return true;
+  });
 }
diff --git a/compiler-rt/test/asan/TestCases/Helpers/initialization-nobug-extra.cpp b/compiler-rt/test/asan/TestCases/Helpers/initialization-nobug-extra.cpp
index 886165affd760..0ce5359b405d0 100644
--- a/compiler-rt/test/asan/TestCases/Helpers/initialization-nobug-extra.cpp
+++ b/compiler-rt/test/asan/TestCases/Helpers/initialization-nobug-extra.cpp
@@ -1,9 +1,9 @@
 // Linker initialized:
 int getAB();
-static int ab = getAB();
+int ab = getAB();
 // Function local statics:
 int countCalls();
-static int one = countCalls();
+int one = countCalls();
 // Trivial constructor, non-trivial destructor:
 int getStructWithDtorValue();
-static int val = getStructWithDtorValue();
+int val = getStructWithDtorValue();
diff --git a/compiler-rt/test/asan/TestCases/initialization-nobug.cpp b/compiler-rt/test/asan/TestCases/initialization-nobug.cpp
index d4dc855148ad3..6cfef022ae57e 100644
--- a/compiler-rt/test/asan/TestCases/initialization-nobug.cpp
+++ b/compiler-rt/test/asan/TestCases/initialization-nobug.cpp
@@ -1,19 +1,16 @@
 // A collection of various initializers which shouldn't trip up initialization
 // order checking.  If successful, this will just return 0.
 
-// RUN: %clangxx_asan -O0 %s %p/Helpers/initialization-nobug-extra.cpp -o %t
-// RUN: %env_asan_opts=check_initialization_order=true %run %t 2>&1
-// RUN: %clangxx_asan -O1 %s %p/Helpers/initialization-nobug-extra.cpp -o %t
-// RUN: %env_asan_opts=check_initialization_order=true %run %t 2>&1
-// RUN: %clangxx_asan -O2 %s %p/Helpers/initialization-nobug-extra.cpp -o %t
-// RUN: %env_asan_opts=check_initialization_order=true %run %t 2>&1
-// RUN: %clangxx_asan -O3 %s %p/Helpers/initialization-nobug-extra.cpp -o %t
-// RUN: %env_asan_opts=check_initialization_order=true %run %t 2>&1
+// RUN: %clangxx_asan -O0 %s %p/Helpers/initialization-nobug-extra.cpp -o %t && %env_asan_opts=check_initialization_order=true:report_globals=3 %run %t 2>&1 | FileCheck %s --implicit-check-not "DynInit"
+// RUN: %clangxx_asan -O1 %s %p/Helpers/initialization-nobug-extra.cpp -o %t && %env_asan_opts=check_initialization_order=true:report_globals=3 %run %t 2>&1 | FileCheck %s --implicit-check-not "DynInit"
+// RUN: %clangxx_asan -O2 %s %p/Helpers/initialization-nobug-extra.cpp -o %t && %env_asan_opts=check_initialization_order=true:report_globals=3 %run %t 2>&1 | FileCheck %s --implicit-check-not "DynInit"
+// RUN: %clangxx_asan -O3 %s %p/Helpers/initialization-nobug-extra.cpp -o %t && %env_asan_opts=check_initialization_order=true:report_globals=3 %run %t 2>&1 | FileCheck %s --implicit-check-not "DynInit"
 
 // Simple access:
 // Make sure that accessing a global in the same TU is safe
 
 bool condition = true;
+__attribute__((noinline, weak))
 int initializeSameTU() {
   return condition ? 0x2a : 052;
 }
@@ -46,3 +43,9 @@ StructWithDtor struct_with_dtor;
 int getStructWithDtorValue() { return struct_with_dtor.value; }
 
 int main() { return 0; }
+
+
+// CHECK: DynInitPoison module: {{.*}}initialization-nobug.cpp
+// CHECK: DynInitUnpoison
+// CHECK: DynInitPoison module: {{.*}}initialization-nobug-extra.cpp
+// CHECK: DynInitUnpoison



More information about the llvm-commits mailing list