[Lldb-commits] [lldb] r260308 - Fixed many issues that were causing differing type definition issues to show up when parsing expressions.
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 9 14:48:36 PST 2016
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160209/b6cf49ce/attachment-0001.html>
More information about the lldb-commits
mailing list