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] Track current dynamic init module #101597

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Aug 2, 2024

This patch allows sequences like:
__asan_before_dynamic_init
__asan_before_dynamic_init
...
__asan_before_dynamic_init
to do minimal incrementa poisoning.

It's NFC as now callbacks invokes in pairs:
__asan_before_dynamic_init
__asan_after_dynamic_init
__asan_before_dynamic_init
__asan_after_dynamic_init
and __asan_after_dynamic_init unpoisons
everything anyway.

For #101837

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 2, 2024

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

Author: Vitaly Buka (vitalybuka)

Changes

It's NFC as now callbacks invokes in pairs:
__asan_before_dynamic_init
__asan_after_dynamic_init
__asan_before_dynamic_init
__asan_after_dynamic_init

And __asan_after_dynamic_init unpoison
everything and reset the current module.


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

1 Files Affected:

  • (modified) compiler-rt/lib/asan/asan_globals.cpp (+28-9)
diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index c7dcc47460e83..84f72a7143f13 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -70,6 +70,9 @@ static ListOfGlobals &GlobalsByIndicator(uptr odr_indicator)
   return (*globals_by_indicator)[odr_indicator];
 }
 
+static const char *current_dynamic_init_module_name
+    SANITIZER_GUARDED_BY(mu_for_globals) = nullptr;
+
 using DynInitGLobalsByModule =
     DenseMap<const char *, IntrusiveList<DynInitGlobal>>;
 
@@ -492,18 +495,29 @@ void __asan_before_dynamic_init(const char *module_name) {
   CHECK(module_name);
   CHECK(AsanInited());
   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);
 
-  DynInitGlobals().forEach([&](auto &kv) {
-    if (kv.first != module_name) {
-      PoisonDynamicGlobals(kv.second);
-    } else {
-      UnpoisonDynamicGlobals(kv.second,
-                             /*mark_initialized=*/!strict_init_order);
-    }
-    return true;
-  });
+  if (current_dynamic_init_module_name == nullptr) {
+    // First call, poison all globals from other modules.
+    DynInitGlobals().forEach([&](auto &kv) {
+      if (kv.first != module_name) {
+        PoisonDynamicGlobals(kv.second);
+      } else {
+        UnpoisonDynamicGlobals(kv.second,
+                               /*mark_initialized=*/!strict_init_order);
+      }
+      return true;
+    });
+  } else {
+    // Module changed.
+    PoisonDynamicGlobals(DynInitGlobals()[current_dynamic_init_module_name]);
+    UnpoisonDynamicGlobals(DynInitGlobals()[module_name],
+                           /*mark_initialized=*/!strict_init_order);
+  }
+  current_dynamic_init_module_name = module_name;
 }
 
 // This method runs immediately after dynamic initialization in each TU, when
@@ -514,6 +528,9 @@ void __asan_after_dynamic_init() {
     return;
   CHECK(AsanInited());
   Lock lock(&mu_for_globals);
+  if (!current_dynamic_init_module_name)
+    return;
+
   if (flags()->report_globals >= 3)
     Printf("DynInitUnpoison\n");
 
@@ -521,4 +538,6 @@ void __asan_after_dynamic_init() {
     UnpoisonDynamicGlobals(kv.second, /*mark_initialized=*/false);
     return true;
   });
+
+  current_dynamic_init_module_name = nullptr;
 }

@vitalybuka
Copy link
Collaborator Author

@artempyanykh

@fmayer
Copy link
Contributor

fmayer commented Aug 2, 2024

Why is this "[NFC]"? This looks like it changes behavior.

because it does not change observable behavior

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@vitalybuka vitalybuka changed the base branch from users/vitalybuka/spr/main.nfcasan-track-current-dynamic-init-module to main August 3, 2024 17:41
Created using spr 1.3.4
@vitalybuka vitalybuka merged commit 3a7861e into main Aug 3, 2024
6 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfcasan-track-current-dynamic-init-module branch August 3, 2024 19:56
kutemeikito added a commit to kutemeikito/llvm-project that referenced this pull request Aug 4, 2024
* 'main' of https://github.com/llvm/llvm-project:
  [RISCV] Improve hasAllNBitUsers for users of SLLI.
  [RISCV] Invert if conditions in the switch in RISCVDAGToDAGISel::hasAllNBitUsers. NFC
  [Transforms] Construct SmallVector with ArrayRef (NFC) (llvm#101851)
  [RISCV] Capitalize some variable names. NFC
  [sanitizer_common] Fix UnwindFast on SPARC (llvm#101634)
  [builtins] Fix divtc3.c etc. compilation on Solaris/SPARC with gcc (llvm#101662)
  [NFC][asan] Track current dynamic init module (llvm#101597)
  [libc] enable most of the entrypoints on aarch64 (llvm#101797)
  [SandboxIR][Tracker] Track InsertIntoBB (llvm#101595)
  [SCEV] Use const SCEV * explicitly in more places.
  [ELF] Move outputSections into Ctx. NFC
  [ELF] Move ElfSym into Ctx. NFC
  [ELF] Move Out into Ctx. NFC
  [test][asan] Fix the test checks
  [NFC][asan] Switch from list to DynInitGlobalsByModule (llvm#101596)

Signed-off-by: Edwiin Kusuma Jaya <kutemeikito0905@gmail.com>
banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
This patch allows sequences like:
`__asan_before_dynamic_init`
`__asan_before_dynamic_init`
...
`__asan_before_dynamic_init`
to do minimal incrementa poisoning.

It's NFC as now callbacks invokes in pairs:
`__asan_before_dynamic_init`
`__asan_after_dynamic_init`
`__asan_before_dynamic_init`
`__asan_after_dynamic_init`
and `__asan_after_dynamic_init` unpoisons
everything anyway.

For llvm#101837
vitalybuka added a commit that referenced this pull request Aug 8, 2024
The pair `__asan_before_dynamic_init` and `__asan_after_dynamic_init` is
executed for each TU. `__asan_after_dynamic_init` unpoisons all globals,
which
makes the time complexity O(N^2), where N is the maximum of the global
count and
the TU count. This is expensive for large binaries.

This patch decreases the time complexity to O(N), when lld and static
runtime is
used on SANITIZER_CAN_USE_PREINIT_ARRAY platforms. This requires:

Enabling incremental poisoning (`__asan_before_dynamic_init` since
#101597). Making most
`__asan_after_dynamic_init` calls do nothing.
kstoimenov pushed a commit to kstoimenov/llvm-project that referenced this pull request Aug 15, 2024
This patch allows sequences like:
`__asan_before_dynamic_init`
`__asan_before_dynamic_init`
...
`__asan_before_dynamic_init`
to do minimal incrementa poisoning.

It's NFC as now callbacks invokes in pairs:
`__asan_before_dynamic_init`
`__asan_after_dynamic_init`
`__asan_before_dynamic_init`
`__asan_after_dynamic_init`
and `__asan_after_dynamic_init` unpoisons
everything anyway.

For llvm#101837
kstoimenov pushed a commit to kstoimenov/llvm-project that referenced this pull request Aug 15, 2024
The pair `__asan_before_dynamic_init` and `__asan_after_dynamic_init` is
executed for each TU. `__asan_after_dynamic_init` unpoisons all globals,
which
makes the time complexity O(N^2), where N is the maximum of the global
count and
the TU count. This is expensive for large binaries.

This patch decreases the time complexity to O(N), when lld and static
runtime is
used on SANITIZER_CAN_USE_PREINIT_ARRAY platforms. This requires:

Enabling incremental poisoning (`__asan_before_dynamic_init` since
llvm#101597). Making most
`__asan_after_dynamic_init` calls do nothing.
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.

4 participants