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\"", _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
