Skip to content

"Reland "[asan] Remove debug tracing from report_globals (#104404)" #105895

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

Conversation

vitalybuka
Copy link
Collaborator

Reland #104404.

In addition to #104404 it raises required
verbosity for stack tracing on global
registration. It confuses a symbolizer test on
Darwin.

This reverts commit 6a8f738.

Created using spr 1.3.4
@vitalybuka vitalybuka merged commit 10407be into main Aug 23, 2024
9 of 11 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/reland-asan-remove-debug-tracing-from-report_globals-104404-1 branch August 23, 2024 22:06
@llvmbot
Copy link
Member

llvmbot commented Aug 23, 2024

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

Author: Vitaly Buka (vitalybuka)

Changes

Reland #104404.

In addition to #104404 it raises required
verbosity for stack tracing on global
registration. It confuses a symbolizer test on
Darwin.

This reverts commit 6a8f738.


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

10 Files Affected:

  • (modified) compiler-rt/lib/asan/asan_flags.inc (+2-5)
  • (modified) compiler-rt/lib/asan/asan_globals.cpp (+8-11)
  • (modified) compiler-rt/test/asan/TestCases/Linux/initialization-nobug-lld.cpp (+1-1)
  • (modified) compiler-rt/test/asan/TestCases/Linux/odr_indicator_unregister.cpp (+1-1)
  • (modified) compiler-rt/test/asan/TestCases/Linux/odr_indicators.cpp (+2-2)
  • (modified) compiler-rt/test/asan/TestCases/Windows/dll_global_dead_strip.c (+2-2)
  • (modified) compiler-rt/test/asan/TestCases/Windows/dll_report_globals_symbolization_at_startup.cpp (+1-1)
  • (modified) compiler-rt/test/asan/TestCases/Windows/global_dead_strip.c (+2-2)
  • (modified) compiler-rt/test/asan/TestCases/Windows/report_globals_vs_freelibrary.cpp (+1-1)
  • (modified) compiler-rt/test/asan/TestCases/initialization-nobug.cpp (+4-4)
diff --git a/compiler-rt/lib/asan/asan_flags.inc b/compiler-rt/lib/asan/asan_flags.inc
index fad1577d912a5e..5e0ced9706e664 100644
--- a/compiler-rt/lib/asan/asan_flags.inc
+++ b/compiler-rt/lib/asan/asan_flags.inc
@@ -36,11 +36,8 @@ ASAN_FLAG(int, max_redzone, 2048,
 ASAN_FLAG(
     bool, debug, false,
     "If set, prints some debugging information and does additional checks.")
-ASAN_FLAG(
-    int, report_globals, 1,
-    "Controls the way to handle globals (0 - don't detect buffer overflow on "
-    "globals, 1 - detect buffer overflow, 2 - print data about registered "
-    "globals).")
+ASAN_FLAG(bool, report_globals, true,
+          "If set, detect and report errors on globals .")
 ASAN_FLAG(bool, check_initialization_order, false,
           "If set, attempts to catch initialization order issues.")
 ASAN_FLAG(
diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index c83b782cb85f89..bf0edce937f06e 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_internal_defs.h"
 #include "sanitizer_common/sanitizer_list.h"
 #include "sanitizer_common/sanitizer_mutex.h"
 #include "sanitizer_common/sanitizer_placement_new.h"
@@ -179,7 +180,7 @@ int GetGlobalsForAddress(uptr addr, Global *globals, u32 *reg_sites,
   int res = 0;
   for (const auto &l : list_of_all_globals) {
     const Global &g = *l.g;
-    if (flags()->report_globals >= 2)
+    if (UNLIKELY(common_flags()->verbosity >= 3))
       ReportGlobal(g, "Search");
     if (IsAddressNearGlobal(addr, g)) {
       internal_memcpy(&globals[res], &g, sizeof(g));
@@ -270,7 +271,7 @@ static inline bool UseODRIndicator(const Global *g) {
 // so we store the globals in a map.
 static void RegisterGlobal(const Global *g) SANITIZER_REQUIRES(mu_for_globals) {
   CHECK(AsanInited());
-  if (flags()->report_globals >= 2)
+  if (UNLIKELY(common_flags()->verbosity >= 3))
     ReportGlobal(*g, "Added");
   CHECK(flags()->report_globals);
   CHECK(AddrIsInMem(g->beg));
@@ -307,7 +308,7 @@ static void RegisterGlobal(const Global *g) SANITIZER_REQUIRES(mu_for_globals) {
 static void UnregisterGlobal(const Global *g)
     SANITIZER_REQUIRES(mu_for_globals) {
   CHECK(AsanInited());
-  if (flags()->report_globals >= 2)
+  if (UNLIKELY(common_flags()->verbosity >= 3))
     ReportGlobal(*g, "Removed");
   CHECK(flags()->report_globals);
   CHECK(AddrIsInMem(g->beg));
@@ -438,7 +439,7 @@ void __asan_register_globals(__asan_global *globals, uptr n) {
   }
   GlobalRegistrationSite site = {stack_id, &globals[0], &globals[n - 1]};
   global_registration_site_vector->push_back(site);
-  if (flags()->report_globals >= 2) {
+  if (UNLIKELY(common_flags()->verbosity >= 4)) {
     PRINT_CURRENT_STACK();
     Printf("=== ID %d; %p %p\n", stack_id, (void *)&globals[0],
            (void *)&globals[n - 1]);
@@ -497,9 +498,7 @@ void __asan_before_dynamic_init(const char *module_name) {
   Lock lock(&mu_for_globals);
   if (current_dynamic_init_module_name == module_name)
     return;
-  if (flags()->report_globals >= 3)
-    Printf("DynInitPoison module: %s\n", module_name);
-
+  VPrintf(2, "DynInitPoison module: %s\n", module_name);
   if (current_dynamic_init_module_name == nullptr) {
     // First call, poison all globals from other modules.
     DynInitGlobals().forEach([&](auto &kv) {
@@ -545,8 +544,7 @@ static void UnpoisonBeforeMain(void) {
       return;
     allow_after_dynamic_init = true;
   }
-  if (flags()->report_globals >= 3)
-    Printf("UnpoisonBeforeMain\n");
+  VPrintf(2, "UnpoisonBeforeMain\n");
   __asan_after_dynamic_init();
 }
 
@@ -570,8 +568,7 @@ void __asan_after_dynamic_init() {
   if (!current_dynamic_init_module_name)
     return;
 
-  if (flags()->report_globals >= 3)
-    Printf("DynInitUnpoison\n");
+  VPrintf(2, "DynInitUnpoison\n");
 
   DynInitGlobals().forEach([&](auto &kv) {
     UnpoisonDynamicGlobals(kv.second, /*mark_initialized=*/false);
diff --git a/compiler-rt/test/asan/TestCases/Linux/initialization-nobug-lld.cpp b/compiler-rt/test/asan/TestCases/Linux/initialization-nobug-lld.cpp
index 5cec029811cbc8..ef82c7a29575eb 100644
--- a/compiler-rt/test/asan/TestCases/Linux/initialization-nobug-lld.cpp
+++ b/compiler-rt/test/asan/TestCases/Linux/initialization-nobug-lld.cpp
@@ -1,4 +1,4 @@
-// RUN: %clangxx_asan -O3 %S/../initialization-nobug.cpp %S/../Helpers/initialization-nobug-extra.cpp -fuse-ld=lld -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/../initialization-nobug.cpp %S/../Helpers/initialization-nobug-extra.cpp -fuse-ld=lld -o %t && %env_asan_opts=check_initialization_order=true:report_globals=1:verbosity=2 %run %t 2>&1 | FileCheck %s --implicit-check-not "DynInit"
 
 // Same as initialization-nobug.cpp, but with lld we expect just one
 // `DynInitUnpoison` executed after `AfterDynamicInit` at the end.
diff --git a/compiler-rt/test/asan/TestCases/Linux/odr_indicator_unregister.cpp b/compiler-rt/test/asan/TestCases/Linux/odr_indicator_unregister.cpp
index 0f2ed6597154bb..b75f5be101ef8a 100644
--- a/compiler-rt/test/asan/TestCases/Linux/odr_indicator_unregister.cpp
+++ b/compiler-rt/test/asan/TestCases/Linux/odr_indicator_unregister.cpp
@@ -4,7 +4,7 @@
 // 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
+// RUN: %env_asan_opts=report_globals=1:detect_odr_violation=1:verbosity=3 %run %t 2>&1 | FileCheck %s
 
 // FIXME: Checks do not match on Android.
 // UNSUPPORTED: android
diff --git a/compiler-rt/test/asan/TestCases/Linux/odr_indicators.cpp b/compiler-rt/test/asan/TestCases/Linux/odr_indicators.cpp
index 8af3ec09be78c4..f28a9f6d07386d 100644
--- a/compiler-rt/test/asan/TestCases/Linux/odr_indicators.cpp
+++ b/compiler-rt/test/asan/TestCases/Linux/odr_indicators.cpp
@@ -1,8 +1,8 @@
 // RUN: %clangxx_asan -fno-sanitize-address-use-odr-indicator -fPIC %s -o %t
-// RUN: %env_asan_opts=report_globals=2 %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK,INDICATOR0
+// RUN: %env_asan_opts=report_globals=1:verbosity=3 %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
+// RUN: %env_asan_opts=report_globals=1:verbosity=3 %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK,INDICATOR1
 
 #include <stdio.h>
 
diff --git a/compiler-rt/test/asan/TestCases/Windows/dll_global_dead_strip.c b/compiler-rt/test/asan/TestCases/Windows/dll_global_dead_strip.c
index a0c96622efeea4..e5bd27bdf65fdf 100644
--- a/compiler-rt/test/asan/TestCases/Windows/dll_global_dead_strip.c
+++ b/compiler-rt/test/asan/TestCases/Windows/dll_global_dead_strip.c
@@ -1,11 +1,11 @@
 // RUN: %clang_cl_asan %Od %p/dll_host.cpp %Fe%t
 //
 // RUN: %clang_cl_nocxx_asan %Gw %LD %Od %s %Fe%t.dll
-// RUN: %env_asan_opts=report_globals=2 %run %t %t.dll 2>&1 | FileCheck %s --check-prefix=NOSTRIP
+// RUN: %env_asan_opts=report_globals=1:verbosity=3 %run %t %t.dll 2>&1 | FileCheck %s --check-prefix=NOSTRIP
 // RUN: %clang_cl_nocxx_asan %Gw %LD -O2 %s %Fe%t.dll \
 // RUN:   %if target={{.*-windows-gnu}} %{ -Wl,--gc-sections %} \
 // RUN:   %else %{ -link -opt:ref %}
-// RUN: %env_asan_opts=report_globals=2 %run %t %t.dll 2>&1 | FileCheck %s --check-prefix=STRIP
+// RUN: %env_asan_opts=report_globals=1:verbosity=3 %run %t %t.dll 2>&1 | FileCheck %s --check-prefix=STRIP
 
 #include <stdio.h>
 
diff --git a/compiler-rt/test/asan/TestCases/Windows/dll_report_globals_symbolization_at_startup.cpp b/compiler-rt/test/asan/TestCases/Windows/dll_report_globals_symbolization_at_startup.cpp
index 06a632e6708b1e..c74b66f2b43b3e 100644
--- a/compiler-rt/test/asan/TestCases/Windows/dll_report_globals_symbolization_at_startup.cpp
+++ b/compiler-rt/test/asan/TestCases/Windows/dll_report_globals_symbolization_at_startup.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cl_asan %LD %Od -DDLL %s %Fe%t.dll \
 // RUN:   %if target={{.*-windows-gnu}} %{ -Wl,--out-implib,%t.lib %}
 // RUN: %clang_cl_asan %Od -DEXE %s %t.lib %Fe%te.exe
-// RUN: %env_asan_opts=report_globals=2 %run %te.exe 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=report_globals=1:verbosity=3 %run %te.exe 2>&1 | FileCheck %s
 
 // FIXME: Currently, the MT runtime build crashes on startup due to dbghelp.dll
 // initialization failure.
diff --git a/compiler-rt/test/asan/TestCases/Windows/global_dead_strip.c b/compiler-rt/test/asan/TestCases/Windows/global_dead_strip.c
index 0e15120a46f776..7f2405fdfc8364 100644
--- a/compiler-rt/test/asan/TestCases/Windows/global_dead_strip.c
+++ b/compiler-rt/test/asan/TestCases/Windows/global_dead_strip.c
@@ -1,9 +1,9 @@
 // RUN: %clang_cl_nocxx_asan %Gw %Od %s %Fe%t.exe
-// RUN: %env_asan_opts=report_globals=2 %t.exe 2>&1 | FileCheck %s --check-prefix=NOSTRIP
+// RUN: %env_asan_opts=report_globals=1:verbosity=3 %t.exe 2>&1 | FileCheck %s --check-prefix=NOSTRIP
 // RUN: %clang_cl_nocxx_asan %Gw -O2 %s %Fe%t.exe \
 // RUN:   %if target={{.*-windows-gnu}} %{ -Wl,--gc-sections %} \
 // RUN:   %else %{ -link -opt:ref %}
-// RUN: %env_asan_opts=report_globals=2 %t.exe 2>&1 | FileCheck %s --check-prefix=STRIP
+// RUN: %env_asan_opts=report_globals=1:verbosity=3 %t.exe 2>&1 | FileCheck %s --check-prefix=STRIP
 
 #include <stdio.h>
 int dead_global = 42;
diff --git a/compiler-rt/test/asan/TestCases/Windows/report_globals_vs_freelibrary.cpp b/compiler-rt/test/asan/TestCases/Windows/report_globals_vs_freelibrary.cpp
index 7cad3f39be1ec2..34ce18e146d677 100644
--- a/compiler-rt/test/asan/TestCases/Windows/report_globals_vs_freelibrary.cpp
+++ b/compiler-rt/test/asan/TestCases/Windows/report_globals_vs_freelibrary.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cl_asan %LD %Od -DDLL %s %Fe%t.dll
 // RUN: %clang_cl_asan %Od -DEXE %s %Fe%te.exe
-// RUN: %env_asan_opts=report_globals=2 %run %te.exe %t.dll 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=report_globals=1:verbosity=3 %run %te.exe %t.dll 2>&1 | FileCheck %s
 
 #include <windows.h>
 #include <stdio.h>
diff --git a/compiler-rt/test/asan/TestCases/initialization-nobug.cpp b/compiler-rt/test/asan/TestCases/initialization-nobug.cpp
index f66d501124bc48..61328b9de28ae6 100644
--- a/compiler-rt/test/asan/TestCases/initialization-nobug.cpp
+++ b/compiler-rt/test/asan/TestCases/initialization-nobug.cpp
@@ -1,10 +1,10 @@
 // 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 && %env_asan_opts=check_initialization_order=true:report_globals=3 %run %t 2>&1 | FileCheck %s --implicit-check-not "DynInitPoison"
-// 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 "DynInitPoison"
-// 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 "DynInitPoison"
-// 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 "DynInitPoison"
+// RUN: %clangxx_asan -O0 %s %p/Helpers/initialization-nobug-extra.cpp -o %t && %env_asan_opts=check_initialization_order=true:report_globals=1:verbosity=2 %run %t 2>&1 | FileCheck %s --implicit-check-not "DynInitPoison"
+// RUN: %clangxx_asan -O1 %s %p/Helpers/initialization-nobug-extra.cpp -o %t && %env_asan_opts=check_initialization_order=true:report_globals=1:verbosity=2 %run %t 2>&1 | FileCheck %s --implicit-check-not "DynInitPoison"
+// RUN: %clangxx_asan -O2 %s %p/Helpers/initialization-nobug-extra.cpp -o %t && %env_asan_opts=check_initialization_order=true:report_globals=1:verbosity=2 %run %t 2>&1 | FileCheck %s --implicit-check-not "DynInitPoison"
+// RUN: %clangxx_asan -O3 %s %p/Helpers/initialization-nobug-extra.cpp -o %t && %env_asan_opts=check_initialization_order=true:report_globals=1:verbosity=2 %run %t 2>&1 | FileCheck %s --implicit-check-not "DynInitPoison"
 
 // Simple access:
 // Make sure that accessing a global in the same TU is safe

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.

2 participants