[Lldb-commits] [lldb] 14642dc - [lldb] Fix member access in GetExpressionPath
Andy Yankovsky via lldb-commits
lldb-commits at lists.llvm.org
Fri Sep 30 04:25:57 PDT 2022
Author: Tonko Sabolčec
Date: 2022-09-30T11:25:07Z
New Revision: 14642dc7405ebd93a46dda1f2dba616395660715
URL: https://github.com/llvm/llvm-project/commit/14642dc7405ebd93a46dda1f2dba616395660715
DIFF: https://github.com/llvm/llvm-project/commit/14642dc7405ebd93a46dda1f2dba616395660715.diff
LOG: [lldb] Fix member access in GetExpressionPath
This change fixes two issues in ValueObject::GetExpressionPath method:
1. Accessing members of struct references used to produce expression
paths such as "str.&str.member" (instead of the expected
"str.member"). This is fixed by assigning the flag tha the child
value is a dereference when calling Dereference() on references
and adjusting logic in expression path creation.
2. If the parent of member access is dereference, the produced
expression path was "*(ptr).member". This is incorrect, since it
dereferences the member instead of the pointer. This is fixed by
wrapping dereference expression into parenthesis, resulting with
"(*(ptr)).member".
Reviewed By: werat, clayborg
Differential Revision: https://reviews.llvm.org/D132734
Added:
lldb/test/API/python_api/expression_path/Makefile
lldb/test/API/python_api/expression_path/TestExpressionPath.py
lldb/test/API/python_api/expression_path/main.cpp
Modified:
lldb/source/Core/ValueObject.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
Removed:
################################################################################
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 19d86bee40e1f..ece787bdbe4c0 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -273,8 +273,6 @@ CompilerType ValueObject::MaybeCalculateCompleteType() {
return compiler_type;
}
-
-
DataExtractor &ValueObject::GetDataExtractor() {
UpdateValueIfNeeded(false);
return m_data;
@@ -409,8 +407,9 @@ ValueObject::GetChildAtIndexPath(llvm::ArrayRef<size_t> idxs,
return root;
}
-lldb::ValueObjectSP ValueObject::GetChildAtIndexPath(
- llvm::ArrayRef<std::pair<size_t, bool>> idxs, size_t *index_of_error) {
+lldb::ValueObjectSP
+ValueObject::GetChildAtIndexPath(llvm::ArrayRef<std::pair<size_t, bool>> idxs,
+ size_t *index_of_error) {
if (idxs.size() == 0)
return GetSP();
ValueObjectSP root(GetSP());
@@ -1185,9 +1184,10 @@ bool ValueObject::DumpPrintableRepresentation(
{
Status error;
lldb::WritableDataBufferSP buffer_sp;
- std::pair<size_t, bool> read_string = ReadPointedString(
- buffer_sp, error, 0, (custom_format == eFormatVectorOfChar) ||
- (custom_format == eFormatCharArray));
+ std::pair<size_t, bool> read_string =
+ ReadPointedString(buffer_sp, error, 0,
+ (custom_format == eFormatVectorOfChar) ||
+ (custom_format == eFormatCharArray));
lldb_private::formatters::StringPrinter::
ReadBufferAndDumpToStreamOptions options(*this);
options.SetData(DataExtractor(
@@ -1552,8 +1552,7 @@ bool ValueObject::GetDeclaration(Declaration &decl) {
return false;
}
-void ValueObject::AddSyntheticChild(ConstString key,
- ValueObject *valobj) {
+void ValueObject::AddSyntheticChild(ConstString key, ValueObject *valobj) {
m_synthetic_children[key] = valobj;
}
@@ -1924,64 +1923,96 @@ void ValueObject::GetExpressionPath(Stream &s,
return;
}
- const bool is_deref_of_parent = IsDereferenceOfParent();
+ // Checks whether a value is dereference of a non-reference parent.
+ // This is used to determine whether to print a dereference operation (*).
+ auto is_deref_of_non_reference = [](ValueObject *value) {
+ if (value == nullptr)
+ return false;
+ ValueObject *value_parent = value->GetParent();
+ if (value_parent) {
+ CompilerType parent_compiler_type = value_parent->GetCompilerType();
+ if (parent_compiler_type) {
+ const uint32_t parent_type_info = parent_compiler_type.GetTypeInfo();
+ if (parent_type_info & eTypeIsReference)
+ return false;
+ }
+ }
+ return value->IsDereferenceOfParent();
+ };
+
+ ValueObject *parent = GetParent();
- if (is_deref_of_parent &&
+ if (is_deref_of_non_reference(this) &&
epformat == eGetExpressionPathFormatDereferencePointers) {
// this is the original format of GetExpressionPath() producing code like
// *(a_ptr).memberName, which is entirely fine, until you put this into
// StackFrame::GetValueForVariableExpressionPath() which prefers to see
- // a_ptr->memberName. the eHonorPointers mode is meant to produce strings
- // in this latter format
+ // a_ptr->memberName. The eHonorPointers mode is meant to produce strings
+ // in this latter format.
s.PutCString("*(");
+ if (parent)
+ parent->GetExpressionPath(s, epformat);
+ s.PutChar(')');
+ return;
}
- ValueObject *parent = GetParent();
-
- if (parent)
- parent->GetExpressionPath(s, epformat);
-
- // if we are a deref_of_parent just because we are synthetic array members
- // made up to allow ptr[%d] syntax to work in variable printing, then add our
- // name ([%d]) to the expression path
- if (m_flags.m_is_array_item_for_pointer &&
- epformat == eGetExpressionPathFormatHonorPointers)
- s.PutCString(m_name.GetStringRef());
-
- if (!IsBaseClass()) {
- if (!is_deref_of_parent) {
- ValueObject *non_base_class_parent = GetNonBaseClassParent();
- if (non_base_class_parent &&
- !non_base_class_parent->GetName().IsEmpty()) {
- CompilerType non_base_class_parent_compiler_type =
- non_base_class_parent->GetCompilerType();
- if (non_base_class_parent_compiler_type) {
- if (parent && parent->IsDereferenceOfParent() &&
- epformat == eGetExpressionPathFormatHonorPointers) {
- s.PutCString("->");
- } else {
- const uint32_t non_base_class_parent_type_info =
- non_base_class_parent_compiler_type.GetTypeInfo();
-
- if (non_base_class_parent_type_info & eTypeIsPointer) {
- s.PutCString("->");
- } else if ((non_base_class_parent_type_info & eTypeHasChildren) &&
- !(non_base_class_parent_type_info & eTypeIsArray)) {
- s.PutChar('.');
- }
+ const bool is_deref_of_parent = IsDereferenceOfParent();
+ bool is_parent_deref_of_non_reference = false;
+ bool print_obj_access = false;
+ bool print_ptr_access = false;
+
+ if (!IsBaseClass() && !is_deref_of_parent) {
+ ValueObject *non_base_class_parent = GetNonBaseClassParent();
+ if (non_base_class_parent && !non_base_class_parent->GetName().IsEmpty()) {
+ CompilerType non_base_class_parent_compiler_type =
+ non_base_class_parent->GetCompilerType();
+ if (non_base_class_parent_compiler_type) {
+ if (parent && parent->IsDereferenceOfParent() &&
+ epformat == eGetExpressionPathFormatHonorPointers) {
+ print_ptr_access = true;
+ } else {
+ const uint32_t non_base_class_parent_type_info =
+ non_base_class_parent_compiler_type.GetTypeInfo();
+
+ if (non_base_class_parent_type_info & eTypeIsPointer) {
+ print_ptr_access = true;
+ } else if ((non_base_class_parent_type_info & eTypeHasChildren) &&
+ !(non_base_class_parent_type_info & eTypeIsArray)) {
+ print_obj_access = true;
}
}
}
-
- const char *name = GetName().GetCString();
- if (name)
- s.PutCString(name);
+ is_parent_deref_of_non_reference =
+ is_deref_of_non_reference(non_base_class_parent);
}
}
- if (is_deref_of_parent &&
- epformat == eGetExpressionPathFormatDereferencePointers) {
- s.PutChar(')');
+ if (parent) {
+ // The parent should be wrapped in parenthesis when doing a member access.
+ // This prevents forming incorrect expressions such as *(ptr).field,
+ // which dereferences the field instead of the ptr, and constructs the
+ // expression in the format (*(ptr)).field. To create expressions compatible
+ // with StackFrame::GetValueForVariableExpressionPath() and reduce amount of
+ // unnecessary parenthesis, this is done only when the parent has the
+ // dereference syntax *(parent).
+ const bool wrap_parent_in_parens = (print_obj_access || print_ptr_access) &&
+ is_parent_deref_of_non_reference;
+ if (wrap_parent_in_parens)
+ s.PutChar('(');
+ parent->GetExpressionPath(s, epformat);
+ if (wrap_parent_in_parens)
+ s.PutChar(')');
+ }
+
+ if (print_obj_access)
+ s.PutChar('.');
+ if (print_ptr_access)
+ s.PutCString("->");
+
+ if (!IsBaseClass() && !is_deref_of_parent) {
+ const char *name = GetName().GetCString();
+ if (name)
+ s.PutCString(name);
}
}
@@ -3108,8 +3139,6 @@ bool ValueObject::CanProvideValue() {
return (!type.IsValid()) || (0 != (type.GetTypeInfo() & eTypeHasValue));
}
-
-
ValueObjectSP ValueObject::Persist() {
if (!UpdateValueIfNeeded())
return nullptr;
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 4adf653b6c508..c55319e7e0499 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6562,6 +6562,8 @@ CompilerType TypeSystemClang::GetChildCompilerTypeAtIndex(
child_is_base_class, tmp_child_is_deref_of_parent, valobj,
language_flags);
} else {
+ child_is_deref_of_parent = true;
+
const char *parent_name =
valobj ? valobj->GetName().GetCString() : nullptr;
if (parent_name) {
diff --git a/lldb/test/API/python_api/expression_path/Makefile b/lldb/test/API/python_api/expression_path/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/python_api/expression_path/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/python_api/expression_path/TestExpressionPath.py b/lldb/test/API/python_api/expression_path/TestExpressionPath.py
new file mode 100644
index 0000000000000..9d230bda99991
--- /dev/null
+++ b/lldb/test/API/python_api/expression_path/TestExpressionPath.py
@@ -0,0 +1,119 @@
+"""Test that SBFrame::GetExpressionPath construct valid expressions"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class SBValueGetExpressionPathTest(TestBase):
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def path(self, value):
+ """Constructs the expression path given the SBValue"""
+ if not value:
+ return None
+ stream = lldb.SBStream()
+ if not value.GetExpressionPath(stream):
+ return None
+ return stream.GetData()
+
+ def test_expression_path(self):
+ """Test that SBFrame::GetExpressionPath construct valid expressions"""
+ self.build()
+ self.setTearDownCleanup()
+
+ exe = self.getBuildArtifact("a.out")
+
+ # Create the target
+ target = self.dbg.CreateTarget(exe)
+ self.assertTrue(target, VALID_TARGET)
+
+ # Set the breakpoints
+ breakpoint = target.BreakpointCreateBySourceRegex(
+ 'Set breakpoint here', lldb.SBFileSpec("main.cpp"))
+ self.assertTrue(breakpoint.GetNumLocations() > 0, VALID_BREAKPOINT)
+
+ # Launch the process, and do not stop at the entry point.
+ process = target.LaunchSimple(
+ None, None, self.get_process_working_directory())
+
+ self.assertTrue(process, PROCESS_IS_VALID)
+
+ # Frame #0 should be at our breakpoint.
+ threads = lldbutil.get_threads_stopped_at_breakpoint(
+ process, breakpoint)
+
+ self.assertEquals(len(threads), 1)
+ self.thread = threads[0]
+ self.frame = self.thread.frames[0]
+ self.assertTrue(self.frame, "Frame 0 is valid.")
+
+ # Find "b" variables in frame
+ b = self.frame.FindVariable("b")
+ bp = self.frame.FindVariable("b_ptr")
+ br = self.frame.FindVariable("b_ref")
+ bpr = self.frame.FindVariable("b_ptr_ref")
+ # Check expression paths
+ self.assertEqual(self.path(b), "b")
+ self.assertEqual(self.path(bp), "b_ptr")
+ self.assertEqual(self.path(br), "b_ref")
+ self.assertEqual(self.path(bpr), "b_ptr_ref")
+
+ # Dereference "b" pointers
+ bp_deref = bp.Dereference()
+ bpr_deref = bpr.Dereference() # a pointer
+ bpr_deref2 = bpr_deref.Dereference() # two Dereference() calls to get object
+ # Check expression paths
+ self.assertEqual(self.path(bp_deref), "*(b_ptr)")
+ self.assertEqual(self.path(bpr_deref), "b_ptr_ref")
+ self.assertEqual(self.path(bpr_deref2), "*(b_ptr_ref)")
+
+ # Access "b" members and check expression paths
+ self.assertEqual(self.path(b.GetChildMemberWithName("x")), "b.x")
+ self.assertEqual(self.path(bp.GetChildMemberWithName("x")), "b_ptr->x")
+ self.assertEqual(self.path(br.GetChildMemberWithName("x")), "b_ref.x")
+ self.assertEqual(self.path(bp_deref.GetChildMemberWithName("x")), "(*(b_ptr)).x")
+ self.assertEqual(self.path(bpr_deref.GetChildMemberWithName("x")), "b_ptr_ref->x")
+ self.assertEqual(self.path(bpr_deref2.GetChildMemberWithName("x")), "(*(b_ptr_ref)).x")
+ # TODO: Uncomment once accessing members on pointer references is supported.
+ # self.assertEqual(self.path(bpr.GetChildMemberWithName("x")), "b_ptr_ref->x")
+
+ # Try few expressions with multiple member access
+ bp_ar_x = bp.GetChildMemberWithName("a_ref").GetChildMemberWithName("x")
+ br_ar_y = br.GetChildMemberWithName("a_ref").GetChildMemberWithName("y")
+ self.assertEqual(self.path(bp_ar_x), "b_ptr->a_ref.x")
+ self.assertEqual(self.path(br_ar_y), "b_ref.a_ref.y")
+ bpr_deref_apr_deref = bpr_deref.GetChildMemberWithName("a_ptr_ref").Dereference()
+ bpr_deref_apr_deref2 = bpr_deref_apr_deref.Dereference()
+ self.assertEqual(self.path(bpr_deref_apr_deref), "b_ptr_ref->a_ptr_ref")
+ self.assertEqual(self.path(bpr_deref_apr_deref2), "*(b_ptr_ref->a_ptr_ref)")
+ bpr_deref_apr_deref_x = bpr_deref_apr_deref.GetChildMemberWithName("x")
+ bpr_deref_apr_deref2_x = bpr_deref_apr_deref2.GetChildMemberWithName("x")
+ self.assertEqual(self.path(bpr_deref_apr_deref_x), "b_ptr_ref->a_ptr_ref->x")
+ self.assertEqual(self.path(bpr_deref_apr_deref2_x), "(*(b_ptr_ref->a_ptr_ref)).x")
+
+ # Find "c" variables in frame
+ c = self.frame.FindVariable("c")
+ cp = self.frame.FindVariable("c_ptr")
+ cr = self.frame.FindVariable("c_ref")
+ cpr = self.frame.FindVariable("c_ptr_ref")
+ # Dereference pointers
+ cp_deref = cp.Dereference()
+ cpr_deref = cpr.Dereference() # a pointer
+ cpr_deref2 = cpr_deref.Dereference() # two Dereference() calls to get object
+ # Check expression paths
+ self.assertEqual(self.path(cp_deref), "*(c_ptr)")
+ self.assertEqual(self.path(cpr_deref), "c_ptr_ref")
+ self.assertEqual(self.path(cpr_deref2), "*(c_ptr_ref)")
+
+ # Access members on "c" variables and check expression paths
+ self.assertEqual(self.path(c.GetChildMemberWithName("x")), "c.x")
+ self.assertEqual(self.path(cp.GetChildMemberWithName("x")), "c_ptr->x")
+ self.assertEqual(self.path(cr.GetChildMemberWithName("x")), "c_ref.x")
+ self.assertEqual(self.path(cp_deref.GetChildMemberWithName("x")), "(*(c_ptr)).x")
+ self.assertEqual(self.path(cpr_deref.GetChildMemberWithName("x")), "c_ptr_ref->x")
+ self.assertEqual(self.path(cpr_deref2.GetChildMemberWithName("x")), "(*(c_ptr_ref)).x")
+ # TODO: Uncomment once accessing members on pointer references is supported.
+ # self.assertEqual(self.path(cpr.GetChildMemberWithName("x")), "c_ptr_ref->x")
\ No newline at end of file
diff --git a/lldb/test/API/python_api/expression_path/main.cpp b/lldb/test/API/python_api/expression_path/main.cpp
new file mode 100644
index 0000000000000..522022bd8125b
--- /dev/null
+++ b/lldb/test/API/python_api/expression_path/main.cpp
@@ -0,0 +1,34 @@
+struct StructA {
+ int x;
+ int y;
+};
+
+struct StructB {
+ int x;
+ StructA &a_ref;
+ StructA *&a_ptr_ref;
+};
+
+struct StructC : public StructB {
+ int y;
+
+ StructC(int x, StructA &a_ref, StructA *&a_ref_ptr, int y)
+ : StructB{x, a_ref, a_ref_ptr}, y(y) {}
+};
+
+int main() {
+ StructA a{1, 2};
+ StructA *a_ptr = &a;
+
+ StructB b{3, a, a_ptr};
+ StructB *b_ptr = &b;
+ StructB &b_ref = b;
+ StructB *&b_ptr_ref = b_ptr;
+
+ StructC c(4, a, a_ptr, 5);
+ StructC *c_ptr = &c;
+ StructC &c_ref = c;
+ StructC *&c_ptr_ref = c_ptr;
+
+ return 0; // Set breakpoint here
+}
\ No newline at end of file
More information about the lldb-commits
mailing list