Frontends consider calls to "XYZ foo()" to be var-args, but this is wrong

[llvmbot]

Bugzilla Link	10713
Resolution	FIXED
Resolved on	Sep 21, 2011 03:09
Version	trunk
OS	Linux
Blocks	#11054
Reporter	LLVM Bugzilla Contributor
CC	@asl,@efriedma-quic,@rjmccall

Extended Description

Clang/llvm-gcc and dragonegg all consider a call to a function declared without
any parameters to be a var-args function call. However this gives the wrong
results when using the stdcall calling convention (with stdcall, the callee pops
arguments except if it is a varargs function; so any mistake in the
"var-argness" of a call is fatal, unlike with the usual C convention where it
doesn't make any difference).

Consider the following testcase. (The second call to bar is to make the stack
adjustments or lack of them clearer in the assembler):

#define STDCALL attribute((stdcall))

void STDCALL foo(); // Var-args or not var-args, that is the question!
void STDCALL bar(int); // Definitely not var-args
void STDCALL varg(int, ...); // Definitely var-args

void g(int x, int y)
{
foo(x, y);
bar(x);
varg(x, y);
bar(y);
}

GCC assembler snippets (compiled with: gcc -m32 -O2 -S):

call	foo
subl	$8, %esp     <= Stack adjust, indicates not var-args!

...
call bar
subl $4, %esp <= Stack adjust; not var-args
...
call varg
movl %ebx, (%esp) <= No stack adjust; var-args
...

Clang etc assembler snippets:

calll	foo
movl	%edi, (%esp) <= No stack adjust, indicates var-args; disagrees with GCC

...
calll bar
subl $4, %esp <= Stack adjust; not var-args; agrees with GCC
...
calll varg
movl %esi, (%esp) <= No stack adjust; var-args; agrees with GCC
...

This difference seems to be the cause of some crashes when running wine compiled
with LLVM.

Note that using C++ and declaring foo "void foo(...)" doesn't make any
difference: GCC still calls it as if it were not var-args!

Digging into the GCC logic, the var-argness of a function type is determined
by the stdarg_p predicate (which returns true for var-args). This special
cases function types with no parameters (as produced by the C front-end for
XYZ foo() and by the C++ front-end for XYZ foo(...)) as not being var-args.

[efriedma-quic]

See also bug 10613.

[llvmbot]

Solved in dragonegg in commit 138663. Moving to clang.

[rjmccall]

Duncan, I think you're over-generalizing here. Some calling conventions don't support unprototyped calls. Other calling conventions specify that such calls follow all the varargs rules — x86-64 is the most obvious example. It sounds like stdcall specifies that such calls act as it they were prototyped with the appropriate default-argument-promoted function type. The latter two choices are both valid by the C spec.

The right rule is to treat this as CC-dependent.

[llvmbot]

I agree - Eli pointed out already that things are more complicated than I
thought.

[llvmbot]

I think it is reasonable to have the LLVM code generators take care of this.
The frontends need to communicate whether the original function was void foo()
or void foo(void). The simplest way to do this is to turn
void foo() into void @​foo(...)
and
void foo(void) into void @​foo()
like is done by clang right now (and used to be done by dragonegg before I
zapped it yesterday!).
Then (eg) the x86 code for the stdcall convention will have to special case
calls to varargs functions where there are no non-varargs parameters, i.e.
if it sees a call to void @​foo(...) it needs to act as if it is not varargs,
while a call to void @​foo(i32 %x, ...) would be varargs.

[rjmccall]

Doesn't that leave calls to "stdcall void foo(...);" broken?

I think there are two ways of dealing with this, and they both require work in the frontend:
(1) Frontends are careful to emit varargs or non-varargs calls as appropriate for the CC they're using. Emitting a "non-varargs" call to an unprototyped function means casting it to the appropriate prototyped type before calling.
(2) We treat stdcall and stdcall_varargs as two different CCs.

#​2 is conceptually cleaner, but it's a pain for targets like MIPS where the default CC acts this way.

[rjmccall]

Doesn't that leave calls to "stdcall void foo(...);" broken?

Oh, no, I see. You said that GCC treats such functions as not really varargs.

In that case, if that's true for all such CCs, this could be done in the backend.

[rjmccall]

Note that, looking at the other bug, I'm not sure that's true for MIPS; the ABI document there certainly seems to suggest that any prototype with ... should use the varargs convention.

[llvmbot]

The backend knows the calling convention, the target and whether the original
function was void foo() or void foo(void) [because in LLVM it is void @​foo(...)
or void @​foo(void)]. That should mean it knows enough. For example, if the calling convention is stdcall, the callee is declared varargs and there are no
parameters [i.e. void @​foo(...)],then the backend should conclude that the callee
pops the arguments (just like for non-varargs). However if callee is declared
varargs with at least one parameter [e.g. void @​foo(i32, ...)] then the caller
pops the arguments. For different calling conventions the logic would be
different.

In short, I'm suggesting that (...) in the function type shouldn't necessarily
force the var-args argument passing method. For functions with no arguments
it can be seen as a way of representing in the IR whether the original function
was declare void foo() or void foo(...).

[rjmccall]

That idea is reasonable if we think that "foo()" and "foo(...)" are treated the same in every calling convention. What I'm saying is that it doesn't look like that's true of the MIPS convention. If so, making the backend special-case some of these types just makes things unnecessarily complicated.

[llvmbot]

I'm not sure why you think it is necessary for all calling conventions to treat
"foo()" and "foo(...)" the same. My point of view here is that what matters is
to pass the information that it was "foo()" and "foo(...)" all the way down to
the code generators, and let them do the right thing depending on the calling
convention. This may mean treating "foo()" and "foo(...)" the same (stdcall) or
differently (cdecl). The question then arises: how to communicate this
information to the code generators? I'm suggesting using the presence of ...
(or not) in the LLVM IR, rather than adding some attribute or whatever.

[rjmccall]

I'm not seeing how your rule gets us anywhere. Suppose we have a function declared as "void foo()". We can't give it the IR type "void ()", because we need to be able to pass it arbitrary arguments, and that type doesn't support any arguments at all. We can't give it the IR type "void (...)", because that type corresponds to the C type "void(...)", and the whole point of this bug is that that might follow a different convention. So we need to do something.

To me, the obvious fix is that the frontend should bitcast the function operand to the right non-variadic type when it emits a call to an unprototyped function under a CC like stdcall/MIPS. So this code:
extern stdcall void foo();
foo(5, 1.0)
would compile to this IR:
call x86_stdcall void bitcast (void (...)* @​foo to void (i32, double)*)(i32 5, double 1.0)

So the code generators follow the nice, simple rule that they use the variadic convention for the CC if the called function type is variadic; conveniently enough, that's what they do right now.

[llvmbot]

I'm not seeing how your rule gets us anywhere. Suppose we have a function
declared as "void foo()". We can't give it the IR type "void ()", because we
need to be able to pass it arbitrary arguments, and that type doesn't support
any arguments at all. We can't give it the IR type "void (...)", because that
type corresponds to the C type "void(...)",

There is no C type "void (...)".

$ cat va.c
void foo(...);
$ gcc -c va.c
va.c:1:10: error: ISO C requires a named argument before ‘...’

Thus no confusion is possible. In C++ you can have "void foo(...)", but in C++
"void foo()" is not the same as in C - it cannot be passed arbitrary arguments.

Indeed internally in gcc, you can't distinguish between C's "void foo()" and
C++'s "void foo(...)". Therefore I think it is perfectly reasonable to output
"void @​foo(...)" in the IR for both.

and the whole point of this bug is that that might follow a different
convention. So we need to do something.

As pointed out above, this is not the issue at all.

As for your comments about the front-end taking care of things: I have some
sympathy for this viewpoint. After all, front-ends already have to take care
of lots of ABI stuff. It's a judgement call as to whether this kind of thing
is best put in the back-end (because it can be done easily enough, and that
makes life simpler for front-ends) or the front-end.

By the way, according to the internet (and it looks like GCC follows this),
a var-args stdcall function is called using the cdecl convention. So maybe
it is simpler to have front-ends make a cdecl call.

[rjmccall]

There is no C type "void (...)".

True, but it's a valid C++ type, and of course the backend has no idea
whether the source code is C or C++.

By the way, according to the internet (and it looks like GCC follows this),
a var-args stdcall function is called using the cdecl convention. So maybe
it is simpler to have front-ends make a cdecl call.

Interesting. Yes, I think that's probably the right move; the different
convention may require different frontend lowering anyway.

[asl]

By the way, according to the internet (and it looks like GCC follows this),
a var-args stdcall function is called using the cdecl convention.
It's not just "according to the internet". It's specified in MSDN: http://msdn.microsoft.com/ru-ru/library/zxk0tw93.aspx :)