DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child

Summary:
With this change DWARFASTParserClang::CompleteTypeFromDWARF returns false if
DWARFASTParserClang::ParseChildMembers returns false. Similarly, it returns
false if any base class is of an incomplete type. This helps in cases like
these:

  class Foo
  {
  public:
    std::string str;
  };
  ...
  Foo f;

If a file with the above code is compiled with a modern clang but without
the -fno-limit-debug-info (or similar) option, then the DWARF has only
a forward declration for std::string. In which case, the type for
"class Foo" cannot be completed. If LLDB does not detect that a child
member has incomplete type, then it wrongly conveys to clang (the LLDB
compiler) that "class Foo" is complete, and consequently crashes due to
an assertion failure in clang when running commands like "p f" or
"frame var f".

Reviewers: clayborg

Subscribers: lldb-commits

Differential Revision: http://reviews.llvm.org/D13066

llvm-svn: 248401

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

@@ -1770,7 +1770,6 @@ DWARFASTParserClang::CompleteTypeFromDWARF (const DWARFDIE &die,
                                                                          type->GetName().AsCString());
     assert (clang_type);
     DWARFAttributes attributes;
-
     switch (tag)
     {
         case DW_TAG_structure_type:
@@ -1817,17 +1816,25 @@ DWARFASTParserClang::CompleteTypeFromDWARF (const DWARFDIE &die,
                     DWARFDIECollection member_function_dies;
 
                     DelayedPropertyList delayed_properties;
-                    ParseChildMembers (sc,
-                                       die,
-                                       clang_type,
-                                       class_language,
-                                       base_classes,
-                                       member_accessibilities,
-                                       member_function_dies,
-                                       delayed_properties,
-                                       default_accessibility,
-                                       is_a_class,
-                                       layout_info);
+                    if (!ParseChildMembers (sc,
+                                            die,
+                                            clang_type,
+                                            class_language,
+                                            base_classes,
+                                            member_accessibilities,
+                                            member_function_dies,
+                                            delayed_properties,
+                                            default_accessibility,
+                                            is_a_class,
+                                            layout_info))
+                    {
+                        auto module = dwarf->GetObjectFile()->GetModule();
+                        module->ReportError (":: Class %s has members with incomplete type.", die.GetName());
+                        if (die.GetCU()->GetProducer() == DWARFCompileUnit::eProducerClang)
+                            module->ReportError(":: Try compiling the source file with -fno-limit-debug-info.");
+
+                        return false;
+                    }
 
                     // Now parse any methods if there were any...
                     size_t num_functions = member_function_dies.Size();
@@ -1902,7 +1909,6 @@ DWARFASTParserClang::CompleteTypeFromDWARF (const DWARFDIE &die,
                         // Make sure all base classes refer to complete types and not
                         // forward declarations. If we don't do this, clang will crash
                         // with an assertion in the call to clang_type.SetBaseClassesForClassType()
-                        bool base_class_error = false;
                         for (auto &base_class : base_classes)
                         {
                             clang::TypeSourceInfo *type_source_info = base_class->getTypeSourceInfo();
@@ -1911,23 +1917,15 @@ DWARFASTParserClang::CompleteTypeFromDWARF (const DWARFDIE &die,
                                 CompilerType base_class_type (&m_ast, type_source_info->getType().getAsOpaquePtr());
                                 if (base_class_type.GetCompleteType() == false)
                                 {
-                                    if (!base_class_error)
-                                    {
-                                        dwarf->GetObjectFile()->GetModule()->ReportError ("DWARF DIE at 0x%8.8x for class '%s' has a base class '%s' that is a forward declaration, not a complete definition.\nPlease file a bug against the compiler and include the preprocessed output for %s",
-                                                                                          die.GetOffset(),
-                                                                                          die.GetName(),
-                                                                                          base_class_type.GetTypeName().GetCString(),
-                                                                                          sc.comp_unit ? sc.comp_unit->GetPath().c_str() : "the source file");
-                                    }
-                                    // We have no choice other than to pretend that the base class
-                                    // is complete. If we don't do this, clang will crash when we
-                                    // call setBases() inside of "clang_type.SetBaseClassesForClassType()"
-                                    // below. Since we provide layout assistance, all ivars in this
-                                    // class and other classes will be fine, this is the best we can do
-                                    // short of crashing.
+                                    auto module = dwarf->GetObjectFile()->GetModule();
+                                    module->ReportError (
+                                        ":: Class '%s' has a base class '%s' which does not have a complete definition.",
+                                        die.GetName(),
+                                        base_class_type.GetTypeName().GetCString());
+                                    if (die.GetCU()->GetProducer() == DWARFCompileUnit::eProducerClang)
+                                         module->ReportError (":: Try compiling the source file with -fno-limit-debug-info.");
 
-                                    ClangASTContext::StartTagDeclarationDefinition (base_class_type);
-                                    ClangASTContext::CompleteTagDeclarationDefinition (base_class_type);
+                                    return false;
                                 }
                             }
                         }
@@ -2354,7 +2352,7 @@ DWARFASTParserClang::ParseFunctionFromDWARF (const SymbolContext& sc,
 }
 
 
-size_t
+bool
 DWARFASTParserClang::ParseChildMembers (const SymbolContext& sc,
                                         const DWARFDIE &parent_die,
                                         CompilerType &class_clang_type,
@@ -2370,7 +2368,7 @@ DWARFASTParserClang::ParseChildMembers (const SymbolContext& sc,
     if (!parent_die)
         return 0;
 
-    size_t count = 0;
+    uint32_t incomplete_member_info_count = 0;
     uint32_t member_idx = 0;
     BitfieldInfo last_field_info;
 
@@ -2704,6 +2702,8 @@ DWARFASTParserClang::ParseChildMembers (const SymbolContext& sc,
                                 }
 
                                 CompilerType member_clang_type = member_type->GetLayoutCompilerType ();
+                                if (!member_clang_type.IsCompleteType() && !member_clang_type.GetCompleteType())
+                                    incomplete_member_info_count += 1;
 
                                 {
                                     // Older versions of clang emit array[0] and array[1] in the same way (<rdar://problem/12566646>).
@@ -2926,7 +2926,7 @@ DWARFASTParserClang::ParseChildMembers (const SymbolContext& sc,
         }
     }
 
-    return count;
+    return incomplete_member_info_count == 0;
 }
 
 

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

@@ -104,7 +104,7 @@ protected:
     ParseTemplateParameterInfos (const DWARFDIE &parent_die,
                                  lldb_private::ClangASTContext::TemplateParameterInfos &template_param_infos);
 
-    size_t
+    bool
     ParseChildMembers (const lldb_private::SymbolContext& sc,
                        const DWARFDIE &die,
                        lldb_private::CompilerType &class_clang_type,

lldb/test/lang/cpp/incomplete-types/Makefile

@@ -0,0 +1,32 @@
+LEVEL = ../../../make
+
+CXX_SOURCES = main.cpp length.cpp
+
+CFLAGS_LIMIT = -g -O0 -c
+CFLAGS_NO_LIMIT = -g -O0 -c
+
+ifneq (,$(findstring clang,$(CC)))
+  CFLAGS_LIMIT += -flimit-debug-info
+  CFLAGS_NO_LIMIT += -fno-limit-debug-info
+endif
+
+all: limit nolimit
+
+limit: main.o length_limit.o
+	$(CXX) main.o length_limit.o -o limit
+
+nolimit: main.o length_nolimit.o
+	$(CXX) main.o length_nolimit.o -o nolimit
+
+main.o: main.cpp
+	$(CXX) $(CFLAGS_LIMIT) main.cpp -o main.o
+
+length_limit.o: length.cpp
+	$(CXX) $(CFLAGS_LIMIT) length.cpp -o length_limit.o
+
+length_nolimit.o: length.cpp
+	$(CXX) $(CFLAGS_NO_LIMIT) length.cpp -o length_nolimit.o
+
+clean: OBJECTS += limit nolimit length_limit.o length_nolimit.o
+
+include $(LEVEL)/Makefile.rules

lldb/test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py

@@ -0,0 +1,75 @@
+import lldb
+from lldbtest import *
+import lldbutil
+
+class TestCppIncompleteTypes(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @dwarf_test
+    @skipIfGcc
+    def test_with_dwarf_limit_debug_info(self):
+        self.buildDwarf()
+        frame = self.get_test_frame('limit')
+
+        value_f = frame.EvaluateExpression("f")
+        self.assertTrue(value_f.IsValid(), "'expr f' results in a valid SBValue object")
+        self.assertFalse(value_f.GetError().Success(), "'expr f' results in an error, but LLDB does not crash")
+
+        value_s = frame.EvaluateExpression("s")
+        self.assertTrue(value_s.IsValid(), "'expr s' results in a valid SBValue object")
+        self.assertFalse(value_s.GetError().Success(), "'expr s' results in an error, but LLDB does not crash")
+
+    @dwarf_test
+    @skipIfGcc
+    def test_with_dwarf_partial_limit_debug_info(self):
+        self.buildDwarf()
+        frame = self.get_test_frame('nolimit')
+
+        value_f = frame.EvaluateExpression("f")
+        self.assertTrue(value_f.IsValid(), "'expr f' results in a valid SBValue object")
+        self.assertTrue(value_f.GetError().Success(), "'expr f' is successful")
+
+        value_s = frame.EvaluateExpression("s")
+        self.assertTrue(value_s.IsValid(), "'expr s' results in a valid SBValue object")
+        self.assertTrue(value_s.GetError().Success(), "'expr s' is successful")
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+    def get_test_frame(self, exe):
+        # Get main source file
+        src_file = "main.cpp"
+        src_file_spec = lldb.SBFileSpec(src_file)
+        self.assertTrue(src_file_spec.IsValid(), "Main source file")
+
+        # Get the path of the executable
+        cwd = os.getcwd()
+        exe_path  = os.path.join(cwd, exe)
+
+        # Load the executable
+        target = self.dbg.CreateTarget(exe_path)
+        self.assertTrue(target.IsValid(), VALID_TARGET)
+
+        # Break on main function
+        main_breakpoint = target.BreakpointCreateBySourceRegex("break here", src_file_spec)
+        self.assertTrue(main_breakpoint.IsValid() and main_breakpoint.GetNumLocations() >= 1, VALID_BREAKPOINT)
+
+        # Launch the process
+        args = None
+        env = None
+        process = target.LaunchSimple(args, env, self.get_process_working_directory())
+        self.assertTrue(process.IsValid(), PROCESS_IS_VALID)
+
+        # Get the thread of the process
+        self.assertTrue(process.GetState() == lldb.eStateStopped, PROCESS_STOPPED)
+        thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+
+        # Get frame for current thread
+        return thread.GetSelectedFrame()
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()

lldb/test/lang/cpp/incomplete-types/length.cpp

@@ -0,0 +1,8 @@
+
+#include "length.h"
+
+size_t
+length (const std::string &str)
+{
+  return str.length();
+}

lldb/test/lang/cpp/incomplete-types/length.h

@@ -0,0 +1,8 @@
+#ifndef __LENGTH_H__
+#define __LENGTH_H__
+
+#include <string>
+
+size_t length (const std::string &str);
+
+#endif

lldb/test/lang/cpp/incomplete-types/main.cpp

@@ -0,0 +1,23 @@
+
+#include "length.h"
+
+class Foo {
+public:
+    Foo(std::string x) : s(x) {}
+
+private:
+    std::string s;
+};
+
+class MyString : public std::string {
+public:
+    MyString(std::string x) : std::string(x) {}
+};
+
+int main()
+{
+    Foo f("qwerty");
+    MyString s("qwerty");
+
+    return length(s); // break here
+}