Author: Jim Ingham Date: 2019-12-20T11:02:24-08:00 New Revision: 2a42a5a2f4144cd99812ad0d230480f94a1d1c92
URL: https://github.com/llvm/llvm-project/commit/2a42a5a2f4144cd99812ad0d230480f94a1d1c92 DIFF: https://github.com/llvm/llvm-project/commit/2a42a5a2f4144cd99812ad0d230480f94a1d1c92.diff LOG: In 'thread step-out' command, only insert a breakpoint in executable memory. Previously, if the current function had a nonstandard stack layout/ABI, and had a valid data pointer in the location where the return address is usually located, data corruption would occur when the breakpoint was written. This could lead to an incorrectly reported crash or silent corruption of the program's state. Now, if the above check fails, the command safely aborts. Differential Revision: https://reviews.llvm.org/D71372 Added: lldb/test/Shell/Unwind/Inputs/thread-step-out-ret-addr-check.s lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test Modified: lldb/include/lldb/Target/ThreadPlanStepOut.h lldb/source/Target/ThreadPlanStepOut.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/ThreadPlanStepOut.h b/lldb/include/lldb/Target/ThreadPlanStepOut.h index 00984db2dca9..576b416c3f2c 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOut.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOut.h @@ -72,6 +72,7 @@ class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere { std::vector<lldb::StackFrameSP> m_stepped_past_frames; lldb::ValueObjectSP m_return_valobj_sp; bool m_calculate_return_value; + StreamString m_constructor_errors; friend lldb::ThreadPlanSP Thread::QueueThreadPlanForStepOut( bool abort_other_plans, SymbolContext *addr_context, bool first_insn, diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp index d7dae446b229..f15a343aaa38 100644 --- a/lldb/source/Target/ThreadPlanStepOut.cpp +++ b/lldb/source/Target/ThreadPlanStepOut.cpp @@ -126,6 +126,25 @@ ThreadPlanStepOut::ThreadPlanStepOut( if (m_return_addr == LLDB_INVALID_ADDRESS) return; + // Perform some additional validation on the return address. + uint32_t permissions = 0; + if (!m_thread.GetProcess()->GetLoadAddressPermissions(m_return_addr, + permissions)) { + m_constructor_errors.Printf("Return address (0x%" PRIx64 + ") permissions not found.", + m_return_addr); + LLDB_LOGF(log, "ThreadPlanStepOut(%p): %s", static_cast<void *>(this), + m_constructor_errors.GetData()); + return; + } else if (!(permissions & ePermissionsExecutable)) { + m_constructor_errors.Printf("Return address (0x%" PRIx64 + ") did not point to executable memory.", + m_return_addr); + LLDB_LOGF(log, "ThreadPlanStepOut(%p): %s", static_cast<void *>(this), + m_constructor_errors.GetData()); + return; + } + Breakpoint *return_bp = m_thread.CalculateTarget() ->CreateBreakpoint(m_return_addr, true, false) .get(); @@ -238,8 +257,13 @@ bool ThreadPlanStepOut::ValidatePlan(Stream *error) { } if (m_return_bp_id == LLDB_INVALID_BREAK_ID) { - if (error) + if (error) { error->PutCString("Could not create return address breakpoint."); + if (m_constructor_errors.GetSize() > 0) { + error->PutCString(" "); + error->PutCString(m_constructor_errors.GetString()); + } + } return false; } diff --git a/lldb/test/Shell/Unwind/Inputs/thread-step-out-ret-addr-check.s b/lldb/test/Shell/Unwind/Inputs/thread-step-out-ret-addr-check.s new file mode 100644 index 000000000000..d18ea24fba4d --- /dev/null +++ b/lldb/test/Shell/Unwind/Inputs/thread-step-out-ret-addr-check.s @@ -0,0 +1,20 @@ + .text + .globl asm_main +asm_main: + sub $0x8, %rsp + movq $0, (%rsp) + push %rsp + jmp _nonstandard_stub + +# Takes a single pointer argument via the stack, which is nonstandard for x64. +# Executing 'thread step-out' here will initially attempt to write a +# breakpoint to that stack address, but should fail because of the executable +# memory check. +_nonstandard_stub: + mov (%rsp), %rdi + mov (%rdi), %rsi + add $1, %rsi + mov %rsi, (%rdi) + + add $0x10, %rsp + ret diff --git a/lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test b/lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test new file mode 100644 index 000000000000..96490faa2de8 --- /dev/null +++ b/lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test @@ -0,0 +1,17 @@ +# Test that `thread step-out` fails when the "return address" +# points to non-executable memory. + +# REQUIRES: target-x86_64, native + +# RUN: %clang_host %p/Inputs/call-asm.c %p/Inputs/thread-step-out-ret-addr-check.s -o %t +# RUN: %lldb %t -s %s -b 2>&1 | FileCheck %s + +breakpoint set -n nonstandard_stub +# CHECK: Breakpoint 1: where = {{.*}}`nonstandard_stub + +process launch +# CHECK: stop reason = breakpoint 1.1 + +thread step-out +# CHECK: Could not create return address breakpoint. +# CHECK: Return address (0x{{[a-f0-9]*}}) did not point to executable memory. _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
