clayborg created this revision.
clayborg added reviewers: jingham, zturner, aprantl.
Herald added a reviewer: serge-sans-paille.
Herald added a subscriber: jdoerfert.

There was a crash that would happen if an IDE would ask for a child of a shared 
pointer via any SB API call that ends up calling 
StackFrame::GetValueForVariableExpressionPath(). The previous code expects an 
error to be set describing why the synthetic child of a type was not able to be 
found, but we have some synthetic child providers that weren't setting the 
error and returning an empty value object shared pointer. This fixes that to 
ensure we don't lose our debug session by crashing, fully tests 
GetValueForVariableExpressionPath functionality, and ensures we don't crash on 
GetValueForVariableExpressionPath() in the future.


https://reviews.llvm.org/D59200

Files:
  packages/Python/lldbsuite/test/functionalities/var_path/Makefile
  packages/Python/lldbsuite/test/functionalities/var_path/TestVarPath.py
  packages/Python/lldbsuite/test/functionalities/var_path/main.cpp
  source/Target/StackFrame.cpp

Index: packages/Python/lldbsuite/test/functionalities/var_path/main.cpp
===================================================================
--- packages/Python/lldbsuite/test/functionalities/var_path/main.cpp
+++ packages/Python/lldbsuite/test/functionalities/var_path/main.cpp
@@ -0,0 +1,15 @@
+#include <memory>
+
+struct Point {
+  int x, y;
+};
+
+int main() {
+  Point pt = { 1, 2 };
+  Point points[] = {{1010,2020}, {3030,4040}, {5050,6060}};
+  Point *pt_ptr = &points[1];
+  Point &pt_ref = pt;
+  std::shared_ptr<Point> pt_sp(new Point{111,222});
+  return 0; // Set a breakpoint here
+}
+
Index: packages/Python/lldbsuite/test/functionalities/var_path/TestVarPath.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/var_path/TestVarPath.py
+++ packages/Python/lldbsuite/test/functionalities/var_path/TestVarPath.py
@@ -0,0 +1,134 @@
+"""
+Make sure the getting a variable path works and doesn't crash.
+"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import re
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestVarPath(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    # If your test case doesn't stress debug info, the
+    # set this to true.  That way it won't be run once for
+    # each debug info format.
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_frame_var(self):
+        self.build()
+        self.do_test()
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(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.assertTrue(v.GetType().GetName() == var_typename, 
+                        "Make sure '%s' has type '%s'" % (var_name, var_typename))
+
+        if '*' in var_typename:
+            valid_prefix = var_name + '->'
+            invalid_prefix = var_name + '.'
+        else:
+            valid_prefix = var_name + '.'
+            invalid_prefix = var_name + '->'
+
+        valid_x_path = valid_prefix + 'x'
+        valid_y_path = valid_prefix + 'y'
+        invalid_x_path = invalid_prefix + 'x'
+        invalid_y_path = invalid_prefix + 'y'
+        invalid_m_path = invalid_prefix + 'm'
+
+        v = frame.GetValueForVariablePath(valid_x_path)
+        self.assertTrue(v.GetError().Success(), "Make sure we find '%s'" % (valid_x_path))
+        self.assertTrue(v.GetValue() == str(x_value), "Make sure '%s' has a value of %i" % (valid_x_path, x_value))
+        self.assertTrue(v.GetType().GetName() == "int", "Make sure '%s' has type 'int'" % (valid_x_path))
+        v = frame.GetValueForVariablePath(invalid_x_path)
+        self.assertTrue(v.GetError().Fail(), "Make sure we don't find '%s'" % (invalid_x_path))
+
+        v = frame.GetValueForVariablePath(valid_y_path)
+        self.assertTrue(v.GetError().Success(), "Make sure we find '%s'" % (valid_y_path))
+        self.assertTrue(v.GetValue() == str(y_value), "Make sure '%s' has a value of %i" % (valid_y_path, y_value))
+        self.assertTrue(v.GetType().GetName() == "int", "Make sure '%s' has type 'int'" % (valid_y_path))
+        v = frame.GetValueForVariablePath(invalid_y_path)
+        self.assertTrue(v.GetError().Fail(), "Make sure we don't find '%s'" % (invalid_y_path))
+
+        v = frame.GetValueForVariablePath(invalid_m_path)
+        self.assertTrue(v.GetError().Fail(), "Make sure we don't find '%s'" % (invalid_m_path))
+
+    def do_test(self):
+        exe = self.getBuildArtifact("a.out")
+
+        # Create a target by the debugger.
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        # Now create a breakpoint in main.cpp at the source matching
+        # "Set a breakpoint here"
+        breakpoint = target.BreakpointCreateBySourceRegex(
+            "Set a breakpoint here", lldb.SBFileSpec("main.cpp"))
+        self.assertTrue(breakpoint and
+                        breakpoint.GetNumLocations() >= 1,
+                        VALID_BREAKPOINT)
+
+        error = lldb.SBError()
+        # This is the launch info.  If you want to launch with arguments or
+        # environment variables, add them using SetArguments or
+        # SetEnvironmentEntries
+
+        launch_info = lldb.SBLaunchInfo(None)
+        process = target.Launch(launch_info, error)
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        # Did we hit our breakpoint?
+        from lldbsuite.test.lldbutil import get_threads_stopped_at_breakpoint
+        threads = get_threads_stopped_at_breakpoint(process, breakpoint)
+        self.assertTrue(
+            len(threads) == 1,
+            "There should be a thread stopped at our breakpoint")
+
+        # The hit count for the breakpoint should be 1.
+        self.assertTrue(breakpoint.GetHitCount() == 1)
+
+        frame = threads[0].GetFrameAtIndex(0)
+
+        v = frame.GetValueForVariablePath('no_such_variable')
+        self.assertTrue(v.GetError().Fail(), "Make sure we don't find 'no_such_variable'")
+
+        # Test an instance
+        self.verify_point(frame, 'pt', 'Point', 1, 2)
+        # Test a pointer
+        self.verify_point(frame, 'pt_ptr', 'Point *', 3030, 4040)
+        # Test using a pointer as an array
+        self.verify_point(frame, 'pt_ptr[-1]', 'Point', 1010, 2020)
+        self.verify_point(frame, 'pt_ptr[0]', 'Point', 3030, 4040)
+        self.verify_point(frame, 'pt_ptr[1]', 'Point', 5050, 6060)
+        # Test arrays
+        v = frame.GetValueForVariablePath('points')
+        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)
+        # Test a reference
+        self.verify_point(frame, 'pt_ref', 'Point &', 1, 2)
+        v = frame.GetValueForVariablePath('pt_sp')
+        self.assertTrue(v.GetError().Success(), "Make sure we find 'pt_sp'")
+        # 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(), 
+                        "Make sure we don't find 'pt_sp->not_valid_child'")
+
+
+
Index: packages/Python/lldbsuite/test/functionalities/var_path/Makefile
===================================================================
--- packages/Python/lldbsuite/test/functionalities/var_path/Makefile
+++ packages/Python/lldbsuite/test/functionalities/var_path/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules
Index: source/Target/StackFrame.cpp
===================================================================
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -645,6 +645,12 @@
               "Failed to dereference sythetic value: %s", deref_error);
           return ValueObjectSP();
         }
+        // Some synthetic plug-ins fail to set the error in Dereference
+        if (!valobj_sp) {
+          error.SetErrorStringWithFormatv(
+              "Failed to dereference sythetic value");
+          return ValueObjectSP();
+        }
         expr_is_ptr = false;
       }
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to