Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra updated this revision to Diff 44274.
sivachandra added a comment.

Rebase


http://reviews.llvm.org/D12809

Files:
  include/lldb/Symbol/ClangASTContext.h
  include/lldb/Symbol/CompilerDeclContext.h
  include/lldb/Symbol/GoASTContext.h
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/TypeSystem.h
  include/lldb/Target/CPPLanguageRuntime.h
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.h
  
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
  source/Plugins/SymbolFile/DWARF/DIERef.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/ClangASTContext.cpp
  source/Symbol/CompilerDeclContext.cpp
  source/Symbol/SymbolFile.cpp

Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -134,6 +134,12 @@
 return 0;
 }
 
+void
+SymbolFile::GetMangledNamesForFunction(const std::string _qualified_name, std::vector _names)
+{
+return;
+}
+
 uint32_t
 SymbolFile::FindTypes (const SymbolContext& sc, const ConstString , const CompilerDeclContext *parent_decl_ctx, bool append, uint32_t max_matches, TypeMap& types)
 {
Index: source/Symbol/CompilerDeclContext.cpp
===
--- source/Symbol/CompilerDeclContext.cpp
+++ source/Symbol/CompilerDeclContext.cpp
@@ -38,6 +38,15 @@
 return ConstString();
 }
 
+ConstString
+CompilerDeclContext::GetScopeQualifiedName () const
+{
+if (IsValid())
+return m_type_system->DeclContextGetScopeQualifiedName(m_opaque_decl_ctx);
+else
+return ConstString();
+}
+
 bool
 CompilerDeclContext::IsStructUnionOrClass () const
 {
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -9969,6 +9969,18 @@
 return ConstString();
 }
 
+ConstString
+ClangASTContext::DeclContextGetScopeQualifiedName (void *opaque_decl_ctx)
+{
+if (opaque_decl_ctx)
+{
+clang::NamedDecl *named_decl = llvm::dyn_cast((clang::DeclContext *)opaque_decl_ctx);
+if (named_decl)
+return ConstString(llvm::StringRef(named_decl->getQualifiedNameAsString()));
+}
+return ConstString();
+}
+
 bool
 ClangASTContext::DeclContextIsClassMethod (void *opaque_decl_ctx,
lldb::LanguageType *language_ptr,
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -208,6 +208,10 @@
bool append,
lldb_private::SymbolContextList& sc_list) override;
 
+void
+GetMangledNamesForFunction (const std::string _qualified_name,
+std::vector _names) override;
+
 uint32_t
 FindTypes (const lldb_private::SymbolContext& sc,
const lldb_private::ConstString ,
@@ -577,6 +581,9 @@
 m_fetched_external_modules:1;
 lldb_private::LazyBool  m_supports_DW_AT_APPLE_objc_complete_type;
 
+typedef std::shared_ptr DIERefSetSP;
+typedef std::unordered_map NameToOffsetMap;
+NameToOffsetMap m_function_scope_qualified_name_map;
 std::unique_ptr m_ranges;
 UniqueDWARFASTTypeMap m_unique_ast_type_map;
 DIEToTypePtr m_die_to_type;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2927,6 +2927,40 @@
 return sc_list.GetSize() - original_size;
 }
 
+void
+SymbolFileDWARF::GetMangledNamesForFunction (const std::string _qualified_name,
+ std::vector _names)
+{
+DWARFDebugInfo* info = DebugInfo();
+uint32_t num_comp_units = 0;
+if (info)
+num_comp_units = info->GetNumCompileUnits();
+
+for (uint32_t i = 0; i < num_comp_units; i++)
+{
+DWARFCompileUnit *cu = info->GetCompileUnitAtIndex(i);
+if (cu == nullptr)
+continue;
+
+SymbolFileDWARFDwo *dwo = cu->GetDwoSymbolFile();
+if (dwo)
+dwo->GetMangledNamesForFunction(scope_qualified_name, mangled_names);
+}
+
+

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added a comment.

Thanks a lot Dwan Perchik, for persisting on my behalf.

And thanks to Sean and Greg for taking a look.

I second Dwan's earlier comment about Greg's code reviews. He has been the best 
reviewer I have worked with, internally and externally.

Will commit this change shortly after rebasing.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Dawn Perchik via lldb-commits]

dawn added a comment.

@spyffe I think this review is waiting on you since you had previously rejected 
it.  Can you please have a look?  It continues to show up on my "Blocking 
Others" list and there's nothing more I can do on my side.  Please review or 
resign out of curtesy to sivachandra???  Thanks in advance.

@sivachandra If spyffe (or anyone for that matter) doesn't respond in a 
reasonable time frame, I would say it's safe to assume he is inactive and you 
can go ahead and commit based on my acceptance.  This patch has been on review 
for months - you've given folks more than enough time to voice any objections.  
That said, it would be nice to get at least one other person's eyes on this.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Greg Clayton via lldb-commits]

clayborg resigned from this revision.
clayborg removed a reviewer: clayborg.
clayborg added a comment.

I looked at this before, and resigned to let Sean Callanan comment on the fix 
since this is in his area of expertise: the expression parser.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Dawn Perchik via lldb-commits]

dawn added a reviewer: clayborg.
dawn added a subscriber: clayborg.
dawn added a comment.

Adding Greg - he is an excellent reviewer and quite responsive (poor guy - ends 
up with way more than his fair share of reviews as a result :( ).

@clayborg would you be willing to be a 2nd pair of eyes here?


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Sean Callanan via lldb-commits]

spyffe accepted this revision.
spyffe added a comment.
This revision is now accepted and ready to land.

The expression parser side fixes look fine to me, and they remove some hacks 
and make things slightly more elegant.
Obviously the ideal would be to have GCC generate full debug info, but to me 
this patch looks like a fine next-best thing.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Dawn Perchik via lldb-commits]

dawn added a comment.

In http://reviews.llvm.org/D12809#301752, @sivachandra wrote:

> @dawn: Thanks for accepting the patch. I guess I still need to wait for 
> sign-off/comments from a CODE_OWNER.


Yeah.  Maybe add more proactive reviewers?   And definitely be a squeaky wheel 
about it.  Keep adding reviewers and spamming them until someone gets irritated 
enough to have a look :)


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Dawn Perchik via lldb-commits]

dawn accepted this revision.
dawn added a comment.

lgtm.  Patch was applied locally and tested with no regressions.  Thanks for 
fixing the patch.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added a comment.

@dawn: Thanks for accepting the patch. I guess I still need to wait for 
sign-off/comments from a CODE_OWNER.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra updated this revision to Diff 41706.
sivachandra added a comment.

Rebased and updated to take in DWO changes.


http://reviews.llvm.org/D12809

Files:
  include/lldb/Symbol/ClangASTContext.h
  include/lldb/Symbol/CompilerDeclContext.h
  include/lldb/Symbol/GoASTContext.h
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/TypeSystem.h
  include/lldb/Target/CPPLanguageRuntime.h
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.h
  
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
  source/Plugins/SymbolFile/DWARF/DIERef.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/ClangASTContext.cpp
  source/Symbol/CompilerDeclContext.cpp
  source/Symbol/SymbolFile.cpp

Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -134,6 +134,12 @@
 return 0;
 }
 
+void
+SymbolFile::GetMangledNamesForFunction(const std::string _qualified_name, std::vector _names)
+{
+return;
+}
+
 uint32_t
 SymbolFile::FindTypes (const SymbolContext& sc, const ConstString , const CompilerDeclContext *parent_decl_ctx, bool append, uint32_t max_matches, TypeMap& types)
 {
Index: source/Symbol/CompilerDeclContext.cpp
===
--- source/Symbol/CompilerDeclContext.cpp
+++ source/Symbol/CompilerDeclContext.cpp
@@ -42,6 +42,15 @@
 return ConstString();
 }
 
+ConstString
+CompilerDeclContext::GetScopeQualifiedName () const
+{
+if (IsValid())
+return m_type_system->DeclContextGetScopeQualifiedName(m_opaque_decl_ctx);
+else
+return ConstString();
+}
+
 bool
 CompilerDeclContext::IsStructUnionOrClass () const
 {
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -9245,6 +9245,18 @@
 return ConstString();
 }
 
+ConstString
+ClangASTContext::DeclContextGetScopeQualifiedName (void *opaque_decl_ctx)
+{
+if (opaque_decl_ctx)
+{
+clang::NamedDecl *named_decl = llvm::dyn_cast((clang::DeclContext *)opaque_decl_ctx);
+if (named_decl)
+return ConstString(llvm::StringRef(named_decl->getQualifiedNameAsString()));
+}
+return ConstString();
+}
+
 bool
 ClangASTContext::DeclContextIsClassMethod (void *opaque_decl_ctx,
lldb::LanguageType *language_ptr,
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 // Other libraries and framework includes
@@ -203,6 +204,10 @@
bool append,
lldb_private::SymbolContextList& sc_list) override;
 
+void
+GetMangledNamesForFunction (const std::string _qualified_name,
+std::vector _names) override;
+
 uint32_t
 FindTypes (const lldb_private::SymbolContext& sc,
const lldb_private::ConstString ,
@@ -560,6 +565,9 @@
 m_fetched_external_modules:1;
 lldb_private::LazyBool  m_supports_DW_AT_APPLE_objc_complete_type;
 
+typedef std::shared_ptr DIERefSetSP;
+typedef std::unordered_map NameToOffsetMap;
+NameToOffsetMap m_function_scope_qualified_name_map;
 std::unique_ptr m_ranges;
 UniqueDWARFASTTypeMap m_unique_ast_type_map;
 DIEToTypePtr m_die_to_type;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2856,6 +2856,40 @@
 return sc_list.GetSize() - original_size;
 }
 
+void
+SymbolFileDWARF::GetMangledNamesForFunction (const std::string _qualified_name,
+ std::vector _names)
+{
+DWARFDebugInfo* info = DebugInfo();
+uint32_t num_comp_units = 0;
+if (info)
+num_comp_units = info->GetNumCompileUnits();
+
+for (uint32_t i = 0; i < num_comp_units; i++)
+{
+DWARFCompileUnit *cu = info->GetCompileUnitAtIndex(i);
+if (cu == nullptr)
+continue;
+
+SymbolFileDWARFDwo 

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added a comment.

A test for this already exists: TestCallStdStringFunction.py


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added a comment.

PTaL
I finally got time to work on this. I have now rebased it and updated it take 
in DWO changes.

I understand this change only enhances debug experience when GCC is used as the 
DWARF producer (the targeted functionality already works as expected when the 
producer is clang). However, GCC is still very important for Android 
development and hence this fix is very useful for us.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Dawn Perchik via lldb-commits]

dawn added a subscriber: dawn.
dawn added a reviewer: evgeny777.
dawn added a comment.

Eugene has been doing some work in this area - perhaps he can accept this patch 
for you.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Dawn Perchik via lldb-commits]

dawn requested changes to this revision.
dawn added a reviewer: dawn.
dawn added a comment.
This revision now requires changes to proceed.

1. Please rebase again - this patch no longer applies cleanly, and fails to 
build after fixing merge conflicts.
2. Please add tests.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Greg Clayton via lldb-commits]

clayborg resigned from this revision.
clayborg edited reviewers, added: jingham; removed: clayborg.
clayborg added a comment.

I will get Sean to take a look.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added a comment.

Ping. I have few follow up changes over whatever goes in here. Would greatly 
appreciate if someone can make a decision on this, or advice on how to take 
this forward.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra updated this revision to Diff 36818.
sivachandra added a comment.

Rebase.


http://reviews.llvm.org/D12809

Files:
  include/lldb/Symbol/ClangASTContext.h
  include/lldb/Symbol/CompilerDeclContext.h
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/TypeSystem.h
  include/lldb/Target/CPPLanguageRuntime.h
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.h
  source/Plugins/SymbolFile/DWARF/DIERef.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/ClangASTContext.cpp
  source/Symbol/CompilerDeclContext.cpp
  source/Symbol/SymbolFile.cpp

Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -134,6 +134,12 @@
 return 0;
 }
 
+void
+SymbolFile::GetMangledNamesForFunction(const std::string _qualified_name, std::vector _names)
+{
+return;
+}
+
 uint32_t
 SymbolFile::FindTypes (const SymbolContext& sc, const ConstString , const CompilerDeclContext *parent_decl_ctx, bool append, uint32_t max_matches, TypeList& types)
 {
Index: source/Symbol/CompilerDeclContext.cpp
===
--- source/Symbol/CompilerDeclContext.cpp
+++ source/Symbol/CompilerDeclContext.cpp
@@ -42,6 +42,15 @@
 return ConstString();
 }
 
+ConstString
+CompilerDeclContext::GetScopeQualifiedName () const
+{
+if (IsValid())
+return m_type_system->DeclContextGetScopeQualifiedName(m_opaque_decl_ctx);
+else
+return ConstString();
+}
+
 bool
 CompilerDeclContext::IsStructUnionOrClass () const
 {
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -9050,6 +9050,18 @@
 return ConstString();
 }
 
+ConstString
+ClangASTContext::DeclContextGetScopeQualifiedName (void *opaque_decl_ctx)
+{
+if (opaque_decl_ctx)
+{
+clang::NamedDecl *named_decl = llvm::dyn_cast((clang::DeclContext *)opaque_decl_ctx);
+if (named_decl)
+return ConstString(llvm::StringRef(named_decl->getQualifiedNameAsString()));
+}
+return ConstString();
+}
+
 bool
 ClangASTContext::DeclContextIsClassMethod (void *opaque_decl_ctx,
lldb::LanguageType *language_ptr,
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 // Other libraries and framework includes
@@ -201,6 +202,10 @@
bool append,
lldb_private::SymbolContextList& sc_list) override;
 
+void
+GetMangledNamesForFunction (const std::string _qualified_name,
+std::vector _names) override;
+
 uint32_t
 FindTypes (const lldb_private::SymbolContext& sc,
const lldb_private::ConstString ,
@@ -583,6 +588,9 @@
 m_fetched_external_modules:1;
 lldb_private::LazyBool  m_supports_DW_AT_APPLE_objc_complete_type;
 
+typedef std::shared_ptr DIERefSetSP;
+typedef std::unordered_map NameToOffsetMap;
+NameToOffsetMap m_function_scope_qualified_name_map;
 std::unique_ptr m_ranges;
 UniqueDWARFASTTypeMap m_unique_ast_type_map;
 DIEToTypePtr m_die_to_type;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2834,6 +2834,24 @@
 return sc_list.GetSize() - original_size;
 }
 
+void
+SymbolFileDWARF::GetMangledNamesForFunction (const std::string _qualified_name,
+ std::vector _names)
+{
+NameToOffsetMap::iterator iter = m_function_scope_qualified_name_map.find(scope_qualified_name);
+if (iter == m_function_scope_qualified_name_map.end())
+return;
+
+DIERefSetSP set_sp = (*iter).second;
+std::set::iterator set_iter;
+for (set_iter = set_sp->begin(); set_iter != set_sp->end(); set_iter++)
+{
+DWARFDIE die = DebugInfo()->GetDIE (*set_iter);
+mangled_names.push_back(ConstString(die.GetMangledName()));
+}
+}
+
+
 uint32_t
 

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added a comment.

Friendly periodic ping. Let me know if once in 2 days is too frequent.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added a comment.

Ping.
The patch in its current state will require a rebase. However, any suggestions 
on how to take this forward would be great. I can rebase and make modifications 
together.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added a comment.

Ping. Any suggestion on how to take this forward?


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added inline comments.


Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:200
@@ -198,1 +199,3 @@
 
+void
+GetMangledNamesForFunction (const std::string _qualified_name,

sivachandra wrote:
> spyffe wrote:
> > Why is this attached to the DWARF?  I would want to attach this to the 
> > ClangExpressionDeclMap because we identify these alternate names during 
> > function name lookup, and we just need to remember them when resolving the 
> > references in IR.  After that, they are no longer needed.
> My thinking was, DWARF is the only thing which knows about the correct 
> mangled name, so keep it close to the code dealing with DWARF. Your 
> suggestion also makes sense, but might (I have not yet thought enough about 
> it) require us to expose DIE info into ClangExpressionDeclMap. I will think 
> more about this approach and get back to you.
I spent some time thinking about this. ClandExpressionDeclMap doesn't really 
explicitly lookup method names. If we have an expression like "v.size()", we 
lookup what "v" is, and that conveys to Clang about the existence of a method 
"size" in its class. The requirement for alternate names kicks in (so to say) 
when we are looking for the address of the method. I am not very clear on how 
we can cleanly keep track of all the methods parsed, while looking up 
variables, in ClangExpressionDeclMap and use that knowledge while looking up 
addresses. Do you have any suggestions?

I agree that it is indeed odd to have a method GetMangledNamesForFunction in 
SymbolFile which is useful only for expression evaluation. How about having a 
temp object that ClangExpressionDeclMap registers with SymbolFile, and cleans 
it up after expression evaluation is done? SymbolFile stuffs in to the object 
all info that ClangExpressionDeclMap could potentially use while parsing the 
DIEs. ExpressionEvaluationIndex?


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Sean Callanan via lldb-commits]

spyffe requested changes to this revision.
spyffe added a comment.

I LOVE the idea of getting rid of those horrid "alternate manglings."  We knew 
what the mangling was during name lookup, we should be able to recognize them 
later!
As listed in my inline comments, I have some concerns about the scope.  This 
knowledge is built up during expression parsing and used during expression 
parsing – we're done.
Thanks for working on this, Siva.



Comment at: source/Expression/ClangExpressionDeclMap.cpp:661
@@ -603,4 +660,3 @@
 {
-FindCodeSymbolInContext(
-demangled, m_parser_vars->m_sym_ctx, 
eFunctionNameTypeFull, sc_list);
-sc_list_size = sc_list.GetSize();
+ConstString best_alternate_mangled_name = 
FindBestAlternateMangledName(demangled, lang_type, sc);
+if (best_alternate_mangled_name)

This should definitely only be done if we can't find the name the original way. 
 I'm always happy to pay extra runtime to fix an expression that would 
otherwise not work – but expressions that would work (the >90% case) shouldn't 
be paying for this.


Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:200
@@ -198,1 +199,3 @@
 
+void
+GetMangledNamesForFunction (const std::string _qualified_name,

Why is this attached to the DWARF?  I would want to attach this to the 
ClangExpressionDeclMap because we identify these alternate names during 
function name lookup, and we just need to remember them when resolving the 
references in IR.  After that, they are no longer needed.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added inline comments.


Comment at: source/Expression/ClangExpressionDeclMap.cpp:661
@@ -603,4 +660,3 @@
 {
-FindCodeSymbolInContext(
-demangled, m_parser_vars->m_sym_ctx, 
eFunctionNameTypeFull, sc_list);
-sc_list_size = sc_list.GetSize();
+ConstString best_alternate_mangled_name = 
FindBestAlternateMangledName(demangled, lang_type, sc);
+if (best_alternate_mangled_name)

spyffe wrote:
> This should definitely only be done if we can't find the name the original 
> way.  I'm always happy to pay extra runtime to fix an expression that would 
> otherwise not work – but expressions that would work (the >90% case) 
> shouldn't be paying for this.
The "original" way is still attempted first at line 643 above. Lines 669 to 674 
below take care of another problem: http://reviews.llvm.org/D12613. Since that 
problem is much more a rare case than that solved in this change, I chose to 
keep this "try" before that "try".


Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:200
@@ -198,1 +199,3 @@
 
+void
+GetMangledNamesForFunction (const std::string _qualified_name,

spyffe wrote:
> Why is this attached to the DWARF?  I would want to attach this to the 
> ClangExpressionDeclMap because we identify these alternate names during 
> function name lookup, and we just need to remember them when resolving the 
> references in IR.  After that, they are no longer needed.
My thinking was, DWARF is the only thing which knows about the correct mangled 
name, so keep it close to the code dealing with DWARF. Your suggestion also 
makes sense, but might (I have not yet thought enough about it) require us to 
expose DIE info into ClangExpressionDeclMap. I will think more about this 
approach and get back to you.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

So let me try to understand. When we are asked during expressions to lookup 
some mangled named for "std::string::length()", it doesn't exist in GCC 
binaries. So we want to then find any alternate manglings and we do this by 
asking the symbol file to find alternate manglings for "std::string::length"? 
I.E. we remove the parens and any arguments and lookup just the fully qualified 
function name?

If so, we should just lookup functions using:

  size_t
  Module::FindFunctions (const ConstString ,
 const CompilerDeclContext *parent_decl_ctx,
 uint32_t name_type_mask, 
 bool symbols_ok,
 bool inlines_ok,
 bool append, 
 SymbolContextList& sc_list);

And then look at all of the symbol contexts in sc_list and find a function that 
we want? I don't really see the need for any of the new 
SymbolFile::GetMangledNamesForFunction() functions.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra added a comment.

I could very well be missing something obvious. However, let me explain what I 
am trying to solve here. Lets take the example of std::vector::size 
method. The DWARF we get when compiled with GCC is as follows:

  < 3><0x20a3>DW_TAG_subprogram
DW_AT_external  yes(1)
DW_AT_name  "size"
DW_AT_decl_file 0x0003 
/usr/include/c++/4.8/bits/stl_vector.h
DW_AT_decl_line 0x0285
DW_AT_linkage_name  
"_ZNKSt6vectorISsSaISsEE4sizeEv"
DW_AT_type  <0x1eb1>
DW_AT_accessibility DW_ACCESS_public
DW_AT_declaration   yes(1)
DW_AT_object_pointer<0x20bc>
DW_AT_sibling   <0x20c2>

If we demangle the linkage name from here, we get "std::vector::size() const". So, the m_function_fullname_index 
of SymbolFileDWARF will have entry for this function with this full name.

However, due to missing debug info elsewhere, the IR generated by clang (the 
LLDB compiler), generates a mangled name like this:
"_ZNKSt6vectorISbIcSt17char_traitsSt15allocatorESt82allocator >E4sizeEv"

This demangles to
"std::vector, std::allocator > >::size() const"

Since neither the clang generated mangled name is present in the ELF symtab, 
and nor its corresponding demangled name is not present in any of the DWARF 
indices, the existing FindFunctions will not be helpful. Also, only functions 
with debug info (those which have an address specified in the DWARF) are 
indexed.

What I am doing in my change is to use the fact that all methods (and their 
types) are grokked while creating the AST for clang (the LLDB compiler). So, 
when a method is grokked, store a map from its scoped name to its DIE. Even if 
there were any discrepancies in the mangled name in the debug info versus that 
generated by the LLDB compiler, the fully scoped names should be the same. In 
which case, use the fully scoped name to get to the DIE and retrieve its 
"actual" mangled name.


http://reviews.llvm.org/D12809



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D12809: Better scheme to lookup alternate mangled name when looking up function address.

[Siva Chandra via lldb-commits]

sivachandra created this revision.
sivachandra added reviewers: clayborg, spyffe.
sivachandra added a subscriber: lldb-commits.

This change is relevant for inferiors compiled with GCC. GCC does not
emit complete debug info std::basic_string<...>, and consequently, Clang
(the LLDB compiler) does not generate correct mangled names for certain
functions.

This change removes the hard-coded alternate names in
ItaniumABILanguageRuntime.cpp.

Before the hard-coded names were put in ItaniumABILanguageRuntime.cpp, one could
not evaluate std::string methods (ex. std::string::length). After putting in
the hard-coded names, one could evaluate them. However, it did not still
enable one to call methods on, say for example, std::vector.
This change makes that possible.

There is some amount of incompleteness in this change. Consider the
following example:

std::string hello("hello"), world("world");
std::map m;
m[hello] = world;

One can still not evaluate the expression "m[hello]" in LLDB. Will
address this issue in another pass.

http://reviews.llvm.org/D12809

Files:
  include/lldb/Symbol/ClangASTContext.h
  include/lldb/Symbol/CompilerDeclContext.h
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/TypeSystem.h
  include/lldb/Target/CPPLanguageRuntime.h
  source/Expression/ClangExpressionDeclMap.cpp
  source/Expression/IRForTarget.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.h
  source/Plugins/SymbolFile/DWARF/DIERef.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/ClangASTContext.cpp
  source/Symbol/CompilerDeclContext.cpp
  source/Symbol/SymbolFile.cpp

Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -136,6 +136,12 @@
 return 0;
 }
 
+void
+SymbolFile::GetMangledNamesForFunction(const std::string _qualified_name, std::vector _names)
+{
+return;
+}
+
 uint32_t
 SymbolFile::FindTypes (const SymbolContext& sc, const ConstString , const CompilerDeclContext *parent_decl_ctx, bool append, uint32_t max_matches, TypeList& types)
 {
Index: source/Symbol/CompilerDeclContext.cpp
===
--- source/Symbol/CompilerDeclContext.cpp
+++ source/Symbol/CompilerDeclContext.cpp
@@ -27,6 +27,15 @@
 return ConstString();
 }
 
+ConstString
+CompilerDeclContext::GetScopeQualifiedName () const
+{
+if (IsValid())
+return m_type_system->DeclContextGetScopeQualifiedName(m_opaque_decl_ctx);
+else
+return ConstString();
+}
+
 bool
 CompilerDeclContext::IsStructUnionOrClass () const
 {
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -8836,6 +8836,18 @@
 return ConstString();
 }
 
+ConstString
+ClangASTContext::DeclContextGetScopeQualifiedName (void *opaque_decl_ctx)
+{
+if (opaque_decl_ctx)
+{
+clang::NamedDecl *named_decl = llvm::dyn_cast((clang::DeclContext *)opaque_decl_ctx);
+if (named_decl)
+return ConstString(llvm::StringRef(named_decl->getQualifiedNameAsString()));
+}
+return ConstString();
+}
+
 bool
 ClangASTContext::DeclContextIsClassMethod (void *opaque_decl_ctx,
lldb::LanguageType *language_ptr,
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 // Other libraries and framework includes
@@ -196,6 +197,10 @@
bool append,
lldb_private::SymbolContextList& sc_list) override;
 
+void
+GetMangledNamesForFunction (const std::string _qualified_name,
+std::vector _names) override;
+
 uint32_t
 FindTypes (const lldb_private::SymbolContext& sc,
const lldb_private::ConstString ,
@@ -578,6 +583,9 @@
 m_fetched_external_modules:1;
 lldb_private::LazyBool  m_supports_DW_AT_APPLE_objc_complete_type;
 
+typedef std::shared_ptr DIERefSetSP;
+typedef std::unordered_map NameToOffsetMap;
+NameToOffsetMap m_function_scope_qualified_name_map;
 std::unique_ptr m_ranges;
 UniqueDWARFASTTypeMap m_unique_ast_type_map;
 DIEToTypePtr m_die_to_type;
Index: