x86_stdcallcc @<n> mangling vs. '\1' prefix [was: x86_stdcallcc and extra name mangling on Windows]

Project Infrastructure LLVM Dev List Archives
#1

Seems like I'm out of luck - the @<n> suffix is added
(AddFastCallStdCallSuffix) in the GlobalValue
Magnler::getNameWithPrefix overload, without paying respect to whether
the name originally had a '\1' prefix or not.

Should this be changed?

David

#2

I think so. There have been other reports lately related to this being wrong.

http://llvm.org/bugs/show_bug.cgi?id=14410

CC’ing Timur since he might know more about this.

#3

I don't remember anything other that what I've written in the bug João
has mentioned.

Probably something like this patch
http://llvm.org/bugs/show_bug.cgi?id=14410#c6
?

#4

My problem is not related to correctly implementing the MS
stdcall/fastcall ABI itself, but rather a (sufficiently) similar one.
Thus, I need to specifically disable mangling in a case where it would
normally be needed.

For the quick-and-dirty patch I'm currently using locally, see below.

David

diff --git a/lib/Target/Mangler.cpp b/lib/Target/Mangler.cpp
index edfd421..e4b6368 100644
--- a/lib/Target/Mangler.cpp
+++ b/lib/Target/Mangler.cpp
@@ -185,9 +185,12 @@ void
Mangler::getNameWithPrefix(SmallVectorImpl<char> &OutName,
     PrefixTy = Mangler::Private;
   else if (GV->hasLinkerPrivateLinkage() || GV->hasLinkerPrivateWeakLinkage())
     PrefixTy = Mangler::LinkerPrivate;

#5

The patch looks incorrect. The code just needs to handle \1 properly
and clang extended to add explicit \1 to the names which does not
require mangling.

I do not think that moving whole mangling to clang is a good idea,
because then everyone who uses LLVM to call WinApi functions will need
to mangle by hands.

#6

The patch looks incorrect. The code just needs to handle \1 properly
and clang extended to add explicit \1 to the names which does not
require mangling.

I think clang already adds \01 to __stdcall names, so only the LLVM
change is remaining.

I do not think that moving whole mangling to clang is a good idea,
because then everyone who uses LLVM to call WinApi functions will need
to mangle by hands.

I agree.

#7

... and LLVM drops \01 in getNameWithPrefixat lib/Target/Mangler.cpp:116
right before returning the rest in OutName.

Ahh you were replying to me, not to David.
How does David's patch look to you?

#8

Anton, what do you think of David’s patch with this test case? OK to commit that?

stdcall-double-mangle.patch (2.45 KB)

#9

Looks good to me, except for
  ; CHECK: MyFunc@0:
should be probably
  ; CHECK: {{^_MyFunc@0:}}
and
+; CHECK: alreadyMangled:
should be
+; CHECK: {{^alreadyMangled:}}

#10

Reid,

I believe I had some concerns over the patch... Let me find / resurrect them :slight_smile: