Skip to content

Commit 0ea80f8

Browse files
committed
[lldb] Correctly check and report error states in StackFrame.cpp
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
1 parent 4b1254e commit 0ea80f8

File tree

1 file changed

+10
-12
lines changed

1 file changed

+10
-12
lines changed

lldb/source/Target/StackFrame.cpp

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -652,15 +652,15 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
652652
Status deref_error;
653653
if (valobj_sp->GetCompilerType().IsReferenceType()) {
654654
valobj_sp = valobj_sp->GetSyntheticValue()->Dereference(deref_error);
655-
if (error.Fail()) {
655+
if (!valobj_sp || deref_error.Fail()) {
656656
error.SetErrorStringWithFormatv(
657657
"Failed to dereference reference type: %s", deref_error);
658658
return ValueObjectSP();
659659
}
660660
}
661661

662662
valobj_sp = valobj_sp->Dereference(deref_error);
663-
if (error.Fail()) {
663+
if (!valobj_sp || deref_error.Fail()) {
664664
error.SetErrorStringWithFormatv(
665665
"Failed to dereference sythetic value: {0}", deref_error);
666666
return ValueObjectSP();
@@ -795,9 +795,9 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
795795
// what we have is *ptr[low]. the most similar C++ syntax is to deref
796796
// ptr and extract bit low out of it. reading array item low would be
797797
// done by saying ptr[low], without a deref * sign
798-
Status error;
799-
ValueObjectSP temp(valobj_sp->Dereference(error));
800-
if (error.Fail()) {
798+
Status deref_error;
799+
ValueObjectSP temp(valobj_sp->Dereference(deref_error));
800+
if (!temp || deref_error.Fail()) {
801801
valobj_sp->GetExpressionPath(var_expr_path_strm);
802802
error.SetErrorStringWithFormat(
803803
"could not dereference \"(%s) %s\"",
@@ -813,9 +813,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
813813
// arr[0] (an operation that is equivalent to deref-ing arr) and
814814
// extract bit low out of it. reading array item low would be done by
815815
// saying arr[low], without a deref * sign
816-
Status error;
817816
ValueObjectSP temp(valobj_sp->GetChildAtIndex(0));
818-
if (error.Fail()) {
817+
if (!temp) {
819818
valobj_sp->GetExpressionPath(var_expr_path_strm);
820819
error.SetErrorStringWithFormat(
821820
"could not get item 0 for \"(%s) %s\"",
@@ -994,9 +993,9 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
994993
// deref ptr and extract bits low thru high out of it. reading array
995994
// items low thru high would be done by saying ptr[low-high], without a
996995
// deref * sign
997-
Status error;
998-
ValueObjectSP temp(valobj_sp->Dereference(error));
999-
if (error.Fail()) {
996+
Status deref_error;
997+
ValueObjectSP temp(valobj_sp->Dereference(deref_error));
998+
if (!temp || deref_error.Fail()) {
1000999
valobj_sp->GetExpressionPath(var_expr_path_strm);
10011000
error.SetErrorStringWithFormat(
10021001
"could not dereference \"(%s) %s\"",
@@ -1011,9 +1010,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
10111010
// get arr[0] (an operation that is equivalent to deref-ing arr) and
10121011
// extract bits low thru high out of it. reading array items low thru
10131012
// high would be done by saying arr[low-high], without a deref * sign
1014-
Status error;
10151013
ValueObjectSP temp(valobj_sp->GetChildAtIndex(0));
1016-
if (error.Fail()) {
1014+
if (!temp) {
10171015
valobj_sp->GetExpressionPath(var_expr_path_strm);
10181016
error.SetErrorStringWithFormat(
10191017
"could not get item 0 for \"(%s) %s\"",

0 commit comments

Comments
 (0)