[Symbolize][MinGW] Support demangling i386 call-conv-decorated C++ names

[alvinhochun]

Summary

On i386 Windows, after C++ names have been Itanium-mangled, the C name
mangling specific to its call convention may also be applied on top.
This change teaches symbolizer to be able to demangle this type of
mangled names.

As part of this change, demanglePE32ExternCFunc has also been modified
to fix unwanted stripping for vectorcall names when the demangled name
is supposed to contain a leading _. Notice that the vectorcall
mangling does not add either an _ or @ prefix. The old code always
tries to strip the prefix first, which for Itanium mangled names in
vectorcall, the leading underscore of the Itanium name gets stripped
instead and breaks the Itanium demangler.

[mstorsjo]

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

674

I see that the demanglePE32ExternCFunc function is restructured a fair bit here, but I kinda fail to see exactly what the intent is - is this a functional change, and in what cases does it change the output/behaviour?

706

This part of the change makes sense!

I'm a bit undecided about whether the current flow in the function is the best/ideal/most correct one etc, but it probably works just right for all practical cases.

(For macho/darwin, where is the leading underscore stripped out?)

llvm/test/DebugInfo/symbolize-demangling-mingw32.s

22

Hmm, both this symbol and g above, aren't technically valid manglings on i386 at all (although they can be considered reasonable internal symbols anyway, where tools/users could use any names they want).

Would it be useful to add a test for a plain __cdecl function with C mangling, i.e. just a plain underscore prefix?

[alvinhochun]

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

674

It fixes some unwanted stripping for vectorcall names. Notice that the vectorcall mangling does not add either an _ or @ prefix. The old code always tries to strip the prefix first, which for Itanium mangled names in vectorcall, the leading underscore of the Itanium name gets stripped instead, which breaks the Itanium demangler.

706

For macho/darwin, where is the leading underscore stripped out?

Looks like it happens way earlier when loading symbols into SymbolizeObjectFile: https://github.com/llvm/llvm-project/blob/462227f1150fc4a12f95a7a101de477c06d35ba7/llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp#L212-L214

llvm/test/DebugInfo/symbolize-demangling-mingw32.s

22

True, I agree it should test a mangled __cdecl C name, and I might as well also add a test each for the other calling conventions.

[mstorsjo]

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

674

Oh, I see - thanks!

I think it'd be good to call out this explicitly in the commit message, as it wasn't obvious to me on the first read through. Now after you've pointed it out, it's quite obvious though :-)

llvm/test/DebugInfo/symbolize-demangling-mingw32.s

22

Yeah, plain C symbols with all calling conventions would be good too - thanks!

[alvinhochun]

Update test and commit message as suggested.

[mstorsjo]

LGTM, thanks!

[alvinhochun]

Thanks. I shall wait for the pre-merge checks to complete before committing. Can this still be backported to 16 now given we are already past rc2?

[mstorsjo]

In D144049#4128546, @alvinhochun wrote:

Thanks. I shall wait for the pre-merge checks to complete before committing. Can this still be backported to 16 now given we are already past rc2?

I guess that's up to @thieta and @tstellar; we can at least file the backport request and let them judge. The change in itself seems very low risk in any case.

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

 
 // Undo these various manglings for Win32 extern "C" functions:
 // cdecl       - _foo
 // stdcall     - _foo@12
 // fastcall    - @foo@12
 // vectorcall  - foo@@12
 // These are all different linkage names for 'foo'.
 StringRef demanglePE32ExternCFunc(StringRef SymbolName) {
-  // Remove any '_' or '@' prefix.
   char Front = SymbolName.empty() ? '\0' : SymbolName[0];
-  if (Front == '_' || Front == '@')
-    SymbolName = SymbolName.drop_front();
 
   // Remove any '@[0-9]+' suffix.
+  bool HasAtNumSuffix = false;
   if (Front != '?') {
     size_t AtPos = SymbolName.rfind('@');
     if (AtPos != StringRef::npos &&
-        all_of(drop_begin(SymbolName, AtPos + 1), isDigit))
+        all_of(drop_begin(SymbolName, AtPos + 1), isDigit)) {
       SymbolName = SymbolName.substr(0, AtPos);
+      HasAtNumSuffix = true;
+    }
   }
 
   // Remove any ending '@' for vectorcall.
-  if (SymbolName.endswith("@"))
+  bool IsVectorCall = false;
+  if (HasAtNumSuffix && SymbolName.endswith("@")) {
     SymbolName = SymbolName.drop_back();
+    IsVectorCall = true;
+  }
+
+  // If not vectorcall, remove any '_' or '@' prefix.
+  if (!IsVectorCall && (Front == '_' || Front == '@'))
+    SymbolName = SymbolName.drop_front();
 
   return SymbolName;
 }
 
 } // end anonymous namespace
 
 std::string
 LLVMSymbolizer::DemangleName(const std::string &Name,
char *DemangledName = microsoftDemangle(
                         MSDF_NoMemberType | MSDF_NoReturnType));
     if (status != 0)
       return Name;
     Result = DemangledName;
     free(DemangledName);
     return Result;
   }
 
-  if (DbiModuleDescriptor && DbiModuleDescriptor->isWin32Module())
-    return std::string(demanglePE32ExternCFunc(Name));
+  if (DbiModuleDescriptor && DbiModuleDescriptor->isWin32Module()) {
+    std::string DemangledCName(demanglePE32ExternCFunc(Name));
+    // On i386 Windows, the C name mangling for different calling conventions
+    // may also be applied on top of the Itanium or Rust name mangling.
+    if (nonMicrosoftDemangle(DemangledCName.c_str(), Result))
+      return Result;
+    return DemangledCName;
+  }
   return Name;
 }
 
 void LLVMSymbolizer::recordAccess(CachedBinary &Bin) {
   if (Bin->getBinary())
     LRUBinaries.splice(LRUBinaries.end(), LRUBinaries, Bin.getIterator());
 }
 

llvm/test/DebugInfo/symbolize-demangling-mingw32.s

+# REQUIRES: x86-registered-target
+
+# RUN: llvm-mc --filetype=obj --triple=i386-w64-windows-gnu %s -o %t.o -g
+
+# RUN: llvm-symbolizer --obj=%t.o 0 1 2 3 4 5 6 7 8 9 10 | FileCheck %s
+
+  .def g
+  .scl 2
+  .type 32
+  .endef
+g:
+  nop
+# CHECK:       {{^g$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:12
+# CHECK-EMPTY:
+
+  .def baz
+  .scl 2
+  .type 32
+  .endef
+baz:
+  nop
+# CHECK-NEXT:  {{^baz$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:22
+# CHECK-EMPTY:
+
+# extern "C" void c() {} // __cdecl
+  .def _c
+  .scl 2
+  .type 32
+  .endef
+_c:
+  nop
+# CHECK-NEXT:  {{^c$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:33
+# CHECK-EMPTY:
+
+# extern "C" void __stdcall c1() {}
+  .def _c1@0
+  .scl 2
+  .type 32
+  .endef
+_c1@0:
+  nop
+# CHECK-NEXT:  {{^c1$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:44
+# CHECK-EMPTY:
+
+# extern "C" void __fastcall c2(void) {}
+  .def @c2@0
+  .scl 2
+  .type 32
+  .endef
+@c2@0:
+  nop
+# CHECK-NEXT:  {{^c2$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:55
+# CHECK-EMPTY:
+
+# extern "C" void __vectorcall c3(void) {}
+  .def c3@@0
+  .scl 2
+  .type 32
+  .endef
+c3@@0:
+  nop
+# CHECK-NEXT:  {{^c3$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:66
+# CHECK-EMPTY:
+
+# void f() {} // __cdecl
+  .def __Z1fv
+  .scl 2
+  .type 32
+  .endef
+__Z1fv:
+  nop
+# CHECK-NEXT:  {{^f\(\)$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:77
+# CHECK-EMPTY:
+
+# void __stdcall f1() {}
+  .def __Z2f1v@0
+  .scl 2
+  .type 32
+  .endef
+__Z2f1v@0:
+  nop
+# CHECK-NEXT:  {{^f1\(\)$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:88
+# CHECK-EMPTY:
+
+# void __fastcall f2(void) {}
+  .def @_Z2f2v@0
+  .scl 2
+  .type 32
+  .endef
+@_Z2f2v@0:
+  nop
+# CHECK-NEXT:  {{^f2\(\)$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:99
+# CHECK-EMPTY:
+
+# void __vectorcall f3(void) {}
+  .def _Z2f3v@@0
+  .scl 2
+  .type 32
+  .endef
+_Z2f3v@@0:
+  nop
+# CHECK-NEXT:  {{^f3\(\)$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:110
+# CHECK-EMPTY:
+
+# Rust
+  .def __RNvC1x1y
+  .scl 2
+  .type 32
+  .endef
+__RNvC1x1y:
+  nop
+# CHECK-NEXT:  {{^x::y$}}
+# CHECK-NEXT:  symbolize-demangling-mingw32.s:121
+# CHECK-EMPTY: