[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