mossberg updated this revision to Diff 233673.
mossberg added a comment.
Output error messages to console in addition to logging.
I wasn't sure, but decided to unconditionally output the `Could not create
return address breakpoint.` message for UX reasons. That message provides a bit
of context for these new, lower level error messages.
Implementation is based on the one in `ThreadPlanCallFunction.cpp`.
I also slightly altered the error message for the "permissions" not found case
to make both error messages more consistent.
Still working on adding the unit test, but I thought I'd submit this for early
feedback.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71372/new/
https://reviews.llvm.org/D71372
Files:
lldb/include/lldb/Target/ThreadPlanStepOut.h
lldb/source/Target/ThreadPlanStepOut.cpp
Index: lldb/source/Target/ThreadPlanStepOut.cpp
===================================================================
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -126,6 +126,25 @@
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 @@
}
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;
}
Index: lldb/include/lldb/Target/ThreadPlanStepOut.h
===================================================================
--- lldb/include/lldb/Target/ThreadPlanStepOut.h
+++ lldb/include/lldb/Target/ThreadPlanStepOut.h
@@ -72,6 +72,7 @@
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,
Index: lldb/source/Target/ThreadPlanStepOut.cpp
===================================================================
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -126,6 +126,25 @@
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 @@
}
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;
}
Index: lldb/include/lldb/Target/ThreadPlanStepOut.h
===================================================================
--- lldb/include/lldb/Target/ThreadPlanStepOut.h
+++ lldb/include/lldb/Target/ThreadPlanStepOut.h
@@ -72,6 +72,7 @@
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,
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
