[WIP] [lldb] change name demangling to be consistent between windows and linx

[lassefolger]

Summary

When printing names in lldb on windows these names contain the full type information while on linux only the name is contained.

This change introduces a flag in the Microsoft demangler to control if the type information should be included.
With the flag enabled demangled name contains only the qualified name, e.g:
without flag -> with flag
int (*array2d)[10] -> array2d
int (*abc::array2d)[10] -> abc::array2d
const int *x -> x

For globals there is a second inconsistency which is not yet addressed by this change. On linux globals (in global namespace) are prefixed with :: while on windows they are not.

[teemperor]

This looks in general good to me. You probably want to add a test for this (llvm/test/Demangle/ms-options.test seems like the right place). I added some folks to review the Demangler changes.

The LLDB itself LGTM, but it seems like this patch is also clang-formatting some lines that are unrelated to this patch, so please just clang-format those files as a preparatory NFC commit (If you don't have commit access then @werat or me can also just do that quickly for you). FWIW, there is a git clang-format script that limits clang-format changes to just the lines you touched.

[mstorsjo]

The demangler change looks fine to me (but a test would indeed be necessary), but I think @thakis is the one who's been most involved with the MS demangler.

[werat]

Thanks for improving this! Can you please add a few examples in the commit description like "before -> after", so it's more obvious what is changing here.

[lassefolger]

add tests for the change and adjust description to include examples

[lassefolger]

created a NFC for formatting: https://reviews.llvm.org/D111934

[thakis]

I don't have an opinion on this change and I don't mind the demangler change, but isn't the type information helpful? The mangled itanium name doesn't include type information which is why it's not printed, but the mangled ms name does include it.

But as I said, I don't have an opinion either way.

[teemperor]

In D111715#3068412, @thakis wrote:

I don't have an opinion on this change and I don't mind the demangler change, but isn't the type information helpful? The mangled itanium name doesn't include type information which is why it's not printed, but the mangled ms name does include it.

But as I said, I don't have an opinion either way.

I think the motivation for this patch is just that LLDB uses the demangler to determine variable names and with the current MS demangler, the variable names returned by LLDB are for example int (*array2d)[10] on Windows instead of array2d (on platforms that use the itanium demangler). And that's clearly not what any user expects when they ask for the name of a variable behind a symbol. Note that type information in LLDB is a separate part of the information, so having the type information in the name gives the user no additional information.

[lassefolger]

rebased after NFC

[rnk]

lgtm

This flag seems reasonable. Microsoft's UnDecorateName API has quite a few of these kinds of things:
https://docs.microsoft.com/en-us/windows/win32/api/dbghelp/nf-dbghelp-undecoratesymbolname

I don't think it makes sense to implement them all, but clearly, people have use cases for this kind of stuff. I think "name only" might be a useful addition at some point.

lldb/source/Core/Mangled.cpp

if (name) {
     m_mangled.Clear();
   }
 }
 
 // Local helpers for different demangling implementations.
 static char *GetMSVCDemangledStr(const char *M) {
   char *demangled_cstr = llvm::microsoftDemangle(
       M, nullptr, nullptr, nullptr, nullptr,
-      llvm::MSDemangleFlags(llvm::MSDF_NoAccessSpecifier |
-                            llvm::MSDF_NoCallingConvention |
-                            llvm::MSDF_NoMemberType));
+      llvm::MSDemangleFlags(
+          llvm::MSDF_NoAccessSpecifier | llvm::MSDF_NoCallingConvention |
+          llvm::MSDF_NoMemberType | llvm::MSDF_NoVariableType));
 
   if (Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DEMANGLE)) {
     if (demangled_cstr && demangled_cstr[0])
       LLDB_LOGF(log, "demangled msvc: %s -> \"%s\"", M, demangled_cstr);
     else
       LLDB_LOGF(log, "demangled msvc: %s -> error", M);
   }
 

llvm/include/llvm/Demangle/Demangle.h

 
 enum MSDemangleFlags {
   MSDF_None = 0,
   MSDF_DumpBackrefs = 1 << 0,
   MSDF_NoAccessSpecifier = 1 << 1,
   MSDF_NoCallingConvention = 1 << 2,
   MSDF_NoReturnType = 1 << 3,
   MSDF_NoMemberType = 1 << 4,
+  MSDF_NoVariableType = 1 << 5,
 };
 
 /// Demangles the Microsoft symbol pointed at by mangled_name and returns it.
 /// Returns a pointer to the start of a null-terminated demangled string on
 /// success, or nullptr on error.
 /// If n_read is non-null and demangling was successful, it receives how many
 /// bytes of the input string were consumed.
 /// buf can point to a *n_buf bytes large buffer where the demangled name is

llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h

 
 enum OutputFlags {
   OF_Default = 0,
   OF_NoCallingConvention = 1,
   OF_NoTagSpecifier = 2,
   OF_NoAccessSpecifier = 4,
   OF_NoMemberType = 8,
   OF_NoReturnType = 16,
+  OF_NoVariableType = 32,
 };
 
 // Types
 enum class PrimitiveKind {
   Void,
   Bool,
   Char,
   Schar,

llvm/lib/Demangle/MicrosoftDemangle.cpp

char *llvm::microsoftDemangle(const char *MangledName, size_t *NMangled,
   if (Flags & MSDF_NoCallingConvention)
     OF = OutputFlags(OF | OF_NoCallingConvention);
   if (Flags & MSDF_NoAccessSpecifier)
     OF = OutputFlags(OF | OF_NoAccessSpecifier);
   if (Flags & MSDF_NoReturnType)
     OF = OutputFlags(OF | OF_NoReturnType);
   if (Flags & MSDF_NoMemberType)
     OF = OutputFlags(OF | OF_NoMemberType);
+  if (Flags & MSDF_NoVariableType)
+    OF = OutputFlags(OF | OF_NoVariableType);
 
   int InternalStatus = demangle_success;
   if (D.Error)
     InternalStatus = demangle_invalid_mangled_name;
   else if (!initializeOutputStream(Buf, N, S, 1024))
     InternalStatus = demangle_memory_alloc_failure;
   else {
     AST->output(S, OF);

llvm/lib/Demangle/MicrosoftDemangleNodes.cpp

default:
     IsStatic = false;
     break;
   }
   if (!(Flags & OF_NoAccessSpecifier) && AccessSpec)
     OS << AccessSpec << ": ";
   if (!(Flags & OF_NoMemberType) && IsStatic)
     OS << "static ";
 
-  if (Type) {
+  if (!(Flags & OF_NoVariableType) && Type) {
     Type->outputPre(OS, Flags);
     outputSpaceIfNecessary(OS);
   }
   Name->output(OS, Flags);
-  if (Type)
+  if (!(Flags & OF_NoVariableType) && Type)
     Type->outputPost(OS, Flags);
 }
 
 void CustomTypeNode::outputPre(OutputStream &OS, OutputFlags Flags) const {
   Identifier->output(OS, Flags);
 }
 void CustomTypeNode::outputPost(OutputStream &OS, OutputFlags Flags) const {}
 

llvm/test/Demangle/ms-options.test

-; RUN: llvm-undname < %s | FileCheck %s
-; RUN: llvm-undname --no-calling-convention < %s | FileCheck %s --check-prefix=CHECK-NO-CALLING-CONV
-; RUN: llvm-undname --no-return-type < %s | FileCheck %s --check-prefix=CHECK-NO-RETURN
-; RUN: llvm-undname --no-access-specifier < %s | FileCheck %s --check-prefix=CHECK-NO-ACCESS
-; RUN: llvm-undname --no-member-type < %s | FileCheck %s --check-prefix=CHECK-NO-MEMBER-TYPE
-; RUN: llvm-undname --no-calling-convention --no-return-type --no-access-specifier --no-member-type < %s | FileCheck %s --check-prefix=CHECK-NO-ALL
-
-?func@MyClass@@UEAAHHH@Z
-; CHECK: public: virtual int __cdecl MyClass::func(int, int)
-; CHECK-NO-CALLING-CONV: public: virtual int MyClass::func(int, int)
-; CHECK-NO-RETURN: public: virtual __cdecl MyClass::func(int, int)
-; CHECK-NO-ACCESS: {{^}}virtual int __cdecl MyClass::func(int, int)
-; CHECK-NO-MEMBER-TYPE: public: int __cdecl MyClass::func(int, int)
-; CHECK-NO-ALL: {{^}}MyClass::func(int, int)
+; RUN: llvm-undname < %s | FileCheck %s
+; RUN: llvm-undname --no-calling-convention < %s | FileCheck %s --check-prefix=CHECK-NO-CALLING-CONV
+; RUN: llvm-undname --no-return-type < %s | FileCheck %s --check-prefix=CHECK-NO-RETURN
+; RUN: llvm-undname --no-access-specifier < %s | FileCheck %s --check-prefix=CHECK-NO-ACCESS
+; RUN: llvm-undname --no-member-type < %s | FileCheck %s --check-prefix=CHECK-NO-MEMBER-TYPE
+; RUN: llvm-undname --no-variable-type < %s | FileCheck %s --check-prefix=CHECK-NO-VARIABLE-TYPE
+; RUN: llvm-undname --no-calling-convention --no-return-type --no-access-specifier --no-member-type --no-variable-type < %s | FileCheck %s --check-prefix=CHECK-NO-ALL
+
+?func@MyClass@@UEAAHHH@Z
+; CHECK: public: virtual int __cdecl MyClass::func(int, int)
+; CHECK-NO-CALLING-CONV: public: virtual int MyClass::func(int, int)
+; CHECK-NO-RETURN: public: virtual __cdecl MyClass::func(int, int)
+; CHECK-NO-ACCESS: {{^}}virtual int __cdecl MyClass::func(int, int)
+; CHECK-NO-MEMBER-TYPE: public: int __cdecl MyClass::func(int, int)
+; CHECK-NO-VARIABLE-TYPE: public: virtual int __cdecl MyClass::func(int, int)
+; CHECK-NO-ALL: {{^}}MyClass::func(int, int)
+
+?array2d@@3PAY09HA
+; CHECK: int (*array2d)[10]
+; CHECK-NO-CALLING-CONV: int (*array2d)[10]
+; CHECK-NO-RETURN: int (*array2d)[10]
+; CHECK-NO-ACCESS: int (*array2d)[10]
+; CHECK-NO-MEMBER-TYPE: int (*array2d)[10]
+; CHECK-NO-VARIABLE-TYPE: array2d
+; CHECK-NO-ALL: array2d
+
+?a@abc@@3PAY09HA
+; CHECK: int (*abc::a)[10]
+; CHECK-NO-CALLING-CONV: int (*abc::a)[10]
+; CHECK-NO-RETURN: int (*abc::a)[10]
+; CHECK-NO-ACCESS: int (*abc::a)[10]
+; CHECK-NO-MEMBER-TYPE: int (*abc::a)[10]
+; CHECK-NO-VARIABLE-TYPE: abc::a
+; CHECK-NO-ALL: abc::a
+
+?x@@3PEAEEA
+; CHECK: unsigned char *x
+; CHECK-NO-CALLING-CONV: unsigned char *x
+; CHECK-NO-RETURN: unsigned char *x
+; CHECK-NO-ACCESS: unsigned char *x
+; CHECK-NO-MEMBER-TYPE: unsigned char *x
+; CHECK-NO-VARIABLE-TYPE: x
+; CHECK-NO-ALL: x

llvm/tools/llvm-undname/llvm-undname.cpp

cl::opt<bool> NoCallingConvention("no-calling-convention", cl::Optional,
                                   cl::Hidden, cl::init(false),
                                   cl::cat(UndNameCategory));
 cl::opt<bool> NoReturnType("no-return-type", cl::Optional,
                            cl::desc("skip return types"), cl::Hidden,
                            cl::init(false), cl::cat(UndNameCategory));
 cl::opt<bool> NoMemberType("no-member-type", cl::Optional,
                            cl::desc("skip member types"), cl::Hidden,
                            cl::init(false), cl::cat(UndNameCategory));
+cl::opt<bool> NoVariableType("no-variable-type", cl::Optional,
+                             cl::desc("skip variable types"), cl::Hidden,
+                             cl::init(false), cl::cat(UndNameCategory));
 cl::opt<std::string> RawFile("raw-file", cl::Optional,
                              cl::desc("for fuzzer data"), cl::Hidden,
                              cl::cat(UndNameCategory));
 cl::opt<bool> WarnTrailing("warn-trailing", cl::Optional,
                            cl::desc("warn on trailing characters"), cl::Hidden,
                            cl::init(false), cl::cat(UndNameCategory));
 cl::list<std::string> Symbols(cl::Positional, cl::desc("<input symbols>"),
                               cl::ZeroOrMore, cl::cat(UndNameCategory));
 
 static bool msDemangle(const std::string &S) {
   int Status;
   MSDemangleFlags Flags = MSDF_None;
   if (DumpBackReferences)
     Flags = MSDemangleFlags(Flags | MSDF_DumpBackrefs);
   if (NoAccessSpecifier)
     Flags = MSDemangleFlags(Flags | MSDF_NoAccessSpecifier);
   if (NoCallingConvention)
     Flags = MSDemangleFlags(Flags | MSDF_NoCallingConvention);
   if (NoReturnType)
     Flags = MSDemangleFlags(Flags | MSDF_NoReturnType);
   if (NoMemberType)
     Flags = MSDemangleFlags(Flags | MSDF_NoMemberType);
+  if (NoVariableType)
+    Flags = MSDemangleFlags(Flags | MSDF_NoVariableType);
 
   size_t NRead;
   char *ResultBuf =
       microsoftDemangle(S.c_str(), &NRead, nullptr, nullptr, &Status, Flags);
   if (Status == llvm::demangle_success) {
     outs() << ResultBuf << "\n";
     outs().flush();
     if (WarnTrailing && NRead < S.size())