Skip to content

Cherry pick data format recursion fix #6986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class DumpValueObjectOptions {
enum class Mode { Always, Formatters, Default, Never } m_mode;
uint32_t m_count;

PointerDepth operator--() const {
PointerDepth Decremented() const {
if (m_count > 0)
return PointerDepth{m_mode, m_count - 1};
return PointerDepth{m_mode, m_count};
Expand Down
3 changes: 1 addition & 2 deletions lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ class ValueObjectPrinter {
bool PrintObjectDescriptionIfNeeded(bool value_printed, bool summary_printed);

bool
ShouldPrintChildren(bool is_failed_description,
DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
ShouldPrintChildren(DumpValueObjectOptions::PointerDepth &curr_ptr_depth);

bool ShouldExpandEmptyAggregates();

Expand Down
77 changes: 39 additions & 38 deletions lldb/source/DataFormatters/ValueObjectPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,6 @@ bool DumpValueObjectOptions::PointerDepth::CanAllowExpansion() const {
}

bool ValueObjectPrinter::ShouldPrintChildren(
bool is_failed_description,
DumpValueObjectOptions::PointerDepth &curr_ptr_depth) {
const bool is_ref = IsRef();
const bool is_ptr = IsPtr();
Expand All @@ -532,6 +531,10 @@ bool ValueObjectPrinter::ShouldPrintChildren(
if (is_uninit)
return false;

// If we have reached the maximum depth we shouldn't print any more children.
if (HasReachedMaximumDepth())
return false;

// if the user has specified an element count, always print children as it is
// explicit user demand being honored
if (m_options.m_pointer_as_array)
Expand All @@ -542,38 +545,37 @@ bool ValueObjectPrinter::ShouldPrintChildren(
if (m_options.m_use_objc)
return false;

if (is_failed_description || !HasReachedMaximumDepth()) {
// We will show children for all concrete types. We won't show pointer
// contents unless a pointer depth has been specified. We won't reference
// contents unless the reference is the root object (depth of zero).
bool print_children = true;
if (TypeSummaryImpl *type_summary = GetSummaryFormatter())
print_children = type_summary->DoesPrintChildren(m_valobj);

// Use a new temporary pointer depth in case we override the current
// pointer depth below...
// We will show children for all concrete types. We won't show pointer
// contents unless a pointer depth has been specified. We won't reference
// contents unless the reference is the root object (depth of zero).

if (is_ptr || is_ref) {
// We have a pointer or reference whose value is an address. Make sure
// that address is not NULL
AddressType ptr_address_type;
if (m_valobj->GetPointerValue(&ptr_address_type) == 0)
return false;
// Use a new temporary pointer depth in case we override the current
// pointer depth below...

const bool is_root_level = m_curr_depth == 0;
if (is_ptr || is_ref) {
// We have a pointer or reference whose value is an address. Make sure
// that address is not NULL
AddressType ptr_address_type;
if (m_valobj->GetPointerValue(&ptr_address_type) == 0)
return false;

if (is_ref && is_root_level) {
// If this is the root object (depth is zero) that we are showing and
// it is a reference, and no pointer depth has been supplied print out
// what it references. Don't do this at deeper depths otherwise we can
// end up with infinite recursion...
return true;
}
const bool is_root_level = m_curr_depth == 0;

return curr_ptr_depth.CanAllowExpansion(false, entry, m_valobj,
m_summary);
if (is_ref && is_root_level && print_children) {
// If this is the root object (depth is zero) that we are showing and
// it is a reference, and no pointer depth has been supplied print out
// what it references. Don't do this at deeper depths otherwise we can
// end up with infinite recursion...
return true;
}

return (!entry || entry->DoesPrintChildren(m_valobj) || m_summary.empty());
return curr_ptr_depth.CanAllowExpansion(false, entry, m_valobj, m_summary);
}
return false;

return print_children || m_summary.empty();
}

bool ValueObjectPrinter::ShouldExpandEmptyAggregates() {
Expand Down Expand Up @@ -610,7 +612,7 @@ void ValueObjectPrinter::PrintChildrenPreamble(bool value_printed,
void ValueObjectPrinter::PrintChild(
ValueObjectSP child_sp,
const DumpValueObjectOptions::PointerDepth &curr_ptr_depth) {
const uint32_t consumed_depth = (!m_options.m_pointer_as_array) ? 1 : 0;
const uint32_t consumed_summary_depth = m_options.m_pointer_as_array ? 0 : 1;
const bool does_consume_ptr_depth =
((IsPtr() && !m_options.m_pointer_as_array) || IsRef());

Expand All @@ -623,15 +625,18 @@ void ValueObjectPrinter::PrintChild(
.SetHideValue(m_options.m_hide_value)
.SetOmitSummaryDepth(child_options.m_omit_summary_depth > 1
? child_options.m_omit_summary_depth -
consumed_depth
consumed_summary_depth
: 0)
.SetElementCount(0);

if (child_sp.get()) {
ValueObjectPrinter child_printer(
child_sp.get(), m_stream, child_options,
does_consume_ptr_depth ? --curr_ptr_depth : curr_ptr_depth,
m_curr_depth + consumed_depth, m_printed_instance_pointers);
auto ptr_depth = curr_ptr_depth;
if (does_consume_ptr_depth)
ptr_depth = curr_ptr_depth.Decremented();

ValueObjectPrinter child_printer(child_sp.get(), m_stream, child_options,
ptr_depth, m_curr_depth + 1,
m_printed_instance_pointers);
child_printer.PrintValueObject();
}
}
Expand Down Expand Up @@ -803,14 +808,10 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool hide_names) {

void ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed,
bool summary_printed) {
// This flag controls whether we tried to display a description for this
// object and failed if that happens, we want to display the children if any.
bool is_failed_description =
!PrintObjectDescriptionIfNeeded(value_printed, summary_printed);
PrintObjectDescriptionIfNeeded(value_printed, summary_printed);

DumpValueObjectOptions::PointerDepth curr_ptr_depth = m_ptr_depth;
const bool print_children =
ShouldPrintChildren(is_failed_description, curr_ptr_depth);
const bool print_children = ShouldPrintChildren(curr_ptr_depth);
const bool print_oneline =
(curr_ptr_depth.CanAllowExpansion() || m_options.m_show_types ||
!m_options.m_allow_oneliner_mode || m_options.m_flat_output ||
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
CXX_SOURCES := main.cpp

include Makefile.rules
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
"""
Tests that frame variable --depth and --element-count options work correctly
together
"""
import lldb
from lldbsuite.test.lldbtest import *
import lldbsuite.test.lldbutil as lldbutil


class TestFrameVarDepthAndElemCount(TestBase):
def test(self):
"""Test that bool types work in the expression parser"""
self.build()
lldbutil.run_to_source_breakpoint(
self, "break here", lldb.SBFileSpec("main.cpp")
)

# Check that we print 5 elements but only 2 levels deep.
self.expect('frame var --depth 2 --element-count 5 -- c',
substrs=[
'[0] = {\n b ={...}\n }',
'[1] = {\n b ={...}\n }',
'[2] = {\n b ={...}\n }',
'[3] = {\n b ={...}\n }',
'[4] = {\n b ={...}\n }',
])

19 changes: 19 additions & 0 deletions lldb/test/API/lang/cpp/frame-var-depth-and-elem-count/main.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#include <cstdio>

struct A {
int i = 42;
};

struct B {
A a;
};

struct C {
B b;
};

int main() {
C *c = new C[5];
puts("break here");
return 0;
}