Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

MS name mangling incorrect for x64 class members #13454

Closed Quuxplusone closed 11 years ago

Quuxplusone commented 12 years ago
Bugzilla Link PR13792
Status RESOLVED FIXED
Importance P normal
Reported by Aaron Ballman (aaron@aaronballman.com)
Reported on 2012-09-07 07:41:33 -0700
Last modified on 2013-08-02 19:00:06 -0700
Version trunk
Hardware PC Windows NT
CC DevOmem@web.de, joao@tritao.eu, llvm-bugs@lists.llvm.org, rnk@google.com, timurrrr@google.com, whunt@google.com
Fixed by commit(s)
Attachments
Blocks PR12477
Blocked by
See also
When compiling for x64 and MS compatibility, the name mangling of class members
is wrong:

class foo {
public:
  int __thiscall f() {
    return 0;
  }
};

clang:
define linkonce_odr x86_thiscallcc i32 @"\01?f@foo@@QAEHXZ"(%class.foo* %this)
nounwind align 2 {

MSVC:
PUBLIC  ?f@foo@@QEAAHXZ                 ; foo::f

Note the EAA in MSVC compared to AE in clang.
Quuxplusone commented 12 years ago

If I had to take a guess as to what's going on, I think we're missing something to mangle before qualifiers (in mangleType for FunctionType) that accounts for the E, and that the calling convention MSVC mangles for instance methods is actually cdecl instead of thiscall.

Q (function class) E (unknown) A (qualifier near) A (cdecl calling convention) H (int) X (void parameter) Z (throw specification)

That's just a guess, however. CCing Timur since I bet he knows.

Quuxplusone commented 12 years ago

I think this is a prefix (__ptr64) that is applied on the CV-class modifier of non-static member functions.

"E" means __ptr64 and "A" no CV modifiers, thus EA. Check MicrosoftCXXNameMangler::mangleQualifiers.

But it does need a little more research to see in exactly what cases this is applied by cl.

Quuxplusone commented 12 years ago
> CCing Timur since I bet he knows.
Nope, I haven't worked on x64 mangling yet.
Quuxplusone commented 12 years ago
Well we do have Ptr64Attr in SemaDeclAttr.cpp than should be created from
__ptr64.

But how to aces it in MicrosoftMangle.cpp to fix this problem ?
I have tried a lot of solutions but could not find any way to access this
information.
Quuxplusone commented 12 years ago
Just checked with undname (Microsoft C++ Name Undecorator).

Undecoration of :- "?f@foo@@QEAAHXZ"
is :- "public: int __cdecl foo::f(void) __ptr64"

Undecoration of :- "?f@foo@@QAEHXZ"
is :- "public: int __thiscall foo::f(void)"

So both the calling convention and __ptr64 are wrong. The calling convention
problem will be fixed by Aaron's patch that should be applied soon.

For the __ptr64, mangling and attribute handling need to be added. I'm not sure
in exactly what cases MSVC adds this, but it seems to be implicit in 64-bit
mode at least in non-static member functions. I don't think Clang adds this
yet, so you'll have to insert it somewhere (maybe on Sema).
Quuxplusone commented 11 years ago

I started looking at this a bit (it seems like a pretty easy starter project). It appears that a 64-bit note 'E' is required for 64-bit pointers in 64-bit mode. I've got a patch that works in almost all cases but I've still got probably a few more hours/another day of debugging to do. I'm also updating some of the mangling tests to check both 32 and 64 bit mangles.

-Warren

Quuxplusone commented 11 years ago

Warren implemented this in the patch I committed in r181825.