Make some libstd++ formatters safer

[wallace]

Summary

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.

[labath]

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp

16

It'd be better to be more explicit about the contents of this "uninitialized" memory.
For example you could allocate a char array of an appropriate size, memset it to zero (or whatever), and then cast its address to a map pointer.

Otherwise you run the risk of flakyness due to random data showing up in that block of memory.

[wallace]

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp

16

good one

[wallace]

labath's comment

lldb/examples/synthetic/gnu_libstdcpp.py

 from __future__ import division
-import re
 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
     # We add wrapping spaces for consistency with the libcxx formatter
     return " Has Value=" + ("true" if has_value else "false") + " "
 
 
 class StdOptionalSynthProvider:
def update(self):
         self.count = None
         try:
             self.head = self.valobj.GetChildMemberWithName('_M_h')
             self.before_begin = self.head.GetChildMemberWithName('_M_before_begin')
             self.next = self.before_begin.GetChildMemberWithName('_M_nxt')
             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):
         try:
             return int(name.lstrip('[').rstrip(']'))
         except:
             return -1
 
def num_children_impl(self):
                 if next_val == self.node_address:
                     return 0
                 if next_val == prev_val:
                     return 1
             size = 1
             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"
             return 0
 
     def get_child_index(self, name):
         logger = lldb.formatters.Logger.Logger()
         try:
def get_child_at_index(self, index):
             return None
 
     def extract_type(self):
         logger = lldb.formatters.Logger.Logger()
         list_type = self.valobj.GetType().GetUnqualifiedType()
         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()
         # preemptively setting this to None - we might end up changing our mind
         # later
         self.count = None
         try:
             self.impl = self.valobj.GetChildMemberWithName('_M_impl')
             self.data_type = self.extract_type()
+            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()
-            self.updateNodes()
         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
 
     def has_children(self):
         return True
 
     '''
      Method is used to identify if a node traversal has reached its end
      and is mandatory to be overriden in each AbstractListSynthProvider subclass
     '''
     def get_end_of_list_address(self):
         raise NotImplementedError
 
 
 class StdForwardListSynthProvider(AbstractListSynthProvider):
 
     def __init__(self, valobj, dict):
         has_prev = False
         super().__init__(valobj, dict, has_prev)
 
     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
 
 
 class StdListSynthProvider(AbstractListSynthProvider):
 
     def __init__(self, valobj, dict):
         has_prev = True
         super().__init__(valobj, dict, has_prev)
 
     def updateNodes(self):
         self.node_address = self.valobj.AddressOf().GetValueAsUnsigned(0)
         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
 
 
 class StdVectorSynthProvider:
 
     class StdVectorImplementation(object):
class StdVectorImplementation(object):
                 # if any of these objects is invalid, it means there is no
                 # point in trying to fetch anything
                 if self.start.IsValid() and self.finish.IsValid(
                 ) and self.end.IsValid() and self.data_type.IsValid():
                     self.count = None
                 else:
                     self.count = 0
             except:
-                pass
+                self.count = 0
             return False
 
     class StdVBoolImplementation(object):
 
         def __init__(self, valobj, bool_type):
             self.valobj = valobj
             self.bool_type = bool_type
             self.valid = False
class StdVBoolImplementation(object):
         def update(self):
             try:
                 m_impl = self.valobj.GetChildMemberWithName('_M_impl')
                 self.m_start = m_impl.GetChildMemberWithName('_M_start')
                 self.m_finish = m_impl.GetChildMemberWithName('_M_finish')
                 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')
+                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
 
     def __init__(self, valobj, dict):
         logger = lldb.formatters.Logger.Logger()
         first_template_arg_type = valobj.GetType().GetTemplateArgumentType(0)
         if str(first_template_arg_type.GetName()) == "bool":
def update(self):
         try:
             # we will set this to True if we find out that discovering a node in the object takes more steps than the overall size of the RB tree
             # if this gets set to True, then we will merrily return None for
             # any child from that moment on
             self.garbage = False
             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()
 
                 # 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):
         logger = lldb.formatters.Logger.Logger()
         if self.count is None:
             self.count = self.num_children_impl()
         return self.count
 

lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp

lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd::
     : SyntheticChildrenFrontEnd(*valobj_sp), m_element_type(), m_tree(nullptr),
       m_num_elements(0), m_next_element(nullptr), m_elements_cache() {
   if (valobj_sp)
     Update();
 }
 
 size_t lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd::
     CalculateNumChildren() {
-  if (m_num_elements != UINT32_MAX)
-    return m_num_elements;
-  return 0;
 }
 
 lldb::ValueObjectSP lldb_private::formatters::
     LibcxxStdUnorderedMapSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
   if (idx >= CalculateNumChildren())
     return lldb::ValueObjectSP();
   if (m_tree == nullptr)
     return lldb::ValueObjectSP();
lldb::ValueObjectSP lldb_private::formatters::
   ExecutionContext exe_ctx = val_hash.first->GetExecutionContextRef().Lock(
       thread_and_frame_only_if_stopped);
   return CreateValueObjectFromData(stream.GetString(), data, exe_ctx,
                                    val_hash.first->GetCompilerType());
 }
 
 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 =
       m_backend.GetChildMemberWithName(ConstString("__table_"), true);
   if (!table_sp)
     return false;
 
   ValueObjectSP p2_sp = table_sp->GetChildMemberWithName(
case 2: {
     break;
   }
   default:
     return false;
   }
 
   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();
   return false;
 }
 
 bool lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd::
     MightHaveChildren() {

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/bitset/TestDataFormatterGenericBitset.py

class GenericBitsetDataFormatterTestCase(TestBase):
     def setUp(self):
         TestBase.setUp(self)
         primes = [1]*300
         primes[0] = primes[1] = 0
         for i in range(2, len(primes)):
             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) + ">"
         elif variant == REFERENCE:
             return "std::bitset<" + str(size) + "> &"
         elif variant == POINTER:
             return "std::bitset<" + str(size) + "> *"
         return ""
 
     def check(self, name, size, variant):
         var = self.frame().FindVariable(name)
         self.assertTrue(var.IsValid())
         self.assertEqual(var.GetNumChildren(), size)
         children = []
         for i in range(size):
             child = var.GetChildAtIndex(i)
             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"""
         self.build(dictionary={stdlib_type: "1"})
 
         lldbutil.run_to_source_breakpoint(self, '// break here',
                 lldb.SBFileSpec("main.cpp", False))
 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py

class TestDataFormatterGenericForwardList(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     def setUp(self):
         TestBase.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"})
         lldbutil.run_to_source_breakpoint(self, '// break here',
                 lldb.SBFileSpec("main.cpp", False))
 
         forward_list = self.namespace + '::forward_list'
         self.expect("frame variable empty",
class TestDataFormatterGenericForwardList(TestBase):
 
     @add_test_categories(["libstdcxx"])
     def test_libstdcpp(self):
         self.do_test(USE_LIBSTDCPP)
 
     @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

 
 class GenericMultiMapDataFormatterTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     def setUp(self):
         TestBase.setUp(self)
         self.namespace = 'std'
-    
     def findVariable(self, name):
         var = self.frame().FindVariable(name)
         self.assertTrue(var.IsValid())
         return var
 
     def getVariableType(self, name):
         var = self.findVariable(name)
         return var.GetType().GetDisplayTypeName()
def do_test_with_run_command(self, stdlib_type):
             self.runCmd(
                 "settings set target.max-children-count 256",
                 check=False)
 
         # Execute the cleanup function during test case tear down.
         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',
                              '{}'])
 
         lldbutil.continue_to_breakpoint(self.process(), bkpt)
 
         self.expect(
             'frame variable ii',
             substrs=[
                 multimap,
                 'size=2',
                 '[0] = (first = 0, second = 0)',
                 '[1] = (first = 1, second = 1)',
             ])
-        
         self.check("ii", 2)
 
         lldbutil.continue_to_breakpoint(self.process(), bkpt)
 
         self.expect('frame variable ii',
                     substrs=[multimap, 'size=4',
                              '[2] = ',
                              'first = 2',
                              'second = 0',
                              '[3] = ',
                              'first = 3',
                              'second = 1'])
-        
         self.check("ii", 4)
 
         lldbutil.continue_to_breakpoint(self.process(), bkpt)
 
         self.expect("frame variable ii",
                     substrs=[multimap, 'size=8',
                              '[5] = ',
                              'first = 5',
class GenericMultiMapDataFormatterTestCase(TestBase):
 
     @add_test_categories(["libstdcxx"])
     def test_with_run_command_libstdcpp(self):
         self.do_test_with_run_command(USE_LIBSTDCPP)
 
     @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

 #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;
 
 int thefoo_rw(int arg = 1)
 {
   if (arg < 0)
     arg = 0;
   if (!arg)
     arg = 1;
   g_the_foo += arg;
   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.
     ii.emplace(2,0);
   ii.emplace(3,1);
   thefoo_rw(1);  // Set break point at this line.
   ii.emplace(4,0);
   ii.emplace(5,1);
   ii.emplace(6,0);
   ii.emplace(7,1);
     thefoo_rw(1);  // Set break point at this line.
     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);
   si.emplace("two",2);
   si.emplace("three",3);
   thefoo_rw(1);  // Set break point at this line.
   si.emplace("four",4);
 
     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");
     is.emplace(1,"is");
     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

-"""
-Test lldb data formatter subsystem.
-"""
-
-
-
-import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 USE_LIBSTDCPP = "USE_LIBSTDCPP"
 USE_LIBCPP = "USE_LIBCPP"
 
 class GenericUnorderedDataFormatterTestCase(TestBase):
def do_test_with_run_command(self, stdlib_type):
             self.runCmd(
                 "settings set target.max-children-count 256",
                 check=False)
 
         # Execute the cleanup function during test case tear down.
         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'])
 
         self.look_for_content_and_continue(
             "mmap", ['%s::unordered_multimap' % ns, 'size=6 {', 'first = 3', 'second = "this"',
                      'first = 2', 'second = "hello"'])
 
def look_for_content_and_continue(self, var_name, patterns):
         self.runCmd("continue")
 
     @add_test_categories(["libstdcxx"])
     def test_with_run_command_libstdcpp(self):
         self.do_test_with_run_command(USE_LIBSTDCPP)
 
     @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

 #include <string>
 #include <unordered_map>
 #include <unordered_set>
 
 int g_the_foo = 0;
 
 int thefoo_rw(int arg = 1) {
   if (arg < 0)
     arg = 0;
   if (!arg)
     arg = 1;
   g_the_foo += arg;
   return g_the_foo;
 }
 
 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");
   map.emplace(4, "is");
   map.emplace(5, "me");
   thefoo_rw(); // Set break point at this line.
 
   std::unordered_multimap<int, std::string> mmap;