Work-around an issue in Arm64 regarding the isolated use of CONTEXT_CONTROL. #313
Conversation
This file contains 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
…ers may become zeroed when CONTEXT_CONTROL is used without CONTEXT_INTEGER.
jaykrell
approved these changes
Nov 27, 2024
| #endif // DETOURS_X86 | ||
|
|
||
| #ifdef DETOURS_X64 | ||
| #define DETOURS_EIP Rip | ||
| #define DETOURS_CONTEXT_FLAGS (CONTEXT_CONTROL | CONTEXT_INTEGER) |
There was a problem hiding this comment.
Should we "just" use this consistently for all architectures?
i.e. also x86?
There was a problem hiding this comment.
Add comment explaining it? I realize there is git blame and PRs. Some orgs are super comment heavy these days and want everything explained on every line. I realize Detours is not that org.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Work-around an issue in Arm64 (and Arm64EC) in which LR and FP registers may become zeroed when CONTEXT_CONTROL is used without CONTEXT_INTEGER.
The addition of the CONTEXT_INTEGER flag does not translate to any real overhead: The kernel performs relatively expensive stack unwind operations for Get and Set ThreadContext touching and scanning over large unwind info datastructures. In scenario the extra few integers register copies (which share cache lines with CONTEXT_CONTROL) can't be observed in the scale and the noise.
This change is adding CONTEXT_INTEGER to the Get and SetThread context calls for both Arm64 and x86_64 so that Arm64EC is also addressed.
This issue is being addressed in the OS as well, but it will take time to disseminate the fix and backport it to all released versions. When the fix is out, this change can arguably be considered redundant, but given there is no real downside, I don't see an objective reason to not add it or making diligent plans to remove it.