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

@@ -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')
+            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()
 
-            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)
 
-            # 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()
+                # 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):

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();

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"""

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)
-    

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)
-    

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;
 }

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)
+        self.do_test_with_run_command(USE_LIBCPP)

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");