werat created this revision.
werat added a reviewer: teemperor.
werat requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Right now `SBValue::Persist()` works properly only for values that refere to 
variables (refer to unit-tests for an example). Constant values (e.g. values 
created by `EvaluateExpression` or `CreateValueFromData`) can be persisted, but 
using them in the expressions leads to an error:

  >>> v = lldb.frame.EvaluateExpression("1+2")
  >>> vp = v.Persist()
  >>> vp.GetName()
  '$1'
  >>> v = lldb.frame.EvaluateExpression("$1")
  >>> v.GetValue()
  '3'
  >>> v = lldb.frame.EvaluateExpression("$1+1")
  >>> v.GetError().GetCString()
  "error: supposed to interpret, but failed: Interpreter couldn't read from 
memory\n"

In this patch we mark constant values as "required materialization" and fix up 
the dematerialization logic to apply side-effects.

Also move Windows-failing use cases to a separate test, so that other tests can 
be executed on Windows too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98370

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/test/API/python_api/sbvalue_persist/TestSBValuePersist.py
  lldb/test/API/python_api/sbvalue_persist/main.cpp

Index: lldb/test/API/python_api/sbvalue_persist/main.cpp
===================================================================
--- lldb/test/API/python_api/sbvalue_persist/main.cpp
+++ lldb/test/API/python_api/sbvalue_persist/main.cpp
@@ -1,14 +1,21 @@
-#include <vector>
 #include <string>
+#include <vector>
 
 void f() {}
 
+struct P {
+  int x;
+  float y;
+} _p;
+
 int main() {
-    int foo = 10;
-    int *bar = new int(4);
-    std::string baz = "85";
-    
-    f(); // break here
-    f(); // break here
-    return 0;
+  int foo = 10;
+  int *bar = new int(4);
+  std::string baz = "85";
+
+  int mem[] = {1, 0x3fc00000}; // P { x = 1, y = 1.5 }
+
+  f(); // break here
+  f(); // break here
+  return 0;
 }
Index: lldb/test/API/python_api/sbvalue_persist/TestSBValuePersist.py
===================================================================
--- lldb/test/API/python_api/sbvalue_persist/TestSBValuePersist.py
+++ lldb/test/API/python_api/sbvalue_persist/TestSBValuePersist.py
@@ -1,7 +1,5 @@
 """Test SBValue::Persist"""
 
-
-
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -13,9 +11,7 @@
     mydir = TestBase.compute_mydir(__file__)
     NO_DEBUG_INFO_TESTCASE = True
 
-    @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772")
-    def test(self):
-        """Test SBValue::Persist"""
+    def _setup(self):
         self.build()
         self.setTearDownCleanup()
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
@@ -40,42 +36,103 @@
         # Execute the cleanup function during test case tear down.
         self.addTearDownHook(cleanup)
 
+    @add_test_categories(['pyapi'])
+    @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772")
+    def test_std_string(self):
+        """Test SBValue::Persist for a variable of type std::string"""
+        self._setup()
+
+        baz = self.frame().FindVariable("baz")
+        self.assertTrue(baz.IsValid(), "baz is not valid")
+
+        bazPersist = baz.Persist()
+        self.assertTrue(bazPersist.IsValid(), "bazPersist is not valid")
+        self.assertEqual(bazPersist.GetSummary(), '"85"')
+
+        self.runCmd("continue")
+
+        self.assertTrue(bazPersist.IsValid(), "bazPersist is not valid")
+        self.assertEqual(bazPersist.GetSummary(), '"85"')
+
+    @add_test_categories(['pyapi'])
+    def test(self):
+        """Test SBValue::Persist"""
+        self._setup()
+
         foo = self.frame().FindVariable("foo")
         bar = self.frame().FindVariable("bar")
-        baz = self.frame().FindVariable("baz")
 
         self.assertTrue(foo.IsValid(), "foo is not valid")
         self.assertTrue(bar.IsValid(), "bar is not valid")
-        self.assertTrue(baz.IsValid(), "baz is not valid")
 
         fooPersist = foo.Persist()
         barPersist = bar.Persist()
-        bazPersist = baz.Persist()
 
         self.assertTrue(fooPersist.IsValid(), "fooPersist is not valid")
         self.assertTrue(barPersist.IsValid(), "barPersist is not valid")
-        self.assertTrue(bazPersist.IsValid(), "bazPersist is not valid")
 
-        self.assertEqual(
-            fooPersist.GetValueAsUnsigned(0), 10,
-            "fooPersist != 10")
-        self.assertEqual(
-            barPersist.GetPointeeData().sint32[0], 4,
-            "barPersist != 4")
-        self.assertEquals(bazPersist.GetSummary(), '"85"', "bazPersist != 85")
+        self.assertEqual(fooPersist.GetValueAsUnsigned(0), 10)
+        self.assertEqual(barPersist.GetPointeeData().sint32[0], 4)
 
         self.runCmd("continue")
 
         self.assertTrue(fooPersist.IsValid(), "fooPersist is not valid")
         self.assertTrue(barPersist.IsValid(), "barPersist is not valid")
-        self.assertTrue(bazPersist.IsValid(), "bazPersist is not valid")
 
-        self.assertEqual(
-            fooPersist.GetValueAsUnsigned(0), 10,
-            "fooPersist != 10")
-        self.assertEqual(
-            barPersist.GetPointeeData().sint32[0], 4,
-            "barPersist != 4")
-        self.assertEquals(bazPersist.GetSummary(), '"85"', "bazPersist != 85")
+        self.assertEqual(fooPersist.GetValueAsUnsigned(0), 10)
+        self.assertEqual(barPersist.GetPointeeData().sint32[0], 4)
 
         self.expect("expr *(%s)" % (barPersist.GetName()), substrs=['= 4'])
+
+    @add_test_categories(['pyapi'])
+    def test_ephemeral(self):
+        self._setup()
+
+        # Test persisting values created from data, i.e. not pointing to any
+        # location in the process memory.
+        var = self.frame().EvaluateExpression("1+1")
+        varPersist = var.Persist()
+        self.assertTrue(varPersist.IsValid(), "varPersist is not valid")
+        self.assertEqual(varPersist.value, "2")
+        # Shortcut for variable name.
+        p = varPersist.name
+
+        val = self.frame().EvaluateExpression("{}".format(p))
+        self.assertEqual(val.value, "2")
+
+        val = self.frame().EvaluateExpression("++{}".format(p))
+        self.assertEqual(val.value, "3")
+
+        val = self.frame().EvaluateExpression("{}++ + {}".format(p, p))
+        self.assertEqual(val.value, "7")
+
+        val = self.frame().EvaluateExpression("{}".format(p))
+        self.assertEqual(val.value, "4")
+
+        val = self.frame().EvaluateExpression("{} + 1".format(p))
+        self.assertEqual(val.value, "5")
+
+    @add_test_categories(['pyapi'])
+    def test_cast_struct(self):
+        self._setup()
+
+        var = self.frame().EvaluateExpression("(P&)mem")
+        varPersist = var.Persist()
+        self.assertTrue(varPersist.IsValid(), "varPersist is not valid")
+        # Shortcut for variable name.
+        p = varPersist.name
+
+        val = self.frame().EvaluateExpression("{}.x".format(p))
+        self.assertEqual(val.value, "1")
+
+        val = self.frame().EvaluateExpression("{}.y".format(p))
+        self.assertEqual(val.value, "1.5")
+
+        val = self.frame().EvaluateExpression("++{}.y".format(p))
+        self.assertEqual(val.value, "2.5")
+
+        val = self.frame().EvaluateExpression("{}.y".format(p))
+        self.assertEqual(val.value, "2.5")
+
+        val = self.frame().EvaluateExpression("mem[1]".format(p))
+        self.assertEqual(val.value, "1069547520")  # 0x3fc00000
Index: lldb/source/Expression/Materializer.cpp
===================================================================
--- lldb/source/Expression/Materializer.cpp
+++ lldb/source/Expression/Materializer.cpp
@@ -284,13 +284,11 @@
                       .getValueOr(0));
 
         // Read the contents of the spare memory area
-
-        m_persistent_variable_sp->ValueUpdated();
+        auto byte_size = m_persistent_variable_sp->GetByteSize().getValueOr(0);
+        auto buffer = std::make_shared<DataBufferHeap>(byte_size, 0);
 
         Status read_error;
-
-        map.ReadMemory(m_persistent_variable_sp->GetValueBytes(), mem,
-                       m_persistent_variable_sp->GetByteSize().getValueOr(0), read_error);
+        map.ReadMemory(buffer->GetBytes(), mem, byte_size, read_error);
 
         if (!read_error.Success()) {
           err.SetErrorStringWithFormat(
@@ -300,8 +298,12 @@
           return;
         }
 
-        m_persistent_variable_sp->m_flags &=
-            ~ExpressionVariable::EVNeedsFreezeDry;
+        m_persistent_variable_sp->m_frozen_sp = ValueObjectConstResult::Create(
+            map.GetBestExecutionContextScope(),
+            m_persistent_variable_sp->GetCompilerType(),
+            m_persistent_variable_sp->GetName(),
+            DataExtractor(buffer, map.GetByteOrder(), map.GetAddressByteSize()),
+            LLDB_INVALID_ADDRESS);
       }
     } else {
       err.SetErrorStringWithFormat(
Index: lldb/source/Core/ValueObject.cpp
===================================================================
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -3131,7 +3131,15 @@
   ExpressionVariableSP persistent_var_sp =
       persistent_state->CreatePersistentVariable(const_result_sp);
   persistent_var_sp->m_live_sp = persistent_var_sp->m_frozen_sp;
-  persistent_var_sp->m_flags |= ExpressionVariable::EVIsProgramReference;
+
+  // ConstResult values don't refer to a location in memory of the target
+  // process, so they need to be allocated explicitly.
+  if (GetValueType() == lldb::eValueTypeConstResult) {
+    persistent_var_sp->m_flags |= ExpressionVariable::EVNeedsAllocation;
+    persistent_var_sp->m_flags |= ExpressionVariable::EVNeedsFreezeDry;
+  } else {
+    persistent_var_sp->m_flags |= ExpressionVariable::EVIsProgramReference;
+  }
 
   return persistent_var_sp->GetValueObject();
 }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to