forked from OSchip/llvm-project
Fix the std::string formatter to report errors in the case where the
string points to unaccessible memory. The formatter tries to get the data field of the std::string, and to check whether that fails it just checks that the ValueObjectSP returned is not empty. But we never return empty ValueObjectSP's to indicate failure, since doing so would lose the Error object that tells you why fetching the ValueObject failed. This patch adds a check for ValueObject::GetError().Success(). I also added a test case for this failure, and reworked the test case a bit (to use run_to_source_breakpoint). I also renamed a couple of single letter locals which don't follow the lldb coding conventions. Differential Revision: https://reviews.llvm.org/D108228
This commit is contained in:
parent
f032690a7c
commit
7afd257ff8
|
@ -558,12 +558,14 @@ enum LibcxxStringLayoutMode {
|
|||
// TODO: Support big-endian architectures.
|
||||
static llvm::Optional<std::pair<uint64_t, ValueObjectSP>>
|
||||
ExtractLibcxxStringInfo(ValueObject &valobj) {
|
||||
ValueObjectSP D(valobj.GetChildAtIndexPath({0, 0, 0, 0}));
|
||||
if (!D)
|
||||
ValueObjectSP dataval_sp(valobj.GetChildAtIndexPath({0, 0, 0, 0}));
|
||||
if (!dataval_sp)
|
||||
return {};
|
||||
if (!dataval_sp->GetError().Success())
|
||||
return {};
|
||||
|
||||
ValueObjectSP layout_decider(
|
||||
D->GetChildAtIndexPath(llvm::ArrayRef<size_t>({0, 0})));
|
||||
dataval_sp->GetChildAtIndexPath(llvm::ArrayRef<size_t>({0, 0})));
|
||||
|
||||
// this child should exist
|
||||
if (!layout_decider)
|
||||
|
@ -581,12 +583,12 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
|
|||
: eLibcxxStringLayoutModeCSD;
|
||||
uint64_t size_mode_value = 0;
|
||||
|
||||
if (ValueObjectSP is_long = D->GetChildAtNamePath(
|
||||
if (ValueObjectSP is_long = dataval_sp->GetChildAtNamePath(
|
||||
{ConstString("__s"), ConstString("__is_long_")})) {
|
||||
using_bitmasks = false;
|
||||
short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
|
||||
if (ValueObjectSP size_member =
|
||||
D->GetChildAtNamePath({ConstString("__s"), ConstString("__size_")}))
|
||||
dataval_sp->GetChildAtNamePath({ConstString("__s"), ConstString("__size_")}))
|
||||
size = size_member->GetValueAsUnsigned(/*fail_value=*/0);
|
||||
else
|
||||
return {};
|
||||
|
@ -599,7 +601,7 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
|
|||
};
|
||||
ValueObjectSP size_mode;
|
||||
for (llvm::ArrayRef<size_t> loc : size_mode_locations) {
|
||||
size_mode = D->GetChildAtIndexPath(loc);
|
||||
size_mode = dataval_sp->GetChildAtIndexPath(loc);
|
||||
if (size_mode && size_mode->GetName() == g_size_name)
|
||||
break;
|
||||
}
|
||||
|
@ -610,7 +612,7 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
|
|||
size_mode_value = (size_mode->GetValueAsUnsigned(0));
|
||||
short_mode = ((size_mode_value & 0x80) == 0);
|
||||
} else {
|
||||
ValueObjectSP size_mode(D->GetChildAtIndexPath({1, 0, 0}));
|
||||
ValueObjectSP size_mode(dataval_sp->GetChildAtIndexPath({1, 0, 0}));
|
||||
if (!size_mode)
|
||||
return {};
|
||||
|
||||
|
@ -619,10 +621,10 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
|
|||
}
|
||||
|
||||
if (short_mode) {
|
||||
ValueObjectSP s(D->GetChildAtIndex(1, true));
|
||||
if (!s)
|
||||
ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
|
||||
if (!short_sp)
|
||||
return {};
|
||||
ValueObjectSP location_sp = s->GetChildAtIndex(
|
||||
ValueObjectSP location_sp = short_sp->GetChildAtIndex(
|
||||
(layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
|
||||
if (using_bitmasks)
|
||||
size = (layout == eLibcxxStringLayoutModeDSC)
|
||||
|
@ -642,7 +644,7 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
|
|||
return std::make_pair(size, location_sp);
|
||||
}
|
||||
|
||||
ValueObjectSP l(D->GetChildAtIndex(0, true));
|
||||
ValueObjectSP l(dataval_sp->GetChildAtIndex(0, true));
|
||||
if (!l)
|
||||
return {};
|
||||
// we can use the layout_decider object as the data pointer
|
||||
|
|
|
@ -19,7 +19,7 @@ class LibcxxStringDataFormatterTestCase(TestBase):
|
|||
# Call super's setUp().
|
||||
TestBase.setUp(self)
|
||||
# Find the line number to break at.
|
||||
self.line = line_number('main.cpp', '// Set break point at this line.')
|
||||
self.main_spec = lldb.SBFileSpec("main.cpp")
|
||||
self.namespace = 'std'
|
||||
|
||||
@add_test_categories(["libc++"])
|
||||
|
@ -30,17 +30,11 @@ class LibcxxStringDataFormatterTestCase(TestBase):
|
|||
def test_with_run_command(self):
|
||||
"""Test that that file and class static variables display correctly."""
|
||||
self.build()
|
||||
self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
|
||||
|
||||
lldbutil.run_break_set_by_file_and_line(
|
||||
self, "main.cpp", self.line, num_expected_locations=-1)
|
||||
|
||||
self.runCmd("run", RUN_SUCCEEDED)
|
||||
|
||||
# The stop reason of the thread should be breakpoint.
|
||||
self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
|
||||
substrs=['stopped',
|
||||
'stop reason = breakpoint'])
|
||||
(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
|
||||
"Set break point at this line.",
|
||||
self.main_spec)
|
||||
frame = thread.frames[0]
|
||||
|
||||
# This is the function to remove the custom formats in order to have a
|
||||
# clean slate for the next test case.
|
||||
|
@ -83,9 +77,9 @@ class LibcxxStringDataFormatterTestCase(TestBase):
|
|||
'(%s::string *) null_str = nullptr'%ns,
|
||||
])
|
||||
|
||||
self.runCmd("n")
|
||||
thread.StepOver()
|
||||
|
||||
TheVeryLongOne = self.frame().FindVariable("TheVeryLongOne")
|
||||
TheVeryLongOne = frame.FindVariable("TheVeryLongOne")
|
||||
summaryOptions = lldb.SBTypeSummaryOptions()
|
||||
summaryOptions.SetCapping(lldb.eTypeSummaryUncapped)
|
||||
uncappedSummaryStream = lldb.SBStream()
|
||||
|
@ -129,3 +123,15 @@ class LibcxxStringDataFormatterTestCase(TestBase):
|
|||
self.expect("frame variable garbage3", substrs=[r'garbage3 = "\xf0\xf0"'])
|
||||
self.expect("frame variable garbage4", substrs=['garbage4 = Summary Unavailable'])
|
||||
self.expect("frame variable garbage5", substrs=['garbage5 = Summary Unavailable'])
|
||||
|
||||
# Finally, make sure that if the string is not readable, we give an error:
|
||||
bkpt_2 = target.BreakpointCreateBySourceRegex("Break here to look at bad string", self.main_spec)
|
||||
self.assertEqual(bkpt_2.GetNumLocations(), 1, "Got one location")
|
||||
threads = lldbutil.continue_to_breakpoint(process, bkpt_2)
|
||||
self.assertEqual(len(threads), 1, "Stopped at second breakpoint")
|
||||
frame = threads[0].frames[0]
|
||||
var = frame.FindVariable("in_str")
|
||||
self.assertTrue(var.GetError().Success(), "Made variable")
|
||||
summary = var.GetSummary()
|
||||
self.assertEqual(summary, "Summary Unavailable", "No summary for bad value")
|
||||
|
||||
|
|
|
@ -57,6 +57,11 @@ static struct {
|
|||
uint64_t data = 0xfffffffffffffffeULL;
|
||||
} garbage_string_long_mode4;
|
||||
|
||||
size_t touch_string(std::string &in_str)
|
||||
{
|
||||
return in_str.size(); // Break here to look at bad string
|
||||
}
|
||||
|
||||
int main()
|
||||
{
|
||||
std::wstring wempty(L"");
|
||||
|
@ -93,5 +98,7 @@ int main()
|
|||
#endif
|
||||
|
||||
S.assign(L"!!!!!"); // Set break point at this line.
|
||||
std::string *not_a_string = (std::string *) 0x0;
|
||||
touch_string(*not_a_string);
|
||||
return 0;
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue