[lldb] Fix TLS support on Darwin platforms #151601
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When I wrote this previously, I was unaware that the TLS function already adds the offset. The test was working previously because the offset was 0 in this case (only 1 thread-local variable). I added another thread-local variable to the test to make sure the offset is indeed handled correctly. rdar://156547548
Member
|
@llvm/pr-subscribers-lldb Author: Alex Langford (bulbazord) ChangesWhen I wrote this previously, I was unaware that the TLS function already adds the offset. The test was working previously because the offset was 0 in this case (only 1 thread-local variable). I added another thread-local variable to the test to make sure the offset is indeed handled correctly. rdar://156547548 Full diff: https://github.com/llvm/llvm-project/pull/151601.diff 3 Files Affected:
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 1270d57423c7b..8deb17c99136e 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -1159,9 +1159,8 @@ DynamicLoaderDarwin::GetThreadLocalData(const lldb::ModuleSP module_sp,
// size_t offset;
// }
//
- // The strategy is to take get_addr, call it with the address of the
- // containing TLS_Thunk structure, and add the offset to the resulting
- // pointer to get the data block.
+ // The strategy is to take get_addr and call it with the address of the
+ // containing TLS_Thunk structure.
//
// On older apple platforms, the key is treated as a pthread_key_t and passed
// to pthread_getspecific. The pointer returned from that call is added to
@@ -1190,7 +1189,7 @@ DynamicLoaderDarwin::GetThreadLocalData(const lldb::ModuleSP module_sp,
const addr_t tls_data = evaluate_tls_address(
thunk_load_addr, llvm::ArrayRef<addr_t>(tls_load_addr));
if (tls_data != LLDB_INVALID_ADDRESS)
- return tls_data + tls_offset;
+ return tls_data;
}
}
diff --git a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
index 5c0f304bdb37e..dad2b19a67170 100644
--- a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -73,6 +73,11 @@ def test(self):
VARIABLES_DISPLAYED_CORRECTLY,
patterns=[r"\(int\) \$.* = 88"],
)
+ self.expect(
+ "expr var_static2",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ patterns=[r"\(int\) \$.* = 66"],
+ )
self.expect(
"expr var_shared",
VARIABLES_DISPLAYED_CORRECTLY,
@@ -104,6 +109,11 @@ def test(self):
VARIABLES_DISPLAYED_CORRECTLY,
patterns=[r"\(int\) \$.* = 44"],
)
+ self.expect(
+ "expr var_static2",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ patterns=[r"\(int\) \$.* = 22"],
+ )
self.expect(
"expr var_shared",
VARIABLES_DISPLAYED_CORRECTLY,
diff --git a/lldb/test/API/lang/c/tls_globals/main.c b/lldb/test/API/lang/c/tls_globals/main.c
index bdfd78c1ac34b..890f823c2046b 100644
--- a/lldb/test/API/lang/c/tls_globals/main.c
+++ b/lldb/test/API/lang/c/tls_globals/main.c
@@ -10,10 +10,12 @@ touch_shared();
// Create some TLS storage within the static executable.
__thread int var_static = 44;
+__thread int var_static2 = 22;
void *fn_static(void *param)
{
var_static *= 2;
+ var_static2 *= 3;
shared_check();
usleep(1); // thread breakpoint
for(;;)
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions c,cpp -- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp lldb/test/API/lang/c/tls_globals/main.cView the diff from clang-format here.diff --git a/lldb/test/API/lang/c/tls_globals/main.c b/lldb/test/API/lang/c/tls_globals/main.c
index fac760b35..f1efaca05 100644
--- a/lldb/test/API/lang/c/tls_globals/main.c
+++ b/lldb/test/API/lang/c/tls_globals/main.c
@@ -15,8 +15,8 @@ __thread int var_static2 = 22;
void *fn_static(void *param)
{
var_static *= 2;
- var_static2 *= 3;
- shared_check();
+ var_static2 *= 3;
+ shared_check();
usleep(1); // thread breakpoint
for(;;)
usleep(1);
|
Michael137
approved these changes
Aug 4, 2025
Member
Michael137
left a comment
Michael137
left a comment
There was a problem hiding this comment.
Based on my reading of the relevant dyld change this makes sense, thanks!
This does mean we won't handle the old TLS format correctly though right?
Member
Author
My understanding is that this should handle the "old" TLS format correctly. When I updated this code 2 years ago, I think I misunderstood the dyld implementation and this worked because of a coincidence. |
medismailben
reviewed
Aug 4, 2025
| void *fn_static(void *param) | ||
| { | ||
| var_static *= 2; | ||
| var_static2 *= 3; |
bulbazord
added a commit
to bulbazord/llvm-project
that referenced
this pull request
Aug 4, 2025
When I wrote this previously, I was unaware that the TLS function already adds the offset. The test was working previously because the offset was 0 in this case (only 1 thread-local variable). I added another thread-local variable to the test to make sure the offset is indeed handled correctly. rdar://156547548 (cherry picked from commit a27d34b)
JDevlieghere
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Aug 5, 2025
When I wrote this previously, I was unaware that the TLS function already adds the offset. The test was working previously because the offset was 0 in this case (only 1 thread-local variable). I added another thread-local variable to the test to make sure the offset is indeed handled correctly. rdar://156547548 (cherry picked from commit a27d34b)
augusto2112
pushed a commit
to augusto2112/llvm-project
that referenced
this pull request
Aug 15, 2025
When I wrote this previously, I was unaware that the TLS function already adds the offset. The test was working previously because the offset was 0 in this case (only 1 thread-local variable). I added another thread-local variable to the test to make sure the offset is indeed handled correctly. rdar://156547548 (cherry picked from commit a27d34b)
augusto2112
pushed a commit
to augusto2112/llvm-project
that referenced
this pull request
Aug 15, 2025
When I wrote this previously, I was unaware that the TLS function already adds the offset. The test was working previously because the offset was 0 in this case (only 1 thread-local variable). I added another thread-local variable to the test to make sure the offset is indeed handled correctly. rdar://156547548 (cherry picked from commit a27d34b)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When I wrote this previously, I was unaware that the TLS function already adds the offset. The test was working previously because the offset was 0 in this case (only 1 thread-local variable). I added another thread-local variable to the test to make sure the offset is indeed handled correctly.
rdar://156547548