[llvm-branch-commits] [lldb] r270104 - Fixed many issues that were causing differing type definition issues to show up when parsing expressions.

Francis Ricci via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu May 19 13:12:19 PDT 2016

Author: fjricci
Date: Thu May 19 15:12:19 2016
New Revision: 270104

URL: http://llvm.org/viewvc/llvm-project?rev=270104&view=rev
Fixed many issues that were causing differing type definition issues to show up when parsing expressions.

1) Turns out we weren't correctly uniquing types for C++. We would search our repository for "lldb_private::Process", but yet store just "Process" in the unique type map. Now we store things correctly and correctly unique types.
2) SymbolFileDWARF::CompleteType() can be called at any time in order to complete a C++ or Objective C class. All public inquiries into the SymbolFile go through SymbolVendor, and SymbolVendor correctly takes the module lock before it call the SymbolFile API call, but when we let CompilerType objects out in the wild, they can complete themselves at any time from the expression parser, so the ValueObjects or (SBValue objects in the public API), and many more places. So we now take the module lock when completing a type to avoid two threads being in the SymbolFileDWARF at the same time.
3) If a class has a template member function like:

    class A
        <template T>
        void Foo(T t);
    The DWARF will _only_ contain a DW_TAG_subprogram for "Foo" if anyone specialized it. This would cause a class definition for A inside a.cpp that used a "int" and "float" overload to look like:
    class A
        void Foo(int t);
        void Foo(double t);
    And a version from b.cpp that used a "float" overload to look like:
    class A
        void Foo(float t);

    And a version from c.cpp that use no overloads to look like:    
    class A
    Then in an expression if you have two variables, one name "a" from a.cpp in liba.dylib, and one named "b" from b.cpp in libb.dylib, you will get conflicting definitions for "A" and your expression will fail. This all stems from the fact that DWARF _only_ emits template specializations, not generic definitions, and they are only emitted if they are used. There are two solutions to this:
    a) When ever you run into ANY class, you must say "just because this class doesn't have templatized member functions, it doesn't mean that any other instances might not have any, so when ever I run into ANY class, I must parse all compile units and parse all instances of class "A" just in case it has member functions that are templatized.". That is really bad because it means you always pull in ALL DWARF that contains most likely exact duplicate definitions of the class "A" and you bloat the memory that the SymbolFileDWARF plug-in uses in LLDB (since you pull in all DIEs from all compile units that contain a "A" definition) uses for little value most of the time.
    b) Modify DWARF to emit generic template member function definitions so that you know from looking at any instance of class "A" wether it has template member functions or not. In order to do this, we would have to have the ability to correctly parse a member function template, but there is a compiler bug: 
    <rdar://problem/24515533> [PR 26553] C++ Debug info should reference DW_TAG_template_type_parameter
    This bugs means that not all of the info needed to correctly make a template member function is in the DWARF. The main source of the problem is if we have DWARF for a template instantiation for "int" like: "void A::Foo<int>(T)" the DWARF comes out as "void A::Foo<int>(int)" (it doesn't mention type "T", it resolves the type to the specialized type to "int"). But if you actually have your function defined as "<template T> void Foo(int t)" and you only use T for local variables inside the function call, we can't correctly make the function prototype up in the clang::ASTContext. 
    So the best we can do for now we just omit all member functions that are templatized from the class definition so that "A" never has any template member functions. This means all defintions of "A" look like:
    class A
    And our expressions will work. You won't be able to call template member fucntions in expressions (not a regression, we weren't able to do this before) and if you are stopped in a templatized member function, we won't know that are are in a method of class "A". All things we should fix, but we need <rdar://problem/24515533> fixed first, followed by:
    <rdar://problem/24515624> Classes should always include a template subprogram definition, even when no template member functions are used
    before we can do anything about it in LLDB.

This bug mainly fixed the following Apple radar:



Modified: lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=270104&r1=270103&r2=270104&view=diff
--- lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (original)
+++ lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Thu May 19 15:12:19 2016
@@ -532,43 +532,32 @@ DWARFASTParserClang::ParseTypeFromDWARF
                     // and clang isn't good and sharing the stack space for variables in different blocks.
                     std::unique_ptr<UniqueDWARFASTType> unique_ast_entry_ap(new UniqueDWARFASTType());
+                    ConstString unique_typename(type_name_const_str);
+                    Declaration unique_decl(decl);
                     if (type_name_const_str)
                         LanguageType die_language = die.GetLanguage();
-                        bool handled = false;
                         if (Language::LanguageIsCPlusPlus(die_language))
+                            // For C++, we rely solely upon the one definition rule that says only
+                            // one thing can exist at a given decl context. We ignore the file and
+                            // line that things are declared on.
                             std::string qualified_name;
                             if (die.GetQualifiedName(qualified_name))
-                            {
-                                handled = true;
-                                ConstString const_qualified_name(qualified_name);
-                                if (dwarf->GetUniqueDWARFASTTypeMap().Find(const_qualified_name, die, Declaration(),
-                                                                           byte_size_valid ? byte_size : -1,
-                                                                           *unique_ast_entry_ap))
-                                {
-                                    type_sp = unique_ast_entry_ap->m_type_sp;
-                                    if (type_sp)
-                                    {
-                                        dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
-                                        return type_sp;
-                                    }
-                                }
-                            }
+                                unique_typename = ConstString(qualified_name);
+                            unique_decl.Clear();
-                        if (!handled)
+                        if (dwarf->GetUniqueDWARFASTTypeMap().Find(unique_typename, die, unique_decl,
+                                                                   byte_size_valid ? byte_size : -1,
+                                                                   *unique_ast_entry_ap))
-                            if (dwarf->GetUniqueDWARFASTTypeMap().Find(type_name_const_str, die, decl,
-                                                                       byte_size_valid ? byte_size : -1,
-                                                                       *unique_ast_entry_ap))
+                            type_sp = unique_ast_entry_ap->m_type_sp;
+                            if (type_sp)
-                                type_sp = unique_ast_entry_ap->m_type_sp;
-                                if (type_sp)
-                                {
-                                    dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
-                                    return type_sp;
-                                }
+                                dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+                                return type_sp;
@@ -799,9 +788,9 @@ DWARFASTParserClang::ParseTypeFromDWARF
                     // and over in the ASTContext for our module
                     unique_ast_entry_ap->m_type_sp = type_sp;
                     unique_ast_entry_ap->m_die = die;
-                    unique_ast_entry_ap->m_declaration = decl;
+                    unique_ast_entry_ap->m_declaration = unique_decl;
                     unique_ast_entry_ap->m_byte_size = byte_size;
-                    dwarf->GetUniqueDWARFASTTypeMap().Insert (type_name_const_str,
+                    dwarf->GetUniqueDWARFASTTypeMap().Insert (unique_typename,
                     if (is_forward_declaration && die.HasChildren())
@@ -1046,6 +1035,7 @@ DWARFASTParserClang::ParseTypeFromDWARF
                     bool is_virtual = false;
                     bool is_explicit = false;
                     bool is_artificial = false;
+                    bool has_template_params = false;
                     DWARFFormValue specification_die_form;
                     DWARFFormValue abstract_origin_die_form;
                     dw_offset_t object_pointer_die_offset = DW_INVALID_OFFSET;
@@ -1170,11 +1160,13 @@ DWARFASTParserClang::ParseTypeFromDWARF
                     clang::DeclContext *containing_decl_ctx = GetClangDeclContextContainingDIE (die, &decl_ctx_die);
                     const clang::Decl::Kind containing_decl_kind = containing_decl_ctx->getDeclKind();
-                    const bool is_cxx_method = DeclKindIsCXXClass (containing_decl_kind);
+                    bool is_cxx_method = DeclKindIsCXXClass (containing_decl_kind);
                     // Start off static. This will be set to false in ParseChildParameters(...)
                     // if we find a "this" parameters as the first parameter
                     if (is_cxx_method)
+                    {
                         is_static = true;
+                    }
                     if (die.HasChildren())
@@ -1185,11 +1177,26 @@ DWARFASTParserClang::ParseTypeFromDWARF
+                                              has_template_params,
+                    bool ignore_containing_context = false;
+                    // Check for templatized class member functions. If we had any DW_TAG_template_type_parameter
+                    // or DW_TAG_template_value_parameter the DW_TAG_subprogram DIE, then we can't let this become
+                    // a method in a class. Why? Because templatized functions are only emitted if one of the
+                    // templatized methods is used in the current compile unit and we will end up with classes
+                    // that may or may not include these member functions and this means one class won't match another
+                    // class definition and it affects our ability to use a class in the clang expression parser. So
+                    // for the greater good, we currently must not allow any template member functions in a class definition.
+//                    if (is_cxx_method && has_template_params)
+//                    {
+//                        ignore_containing_context = true;
+//                        is_cxx_method = false;
+//                    }
                     // clang_type will get the function prototype clang type after this call
                     clang_type = m_ast.CreateFunctionType (return_clang_type,
@@ -1197,7 +1204,6 @@ DWARFASTParserClang::ParseTypeFromDWARF
-                    bool ignore_containing_context = false;
                     if (type_name_cstr)
@@ -1957,6 +1963,10 @@ DWARFASTParserClang::CompleteTypeFromDWA
                                             lldb_private::Type *type,
                                             CompilerType &clang_type)
+    SymbolFileDWARF *dwarf = die.GetDWARF();
+    lldb_private::Mutex::Locker locker(dwarf->GetObjectFile()->GetModule()->GetMutex());
     // Disable external storage for this type so we don't get anymore
     // clang::ExternalASTSource queries for this type.
     m_ast.SetHasExternalStorage (clang_type.GetOpaqueQualType(), false);
@@ -1966,8 +1976,6 @@ DWARFASTParserClang::CompleteTypeFromDWA
     const dw_tag_t tag = die.Tag();
-    SymbolFileDWARF *dwarf = die.GetDWARF();
     Log *log = nullptr; // (LogChannelDWARF::GetLogIfAny(DWARF_LOG_DEBUG_INFO|DWARF_LOG_TYPE_COMPLETION));
     if (log)
         dwarf->GetObjectFile()->GetModule()->LogMessageVerboseBacktrace (log,
@@ -2484,6 +2492,7 @@ DWARFASTParserClang::ParseFunctionFromDW
                 // never mangled.
                 bool is_static = false;
                 bool is_variadic = false;
+                bool has_template_params = false;
                 unsigned type_quals = 0;
                 std::vector<CompilerType> param_types;
                 std::vector<clang::ParmVarDecl*> param_decls;
@@ -2500,6 +2509,7 @@ DWARFASTParserClang::ParseFunctionFromDW
+                                     has_template_params,
@@ -3166,6 +3176,7 @@ DWARFASTParserClang::ParseChildParameter
                                            bool skip_artificial,
                                            bool &is_static,
                                            bool &is_variadic,
+                                           bool &has_template_params,
                                            std::vector<CompilerType>& function_param_types,
                                            std::vector<clang::ParmVarDecl*>& function_param_decls,
                                            unsigned &type_quals)
@@ -3324,6 +3335,7 @@ DWARFASTParserClang::ParseChildParameter
                 // in SymbolFileDWARF::ParseType() so this was removed. If we ever need
                 // the template params back, we can add them back.
                 // ParseTemplateDIE (dwarf_cu, die, template_param_infos);
+                has_template_params = true;

Modified: lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h?rev=270104&r1=270103&r2=270104&view=diff
--- lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h (original)
+++ lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h Thu May 19 15:12:19 2016
@@ -133,6 +133,7 @@ protected:
                           bool skip_artificial,
                           bool &is_static,
                           bool &is_variadic,
+                          bool &has_template_params,
                           std::vector<lldb_private::CompilerType>& function_args,
                           std::vector<clang::ParmVarDecl*>& function_param_decls,
                           unsigned &type_quals);

Modified: lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=270104&r1=270103&r2=270104&view=diff
--- lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/branches/release_38/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Thu May 19 15:12:19 2016
@@ -1590,6 +1590,8 @@ SymbolFileDWARF::HasForwardDeclForClangT
 SymbolFileDWARF::CompleteType (CompilerType &compiler_type)
+    lldb_private::Mutex::Locker locker(GetObjectFile()->GetModule()->GetMutex());
     TypeSystem *type_system = compiler_type.GetTypeSystem();
     if (type_system)

More information about the llvm-branch-commits mailing list