Skip to content

[sanitizer] Change GetDTLSRange #108345

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

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

vitalybuka
Copy link
Collaborator

We only need to change size, tls_beg should be unchanged.

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Sep 12, 2024

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

Author: Vitaly Buka (vitalybuka)

Changes

We only need to change size, tls_beg should be unchanged.


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

1 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp (+11-9)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
index e5839f6f497474..17b9ff4aa6c40b 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
@@ -110,15 +110,15 @@ SANITIZER_WEAK_ATTRIBUTE
 const void *__sanitizer_get_allocated_begin(const void *p);
 }
 
-static bool GetDTLSRange(uptr &tls_beg, uptr &tls_size) {
+static uptr GetDTLSRange(uptr tls_beg) {
   const void *start = __sanitizer_get_allocated_begin((void *)tls_beg);
   if (!start)
-    return false;
-  tls_beg = (uptr)start;
-  tls_size = __sanitizer_get_allocated_size(start);
+    return 0;
+  CHECK_EQ(start, (void *)tls_beg);
+  uptr tls_size = __sanitizer_get_allocated_size(start);
   VReport(2, "__tls_get_addr: glibc DTLS suspected; tls={%p,0x%zx}\n",
           (void *)tls_beg, tls_size);
-  return true;
+  return tls_size;
 }
 
 DTLS::DTV *DTLS_on_tls_get_addr(void *arg_void, void *res,
@@ -142,10 +142,12 @@ DTLS::DTV *DTLS_on_tls_get_addr(void *arg_void, void *res,
     // creation.
     VReport(2, "__tls_get_addr: static tls: %p\n", (void *)tls_beg);
     tls_size = 0;
-  } else if (!GetDTLSRange(tls_beg, tls_size)) {
-    VReport(2, "__tls_get_addr: Can't guess glibc version\n");
-    // This may happen inside the DTOR of main thread, so just ignore it.
-    tls_size = 0;
+  } else {
+    tls_size = GetDTLSRange(tls_beg);
+    if (tls_size) {
+      VReport(2, "__tls_get_addr: Can't guess glibc version\n");
+      // This may happen inside the DTOR of main thread, so just ignore it.
+    }
   }
   dtv->beg = tls_beg;
   dtv->size = tls_size;

Created using spr 1.3.4
@vitalybuka vitalybuka merged commit ee92645 into main Sep 12, 2024
7 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/sanitizer-change-getdtlsrange branch September 12, 2024 17:27
vitalybuka added a commit that referenced this pull request Sep 18, 2024
The check is too strict. It works for 2.38 I have,
but not for older glibc which used different
allocation code.

The check was introduced with #108345.
tmsri pushed a commit to tmsri/llvm-project that referenced this pull request Sep 19, 2024
The check is too strict. It works for 2.38 I have,
but not for older glibc which used different
allocation code.

The check was introduced with llvm#108345.
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