Any tests for this? On Tue, Feb 9, 2016 at 2:40 PM Greg Clayton via lldb-commits < lldb-commits@lists.llvm.org> wrote:
> Author: gclayton > Date: Tue Feb 9 16:36:24 2016 > New Revision: 260308 > > URL: http://llvm.org/viewvc/llvm-project?rev=260308&view=rev > Log: > 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: > > <rdar://problem/24483905> > > > Modified: > lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp > lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h > lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp > > Modified: > lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=260308&r1=260307&r2=260308&view=diff > > ============================================================================== > --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp > (original) > +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Tue > Feb 9 16:36:24 2016 > @@ -555,43 +555,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; > } > } > } > @@ -822,9 +811,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, > > *unique_ast_entry_ap); > > if (is_forward_declaration && die.HasChildren()) > @@ -1069,6 +1058,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; > @@ -1193,11 +1183,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()) > { > @@ -1208,11 +1200,26 @@ DWARFASTParserClang::ParseTypeFromDWARF > skip_artificial, > is_static, > is_variadic, > + has_template_params, > function_param_types, > function_param_decls, > type_quals); > } > > + 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, > > function_param_types.data(), > @@ -1220,7 +1227,6 @@ DWARFASTParserClang::ParseTypeFromDWARF > is_variadic, > type_quals); > > - bool ignore_containing_context = false; > > if (type_name_cstr) > { > @@ -1980,6 +1986,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); > @@ -1989,8 +1999,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, > @@ -2507,6 +2515,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; > @@ -2523,6 +2532,7 @@ DWARFASTParserClang::ParseFunctionFromDW > true, > is_static, > is_variadic, > + has_template_params, > param_types, > param_decls, > type_quals); > @@ -3189,6 +3199,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) > @@ -3347,6 +3358,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; > break; > > default: > > Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h?rev=260308&r1=260307&r2=260308&view=diff > > ============================================================================== > --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h > (original) > +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h Tue > Feb 9 16:36:24 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/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=260308&r1=260307&r2=260308&view=diff > > ============================================================================== > --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp > (original) > +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Tue > Feb 9 16:36:24 2016 > @@ -1623,6 +1623,8 @@ SymbolFileDWARF::HasForwardDeclForClangT > bool > SymbolFileDWARF::CompleteType (CompilerType &compiler_type) > { > + lldb_private::Mutex::Locker > locker(GetObjectFile()->GetModule()->GetMutex()); > + > TypeSystem *type_system = compiler_type.GetTypeSystem(); > if (type_system) > { > > > _______________________________________________ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
