[Lldb-commits] [PATCH] D27223: Expression evaluation for overloaded C functions (redux)

[Luke Drummond via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL290117: Expression evaluation for overloaded C functions 
(redux) (authored by ldrumm).

Changed prior to commit:
  https://reviews.llvm.org/D27223?vs=81753=81967#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27223

Files:
  lldb/trunk/include/lldb/Core/FastDemangle.h
  lldb/trunk/source/Core/FastDemangle.cpp
  lldb/trunk/source/Expression/IRExecutionUnit.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h

Index: lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -12,6 +12,7 @@
 
 // C Includes
 // C++ Includes
+#include 
 #include 
 
 // Other libraries and framework includes
@@ -93,7 +94,6 @@
   }
 
   std::unique_ptr GetTypeScavenger() override;
-  
   lldb::TypeCategoryImplSP GetFormatters() override;
 
   HardcodedFormatters::HardcodedSummaryFinder GetHardcodedSummaries() override;
@@ -142,6 +142,12 @@
   static uint32_t FindEquivalentNames(ConstString type_name,
   std::vector );
 
+  // Given a mangled function name, calculates some alternative manglings since
+  // the compiler mangling may not line up with the symbol we are expecting
+  static uint32_t
+  FindAlternateFunctionManglings(const ConstString mangled,
+ std::set );
+
   //--
   // PluginInterface protocol
   //--
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -10,17 +10,22 @@
 #include "CPlusPlusLanguage.h"
 
 // C Includes
-// C++ Includes
 #include 
 #include 
+
+// C++ Includes
 #include 
+#include 
 #include 
+#include 
 
 // Other libraries and framework includes
 #include "llvm/ADT/StringRef.h"
 
 // Project includes
 #include "lldb/Core/ConstString.h"
+#include "lldb/Core/FastDemangle.h"
+#include "lldb/Core/Log.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/RegularExpression.h"
 #include "lldb/Core/UniqueCStringMap.h"
@@ -440,6 +445,101 @@
   return count;
 }
 
+/// Given a mangled function `mangled`, replace all the primitive function type
+/// arguments of `search` with type `replace`.
+static ConstString SubsPrimitiveParmItanium(llvm::StringRef mangled,
+llvm::StringRef search,
+llvm::StringRef replace) {
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE);
+
+  const size_t max_len =
+  mangled.size() + mangled.count(search) * replace.size() + 1;
+
+  // Make a temporary buffer to fix up the mangled parameter types and copy the
+  // original there
+  std::string output_buf;
+  output_buf.reserve(max_len);
+  output_buf.insert(0, mangled.str());
+  ptrdiff_t replaced_offset = 0;
+
+  auto swap_parms_hook = [&](const char *parsee) {
+if (!parsee || !*parsee)
+  return;
+
+// Check whether we've found a substitutee
+llvm::StringRef s(parsee);
+if (s.startswith(search)) {
+  // account for the case where a replacement is of a different length to
+  // the original
+  replaced_offset += replace.size() - search.size();
+
+  ptrdiff_t replace_idx = (mangled.size() - s.size()) + replaced_offset;
+  output_buf.erase(replace_idx, search.size());
+  output_buf.insert(replace_idx, replace.str());
+}
+  };
+
+  // FastDemangle will call our hook for each instance of a primitive type,
+  // allowing us to perform substitution
+  const char *const demangled =
+  FastDemangle(mangled.str().c_str(), mangled.size(), swap_parms_hook);
+
+  if (log)
+log->Printf("substituted mangling for %s:{%s} %s:{%s}\n",
+mangled.str().c_str(), demangled, output_buf.c_str(),
+FastDemangle(output_buf.c_str()));
+
+  return output_buf == mangled ? ConstString() : ConstString(output_buf);
+}
+
+uint32_t CPlusPlusLanguage::FindAlternateFunctionManglings(
+const ConstString mangled_name, std::set ) {
+  const auto start_size = alternates.size();
+  /// Get a basic set of alternative manglings for the given symbol `name`, by
+  /// making a few basic possible substitutions on basic types, storage duration
+  /// and `const`ness for the given symbol. The output parameter `alternates`
+  /// is filled with a best-guess, non-exhaustive set of different 

[Lldb-commits] [PATCH] D27223: Expression evaluation for overloaded C functions (redux)

[Greg Clayton via Phabricator via lldb-commits]

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

Looks good.


https://reviews.llvm.org/D27223



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

[Lldb-commits] [PATCH] D27223: Expression evaluation for overloaded C functions (redux)

[Luke Drummond via Phabricator via lldb-commits]

ldrumm added inline comments.



Comment at: include/lldb/Core/FastDemangle.h:22
+char *
+FastDemangle(const char *mangled_name, size_t mangled_name_length,
+ std::function primitive_type_hook = nullptr);

alexshap wrote:
> some thoughts: 
> (also i assume i might be mistaken / missing smth) 
> in ./source/Core/Mangled.cpp at least 3 demanglers are used:
> FastDemangle, itaniumDemangle, abi::__cxa_demangle .
> Adding primitive_type_hook to FastDemangle makes  the interface 
> more complicated & inconsistent (although yes, it's not particularly 
> consistent right now). 
You're right there are a bunch of manglers in use, and this is suboptimal. 
There is recent traction in LLVM to switch to using LLDB's FastDemangler once 
it supports everything; in terms of making the interface messy - you're again 
spot-on. None of the 4-or-so manglers visible to lldb have a symmetric API to 
support things like demangle -> remangle, so this is the least intrusive way I 
could come up with that didn't involve adding a new demangler to the list. I 
did think of several other possible approaches, including using the llvm 
demangler (which has no real public API apart from `llvm::itaniumDemangle` 
where the internal datastructure  (`struct Db`) is not exposed, and modifying 
the llvm demangler to fix a problem in LLDB is going to be a much harder sell.



Comment at: 
packages/Python/lldbsuite/test/expression_command/call-overloaded-c-fuction/main.c:17
+return 0; // break here
+}

alexshap wrote:
> does this patch work when the functions come from a shared library ? 
> (i mean if we have libFoo.so which contains the implementations of 
> get_arg_type(float), get_arg_type(int), and then in lldb we want to call "p 
> get_arg_type(0.12f)")
Yes. The only thing that matters to the current implementation is that we know 
the mangled symbol name.



Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:449
+/// Given a mangled function `mangled`, replace all the primitive function type
+/// arguments of `mangled` with type `replace`.
+static ConstString SubsPrimitiveParmItanium(const char *mangled,

clayborg wrote:
> Don't you mean `search` instead of `mangled` above?
I did. That was a silly mistake.



Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:465-466
+  std::unique_ptr output_buf(new char[max_len]);
+  ::memset(output_buf.get(), '\0', max_len);
+  ::strncpy(output_buf.get(), mangled, max_len - 1);
+

clayborg wrote:
> Remove both lines above if you use a std::string. Also, why zero it first and 
> then copy the string over it?
The reason I set it to zero first was that I didn't know how many replacements 
would be made or how many times `swap_parms_hook` would be called, so this case 
guarantees it's always NUL-terminated. I've switched to std::string as you 
suggested, which conveniently sidesteps the issue :)



Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:471
+
+  auto swap_parms_hook = [&](const char *s) {
+if (!s || !*s)

clayborg wrote:
> Does this only get called with parameters? Or does it get called for 
> everything?
I'm not sure I follow; `s` should always be a valid pointer if that's what you 
mean?



Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:476
+// Check whether we've found a substitutee
+if (::strncmp(s, search, search_len) == 0) {
+  // account for the case where a replacement is of a different length

clayborg wrote:
> Use starts_with StringRef function.
> 
> You also need to verify that there is a word boundary at the end of this. If 
> you are searching for "F", and replacing it with "E" and you get "Foo, Bar) 
> const" in `s` then this will match you will change it to "Eoo, Bar) const" 
> right? So you need to check `s[search.size]` and make sure it is a non 
> identifier character. Unless this function only gets called with identifier 
> boundaries. From my quick look at the FastDemangle changes, it seems that 
> this will get called with the entire remaining part of the string.
This function only gets called when the demangler is trying to resolve a 
builtin type, so it only get called in a context where it's encoding a type, 
not an identifier. I could be wrong, but as far as I can tell, from reading the 
spec for itanium demangling 
(https://mentorembedded.github.io/cxx-abi/abi.html#mangle.builtin-type), and 
the code for the FastDemangler, this should be safe in the case you mentioned.


https://reviews.llvm.org/D27223



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

[Lldb-commits] [PATCH] D27223: Expression evaluation for overloaded C functions (redux)

[Luke Drummond via Phabricator via lldb-commits]

ldrumm updated this revision to Diff 81753.
ldrumm marked 6 inline comments as done.
ldrumm added a comment.

updated to use StringRef-based parser rather than char pointers


https://reviews.llvm.org/D27223

Files:
  include/lldb/Core/FastDemangle.h
  
packages/Python/lldbsuite/test/expression_command/call-overloaded-c-fuction/Makefile
  
packages/Python/lldbsuite/test/expression_command/call-overloaded-c-fuction/TestCallOverloadedCFunction.py
  
packages/Python/lldbsuite/test/expression_command/call-overloaded-c-fuction/main.c
  source/Core/FastDemangle.cpp
  source/Expression/IRExecutionUnit.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h

Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -12,6 +12,7 @@
 
 // C Includes
 // C++ Includes
+#include 
 #include 
 
 // Other libraries and framework includes
@@ -93,7 +94,6 @@
   }
 
   std::unique_ptr GetTypeScavenger() override;
-  
   lldb::TypeCategoryImplSP GetFormatters() override;
 
   HardcodedFormatters::HardcodedSummaryFinder GetHardcodedSummaries() override;
@@ -142,6 +142,12 @@
   static uint32_t FindEquivalentNames(ConstString type_name,
   std::vector );
 
+  // Given a mangled function name, calculates some alternative manglings since
+  // the compiler mangling may not line up with the symbol we are expecting
+  static uint32_t
+  FindAlternateFunctionManglings(const ConstString mangled,
+ std::set );
+
   //--
   // PluginInterface protocol
   //--
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -10,17 +10,22 @@
 #include "CPlusPlusLanguage.h"
 
 // C Includes
-// C++ Includes
 #include 
 #include 
+
+// C++ Includes
 #include 
+#include 
 #include 
+#include 
 
 // Other libraries and framework includes
 #include "llvm/ADT/StringRef.h"
 
 // Project includes
 #include "lldb/Core/ConstString.h"
+#include "lldb/Core/FastDemangle.h"
+#include "lldb/Core/Log.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/RegularExpression.h"
 #include "lldb/Core/UniqueCStringMap.h"
@@ -440,6 +445,101 @@
   return count;
 }
 
+/// Given a mangled function `mangled`, replace all the primitive function type
+/// arguments of `search` with type `replace`.
+static ConstString SubsPrimitiveParmItanium(llvm::StringRef mangled,
+llvm::StringRef search,
+llvm::StringRef replace) {
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE);
+
+  const size_t max_len =
+  mangled.size() + mangled.count(search) * replace.size() + 1;
+
+  // Make a temporary buffer to fix up the mangled parameter types and copy the
+  // original there
+  std::string output_buf;
+  output_buf.reserve(max_len);
+  output_buf.insert(0, mangled.str());
+  ptrdiff_t replaced_offset = 0;
+
+  auto swap_parms_hook = [&](const char *parsee) {
+if (!parsee || !*parsee)
+  return;
+
+// Check whether we've found a substitutee
+llvm::StringRef s(parsee);
+if (s.startswith(search)) {
+  // account for the case where a replacement is of a different length to
+  // the original
+  replaced_offset += replace.size() - search.size();
+
+  ptrdiff_t replace_idx = (mangled.size() - s.size()) + replaced_offset;
+  output_buf.erase(replace_idx, search.size());
+  output_buf.insert(replace_idx, replace.str());
+}
+  };
+
+  // FastDemangle will call our hook for each instance of a primitive type,
+  // allowing us to perform substitution
+  const char *const demangled =
+  FastDemangle(mangled.str().c_str(), mangled.size(), swap_parms_hook);
+
+  if (log)
+log->Printf("substituted mangling for %s:{%s} %s:{%s}\n",
+mangled.str().c_str(), demangled, output_buf.c_str(),
+FastDemangle(output_buf.c_str()));
+
+  return output_buf == mangled ? ConstString() : ConstString(output_buf);
+}
+
+uint32_t CPlusPlusLanguage::FindAlternateFunctionManglings(
+const ConstString mangled_name, std::set ) {
+  const auto start_size = alternates.size();
+  /// Get a basic set of alternative manglings for the given symbol `name`, by
+  /// making a few basic possible substitutions on basic types, storage duration
+  /// and `const`ness for the given symbol. The output parameter `alternates`
+  /// is filled with a 

[Lldb-commits] [PATCH] D27223: Expression evaluation for overloaded C functions (redux)

[Alexander Shaposhnikov via Phabricator via lldb-commits]

alexshap added inline comments.



Comment at: include/lldb/Core/FastDemangle.h:22
+char *
+FastDemangle(const char *mangled_name, size_t mangled_name_length,
+ std::function primitive_type_hook = nullptr);

some thoughts: 
(also i assume i might be mistaken / missing smth) 
in ./source/Core/Mangled.cpp at least 3 demanglers are used:
FastDemangle, itaniumDemangle, abi::__cxa_demangle .
Adding primitive_type_hook to FastDemangle makes  the interface 
more complicated & inconsistent (although yes, it's not particularly consistent 
right now). 



Comment at: 
packages/Python/lldbsuite/test/expression_command/call-overloaded-c-fuction/main.c:17
+return 0; // break here
+}

does this patch work when the functions come from a shared library ? 
(i mean if we have libFoo.so which contains the implementations of 
get_arg_type(float), get_arg_type(int), and then in lldb we want to call "p 
get_arg_type(0.12f)")


https://reviews.llvm.org/D27223



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