Skip to content

Commit 09c64e5

Browse files
[lldb] Restore register state if PrepareTrivialCall fails (#129038)
Fixes #124269 PrepareTrivalCall always had the possibility of failing, but given that it only wrote to general purpose registers, if it did, you had bigger problems. When it failed, we did not mark the thread plan valid and when it was torn down we didn't try to restore the register state. This meant that if you tried to continue, the program was unlikely to work. When I added AArch64 GCS support, I needed to handle the situation where the GCS pointer points to unmapped memory and we fail to write the extra entry we need. So I added code to restore the gcspr_el0 register specifically if this happened, and ordered the operations so that we tried this first. In this change I've made the teardown of an invalid thread plan restore the register state if one was saved. It may be there isn't one if ConstructorSetup fails, but this is ok because that function does not modify anything. Now that we're doing that, I don't need the GCS specific code anymore, and all thread plans are protected from this in the rare event something does fail. Testing is done by the existing GCS test case that points the gcspr into unmapped memory which causes PrepareTrivialCall to fail. I tried adding a simulated test using a mock gdb server. This was not possible because they all use DynamicLoaderStatic which disables all JIT features.
1 parent 2639dea commit 09c64e5

File tree

2 files changed

+13
-9
lines changed

2 files changed

+13
-9
lines changed

lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,7 @@ static Status PushToLinuxGuardedControlStack(addr_t return_addr,
102102
size_t wrote = thread.GetProcess()->WriteMemory(gcspr_el0, &return_addr,
103103
sizeof(return_addr), error);
104104
if ((wrote != sizeof(return_addr) || error.Fail())) {
105-
// When PrepareTrivialCall fails, the register context is not restored,
106-
// unlike when an expression fails to execute. This is arguably a bug,
107-
// see https://github.com/llvm/llvm-project/issues/124269.
108-
// For now we are handling this here specifically. We can assume this
109-
// write will work as the one to decrement the register did.
110-
reg_ctx->WriteRegisterFromUnsigned(gcspr_el0_info, gcspr_el0 + 8);
105+
// gcspr_el0 will be restored by the ThreadPlan's DoTakedown.
111106
return Status("Failed to write new Guarded Control Stack entry.");
112107
}
113108

@@ -150,8 +145,6 @@ bool ABISysV_arm64::PrepareTrivialCall(Thread &thread, addr_t sp,
150145
if (args.size() > 8)
151146
return false;
152147

153-
// Do this first, as it's got the most chance of failing (though still very
154-
// low).
155148
if (GetProcessSP()->GetTarget().GetArchitecture().GetTriple().isOSLinux()) {
156149
Status err = PushToLinuxGuardedControlStack(return_addr, reg_ctx, thread);
157150
// If we could not manage the GCS, the expression will certainly fail,

lldb/source/Target/ThreadPlanCallFunction.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,20 @@ void ThreadPlanCallFunction::ReportRegisterState(const char *message) {
174174

175175
void ThreadPlanCallFunction::DoTakedown(bool success) {
176176
Log *log = GetLog(LLDBLog::Step);
177+
Thread &thread = GetThread();
177178

178179
if (!m_valid) {
180+
// If ConstructorSetup was succesfull but PrepareTrivialCall was not,
181+
// we will have a saved register state and potentially modified registers.
182+
// Restore those.
183+
if (m_stored_thread_state.register_backup_sp)
184+
if (!thread.RestoreRegisterStateFromCheckpoint(m_stored_thread_state))
185+
LLDB_LOGF(
186+
log,
187+
"ThreadPlanCallFunction(%p): Failed to restore register state from "
188+
"invalid plan that contained a saved register state.",
189+
static_cast<void *>(this));
190+
179191
// Don't call DoTakedown if we were never valid to begin with.
180192
LLDB_LOGF(log,
181193
"ThreadPlanCallFunction(%p): Log called on "
@@ -185,7 +197,6 @@ void ThreadPlanCallFunction::DoTakedown(bool success) {
185197
}
186198

187199
if (!m_takedown_done) {
188-
Thread &thread = GetThread();
189200
if (success) {
190201
SetReturnValue();
191202
}

0 commit comments

Comments
 (0)