Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

generic thunk code fails #3133

Open Quuxplusone opened 16 years ago

Quuxplusone commented 16 years ago
Bugzilla Link PR2861
Status NEW
Importance P normal
Reported by Nick Lewycky (nicholas@mxc.ca)
Reported on 2008-10-04 20:40:10 -0700
Last modified on 2017-09-15 09:55:25 -0700
Version trunk
Hardware PC Linux
CC baldrick@free.fr, clattner@nondot.org, efriedma@quicinc.com, evan.cheng@apple.com, llvm-bugs@lists.llvm.org, nlewycky@google.com
Fixed by commit(s)
Attachments thunk_error.cpp (428 bytes, text/x-c++src)
Blocks
Blocked by
See also
Created attachment 2059
testcase

This delta-reduced testcase triggers a problem with llvm-g++ that doesn't
happen with FSF g++ 4.2:

$ g++ thunk_error.cpp -c
thunk_error.cpp:21: error: generic thunk code fails for method ‘virtual void
RPCCore::Ev(const char*, int, int, const char*, ...) const’ which uses ‘...’
$ g++-4.2 thunk_error.cpp -c
$ g++-4.2 -v
[...]
gcc version 4.2.4 (Debian 4.2.4-3)
Quuxplusone commented 16 years ago

Attached thunk_error.cpp (428 bytes, text/x-c++src): testcase

Quuxplusone commented 16 years ago

This comes out of cp/method.c use_thunk. In non-LLVM builds, it will run a target hook (x86_output_mi_thunk in my case) which can handle varargs. For LLVM, this is disabled and the generic code can't handle varargs.

It's not clear to me what the thunk is needed for or how to lower it to LLVM IR.

Quuxplusone commented 15 years ago

Briefly, what use_thunk is doing is writing out a function that adjusts the 'this' pointer, then calling the thunkee with the same parameters (after modification).

The "generic" code path that we're relying on can't handle varargs since there's no way to copy varargs from one function to another. There are, however, ABI specific tricks to implement this.

There's no current way in LLVM IR to lower this.

What we should probably do is in the vararg case, call the target-specific output hook, and construct an InlineAsm block. That's not pretty, but will get it working.

Quuxplusone commented 15 years ago

The use of asm is probably a good solution for now, but we have to find a way to properly represent this in llvm.

One easy solution is to add thunks to the llvm IL. If I understand correctly, all that is needed is the name of the thunk, the offset and the function to jump to.

Another option that is probably a bit harder is to make it possible for a tail call to reuse the varargs of the caller. This would make it possible for the pointer update to be just a regular llvm instruction.

I have a small preference for the second option since it is a bit more generic.

Quuxplusone commented 15 years ago

After reading through the thread for the equivalent problem with the gcc lto folks, http://gcc.gnu.org/ml/gcc-patches/2008-12/msg00953.html , I think the cleanest solution for us, for now, is to emit a new copy of the function with the %this adjustment. That means we'll get duplicated code for each time this occurs, but that's not easily avoidable.

Quuxplusone commented 14 years ago

Clang also gets this wrong. It silently drops the variadic arguments.

Quuxplusone commented 13 years ago

This was fixed in clang pretty recently; throwing this into the dragonegg component, so someone can check the status there.