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

__asan_register_elf_globals: properly check the "no instrumented global variable" case #96529

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jun 24, 2024

On ELF platforms, the instrumentation registers global variables using
__asan_register_elf_globals for the default UseGlobalsGC case. If
all instrumented global variables in a module are discarded by linker
GC, we will have start == stop.

Normally start == 0, but start != 0 is possible with a linker script
retaining asan_globals. The called __asan_register_globals would
access out-of-bounds globals[n-1], though there is likely no runtime
failure.

Created using spr 1.3.5-bogner
@llvmbot
Copy link

llvmbot commented Jun 24, 2024

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

Author: Fangrui Song (MaskRay)

Changes

On ELF platforms, the instrumentation registers global variables using
__asan_register_elf_globals for the default UseGlobalsGC case. If
all instrumented global variables in a module are discarded by linker
GC, we will have start == stop.

Normally start == 0, but start != 0 is possible with a linker script
retaining asan_globals. The called __asan_register_globals would
access out-of-bounds globals[n-1], though there is likely no runtime
failure.


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

1 Files Affected:

  • (modified) compiler-rt/lib/asan/asan_globals.cpp (+4-4)
diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
index 6ac64c4b776bb..d413b1ebc9fc0 100644
--- a/compiler-rt/lib/asan/asan_globals.cpp
+++ b/compiler-rt/lib/asan/asan_globals.cpp
@@ -344,8 +344,8 @@ void __asan_unregister_image_globals(uptr *flag) {
 }
 
 void __asan_register_elf_globals(uptr *flag, void *start, void *stop) {
-  if (*flag) return;
-  if (!start) return;
+  if (*flag || start == stop)
+    return;
   CHECK_EQ(0, ((uptr)stop - (uptr)start) % sizeof(__asan_global));
   __asan_global *globals_start = (__asan_global*)start;
   __asan_global *globals_stop = (__asan_global*)stop;
@@ -354,8 +354,8 @@ void __asan_register_elf_globals(uptr *flag, void *start, void *stop) {
 }
 
 void __asan_unregister_elf_globals(uptr *flag, void *start, void *stop) {
-  if (!*flag) return;
-  if (!start) return;
+  if (!*flag || start == stop)
+    return;
   CHECK_EQ(0, ((uptr)stop - (uptr)start) % sizeof(__asan_global));
   __asan_global *globals_start = (__asan_global*)start;
   __asan_global *globals_stop = (__asan_global*)stop;

Copy link
Collaborator

@hctim hctim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MaskRay MaskRay merged commit bd5b775 into main Jun 25, 2024
10 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/__asan_register_elf_globals-properly-check-the-no-instrumented-global-variable-case branch June 25, 2024 20:14
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
…al variable" case

On ELF platforms, the instrumentation registers global variables using
`__asan_register_elf_globals` for the default `UseGlobalsGC` case. If
all instrumented global variables in a module are discarded by linker
GC, we will have `start == stop`.

Normally `start == 0`, but `start != 0` is possible with a linker script
retaining `asan_globals`. The called `__asan_register_globals` would
access out-of-bounds `globals[n-1]`, though there is likely no runtime
failure.

Pull Request: llvm#96529
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