[Lldb-commits] [PATCH] D79404: Fix error handling after [<index>] in 'frame variable'

[Jaroslav Sevcik via Phabricator via lldb-commits]

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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79404.262056.patch
Type: text/x-patch
Size: 3180 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200505/89151e3c/attachment.bin>