[Lldb-commits] [lldb] 0e9879e - [lldb] Correctly check and report error states in StackFrame.cpp (#74414)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Dec 12 04:50:22 PST 2023
Author: Pete Lawrence
Date: 2023-12-12T09:50:18-03:00
New Revision: 0e9879ed9711ac280c5e1ea47f77a033393d6baa
URL: https://github.com/llvm/llvm-project/commit/0e9879ed9711ac280c5e1ea47f77a033393d6baa
DIFF: https://github.com/llvm/llvm-project/commit/0e9879ed9711ac280c5e1ea47f77a033393d6baa.diff
LOG: [lldb] Correctly check and report error states in StackFrame.cpp (#74414)
This commits fixes a few subtle bugs where the method:
1. Declares a local `Status error` which eclipses the method's parameter
`Status &error`.
- The method then sets the error state to the local `error` and returns
without ever touching the parameter `&error`.
- This effectively traps the error state and its message from ever
reaching the caller.
- I also threw in a null pointer check in case the callee doesn't set
its `Status` parameter but returns `0`/`nullptr`.
2. Declares a local `Status deref_error` (good), passes it to the
`Dereference` method (also good), but then checks the status of the
method's `Status &error` parameter (not good).
- The fix checks `deref_error` instead and also checks for a `nullptr`
return value.
- There's a good opportunity here for a future PR that changes the
`Dereference` method to fold an error state into the `ValueObject`
return value's `m_error` instead of using a parameter.
3. Declares another local `Status error`, which it doesn't pass to a
method (because there isn't a parameter for it), and then checks for an
error condition that never happens.
- The fix just checks the callee's return value, because that's all it
has to go on.
- This likely comes from a copy/paste from issue 1 above.
rdar://119155810
Added:
Modified:
lldb/source/Target/StackFrame.cpp
Removed:
################################################################################
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index f0d78d8aa5fefc..50cf01e63cd493 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -652,7 +652,7 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
Status deref_error;
if (valobj_sp->GetCompilerType().IsReferenceType()) {
valobj_sp = valobj_sp->GetSyntheticValue()->Dereference(deref_error);
- if (error.Fail()) {
+ if (!valobj_sp || deref_error.Fail()) {
error.SetErrorStringWithFormatv(
"Failed to dereference reference type: %s", deref_error);
return ValueObjectSP();
@@ -660,7 +660,7 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
}
valobj_sp = valobj_sp->Dereference(deref_error);
- if (error.Fail()) {
+ if (!valobj_sp || deref_error.Fail()) {
error.SetErrorStringWithFormatv(
"Failed to dereference sythetic value: {0}", deref_error);
return ValueObjectSP();
@@ -795,9 +795,9 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
// what we have is *ptr[low]. the most similar C++ syntax is to deref
// ptr and extract bit low out of it. reading array item low would be
// done by saying ptr[low], without a deref * sign
- Status error;
- ValueObjectSP temp(valobj_sp->Dereference(error));
- if (error.Fail()) {
+ Status deref_error;
+ ValueObjectSP temp(valobj_sp->Dereference(deref_error));
+ if (!temp || deref_error.Fail()) {
valobj_sp->GetExpressionPath(var_expr_path_strm);
error.SetErrorStringWithFormat(
"could not dereference \"(%s) %s\"",
@@ -813,9 +813,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
// arr[0] (an operation that is equivalent to deref-ing arr) and
// extract bit low out of it. reading array item low would be done by
// saying arr[low], without a deref * sign
- Status error;
ValueObjectSP temp(valobj_sp->GetChildAtIndex(0));
- if (error.Fail()) {
+ if (!temp) {
valobj_sp->GetExpressionPath(var_expr_path_strm);
error.SetErrorStringWithFormat(
"could not get item 0 for \"(%s) %s\"",
@@ -994,9 +993,9 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
// deref ptr and extract bits low thru high out of it. reading array
// items low thru high would be done by saying ptr[low-high], without a
// deref * sign
- Status error;
- ValueObjectSP temp(valobj_sp->Dereference(error));
- if (error.Fail()) {
+ Status deref_error;
+ ValueObjectSP temp(valobj_sp->Dereference(deref_error));
+ if (!temp || deref_error.Fail()) {
valobj_sp->GetExpressionPath(var_expr_path_strm);
error.SetErrorStringWithFormat(
"could not dereference \"(%s) %s\"",
@@ -1011,9 +1010,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
// get arr[0] (an operation that is equivalent to deref-ing arr) and
// extract bits low thru high out of it. reading array items low thru
// high would be done by saying arr[low-high], without a deref * sign
- Status error;
ValueObjectSP temp(valobj_sp->GetChildAtIndex(0));
- if (error.Fail()) {
+ if (!temp) {
valobj_sp->GetExpressionPath(var_expr_path_strm);
error.SetErrorStringWithFormat(
"could not get item 0 for \"(%s) %s\"",
More information about the lldb-commits
mailing list