Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC][asan] Use Thread Safety Analysis in asan_globals.cpp #101574

Conversation

vitalybuka
Copy link
Collaborator

No description provided.

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Aug 1, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Vitaly Buka (vitalybuka)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/101574.diff

1 Files Affected:

  • (modified) compiler-rt/lib/asan/asan_globals.cpp (+22-14)
diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index 0445a1d44f682..9766488fed0c9 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 {
 
@@ -40,8 +41,9 @@ typedef IntrusiveList<GlobalListNode> ListOfGlobals;
 typedef DenseMap<uptr, ListOfGlobals> MapOfGlobals;
 
 static Mutex mu_for_globals;
-static ListOfGlobals list_of_all_globals;
-static MapOfGlobals map_of_globals_by_indicator;
+static ListOfGlobals list_of_all_globals SANITIZER_GUARDED_BY(mu_for_globals);
+static MapOfGlobals map_of_globals_by_indicator
+    SANITIZER_GUARDED_BY(mu_for_globals);
 
 static const int kDynamicInitGlobalsInitialCapacity = 512;
 struct DynInitGlobal {
@@ -50,7 +52,8 @@ struct DynInitGlobal {
 };
 typedef InternalMmapVector<DynInitGlobal> VectorOfGlobals;
 // Lazy-initialized and never deleted.
-static VectorOfGlobals *dynamic_init_globals;
+static VectorOfGlobals *dynamic_init_globals
+    SANITIZER_GUARDED_BY(mu_for_globals);
 
 // We want to remember where a certain range of globals was registered.
 struct GlobalRegistrationSite {
@@ -147,7 +150,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;
@@ -175,7 +179,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.
@@ -213,7 +218,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");
@@ -253,7 +258,8 @@ static void RegisterGlobal(const Global *g) {
   }
 }
 
-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");
@@ -275,8 +281,10 @@ static void UnregisterGlobal(const Global *g) {
 }
 
 void StopInitOrderChecking() {
+  if (!flags()->check_initialization_order)
+    return;
   Lock lock(&mu_for_globals);
-  if (!flags()->check_initialization_order || !dynamic_init_globals)
+  if (!dynamic_init_globals)
     return;
   flags()->check_initialization_order = false;
   for (uptr i = 0, n = dynamic_init_globals->size(); i < n; ++i) {
@@ -441,14 +449,14 @@ 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);
   CHECK(AsanInited());
   Lock lock(&mu_for_globals);
+  if (!dynamic_init_globals)
+    return;
   if (flags()->report_globals >= 3)
     Printf("DynInitPoison module: %s\n", module_name);
   for (uptr i = 0, n = dynamic_init_globals->size(); i < n; ++i) {
@@ -467,12 +475,12 @@ void __asan_before_dynamic_init(const char *module_name) {
 // 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);
+  if (!dynamic_init_globals)
+    return;
   // 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];

@vitalybuka vitalybuka requested a review from kstoimenov August 1, 2024 22:49
@vitalybuka
Copy link
Collaborator Author

@artempyanykh

@vitalybuka vitalybuka requested a review from thurstond August 1, 2024 22:49
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
@vitalybuka vitalybuka merged commit 46bc11d into main Aug 2, 2024
3 of 5 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfcasan-use-thread-safety-analysis-in-asan_globalscpp branch August 2, 2024 06:39
banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
Reviewers: thurstond, kstoimenov

Reviewed By: kstoimenov

Pull Request: llvm#101574
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants