jingham created this revision.
jingham added a reviewer: JDevlieghere.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D108228
Files:
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===================================================================
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -57,6 +57,11 @@
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 @@
#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;
}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===================================================================
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -19,7 +19,7 @@
# 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 @@
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 @@
'(%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 @@
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")
+
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -555,12 +555,14 @@
/// extract its data payload. Return the size + payload pair.
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)
@@ -576,13 +578,13 @@
uint64_t size_mode_value = 0;
if (layout == eLibcxxStringLayoutModeDSC) {
- ValueObjectSP size_mode(D->GetChildAtIndexPath({1, 1, 0}));
+ ValueObjectSP size_mode(dataval_sp->GetChildAtIndexPath({1, 1, 0}));
if (!size_mode)
return {};
if (size_mode->GetName() != g_size_name) {
// we are hitting the padding structure, move along
- size_mode = D->GetChildAtIndexPath({1, 1, 1});
+ size_mode = dataval_sp->GetChildAtIndexPath({1, 1, 1});
if (!size_mode)
return {};
}
@@ -590,7 +592,7 @@
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 {};
@@ -599,10 +601,10 @@
}
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);
const uint64_t size = (layout == eLibcxxStringLayoutModeDSC)
? size_mode_value
@@ -621,7 +623,7 @@
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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
