[Lldb-commits] [PATCH] Use the DWARF linkage name when importing C++ methods

[Nelson Elhage via lldb-commits]

When importing C++ methods into clang AST nodes from the DWARF symbol
table, preserve the DW_AT_linkage_name and use it as the linker ("asm")
name for the symbol.

Concretely, this enables `expression` to call into names that use the GNU
`abi_tag` extension, and enables lldb to call into code using std::string
or std::list from recent versions of libstdc++. See
https://bugs.llvm.org/show_bug.cgi?id=35310 . It also seems broadly more
robust than relying on the DWARF->clang->codegen pipeline to roundtrip
properly, but I'm not immediately aware of any other cases in which it
makes a difference.

- Nelson
Index: include/lldb/Symbol/ClangASTContext.h
===
--- include/lldb/Symbol/ClangASTContext.h	(revision 318614)
+++ include/lldb/Symbol/ClangASTContext.h	(working copy)
@@ -821,6 +821,7 @@
 
   clang::CXXMethodDecl *
   AddMethodToCXXRecordType(lldb::opaque_compiler_type_t type, const char *name,
+   const char *mangled_name,
const CompilerType &method_type,
lldb::AccessType access, bool is_virtual,
bool is_static, bool is_inline, bool is_explicit,
Index: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp	(revision 318614)
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp	(working copy)
@@ -2165,7 +2165,7 @@
 CXXMethodDecl *method_decl =
 ClangASTContext::GetASTContext(m_ast_context)
 ->AddMethodToCXXRecordType(
-copied_clang_type.GetOpaqueQualType(), "$__lldb_expr",
+copied_clang_type.GetOpaqueQualType(), "$__lldb_expr", NULL,
 method_type, lldb::eAccessPublic, is_virtual, is_static,
 is_inline, is_explicit, is_attr_used, is_artificial);
 
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp	(revision 318614)
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp	(working copy)
@@ -220,6 +220,7 @@
   bool is_forward_declaration = false;
   DWARFAttributes attributes;
   const char *type_name_cstr = NULL;
+  const char *mangled_name_cstr = NULL;
   ConstString type_name_const_str;
   Type::ResolveState resolve_state = Type::eResolveStateUnresolved;
   uint64_t byte_size = 0;
@@ -1135,9 +1136,8 @@
 
   case DW_AT_linkage_name:
   case DW_AT_MIPS_linkage_name:
-break; // mangled =
-   // form_value.AsCString(&dwarf->get_debug_str_data());
-   // break;
+mangled_name_cstr = form_value.AsCString();
+break;
   case DW_AT_type:
 type_die_form = form_value;
 break;
@@ -1498,9 +1498,10 @@
   clang::CXXMethodDecl *cxx_method_decl =
   m_ast.AddMethodToCXXRecordType(
   class_opaque_type.GetOpaqueQualType(),
-  type_name_cstr, clang_type, accessibility,
-  is_virtual, is_static, is_inline, is_explicit,
-  is_attr_used, is_artificial);
+  type_name_cstr, mangled_name_cstr, clang_type,
+  accessibility, is_virtual, is_static,
+  is_inline, is_explicit, is_attr_used,
+  is_artificial);
 
   type_handled = cxx_method_decl != NULL;
 
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp	(revision 318614)
+++ source/Symbol/ClangASTContext.cpp	(working copy)
@@ -7960,7 +7960,7 @@
 }
 
 clang::CXXMethodDecl *ClangASTContext::AddMethodToCXXRecordType(
-lldb::opaque_compiler_type_t type, const char *name,
+lldb::opaque_compiler_type_t type, const char *name, const char *mangled_name,
 const CompilerType &method_clang_type, lldb::AccessType access,
 bool is_virtual, bool is_static, bool is_inline, bool is_explicit,
 bool is_attr_used, bool is_artificial) {
@@ -8080,6 +8080,11 @@
   if (is_attr_used)
 cxx_method_decl->addAttr(clang::UsedAttr::CreateImplicit(*getASTContext()));
 
+  if (mangled_name != NULL) {
+cxx_method_decl->addAttr(
+clang::AsmLabelAttr::CreateImplicit(*getASTContext(), mangled_name));
+  }
+
   // Populate the method decl with parameter decls
 
   llvm::SmallVector params;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo

Re: [Lldb-commits] [PATCH] Use the DWARF linkage name when importing C++ methods

[Nelson Elhage via lldb-commits]

Thanks, created here: https://reviews.llvm.org/D40283

I got here via
http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch ;
Should that documentation be updated? Would a patch be appropriate?

- Nelson

On Mon, Nov 20, 2017 at 7:57 AM Greg Clayton  wrote:

> Please submit a review of this patch through reviews.llvm.org.
>
> - svn diff -x -U9 > /tmp/a.patch
> - login or create a login if you don't already have one
> - Select Differential from the top left corner
> - Click "+ Create Diff" in upper right
> - Click "Choose File" and select /tmp/a.patch, no need to fill anything in
> the Raw Diff field or Repository then click the "Create Diff" button
> - Review your code and click the button to continue
> - Fill in title, description and add the names of reviewers and add
> lldb-commits to the reviewers
>
> On Nov 19, 2017, at 10:33 PM, Nelson Elhage via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
> When importing C++ methods into clang AST nodes from the DWARF symbol
> table, preserve the DW_AT_linkage_name and use it as the linker ("asm")
> name for the symbol.
>
> Concretely, this enables `expression` to call into names that use the GNU
> `abi_tag` extension, and enables lldb to call into code using std::string
> or std::list from recent versions of libstdc++. See
> https://bugs.llvm.org/show_bug.cgi?id=35310 . It also seems broadly more
> robust than relying on the DWARF->clang->codegen pipeline to roundtrip
> properly, but I'm not immediately aware of any other cases in which it
> makes a difference.
>
> - Nelson
>
> ___
> 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

Re: [Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

[Nelson Elhage via lldb-commits]

I can try to take some time to benchmark, hopefully this weekend or so.

I do want to reiterate that this without this change debugging binaries
linked against libstdc++ is badly degraded, since recent libstdc++  uses
ABI tags on ~any methods that touch a std::string.

- Nelson

On Thu, Jan 11, 2018 at 7:18 AM Davide Italiano 
wrote:

> On Thu, Jan 11, 2018 at 2:01 AM, Pavel Labath via lldb-commits
>  wrote:
> > On 10 January 2018 at 22:51, Greg Clayton  wrote:
> >> The right solution seems to be adding some sort of custom GNU ABI tag
> to the DWARF. I know that won't help with existing binaries, but it sounds
> too expensive to set the ASM name for everything.
> >>
> >
> > What makes you think it will be expensive? I don't know much about
> > clang internals, but I think something has to provide a mangled name
> > in any case. So, if we don't specify a mangled name, clang will have
> > to do the mangling itself. For all I know, setting the asm attribute
> > may actually speed things up, as clang could avoid doing some extra
> > work.
> >
> > I think it would be interesting to try to measure the
> > performance/memory footprint impact of just setting the asm attribute
> > everywhere.
> >
>
> Indeed. If somebody has the time to put into this I think it's
> something worth benchmarking before declaring victory (or defeat).
>
> Thanks,
>
> --
> Davide
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits