[Lldb-commits] [lldb] r260308 - Fixed many issues that were causing differing type definition issues to show up when parsing expressions.
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 9 15:26:35 PST 2016
Not yet, but I will try to reduce one down. The best I have right now is to debug clang, set a breakpoint at "Sema::SelectBestMethod" and then run "p Sel" when you hit the breakpoint. This is of course only by running with a file:
lldb -- "/tmp/local/clang/install/bin/clang-3.8" -cc1 -triple arm64-apple-ios5.0.0 -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -Werror=implicit-function-declaration -fsyntax-only -disable-free -main-file-name ios10.mi -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim -masm-verbose -target-cpu cyclone -target-feature +neon -target-feature +crc -target-feature +crypto -target-feature +zcm -target-feature +zcz -target-abi darwinpcs -target-linker-version 262.4 -v -dwarf-column-info -resource-dir /tmp/local/clang/install/bin/../lib/clang/3.8.0 -fdebug-compilation-dir /tmp -ferror-limit 19 -fmessage-length 80 -stack-protector 1 -fallow-half-arguments-and-returns -fblocks -fobjc-runtime=ios-5.0.0 -fencode-extended-block-signature -fobjc-exceptions -fexceptions -fmax-type-align=16 -fdiagnostics-show-option -fcolor-diagnostics -x objective-c-cpp-output ios10.mi
So not a very reduced test case. I will work on getting one in ASAP but I would like it to be more self contained and controlled. The expression also takes way too long (30-90 seconds) because of the changes that were previously added (revision 247746) that parse everything in a decl context. If that proves to be what is slowing down expressions, I will need to do something about it as it is affecting our expression performance.
So stay tuned, I'll follow up with a test soon.
> On Feb 9, 2016, at 2:48 PM, Zachary Turner <zturner at google.com> wrote:
>
> Any tests for this?
>
> On Tue, Feb 9, 2016 at 2:40 PM Greg Clayton via lldb-commits <lldb-commits at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
More information about the lldb-commits
mailing list