[Lldb-commits] [lldb] cf5ed6d - Fix error handling after [<index>] in 'frame variable'

[Pavel Labath via lldb-commits]

Author: Jaroslav Sevcik
Date: 2020-05-06T11:03:46+02:00
New Revision: cf5ed6dc59ec8e416cca91c8145b1f09d891f130

URL: https://github.com/llvm/llvm-project/commit/cf5ed6dc59ec8e416cca91c8145b1f09d891f130
DIFF: https://github.com/llvm/llvm-project/commit/cf5ed6dc59ec8e416cca91c8145b1f09d891f130.diff

LOG: Fix error handling after [<index>] in 'frame variable'

Summary:
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'.

Reviewed By: teemperor, labath

Differential Revision: https://reviews.llvm.org/D79404

Added: 
    

Modified: 
    lldb/source/Target/StackFrame.cpp
    lldb/test/API/functionalities/var_path/TestVarPath.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index 48164edcc42f..3d6cc5dc90b3 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -606,7 +606,7 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
   }
 
   // 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 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
           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 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
         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 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
 
     if (child_valobj_sp)
       valobj_sp = child_valobj_sp;
-
-    if (var_expr.empty())
-      break;
   }
   if (valobj_sp) {
     if (deref) {

diff  --git a/lldb/test/API/functionalities/var_path/TestVarPath.py b/lldb/test/API/functionalities/var_path/TestVarPath.py
index ff469cb883ad..e155c25f1dfe 100644
--- a/lldb/test/API/functionalities/var_path/TestVarPath.py
+++ b/lldb/test/API/functionalities/var_path/TestVarPath.py
@@ -25,7 +25,7 @@ def test_frame_var(self):
     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 @@ def do_test(self):
         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 @@ def do_test(self):
         # 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'")