-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[lldb] Tighten ABI assert in StopInfoMachException::DeterminePtrauthFailure
(NFC)
#95015
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
[lldb] Tighten ABI assert in StopInfoMachException::DeterminePtrauthFailure
(NFC)
#95015
Conversation
…ailure (NFC) This patch tightens the assert check for the ABISP object in `StopInfoMachException::DeterminePtrauthFailure`. This causes some failure when debugging on a system that doesn't have pointer authentification support, like on Intel for instance. rdar://129401926 Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
@llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) ChangesThis patch tightens the assert check for the ABISP object in This causes some failure when debugging on a system that doesn't have pointer authentification support, like on Intel for instance. rdar://129401926 Full diff: https://github.com/llvm/llvm-project/pull/95015.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index 75504323b4fdf..25cee369d7ee3 100644
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -92,9 +92,7 @@ bool StopInfoMachException::DeterminePtrauthFailure(ExecutionContext &exe_ctx) {
Target &target = *exe_ctx.GetTargetPtr();
Process &process = *exe_ctx.GetProcessPtr();
- ABISP abi_sp = process.GetABI();
const ArchSpec &arch = target.GetArchitecture();
- assert(abi_sp && "Missing ABI info");
// Check for a ptrauth-enabled target.
const bool ptrauth_enabled_target =
@@ -110,6 +108,9 @@ bool StopInfoMachException::DeterminePtrauthFailure(ExecutionContext &exe_ctx) {
strm.Printf("Note: Possible pointer authentication failure detected.\n");
};
+ ABISP abi_sp = process.GetABI();
+ assert(abi_sp && "Missing ABI info");
+
// Check if we have a "brk 0xc47x" trap, where the value that failed to
// authenticate is in x16.
Address current_address = current_frame->GetFrameCodeAddress();
|
…Failure` (NFC) (llvm#95015) This patch tightens the assert check for the ABISP object in `StopInfoMachException::DeterminePtrauthFailure`. This causes some failure when debugging on a system that doesn't have pointer authentification support, like on Intel for instance. rdar://129401926 Signed-off-by: Med Ismail Bennani <ismail@bennani.ma> (cherry picked from commit 93b91dd)
this is fine but fwiw there are |
@@ -110,6 +108,9 @@ bool StopInfoMachException::DeterminePtrauthFailure(ExecutionContext &exe_ctx) { | |||
strm.Printf("Note: Possible pointer authentication failure detected.\n"); | |||
}; | |||
|
|||
ABISP abi_sp = process.GetABI(); | |||
assert(abi_sp && "Missing ABI info"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really want to crash the debugger if there is no abi plug-in? I would like us to not crash by reporting an error to the debug console and being able to let the debugger live and just check abi_sp
before we use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clayborg Would you be fine with that ?
assert(abi_sp && "Missing ABI info"); | |
if(!abi_sp) { | |
// log missing ABI info | |
return false; | |
} |
@jasonmolenda would we still need to call FixAnyAddress
here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh I'm not entirely sure why we're here at all for an x86_64 target? this method is only be called when the target cpu is llvm::Triple::aarch64. It fetches the value of the x16 register just before this, and returns if no such register is found I think? But yes, I would change these to not get the ABI/assert. Instead, call Process::FixCodeAddress at the point we're clearing the non-addressable bits. Process::FixCodeAddress will silently return the same value if there is no ABI (and if it gets an ABI without a FixCodeAddress override method, it will also return the same value).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, FWIW, this is happening with crashlog
command when loading an arm64 target on an intel machine. That could explain why we're seeing this behavior.
…Failure` (NFC) (llvm#95015) This patch tightens the assert check for the ABISP object in `StopInfoMachException::DeterminePtrauthFailure`. This causes some failure when debugging on a system that doesn't have pointer authentification support, like on Intel for instance. rdar://129401926 Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch tightens the assert check for the ABISP object in
StopInfoMachException::DeterminePtrauthFailure
.This causes some failure when debugging on a system that doesn't have pointer authentification support, like on Intel for instance.
rdar://129401926