[Lldb-commits] [lldb] e1462d1 - Don't produce a dynamic value if there was an error creating it.

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Fri Mar 10 10:22:19 PST 2023


Author: Jim Ingham
Date: 2023-03-10T10:21:50-08:00
New Revision: e1462d14b1e4be329a95bdd08181b97a7a3ad6e6

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

LOG: Don't produce a dynamic value if there was an error creating it.

We used to make a dynamic value that "pretended to be its parent"
but that's hard for some of the more complex ValueObject types, and
it's better in this case just to return no dynamic value.

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

Added: 
    lldb/test/API/python_api/value/addr_of_void_star/Makefile
    lldb/test/API/python_api/value/addr_of_void_star/TestValueAPIAddressOfVoidStar.py
    lldb/test/API/python_api/value/addr_of_void_star/main.c

Modified: 
    lldb/source/Core/ValueObject.cpp
    lldb/source/Core/ValueObjectConstResult.cpp
    lldb/source/Core/ValueObjectDynamicValue.cpp
    lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 6e79a04d024e5..1be7e2c322f0d 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -1859,7 +1859,7 @@ ValueObjectSP ValueObject::GetDynamicValue(DynamicValueType use_dynamic) {
   if (!IsDynamic() && m_dynamic_value == nullptr) {
     CalculateDynamicValue(use_dynamic);
   }
-  if (m_dynamic_value)
+  if (m_dynamic_value && m_dynamic_value->GetError().Success())
     return m_dynamic_value->GetSP();
   else
     return ValueObjectSP();

diff  --git a/lldb/source/Core/ValueObjectConstResult.cpp b/lldb/source/Core/ValueObjectConstResult.cpp
index 640abdddfcad5..17a725dcc7dd8 100644
--- a/lldb/source/Core/ValueObjectConstResult.cpp
+++ b/lldb/source/Core/ValueObjectConstResult.cpp
@@ -287,7 +287,7 @@ ValueObjectConstResult::GetDynamicValue(lldb::DynamicValueType use_dynamic) {
       if (process && process->IsPossibleDynamicValue(*this))
         m_dynamic_value = new ValueObjectDynamicValue(*this, use_dynamic);
     }
-    if (m_dynamic_value)
+    if (m_dynamic_value && m_dynamic_value->GetError().Success())
       return m_dynamic_value->GetSP();
   }
   return ValueObjectSP();

diff  --git a/lldb/source/Core/ValueObjectDynamicValue.cpp b/lldb/source/Core/ValueObjectDynamicValue.cpp
index 0659c771918a5..9db50aeeec9ea 100644
--- a/lldb/source/Core/ValueObjectDynamicValue.cpp
+++ b/lldb/source/Core/ValueObjectDynamicValue.cpp
@@ -187,17 +187,19 @@ bool ValueObjectDynamicValue::UpdateValue() {
     m_type_impl.Clear();
   }
 
-  // If we don't have a dynamic type, then make ourselves just a echo of our
-  // parent. Or we could return false, and make ourselves an echo of our
-  // parent?
+  // If we don't have a dynamic type, set ourselves to be invalid and return
+  // false.  We used to try to produce a dynamic ValueObject that behaved "like"
+  // its parent, but that failed for ValueObjectConstResult, which is too 
+  // complex a beast to try to emulate.  If we return an invalid ValueObject,
+  // clients will end up getting the static value instead, which behaves
+  // correctly.
   if (!found_dynamic_type) {
     if (m_dynamic_type_info)
       SetValueDidChange(true);
     ClearDynamicTypeInformation();
     m_dynamic_type_info.Clear();
-    m_value = m_parent->GetValue();
-    m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
-    return m_error.Success();
+    m_error.SetErrorString("no dynamic type found");
+    return false;
   }
 
   Value old_value(m_value);

diff  --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
index f05997fa03af8..2879aa64fd7c3 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
@@ -583,7 +583,11 @@ ValueObjectSP ItaniumABILanguageRuntime::GetExceptionObjectForThread(
   ValueObjectSP exception = ValueObject::CreateValueObjectFromData(
       "exception", exception_isw.GetAsData(m_process->GetByteOrder()), exe_ctx,
       voidstar);
-  exception = exception->GetDynamicValue(eDynamicDontRunTarget);
+  ValueObjectSP dyn_exception 
+      = exception->GetDynamicValue(eDynamicDontRunTarget);
+  // If we succeed in making a dynamic value, return that:
+  if (dyn_exception) 
+     return dyn_exception;
 
   return exception;
 }

diff  --git a/lldb/test/API/python_api/value/addr_of_void_star/Makefile b/lldb/test/API/python_api/value/addr_of_void_star/Makefile
new file mode 100644
index 0000000000000..10495940055b6
--- /dev/null
+++ b/lldb/test/API/python_api/value/addr_of_void_star/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git a/lldb/test/API/python_api/value/addr_of_void_star/TestValueAPIAddressOfVoidStar.py b/lldb/test/API/python_api/value/addr_of_void_star/TestValueAPIAddressOfVoidStar.py
new file mode 100644
index 0000000000000..701b569e63576
--- /dev/null
+++ b/lldb/test/API/python_api/value/addr_of_void_star/TestValueAPIAddressOfVoidStar.py
@@ -0,0 +1,38 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ValueAPIVoidStarTestCase(TestBase):
+
+    def test(self):
+        self.build()
+        
+        target, process, thread, _ = lldbutil.run_to_source_breakpoint(self,
+                                                                       "Break at this line",
+                                                                       lldb.SBFileSpec("main.c"))
+        frame = thread.GetFrameAtIndex(0)
+
+        # Verify that the expression result for a void * behaves the same way as the
+        # variable value.
+
+        var_val = frame.FindVariable("void_ptr")
+        self.assertSuccess(var_val.GetError(), "Var version made correctly")
+        
+        expr_val = frame.EvaluateExpression("void_ptr")
+        self.assertSuccess(expr_val.GetError(), "Expr version succeeds")
+
+        # The pointer values should be equal:
+        self.assertEqual(var_val.unsigned, expr_val.unsigned, "Values are equal")
+
+        # Both versions should have valid AddressOf, and they should be the same.
+
+        val_addr_of = var_val.AddressOf()
+        self.assertNotEqual(val_addr_of, lldb.LLDB_INVALID_ADDRESS, "Var addr of right")
+
+        expr_addr_of = expr_val.AddressOf()
+        self.assertNotEqual(expr_addr_of, lldb.LLDB_INVALID_ADDRESS, "Expr addr of right")
+
+        # The AddressOf values should also be equal.
+        self.assertEqual(expr_addr_of.unsigned, val_addr_of.unsigned, "Addr of equal")
+

diff  --git a/lldb/test/API/python_api/value/addr_of_void_star/main.c b/lldb/test/API/python_api/value/addr_of_void_star/main.c
new file mode 100644
index 0000000000000..e60220a800f71
--- /dev/null
+++ b/lldb/test/API/python_api/value/addr_of_void_star/main.c
@@ -0,0 +1,6 @@
+int main (int argc, char const *argv[]) {
+  char *char_ptr = "Some pointer here";
+  void *void_ptr = &char_ptr;
+  
+  return 0; // Break at this line
+}


        


More information about the lldb-commits mailing list