From a48501150b9ef64fd61d24f8cef2645237facc44 Mon Sep 17 00:00:00 2001 From: Walter Erquinigo Date: Tue, 23 Nov 2021 09:32:30 -0800 Subject: [PATCH] Make some libstd++ formatters safer We need to add checks that ensure that some core variables are valid, so that we avoid printing out garbage data. The worst that could happen is that an non-initialized variable is being printed as something with 123123432 children instead of 0. Differential Revision: https://reviews.llvm.org/D114458 --- lldb/examples/synthetic/gnu_libstdcpp.py | 94 +++++++++++++--------- .../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 13 +-- .../bitset/TestDataFormatterGenericBitset.py | 4 +- .../TestDataFormatterGenericForwardList.py | 3 +- .../multimap/TestDataFormatterGenericMultiMap.py | 18 ++++- .../data-formatter-stl/generic/multimap/main.cpp | 28 +++---- .../unordered/TestDataFormatterGenericUnordered.py | 15 ++-- .../data-formatter-stl/generic/unordered/main.cpp | 6 +- 8 files changed, 107 insertions(+), 74 deletions(-) diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py b/lldb/examples/synthetic/gnu_libstdcpp.py index efeb4fe..e2abc45 100644 --- a/lldb/examples/synthetic/gnu_libstdcpp.py +++ b/lldb/examples/synthetic/gnu_libstdcpp.py @@ -2,10 +2,9 @@ from __future__ import division import lldb.formatters.Logger # C++ STL formatters for LLDB -# These formatters are based upon the version of the GNU libstdc++ -# as it ships with Mac OS X 10.6.8 thru 10.8.0 -# You are encouraged to look at the STL implementation for your platform -# before relying on these formatters to do the right thing for your setup +# As there are many versions of the libcstd++, you are encouraged to look at the STL +# implementation for your platform before relying on these formatters to do the right +# thing for your setup def StdOptionalSummaryProvider(valobj, dict): has_value = valobj.GetNumChildren() > 0 @@ -76,8 +75,10 @@ class StdUnorderedMapSynthProvider: self.data_type = self.extract_type() self.skip_size = self.next.GetType().GetByteSize() self.data_size = self.data_type.GetByteSize() + if (not self.data_type.IsValid()) or (not self.next.IsValid()): + self.count = 0 except: - pass + self.count = 0 return False def get_child_index(self, name): @@ -208,8 +209,11 @@ class AbstractListSynthProvider: current = self.next while current.GetChildMemberWithName( '_M_next').GetValueAsUnsigned(0) != self.get_end_of_list_address(): - size = size + 1 current = current.GetChildMemberWithName('_M_next') + if not current.IsValid(): + break + size = size + 1 + return size except: logger >> "Error determining the size" @@ -250,10 +254,8 @@ class AbstractListSynthProvider: if list_type.IsReferenceType(): list_type = list_type.GetDereferencedType() if list_type.GetNumberOfTemplateArguments() > 0: - data_type = list_type.GetTemplateArgumentType(0) - else: - data_type = None - return data_type + return list_type.GetTemplateArgumentType(0) + return lldb.SBType() def update(self): logger = lldb.formatters.Logger.Logger() @@ -263,15 +265,20 @@ class AbstractListSynthProvider: try: self.impl = self.valobj.GetChildMemberWithName('_M_impl') self.data_type = self.extract_type() - self.data_size = self.data_type.GetByteSize() - self.updateNodes() + if (not self.data_type.IsValid()) or (not self.impl.IsValid()): + self.count = 0 + elif not self.updateNodes(): + self.count = 0 + else: + self.data_size = self.data_type.GetByteSize() except: - pass + self.count = 0 return False ''' Method is used to extract the list pointers into the variables (e.g self.node, self.next, and optionally to self.prev) - and is mandatory to be overriden in each AbstractListSynthProvider subclass + and is mandatory to be overriden in each AbstractListSynthProvider subclass. + This should return True or False depending on wheter it found valid data. ''' def updateNodes(self): raise NotImplementedError @@ -296,6 +303,9 @@ class StdForwardListSynthProvider(AbstractListSynthProvider): def updateNodes(self): self.node = self.impl.GetChildMemberWithName('_M_head') self.next = self.node.GetChildMemberWithName('_M_next') + if (not self.node.IsValid()) or (not self.next.IsValid()): + return False + return True def get_end_of_list_address(self): return 0 @@ -312,6 +322,9 @@ class StdListSynthProvider(AbstractListSynthProvider): self.node = self.impl.GetChildMemberWithName('_M_node') self.prev = self.node.GetChildMemberWithName('_M_prev') self.next = self.node.GetChildMemberWithName('_M_next') + if self.node_address == 0 or (not self.node.IsValid()) or (not self.next.IsValid()) or (not self.prev.IsValid()): + return False + return True def get_end_of_list_address(self): return self.node_address @@ -398,7 +411,7 @@ class StdVectorSynthProvider: else: self.count = 0 except: - pass + self.count = 0 return False class StdVBoolImplementation(object): @@ -443,7 +456,10 @@ class StdVectorSynthProvider: self.start_p = self.m_start.GetChildMemberWithName('_M_p') self.finish_p = self.m_finish.GetChildMemberWithName('_M_p') self.offset = self.m_finish.GetChildMemberWithName('_M_offset') - self.valid = True + if self.offset.IsValid() and self.start_p.IsValid() and self.finish_p.IsValid(): + self.valid = True + else: + self.valid = False except: self.valid = False return False @@ -530,29 +546,31 @@ class StdMapLikeSynthProvider: self.Mt = self.valobj.GetChildMemberWithName('_M_t') self.Mimpl = self.Mt.GetChildMemberWithName('_M_impl') self.Mheader = self.Mimpl.GetChildMemberWithName('_M_header') - - map_type = self.valobj.GetType() - if map_type.IsReferenceType(): - logger >> "Dereferencing type" - map_type = map_type.GetDereferencedType() - - # Get the type of std::pair. It is the first template - # argument type of the 4th template argument to std::map. - allocator_type = map_type.GetTemplateArgumentType(3) - self.data_type = allocator_type.GetTemplateArgumentType(0) - if not self.data_type: - # GCC does not emit DW_TAG_template_type_parameter for - # std::allocator<...>. For such a case, get the type of - # std::pair from a member of std::map. - rep_type = self.valobj.GetChildMemberWithName('_M_t').GetType() - self.data_type = rep_type.GetTypedefedType().GetTemplateArgumentType(1) - - # from libstdc++ implementation of _M_root for rbtree - self.Mroot = self.Mheader.GetChildMemberWithName('_M_parent') - self.data_size = self.data_type.GetByteSize() - self.skip_size = self.Mheader.GetType().GetByteSize() + if not self.Mheader.IsValid(): + self.count = 0 + else: + map_type = self.valobj.GetType() + if map_type.IsReferenceType(): + logger >> "Dereferencing type" + map_type = map_type.GetDereferencedType() + + # Get the type of std::pair. It is the first template + # argument type of the 4th template argument to std::map. + allocator_type = map_type.GetTemplateArgumentType(3) + self.data_type = allocator_type.GetTemplateArgumentType(0) + if not self.data_type: + # GCC does not emit DW_TAG_template_type_parameter for + # std::allocator<...>. For such a case, get the type of + # std::pair from a member of std::map. + rep_type = self.valobj.GetChildMemberWithName('_M_t').GetType() + self.data_type = rep_type.GetTypedefedType().GetTemplateArgumentType(1) + + # from libstdc++ implementation of _M_root for rbtree + self.Mroot = self.Mheader.GetChildMemberWithName('_M_parent') + self.data_size = self.data_type.GetByteSize() + self.skip_size = self.Mheader.GetType().GetByteSize() except: - pass + self.count = 0 return False def num_children(self): diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp index 3a44197..57c5ba8 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp @@ -62,9 +62,7 @@ lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd:: size_t lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd:: CalculateNumChildren() { - if (m_num_elements != UINT32_MAX) - return m_num_elements; - return 0; + return m_num_elements; } lldb::ValueObjectSP lldb_private::formatters:: @@ -160,7 +158,7 @@ lldb::ValueObjectSP lldb_private::formatters:: bool lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd:: Update() { - m_num_elements = UINT32_MAX; + m_num_elements = 0; m_next_element = nullptr; m_elements_cache.clear(); ValueObjectSP table_sp = @@ -195,8 +193,13 @@ bool lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd:: if (!num_elements_sp) return false; - m_num_elements = num_elements_sp->GetValueAsUnsigned(0); + m_tree = table_sp->GetChildAtNamePath(next_path).get(); + if (m_tree == nullptr) + return false; + + m_num_elements = num_elements_sp->GetValueAsUnsigned(0); + if (m_num_elements > 0) m_next_element = table_sp->GetChildAtNamePath(next_path).get(); diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/bitset/TestDataFormatterGenericBitset.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/bitset/TestDataFormatterGenericBitset.py index 7dac8ec..3d7ff34 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/bitset/TestDataFormatterGenericBitset.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/bitset/TestDataFormatterGenericBitset.py @@ -27,7 +27,7 @@ class GenericBitsetDataFormatterTestCase(TestBase): for j in range(2*i, len(primes), i): primes[j] = 0 self.primes = primes - + def getBitsetVariant(self, size, variant): if variant == VALUE: return "std::bitset<" + str(size) + ">" @@ -47,7 +47,7 @@ class GenericBitsetDataFormatterTestCase(TestBase): children.append(ValueCheck(value=str(bool(child.GetValueAsUnsigned())).lower())) self.assertEqual(child.GetValueAsUnsigned(), self.primes[i], "variable: %s, index: %d"%(name, size)) - self.expect_var_path(name,type=self.getBitsetVariant(size,variant),children=children) + self.expect_var_path(name,type=self.getBitsetVariant(size,variant),children=children) def do_test_value(self, stdlib_type): """Test that std::bitset is displayed correctly""" diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py index d54aedd..28a01f1 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py @@ -21,7 +21,7 @@ class TestDataFormatterGenericForwardList(TestBase): self.line = line_number('main.cpp', '// break here') self.namespace = 'std' - + def do_test(self, stdlib_type): """Test that std::forward_list is displayed correctly""" self.build(dictionary={stdlib_type: "1"}) @@ -59,4 +59,3 @@ class TestDataFormatterGenericForwardList(TestBase): @add_test_categories(["libc++"]) def test_libcpp(self): self.do_test(USE_LIBCPP) - diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py index b909649..84be87a 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py @@ -20,7 +20,7 @@ class GenericMultiMapDataFormatterTestCase(TestBase): def setUp(self): TestBase.setUp(self) self.namespace = 'std' - + def findVariable(self, name): var = self.frame().FindVariable(name) self.assertTrue(var.IsValid()) @@ -70,6 +70,17 @@ class GenericMultiMapDataFormatterTestCase(TestBase): self.addTearDownHook(cleanup) multimap = self.namespace + "::multimap" + + self.expect('frame variable ii', + substrs=[multimap, 'size=0', + '{}']) + + self.expect('frame variable si', + substrs=[multimap, 'size=0', + '{}']) + + lldbutil.continue_to_breakpoint(self.process(), bkpt) + self.expect('frame variable ii', substrs=[multimap, 'size=0', '{}']) @@ -84,7 +95,7 @@ class GenericMultiMapDataFormatterTestCase(TestBase): '[0] = (first = 0, second = 0)', '[1] = (first = 1, second = 1)', ]) - + self.check("ii", 2) lldbutil.continue_to_breakpoint(self.process(), bkpt) @@ -97,7 +108,7 @@ class GenericMultiMapDataFormatterTestCase(TestBase): '[3] = ', 'first = 3', 'second = 1']) - + self.check("ii", 4) lldbutil.continue_to_breakpoint(self.process(), bkpt) @@ -326,4 +337,3 @@ class GenericMultiMapDataFormatterTestCase(TestBase): @add_test_categories(["libc++"]) def test_with_run_command_libcpp(self): self.do_test_with_run_command(USE_LIBCPP) - diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/main.cpp index 27bdc0a..a30daa7 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/main.cpp @@ -1,10 +1,10 @@ #include #include -#define intint_map std::multimap -#define strint_map std::multimap -#define intstr_map std::multimap -#define strstr_map std::multimap +#define intint_map std::multimap +#define strint_map std::multimap +#define intstr_map std::multimap +#define strstr_map std::multimap int g_the_foo = 0; @@ -18,10 +18,10 @@ int thefoo_rw(int arg = 1) return g_the_foo; } -int main() +int main() // Set break point at this line. { intint_map ii; - + ii.emplace(0,0); // Set break point at this line. ii.emplace(1,1); thefoo_rw(1); // Set break point at this line. @@ -36,10 +36,10 @@ int main() ii.emplace(85,1234567); ii.clear(); - + strint_map si; thefoo_rw(1); // Set break point at this line. - + si.emplace("zero",0); thefoo_rw(1); // Set break point at this line. si.emplace("one",1); @@ -50,7 +50,7 @@ int main() si.clear(); thefoo_rw(1); // Set break point at this line. - + intstr_map is; thefoo_rw(1); // Set break point at this line. is.emplace(85,"goofy"); @@ -58,20 +58,20 @@ int main() is.emplace(2,"smart"); is.emplace(3,"!!!"); thefoo_rw(1); // Set break point at this line. - + is.clear(); thefoo_rw(1); // Set break point at this line. - + strstr_map ss; thefoo_rw(1); // Set break point at this line. - + ss.emplace("ciao","hello"); ss.emplace("casa","house"); ss.emplace("gatto","cat"); thefoo_rw(1); // Set break point at this line. ss.emplace("a Mac..","..is always a Mac!"); - + ss.clear(); - thefoo_rw(1); // Set break point at this line. + thefoo_rw(1); // Set break point at this line. return 0; } diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py index 664ff69..6468366 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py @@ -1,10 +1,3 @@ -""" -Test lldb data formatter subsystem. -""" - - - -import lldb from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil @@ -49,6 +42,12 @@ class GenericUnorderedDataFormatterTestCase(TestBase): self.addTearDownHook(cleanup) ns = self.namespace + + # We check here that the map shows 0 children even with corrupt data. + self.look_for_content_and_continue( + "corrupt_map", ['%s::unordered_map' % + ns, 'size=0 {}']) + self.look_for_content_and_continue( "map", ['%s::unordered_map' % ns, 'size=5 {', 'hello', 'world', 'this', 'is', 'me']) @@ -84,4 +83,4 @@ class GenericUnorderedDataFormatterTestCase(TestBase): @add_test_categories(["libc++"]) def test_with_run_command_libcpp(self): - self.do_test_with_run_command(USE_LIBCPP) \ No newline at end of file + self.do_test_with_run_command(USE_LIBCPP) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp index 36ebefc..00d37dc 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp @@ -14,7 +14,11 @@ int thefoo_rw(int arg = 1) { } int main() { - std::unordered_map map; + + char buffer[sizeof(std::unordered_map)] = {0}; + std::unordered_map &corrupt_map = *(std::unordered_map *)buffer; + + std::unordered_map map; // Set break point at this line. map.emplace(1, "hello"); map.emplace(2, "world"); map.emplace(3, "this"); -- 2.7.4