Skip to content

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

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

@vitalybuka vitalybuka merged commit e185850 into main Aug 24, 2024
5 of 6 checks passed
@vitalybuka vitalybuka deleted the revert-105895-users/vitalybuka/spr/reland-asan-remove-debug-tracing-from-report_globals-104404-1 branch August 24, 2024 06:29
@llvmbot
Copy link
Member

llvmbot commented Aug 24, 2024

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

Author: Vitaly Buka (vitalybuka)

Changes

Reverts llvm/llvm-project#105895

Still breaks the test https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-RA/1864/


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

10 Files Affected:

  • (modified) compiler-rt/lib/asan/asan_flags.inc (+5-2)
  • (modified) compiler-rt/lib/asan/asan_globals.cpp (+11-8)
  • (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 5e0ced9706e664..fad1577d912a5e 100644
--- a/compiler-rt/lib/asan/asan_flags.inc
+++ b/compiler-rt/lib/asan/asan_flags.inc
@@ -36,8 +36,11 @@ ASAN_FLAG(int, max_redzone, 2048,
 ASAN_FLAG(
     bool, debug, false,
     "If set, prints some debugging information and does additional checks.")
-ASAN_FLAG(bool, report_globals, true,
-          "If set, detect and report errors on globals .")
+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, 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 bf0edce937f06e..c83b782cb85f89 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -22,7 +22,6 @@
 #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"
@@ -180,7 +179,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 (UNLIKELY(common_flags()->verbosity >= 3))
+    if (flags()->report_globals >= 2)
       ReportGlobal(g, "Search");
     if (IsAddressNearGlobal(addr, g)) {
       internal_memcpy(&globals[res], &g, sizeof(g));
@@ -271,7 +270,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 (UNLIKELY(common_flags()->verbosity >= 3))
+  if (flags()->report_globals >= 2)
     ReportGlobal(*g, "Added");
   CHECK(flags()->report_globals);
   CHECK(AddrIsInMem(g->beg));
@@ -308,7 +307,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 (UNLIKELY(common_flags()->verbosity >= 3))
+  if (flags()->report_globals >= 2)
     ReportGlobal(*g, "Removed");
   CHECK(flags()->report_globals);
   CHECK(AddrIsInMem(g->beg));
@@ -439,7 +438,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 (UNLIKELY(common_flags()->verbosity >= 4)) {
+  if (flags()->report_globals >= 2) {
     PRINT_CURRENT_STACK();
     Printf("=== ID %d; %p %p\n", stack_id, (void *)&globals[0],
            (void *)&globals[n - 1]);
@@ -498,7 +497,9 @@ void __asan_before_dynamic_init(const char *module_name) {
   Lock lock(&mu_for_globals);
   if (current_dynamic_init_module_name == module_name)
     return;
-  VPrintf(2, "DynInitPoison module: %s\n", module_name);
+  if (flags()->report_globals >= 3)
+    Printf("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) {
@@ -544,7 +545,8 @@ static void UnpoisonBeforeMain(void) {
       return;
     allow_after_dynamic_init = true;
   }
-  VPrintf(2, "UnpoisonBeforeMain\n");
+  if (flags()->report_globals >= 3)
+    Printf("UnpoisonBeforeMain\n");
   __asan_after_dynamic_init();
 }
 
@@ -568,7 +570,8 @@ void __asan_after_dynamic_init() {
   if (!current_dynamic_init_module_name)
     return;
 
-  VPrintf(2, "DynInitUnpoison\n");
+  if (flags()->report_globals >= 3)
+    Printf("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 ef82c7a29575eb..5cec029811cbc8 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=1:verbosity=2 %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=3 %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 b75f5be101ef8a..0f2ed6597154bb 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=1:detect_odr_violation=1:verbosity=3 %run %t 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=report_globals=2:detect_odr_violation=1 %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 f28a9f6d07386d..8af3ec09be78c4 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=1:verbosity=3 %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK,INDICATOR0
+// 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=1:verbosity=3 %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK,INDICATOR1
+// RUN: %env_asan_opts=report_globals=2 %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 e5bd27bdf65fdf..a0c96622efeea4 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=1:verbosity=3 %run %t %t.dll 2>&1 | FileCheck %s --check-prefix=NOSTRIP
+// RUN: %env_asan_opts=report_globals=2 %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=1:verbosity=3 %run %t %t.dll 2>&1 | FileCheck %s --check-prefix=STRIP
+// RUN: %env_asan_opts=report_globals=2 %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 c74b66f2b43b3e..06a632e6708b1e 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=1:verbosity=3 %run %te.exe 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=report_globals=2 %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 7f2405fdfc8364..0e15120a46f776 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=1:verbosity=3 %t.exe 2>&1 | FileCheck %s --check-prefix=NOSTRIP
+// RUN: %env_asan_opts=report_globals=2 %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=1:verbosity=3 %t.exe 2>&1 | FileCheck %s --check-prefix=STRIP
+// RUN: %env_asan_opts=report_globals=2 %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 34ce18e146d677..7cad3f39be1ec2 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=1:verbosity=3 %run %te.exe %t.dll 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=report_globals=2 %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 61328b9de28ae6..f66d501124bc48 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=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"
+// 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"
 
 // 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