forked from OSchip/llvm-project
[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
This commit is contained in:
Tonko Sabolčec
Andy Yankovsky
parent
c720fad16f
commit
14642dc740
|
|
@ -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();
|
||||
};
|
||||
|
||||
if (is_deref_of_parent &&
|
||||
ValueObject *parent = GetParent();
|
||||
|
||||
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();
|
||||
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 (parent)
|
||||
parent->GetExpressionPath(s, epformat);
|
||||
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 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('.');
|
||||
}
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,3 @@
|
|||
CXX_SOURCES := main.cpp
|
||||
|
||||
include Makefile.rules
|
||||
|
|
@ -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")
|
||||
|
|
@ -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
|
||||
}
|
||||
Loading…
Reference in New Issue