jarin created this revision. jarin added reviewers: teemperor, labath. Herald added subscribers: lldb-commits, arphaman. Herald added a project: LLDB. jarin retitled this revision from "Fix handling of [<index>] in 'frame variable'" to "Fix error handling after [<index>] in 'frame variable'".
This fixes a bug where frame var a[0]+5 returns the value a[0] without any warning because the current logic simply ignores everything after ']' as long as there is no '.', '-' or '[' in the rest of the string. The fix simplifies the termination condition of the expression path parsing loop to check if have a non-empty remaining string to parse. Previously, the condition checked if a separator was found. That condition coincided with the remaining string-to-parse condition except for the buggy indexed case where non-empty string was left ("+5" in the example above), but the separator index was 'npos'. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D79404 Files: lldb/source/Target/StackFrame.cpp lldb/test/API/functionalities/var_path/TestVarPath.py Index: lldb/test/API/functionalities/var_path/TestVarPath.py =================================================================== --- lldb/test/API/functionalities/var_path/TestVarPath.py +++ lldb/test/API/functionalities/var_path/TestVarPath.py @@ -25,7 +25,7 @@ def verify_point(self, frame, var_name, var_typename, x_value, y_value): v = frame.GetValueForVariablePath(var_name) self.assertTrue(v.GetError().Success(), "Make sure we find '%s'" % (var_name)) - self.assertEquals(v.GetType().GetName(), var_typename, + self.assertEquals(v.GetType().GetName(), var_typename, "Make sure '%s' has type '%s'" % (var_name, var_typename)) if '*' in var_typename: @@ -76,11 +76,14 @@ self.verify_point(frame, 'pt_ptr[1]', 'Point', 5050, 6060) # Test arrays v = frame.GetValueForVariablePath('points') - self.assertTrue(v.GetError().Success(), + self.assertTrue(v.GetError().Success(), "Make sure we find 'points'") self.verify_point(frame, 'points[0]', 'Point', 1010, 2020) self.verify_point(frame, 'points[1]', 'Point', 3030, 4040) self.verify_point(frame, 'points[2]', 'Point', 5050, 6060) + v = frame.GetValueForVariablePath('points[0]+5') + self.assertTrue(v.GetError().Fail(), + "Make sure we do not ignore characters between ']' and the end") # Test a reference self.verify_point(frame, 'pt_ref', 'Point &', 1, 2) v = frame.GetValueForVariablePath('pt_sp') @@ -88,7 +91,7 @@ # Make sure we don't crash when looking for non existant child # in type with synthetic children. This used to cause a crash. v = frame.GetValueForVariablePath('pt_sp->not_valid_child') - self.assertTrue(v.GetError().Fail(), + self.assertTrue(v.GetError().Fail(), "Make sure we don't find 'pt_sp->not_valid_child'") Index: lldb/source/Target/StackFrame.cpp =================================================================== --- lldb/source/Target/StackFrame.cpp +++ lldb/source/Target/StackFrame.cpp @@ -606,7 +606,7 @@ } // We are dumping at least one child - while (separator_idx != std::string::npos) { + while (!var_expr.empty()) { // Calculate the next separator index ahead of time ValueObjectSP child_valobj_sp; const char separator_type = var_expr[0]; @@ -940,7 +940,6 @@ return ValueObjectSP(); } - separator_idx = var_expr.find_first_of(".-["); if (use_dynamic != eNoDynamicValues) { ValueObjectSP dynamic_value_sp( child_valobj_sp->GetDynamicValue(use_dynamic)); @@ -1025,7 +1024,6 @@ return ValueObjectSP(); } - separator_idx = var_expr.find_first_of(".-["); if (use_dynamic != eNoDynamicValues) { ValueObjectSP dynamic_value_sp( child_valobj_sp->GetDynamicValue(use_dynamic)); @@ -1051,9 +1049,6 @@ if (child_valobj_sp) valobj_sp = child_valobj_sp; - - if (var_expr.empty()) - break; } if (valobj_sp) { if (deref) {
Index: lldb/test/API/functionalities/var_path/TestVarPath.py =================================================================== --- lldb/test/API/functionalities/var_path/TestVarPath.py +++ lldb/test/API/functionalities/var_path/TestVarPath.py @@ -25,7 +25,7 @@ def verify_point(self, frame, var_name, var_typename, x_value, y_value): v = frame.GetValueForVariablePath(var_name) self.assertTrue(v.GetError().Success(), "Make sure we find '%s'" % (var_name)) - self.assertEquals(v.GetType().GetName(), var_typename, + self.assertEquals(v.GetType().GetName(), var_typename, "Make sure '%s' has type '%s'" % (var_name, var_typename)) if '*' in var_typename: @@ -76,11 +76,14 @@ self.verify_point(frame, 'pt_ptr[1]', 'Point', 5050, 6060) # Test arrays v = frame.GetValueForVariablePath('points') - self.assertTrue(v.GetError().Success(), + self.assertTrue(v.GetError().Success(), "Make sure we find 'points'") self.verify_point(frame, 'points[0]', 'Point', 1010, 2020) self.verify_point(frame, 'points[1]', 'Point', 3030, 4040) self.verify_point(frame, 'points[2]', 'Point', 5050, 6060) + v = frame.GetValueForVariablePath('points[0]+5') + self.assertTrue(v.GetError().Fail(), + "Make sure we do not ignore characters between ']' and the end") # Test a reference self.verify_point(frame, 'pt_ref', 'Point &', 1, 2) v = frame.GetValueForVariablePath('pt_sp') @@ -88,7 +91,7 @@ # Make sure we don't crash when looking for non existant child # in type with synthetic children. This used to cause a crash. v = frame.GetValueForVariablePath('pt_sp->not_valid_child') - self.assertTrue(v.GetError().Fail(), + self.assertTrue(v.GetError().Fail(), "Make sure we don't find 'pt_sp->not_valid_child'") Index: lldb/source/Target/StackFrame.cpp =================================================================== --- lldb/source/Target/StackFrame.cpp +++ lldb/source/Target/StackFrame.cpp @@ -606,7 +606,7 @@ } // We are dumping at least one child - while (separator_idx != std::string::npos) { + while (!var_expr.empty()) { // Calculate the next separator index ahead of time ValueObjectSP child_valobj_sp; const char separator_type = var_expr[0]; @@ -940,7 +940,6 @@ return ValueObjectSP(); } - separator_idx = var_expr.find_first_of(".-["); if (use_dynamic != eNoDynamicValues) { ValueObjectSP dynamic_value_sp( child_valobj_sp->GetDynamicValue(use_dynamic)); @@ -1025,7 +1024,6 @@ return ValueObjectSP(); } - separator_idx = var_expr.find_first_of(".-["); if (use_dynamic != eNoDynamicValues) { ValueObjectSP dynamic_value_sp( child_valobj_sp->GetDynamicValue(use_dynamic)); @@ -1051,9 +1049,6 @@ if (child_valobj_sp) valobj_sp = child_valobj_sp; - - if (var_expr.empty()) - break; } if (valobj_sp) { if (deref) {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits