[Lldb-commits] [lldb] 7afd257 - Fix the std::string formatter to report errors in the case where the
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Tue May 17 08:22:36 PDT 2022
Author: Jim Ingham
Date: 2022-05-17T08:22:30-07:00
New Revision: 7afd257ff8a45a348767b4432abc6f4f105fc376
URL: https://github.com/llvm/llvm-project/commit/7afd257ff8a45a348767b4432abc6f4f105fc376
DIFF: https://github.com/llvm/llvm-project/commit/7afd257ff8a45a348767b4432abc6f4f105fc376.diff
LOG: 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
Added:
Modified:
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
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index be2e4da80881a..f8052f62babd3 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -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
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
index 93e3a2ca1a9e2..168297cc0611f 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -19,7 +19,7 @@ def setUp(self):
# 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 setUp(self):
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 @@ def cleanup():
'(%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 @@ def cleanup():
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")
+
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
index 365af0522c113..f88afc1f3a8e8 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -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;
}
More information about the lldb-commits
mailing list