[Symbolize] Use the local MSVC C++ demangler instead of relying on dbghelp. NFC.

[mstorsjo]

Summary

This allows making a couple llvm-symbolizer tests run in all environments.

Keeping the existing exact prefix checks to make sure this is a strict NFC patch.

[rnk]

lgtm

[mstorsjo]

Had to revert this one, as it broke sanitizer tests: http://lab.llvm.org:8011/builders/sanitizer-windows/builds/52441

So apparently it seems (again) like it would be useful if the llvm microsoft demangler could be configured to leave out parts of the output. In D68134 (in LLDB) we can consider changing the testcases to include the new demangler output, but there was a concern that there might be cases within LLDB that try to parse the demangled function names. And here it significantly changes the libfuzzer deduplication token output, which I think also isn't ideal.

@thakis - you brought it up earlier that adding options to it could be possible. Do you, who I think is familiar with that demangler, have time to make a PoC of it? Otherwise I might have a stab at it sometimes later...

[thakis]

In D68133#1694180, @mstorsjo wrote:

Had to revert this one, as it broke sanitizer tests: http://lab.llvm.org:8011/builders/sanitizer-windows/builds/52441

So apparently it seems (again) like it would be useful if the llvm microsoft demangler could be configured to leave out parts of the output. In D68134 (in LLDB) we can consider changing the testcases to include the new demangler output, but there was a concern that there might be cases within LLDB that try to parse the demangled function names. And here it significantly changes the libfuzzer deduplication token output, which I think also isn't ideal.

@thakis - you brought it up earlier that adding options to it could be possible. Do you, who I think is familiar with that demangler, have time to make a PoC of it? Otherwise I might have a stab at it sometimes later...

That links is dead. Which customization is needed here? microsoftDemangle() already gets a Flags argument; we can just add more flags to it.

[thakis]

But again, maybe the tests are just overly strict here?

[mstorsjo]

In D68133#1700276, @thakis wrote:

That links is dead. Which customization is needed here?

Ah crap, I should have quoted the relevant bits...

IIRC, it at least had extra public: at the start of the symbol, extra return type and calling convention specifiers. D68134 has a more concrete case of what needs to change in LLDB if the llvm demangler is used as-is there.

microsoftDemangle() already gets a Flags argument; we can just add more flags to it.

Oh, that's nice! Then this shoudln't probably be too big a deal to extend.

In D68133#1700277, @thakis wrote:

But again, maybe the tests are just overly strict here?

I think for this particular case, for the fuzzer sanitizer's backtrace disambiguation log, there might be a risk that something is expecting to process the log, which might not be ready to handle extra unexpected keywords. I might be overly cautious though.

In the LLDB case, a concern was voiced that some parts of LLDB might try to parse the demangled symbol names, and not expect those extra keywords there.

[mstorsjo]

In D68133#1700289, @mstorsjo wrote:

In D68133#1700276, @thakis wrote:

That links is dead. Which customization is needed here?

Ah crap, I should have quoted the relevant bits...

IIRC, it at least had extra public: at the start of the symbol, extra return type and calling convention specifiers. D68134 has a more concrete case of what needs to change in LLDB if the llvm demangler is used as-is there.

I guess ideally we would have individual flags for all the UnDecorateSymbolName flags used here: UNDNAME_NO_ACCESS_SPECIFIERS, UNDNAME_NO_ALLOCATION_LANGUAGE, UNDNAME_NO_THROW_SIGNATURES, UNDNAME_NO_MEMBER_TYPE, UNDNAME_NO_MS_KEYWORDS, UNDNAME_NO_FUNCTION_RETURNS.

[thakis]

for the fuzzer sanitizer's backtrace disambiguation log, there might be a risk that something is expecting to process the log, which might not be ready to handle extra unexpected keywords. I might be overly cautious though.

In the LLDB case, a concern was voiced that some parts of LLDB might try to parse the demangled symbol names, and not expect those extra keywords there.

I think LLDB/win and fuzzers/win might have few enough clients that we can just trying to change it and see what breaks.

[mstorsjo]

Using specific flags to microsoftDemangle, to avoid spurious changes in libfuzzer tests when llvm-symbolizer is used there. (This means that it can't use the nice unified demangle function frontend that handles both itanium and ms style, and which does the char*->std::string wrapping.)

[mstorsjo]

@rnk - does this update of the patch seem ok to you?

[rnk]

lgtm

[mstorsjo]

It seems like this still broke a buildbot. @rnk, as it's your bot, can you look into it before we revert this? The logs don't really show enough detail... http://lab.llvm.org:8011/builders/sanitizer-windows/builds/53114/steps/stage%201%20check/logs/stdio

[rnk]

In D68133#1712212, @mstorsjo wrote:

It seems like this still broke a buildbot. @rnk, as it's your bot, can you look into it before we revert this? The logs don't really show enough detail... http://lab.llvm.org:8011/builders/sanitizer-windows/builds/53114/steps/stage%201%20check/logs/stdio

Yep, done in r375147. The demangler difference is:

MS:   f(char,char *,char *)
LLVM: f(char, char *, char *)

[mstorsjo]

In D68133#1713254, @rnk wrote:

In D68133#1712212, @mstorsjo wrote:

It seems like this still broke a buildbot. @rnk, as it's your bot, can you look into it before we revert this? The logs don't really show enough detail... http://lab.llvm.org:8011/builders/sanitizer-windows/builds/53114/steps/stage%201%20check/logs/stdio

Yep, done in r375147. The demangler difference is:

MS:   f(char,char *,char *)
LLVM: f(char, char *, char *)

Awesome, thanks! I guess that should have been deducible from the buildbot log, but I overlooked the spacing.

llvm/trunk/lib/DebugInfo/Symbolize/Symbolize.cpp

 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include <algorithm>
 #include <cassert>
 #include <cstring>
 
-#if defined(_MSC_VER)
-#include <Windows.h>
-
-// This must be included after windows.h.
-#include <DbgHelp.h>
-#pragma comment(lib, "dbghelp.lib")
-
-// Windows.h conflicts with our COFF header definitions.
-#ifdef IMAGE_FILE_MACHINE_I386
-#undef IMAGE_FILE_MACHINE_I386
-#endif
-#endif
-
 namespace llvm {
 namespace symbolize {
 
 Expected<DILineInfo>
 LLVMSymbolizer::symbolizeCodeCommon(SymbolizableModule *Info,
                                     object::SectionedAddress ModuleOffset) {
   // A null module means an error has already been reported. Return an empty
   // result.

 
 } // 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);
-    return Result;
-  }
-
-#if defined(_MSC_VER)
-  if (!Name.empty() && Name.front() == '?') {
-    // Only do MSVC C++ demangling on symbols starting with '?'.
-    char DemangledName[1024] = {0};
-    DWORD result = ::UnDecorateSymbolName(
-        Name.c_str(), DemangledName, 1023,
-        UNDNAME_NO_ACCESS_SPECIFIERS |       // Strip public, private, protected
-            UNDNAME_NO_ALLOCATION_LANGUAGE | // Strip __thiscall, __stdcall, etc
-            UNDNAME_NO_THROW_SIGNATURES |    // Strip throw() specifications
-            UNDNAME_NO_MEMBER_TYPE | // Strip virtual, static, etc specifiers
-            UNDNAME_NO_MS_KEYWORDS | // Strip all MS extension keywords
-            UNDNAME_NO_FUNCTION_RETURNS); // Strip function return types
-    return (result == 0) ? Name : std::string(DemangledName);
-  }
-#endif
+  // MSVC C++ mangled symbols start with '?', while itanium mangled ones
+  // start with _Z.
+  if (Name.substr(0, 2) == "_Z" || (!Name.empty() && Name.front() == '?'))
+    return demangle(Name);
+
   if (DbiModuleDescriptor && DbiModuleDescriptor->isWin32Module())
     return std::string(demanglePE32ExternCFunc(Name));
   return Name;
 }
 
 } // namespace symbolize
 } // namespace llvm

llvm/trunk/test/tools/llvm-symbolizer/coff-dwarf.test

 RUN: llvm-symbolizer 0x5009 0x5038 --inlining --relative-address -obj="%p/Inputs/coff-dwarf.exe" \
 RUN:     | FileCheck %s
 RUN: llvm-symbolizer 0x5009 0x5038 --inlines --relative-address -obj="%p/Inputs/coff-dwarf.exe" \
 RUN:     | FileCheck %s
 RUN: llvm-symbolizer 0x5009 0x5038 -i --relative-address -obj="%p/Inputs/coff-dwarf.exe" \
 RUN:     | FileCheck %s
 
-This test relies on UnDecorateSymbolName, which is Windows-only.
-REQUIRES: target-windows, system-windows
-
 CHECK: foo(void)
 CHECK: coff-dwarf.cpp:7
 CHECK: bar(void)
 CHECK: coff-dwarf.cpp:13
 CHECK: main
 CHECK: coff-dwarf.cpp:17

llvm/trunk/test/tools/llvm-symbolizer/coff-exports.test

 RUN: llvm-symbolizer 0x500A 0x5038 0x504B --inlining --relative-address -obj="%p/Inputs/coff-exports.exe" \
 RUN:   | FileCheck %s
 RUN: llvm-symbolizer 0x500A 0x5038 0x504B --inlines --relative-address -obj="%p/Inputs/coff-exports.exe" \
 RUN:   | FileCheck %s
 RUN: llvm-symbolizer 0x500A 0x5038 0x504B -i --relative-address -obj="%p/Inputs/coff-exports.exe" \
 RUN:   | FileCheck %s
 
-This test relies on UnDecorateSymbolName, which is Win32-only.
-REQUIRES: system-windows
-REQUIRES: target-windows
-FIXME: This test depends on host, not target.
-
 We get the expected stack trace, except 'foo' appears for the 'bar' frame
 because 'bar' isn't in the export table.
 
 CHECK: foo(void)
 CHECK: ??:0:0
 CHECK: foo(void)
 CHECK: ??:0:0
 CHECK: main
 CHECK: ??:0:0