[Symbolize] Demangle Rust symbols

[tmiasko]

Summary

Extract and reuse demangling functionality common to llvm-cxxfilt,
LLVMSymbolizer::DemangleName and llvm::demangle.

From functional perspective it introduces following changes:

• Supports Rust symbols in LLVMSymbolizer::DemangleName.
• Accepts Rust symbols with an extra underscore in llvm::demangle.

[dblaikie]

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

654–659

Can/should we make this somehow a generic piece of code that could be used here and in llvm-cxxfilt, so it's only written in one place? (eg: there's patches to add D demangling going in at the moment & so would be handy if adding support only had to go in one place rather than 2 or more)

Speaking of the D demangling support - I was hoping it could be broken down into pieces like the Rust work you did (really appreciate that, btw) - I thought the original Rust proposal was a fully featured single patch that then got broken down, am I remembering that correctly? I couldn't find the original single patch version - if you happen to have a pointer to it, I'd appreciate it for comparison/understanding of how things went there.

[tmiasko]

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

654–659

Reusing the llvm::demangle might be an option, but looking back in discussion from D68133, so far code here was intentionally using microsoftDemangle directly with different options. Maybe at least llvm-cxxfilt could use llvm::demangle? But then again llvm-cxxfilt, its not demangling MSVC symbols right now, and I don't have particular insight whether it should and if so how exactly.

The single patch version of Rust demangling support was in D99981.

[dblaikie]

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

654–659

seems OK to me to use a generic demangling for all these tools - but at least llvm-cxxfilt and symbolize here are currently both doing everything except MSVC mangling - so maybe some "unix-y demangle" function would abstract these two callers at least.

Cross referencing to: D110577 - that patch and this one are both adding a case to two places that seem like they should be kept in sync, and I'd like to see that done probably by this patch, so that 110577 doesn't have to touch two places.

[jhenderson]

Nothing to add myself, but +1 to reducing code duplication around this.

There are many other tools that do symbol demangling too (e.g. llvm-objdump, llvm-nm, llvm-readobj). Maybe it should be shared with those too somehow?

[tmiasko]

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

654–659

The function here is handling MSVC mangling, though. Hence, my comment about an earlier attempt to use llvm::demangle in this function.

[dblaikie]

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

654–659

ah, sorry, right, misread. I think it'd be fine/perhaps helpful to add llvm-cxxfilt support for MSVC demangling too (though maybe it's too ambiguous for a general "filter any string starting with '?' to try to demangle it") - but if not, at least the first part of LLVMSymbolizer::DemangleName could defer to a function that wraps up the itanium-ish demanglings over C++, Rust, and pending D support. Returning some "it wasn't mangled in any way we recognize" value, which in this caller would then fallback to the MSVC case.

[tmiasko]

Extract and reuse common demangling functionality.

[tmiasko]

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

654–659

I extracted a common demangling code into a new function.

I tried to limit the functional changes to demangling of Rust symbols only. The
code could be unified a bit further with additional functional changes, but I
would rather not attempt to do this here.

The current function name is non-great and I would welcome suggestions.

[jhenderson]

Why the functional change to allow double underscore Rust symbols?

llvm/lib/Demangle/Demangle.cpp

28

I'm not sure you need the llvm qualifiers, since you're already in the llvm namespace?

31–32

Previously you didn't need this second call. I'm not sure I follow why you need it now?

44

Do we have access to llvm::Optional here? Not sure, as I know there are some layering issues specific to the demangler.

If we do, I think returning an optional string would be better than passing in a reference.

[tmiasko]

Remove unnecessary qualifiers

llvm/lib/Demangle/Demangle.cpp

31–32

The llvm::demangle and llvm-cxxfilt differ in a treatment of the leading underscore. For Itanium mangling llvm-cxxfilt accepts only symbols starting with _Z or ___Z, but has an option to strip an extra underscore first. On the other hand llvm::demangle accepts all variants by default, i.e., not only _Z and __Z but also __Z and ____Z.

To retain the existing behaviour while sharing the code that checks mangling prefixes, the new function expects symbols without an extra underscore. Thus when first attempt at demangling fails, the llvm::demangle tries once again with the leading underscore stripped.

The implementation strategy also subtly changed, since now responsibility for stripping the extra underscore moved from the inside of itaniumDemangle to the outside.

44

The demangle library currently doesn't have any dependencies on other LLVM components. Otherwise the optional string would be my preferred choice here as well.

[tmiasko]

In D110664#3057345, @jhenderson wrote:

Why the functional change to allow double underscore Rust symbols?

To support symbols with extra decoration on Mach-O for code paths that use llvm::demangle and expect it to strip the extra underscore. One example would be llvm-objdump.

[jhenderson]

I think this all looks okay to me, but I'd appreciate @dblaikie confirming too.

[dblaikie]

Looks OK - might be beneficial to split it up into the refactor commit first (pulling out the functionality from llvm::demangle), then the additional functionality (reusing that in LLVMSymbolizer).

[tmiasko]

Thanks for the review.

The patch landed split in parts as:

• rG41a6fc8438f306aa562908027de64061353778ce
• rGa3813438ae1c2d688f7bccc4501a172f17b7a505
• rG48ce523a26b7a5a3dc4cff616c93ed951244746b

llvm/include/llvm/Demangle/Demangle.h

 
 /// Attempt to demangle a string using different demangling schemes.
 /// The function uses heuristics to determine which demangling scheme to use.
 /// \param MangledName - reference to string to demangle.
 /// \returns - the demangled string, or a copy of the input string if no
 /// demangling occurred.
 std::string demangle(const std::string &MangledName);
 
+bool nonMicrosoftDemangle(const char *MangledName, std::string &Result);
+
 /// "Partial" demangler. This supports demangling a string into an AST
 /// (typically an intermediate stage in itaniumDemangle) and querying certain
 /// properties or partially printing the demangled name.
 struct ItaniumPartialDemangler {
   ItaniumPartialDemangler();
 
   ItaniumPartialDemangler(ItaniumPartialDemangler &&Other);
   ItaniumPartialDemangler &operator=(ItaniumPartialDemangler &&Other);

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

StringRef demanglePE32ExternCFunc(StringRef SymbolName) {
 
   return SymbolName;
 }
 
 } // end anonymous namespace
 
 std::string
 LLVMSymbolizer::DemangleName(const std::string &Name,
                              const SymbolizableModule *DbiModuleDescriptor) {
-  // We can spoil names of symbols with C linkage, so use an heuristic
-  // approach to check if the name should be demangled.
-  if (Name.substr(0, 2) == "_Z") {
-    int status = 0;
-    char *DemangledName =
-        itaniumDemangle(Name.c_str(), nullptr, nullptr, &status);
-    if (status != 0)
-      return Name;
-    std::string Result = DemangledName;
-    free(DemangledName);
+  std::string Result;
+  if (nonMicrosoftDemangle(Name.c_str(), Result))
     return Result;
-  }
 
   if (!Name.empty() && Name.front() == '?') {
     // Only do MSVC C++ demangling on symbols starting with '?'.
     int status = 0;
     char *DemangledName = microsoftDemangle(
         Name.c_str(), nullptr, nullptr, nullptr, &status,
         MSDemangleFlags(MSDF_NoAccessSpecifier | MSDF_NoCallingConvention |
                         MSDF_NoMemberType | MSDF_NoReturnType));
     if (status != 0)
       return Name;
-    std::string Result = DemangledName;
+    Result = DemangledName;
     free(DemangledName);
     return Result;
   }
 
   if (DbiModuleDescriptor && DbiModuleDescriptor->isWin32Module())
     return std::string(demanglePE32ExternCFunc(Name));
   return Name;
 }
 
 } // namespace symbolize
 } // namespace llvm

llvm/lib/Demangle/Demangle.cpp

 //===-- Demangle.cpp - Common demangling functions ------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file This file contains definitions of common demangling functions.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Demangle/Demangle.h"
 #include <cstdlib>
+#include <cstring>
 
-static bool isItaniumEncoding(const std::string &MangledName) {
-  size_t Pos = MangledName.find_first_not_of('_');
-  // A valid Itanium encoding requires 1-4 leading underscores, followed by 'Z'.
-  return Pos > 0 && Pos <= 4 && MangledName[Pos] == 'Z';
+static bool isItaniumEncoding(const char *S) {
+  // Itanium encoding requires 1 or 3 leading underscores, followed by 'Z'.
+  return std::strncmp(S, "_Z", 2) == 0 || std::strncmp(S, "___Z", 4) == 0;
 }
 
-static bool isRustEncoding(const std::string &MangledName) {
-  return MangledName.size() >= 2 && MangledName[0] == '_' &&
-         MangledName[1] == 'R';
-}
+static bool isRustEncoding(const char *S) { return S[0] == '_' && S[1] == 'R'; }
 
 std::string llvm::demangle(const std::string &MangledName) {
-  char *Demangled;
+  std::string Result;
+  const char *S = MangledName.c_str();
+
+  if (nonMicrosoftDemangle(S, Result))
+    return Result;
+
+  if (S[0] == '_' && nonMicrosoftDemangle(S + 1, Result))
+    return Result;
+
+  if (char *Demangled =
+          microsoftDemangle(S, nullptr, nullptr, nullptr, nullptr)) {
+    Result = Demangled;
+    std::free(Demangled);
+    return Result;
+  }
+
+  return MangledName;
+}
+
+bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) {
+  char *Demangled = nullptr;
   if (isItaniumEncoding(MangledName))
-    Demangled = itaniumDemangle(MangledName.c_str(), nullptr, nullptr, nullptr);
+    Demangled = itaniumDemangle(MangledName, nullptr, nullptr, nullptr);
   else if (isRustEncoding(MangledName))
-    Demangled = rustDemangle(MangledName.c_str(), nullptr, nullptr, nullptr);
-  else
-    Demangled = microsoftDemangle(MangledName.c_str(), nullptr, nullptr,
-                                  nullptr, nullptr);
+    Demangled = rustDemangle(MangledName, nullptr, nullptr, nullptr);
 
   if (!Demangled)
-    return MangledName;
+    return false;
 
-  std::string Ret = Demangled;
+  Result = Demangled;
   std::free(Demangled);
-  return Ret;
+  return true;
 }

llvm/test/DebugInfo/symbolize-demangling.s

 # REQUIRES: x86-registered-target
 
 # RUN: llvm-mc --filetype=obj --triple=x86_64-pc-linux %s -o %t.o -g
 
-# RUN: llvm-symbolizer --obj=%t.o 0 1 2 | FileCheck %s
+# RUN: llvm-symbolizer --obj=%t.o 0 1 2 3 | FileCheck %s
 
 # CHECK:       f()
-# CHECK-NEXT:  symbolize-demangling.s:20
+# CHECK-NEXT:  symbolize-demangling.s:24
 # CHECK-EMPTY:
 # CHECK-NEXT:  {{^g$}}
-# CHECK-NEXT:  symbolize-demangling.s:22
+# CHECK-NEXT:  symbolize-demangling.s:26
 # CHECK-EMPTY:
 # CHECK-NEXT:  {{^baz$}}
-# CHECK-NEXT:  symbolize-demangling.s:24
+# CHECK-NEXT:  symbolize-demangling.s:28
+# CHECK-EMPTY:
+# CHECK-NEXT:  {{^x::y$}}
+# CHECK-NEXT:  symbolize-demangling.s:30
 
 .type _Z1fv,@function
 .type g,@function
 .type baz,@function
+.type _RNvC1x1y,@function
 _Z1fv:
   nop
 g:
   nop
 baz:
   nop
+_RNvC1x1y:
+  nop

llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp

 static StringRef ToolName;
 
 static void error(const Twine &Message) {
   WithColor::error(errs(), ToolName) << Message << '\n';
   exit(1);
 }
 
 static std::string demangle(const std::string &Mangled) {
-  int Status;
-  std::string Prefix;
-
   const char *DecoratedStr = Mangled.c_str();
   if (StripUnderscore)
     if (DecoratedStr[0] == '_')
       ++DecoratedStr;
-  size_t DecoratedLength = strlen(DecoratedStr);
 
+  std::string Result;
+  if (nonMicrosoftDemangle(DecoratedStr, Result))
+    return Result;
+
+  std::string Prefix;
   char *Undecorated = nullptr;
 
-  if (Types ||
-      ((DecoratedLength >= 2 && strncmp(DecoratedStr, "_Z", 2) == 0) ||
-       (DecoratedLength >= 4 && strncmp(DecoratedStr, "___Z", 4) == 0)))
-    Undecorated = itaniumDemangle(DecoratedStr, nullptr, nullptr, &Status);
+  if (Types)
+    Undecorated = itaniumDemangle(DecoratedStr, nullptr, nullptr, nullptr);
 
-  if (!Undecorated &&
-      (DecoratedLength > 6 && strncmp(DecoratedStr, "__imp_", 6) == 0)) {
+  if (!Undecorated && strncmp(DecoratedStr, "__imp_", 6) == 0) {
     Prefix = "import thunk for ";
-    Undecorated = itaniumDemangle(DecoratedStr + 6, nullptr, nullptr, &Status);
-  }
-
-  if (!Undecorated &&
-      (DecoratedLength >= 2 && strncmp(DecoratedStr, "_R", 2) == 0)) {
-    Undecorated = rustDemangle(DecoratedStr, nullptr, nullptr, &Status);
+    Undecorated = itaniumDemangle(DecoratedStr + 6, nullptr, nullptr, nullptr);
   }
 
-  std::string Result(Undecorated ? Prefix + Undecorated : Mangled);
+  Result = Undecorated ? Prefix + Undecorated : Mangled;
   free(Undecorated);
   return Result;
 }
 
 // Split 'Source' on any character that fails to pass 'IsLegalChar'.  The
 // returned vector consists of pairs where 'first' is the delimited word, and
 // 'second' are the delimiters following that word.
 static void SplitStringDelims(

llvm/unittests/Demangle/DemangleTest.cpp

TEST(Demangle, demangleTest) {
   EXPECT_EQ(demangle("__Z3fooi"), "foo(int)");
   EXPECT_EQ(demangle("___Z3fooi_block_invoke"),
             "invocation function for block in foo(int)");
   EXPECT_EQ(demangle("____Z3fooi_block_invoke"),
             "invocation function for block in foo(int)");
   EXPECT_EQ(demangle("?foo@@YAXH@Z"), "void __cdecl foo(int)");
   EXPECT_EQ(demangle("foo"), "foo");
   EXPECT_EQ(demangle("_RNvC3foo3bar"), "foo::bar");
+  EXPECT_EQ(demangle("__RNvC3foo3bar"), "foo::bar");
 
   // Regression test for demangling of optional template-args for vendor
   // extended type qualifier (https://bugs.llvm.org/show_bug.cgi?id=48009)
   EXPECT_EQ(demangle("_Z3fooILi79EEbU7_ExtIntIXT_EEi"),
             "bool foo<79>(int _ExtInt<79>)");
 }