sivachandra updated this revision to Diff 35444.
sivachandra added a comment.

Address comments.


http://reviews.llvm.org/D13066

Files:
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  test/lang/cpp/incomplete-types/Makefile
  test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py
  test/lang/cpp/incomplete-types/length.cpp
  test/lang/cpp/incomplete-types/length.h
  test/lang/cpp/incomplete-types/main.cpp

Index: test/lang/cpp/incomplete-types/main.cpp
===================================================================
--- /dev/null
+++ 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
+}
Index: test/lang/cpp/incomplete-types/length.h
===================================================================
--- /dev/null
+++ 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
Index: test/lang/cpp/incomplete-types/length.cpp
===================================================================
--- /dev/null
+++ test/lang/cpp/incomplete-types/length.cpp
@@ -0,0 +1,8 @@
+
+#include "length.h"
+
+size_t
+length (const std::string &str)
+{
+  return str.length();
+}
Index: test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py
===================================================================
--- /dev/null
+++ 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()
Index: test/lang/cpp/incomplete-types/Makefile
===================================================================
--- /dev/null
+++ 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
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -104,7 +104,7 @@
     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,
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1770,7 +1770,6 @@
                                                                          type->GetName().AsCString());
     assert (clang_type);
     DWARFAttributes attributes;
-
     switch (tag)
     {
         case DW_TAG_structure_type:
@@ -1817,17 +1816,25 @@
                     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,32 +1909,23 @@
                         // 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();
                             if (type_source_info)
                             {
                                 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.
-
-                                    ClangASTContext::StartTagDeclarationDefinition (base_class_type);
-                                    ClangASTContext::CompleteTagDeclarationDefinition (base_class_type);
+                                    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.");
+
+                                    return false;
                                 }
                             }
                         }
@@ -2354,7 +2352,7 @@
 }
 
 
-size_t
+bool
 DWARFASTParserClang::ParseChildMembers (const SymbolContext& sc,
                                         const DWARFDIE &parent_die,
                                         CompilerType &class_clang_type,
@@ -2370,7 +2368,7 @@
     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 @@
                                 }
 
                                 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 @@
         }
     }
 
-    return count;
+    return incomplete_member_info_count == 0;
 }
 
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to