[Lldb-commits] [lldb] a485011 - Make some libstd++ formatters safer

Walter Erquinigo via lldb-commits lldb-commits at lists.llvm.org
Tue Nov 23 13:58:39 PST 2021


Author: Walter Erquinigo
Date: 2021-11-23T13:52:32-08:00
New Revision: a48501150b9ef64fd61d24f8cef2645237facc44

URL: https://github.com/llvm/llvm-project/commit/a48501150b9ef64fd61d24f8cef2645237facc44
DIFF: https://github.com/llvm/llvm-project/commit/a48501150b9ef64fd61d24f8cef2645237facc44.diff

LOG: 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

Added: 
    

Modified: 
    lldb/examples/synthetic/gnu_libstdcpp.py
    lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
    lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/bitset/TestDataFormatterGenericBitset.py
    lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py
    lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py
    lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/main.cpp
    lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
    lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/examples/synthetic/gnu_libstdcpp.py b/lldb/examples/synthetic/gnu_libstdcpp.py
index efeb4fe29546..e2abc452bebd 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -2,10 +2,9 @@
 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 @@ def update(self):
             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 @@ def num_children_impl(self):
             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 @@ def extract_type(self):
         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 @@ def update(self):
         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 @@ def __init__(self, valobj, dict):
     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 @@ def updateNodes(self):
         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 @@ def update(self):
                 else:
                     self.count = 0
             except:
-                pass
+                self.count = 0
             return False
 
     class StdVBoolImplementation(object):
@@ -443,7 +456,10 @@ def update(self):
                 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 @@ def update(self):
             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<key, value>. 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<key, value>. 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 3a441973fc73..57c5ba87c397 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 7dac8ec06c0b..3d7ff340caab 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 @@ def setUp(self):
             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 @@ def check(self, name, size, variant):
             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 d54aedd4960b..28a01f17c684 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 @@ def setUp(self):
         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 @@ def test_libstdcpp(self):
     @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 b90964951fee..84be87ae478e 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 @@ def cleanup():
         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 @@ def cleanup():
                 '[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 @@ def cleanup():
                              '[3] = ',
                              'first = 3',
                              'second = 1'])
-        
+
         self.check("ii", 4)
 
         lldbutil.continue_to_breakpoint(self.process(), bkpt)
@@ -326,4 +337,3 @@ def test_with_run_command_libstdcpp(self):
     @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 27bdc0a57729..a30daa7a5bfe 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 <string>
 #include <map>
 
-#define intint_map std::multimap<int, int> 
-#define strint_map std::multimap<std::string, int> 
-#define intstr_map std::multimap<int, std::string> 
-#define strstr_map std::multimap<std::string, std::string> 
+#define intint_map std::multimap<int, int>
+#define strint_map std::multimap<std::string, int>
+#define intstr_map std::multimap<int, std::string>
+#define strstr_map std::multimap<std::string, std::string>
 
 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 664ff6921129..646836696ff0 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 @@ def cleanup():
         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 @@ def test_with_run_command_libstdcpp(self):
 
     @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 36ebefcd747b..00d37dcb4bd0 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<int, std::string> map;
+
+  char buffer[sizeof(std::unordered_map<int, std::string>)] = {0};
+  std::unordered_map<int, std::string> &corrupt_map = *(std::unordered_map<int, std::string> *)buffer;
+
+  std::unordered_map<int, std::string> map; // Set break point at this line.
   map.emplace(1, "hello");
   map.emplace(2, "world");
   map.emplace(3, "this");


        


More information about the lldb-commits mailing list