linux/arm64: Verify TLS resolver code#108419
Merged
kunalspathak merged 2 commits intodotnet:mainfrom Oct 4, 2024
Merged
Conversation
Contributor
|
Tagging subscribers to this area: @mangod9 |
Contributor
Author
|
FYI @shushanhf - I did not touch loongarch code because seems the instruction stream is different, but you might want to do something similar. |
3 tasks
Contributor
Author
|
I have also moved |
This was referenced Oct 4, 2024
davidwrighton
approved these changes
Oct 4, 2024
Member
davidwrighton
left a comment
There was a problem hiding this comment.
I validated locally and it appears to work.
Contributor
Author
|
/backport to release/9.0 |
Contributor
|
Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/11188481884 |
4 tasks
sirntar
pushed a commit
to sirntar/runtime
that referenced
this pull request
Oct 8, 2024
* linux/arm64: Verify TLS resolver code * Move the flag at the top so customer can turn it off to get unblocked
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
For linux/arm64, in #106052 , we added a piece of code to retrieve the TLS resolver to check if it is static or dynamic resolver and disable the TLS optimization if it was dynamic. In that, we disassemble the resolver address to check the instruction stream in order to determine if it is static or dynamic resolver. However, when SingleApps are compiled, we noticed that the TLS resolver address retrieval code was linked with different code sequence because of which it did not return the correct address of resolver that the caller was expecting. The caller simply tries to de-reference the value (it assumes it is a function pointer), but with invalid value, we would segfault. I added a check for the TLS resolver retrieval code itself to make sure that it is in correct format.
On my local machine, I was able to repro the original issue, but when I tried reproing it by pointing it to locally build
singleAppHost, the resolver code did return an incorrect address, but it was not a totally invalid one and the code sequence was getting dereferenced and disassembled. So, I was not able to see the segfault, but the existing checks were able to find out that the resolver is incorrect and was not doing the optimization. With the fix, I did verify that we would bail out even before invoking the resolver code.I did locally verify that JIT linux/arm64 scenario still triggers the TLS optimization.
I do not have much knowledge about the SingleAppHost test, and there is a test hole because of which we were not able to catch this issue earlier. @agocke / @elinor-fung - I will need your help in writing and enabling a test for it. We can use the existing environment variable
DOTNET_DisableOptimizedThreadStaticAccessto make sure we hit the code path and validate the code.The issue was reported by the customer in #108327.