va_start() is mishandled on Windows

[timurrrr]

Bugzilla Link	12162
Resolution	DUPLICATE
Resolved on	Sep 21, 2015 22:00
Version	trunk
OS	Windows NT
Blocks	llvm/llvm-bugzilla-archive#13707
CC	@AaronBallman,@asl,@majnemer,@efriedma-quic,@tritao,@kcc,@nico,@rnk

Extended Description

$ more va_args.c # Reproducer
#include <stdarg.h>
#include <stdio.h>

void foo(int A, ...) {
int B;
va_list L;
va_start(L, A);
B = va_arg(L, int);
printf("foo(%d, %d)\n", A, B);
va_end(L);
}

int main(void) { foo(1, 2); }

$ cl /nologo va_args.c && va_args.exe # Expected result
foo(1, 2)

$ clang.exe --version
clang version 3.1 (trunk 150957)
Target: i686-pc-win32
Thread model: posix

$ clang.exe va_args.c && ./a.out # Actual result
foo(1, 1638208)

$ clang.exe va_args.c -S -emit-llvm

$ more va_args.s
...
define void @​foo(i32 %A, ...) nounwind {
%1 = alloca i32, align 4
%B = alloca i32, align 4
%L = alloca i8*, align 4
store i32 %A, i32* %1, align 4
%2 = bitcast i32* %1 to i8*
%3 = getelementptr inbounds i8* %2, i32 4 # %3 = &A + 1
store i8* %3, i8** %L, align 4 # L = &A + 1
%4 = load i8** %L, align 4
%5 = getelementptr inbounds i8* %4, i32 4 # %5 = (&A + 1) + 1
store i8* %5, i8** %L, align 4 # L = &A + 2
%6 = getelementptr inbounds i8* %5, i32 -4 # load ((&A + 2) - 1) == (&A + 1) ?
%7 = bitcast i8* %6 to i32*
%8 = load i32* %7, align 4
store i32 %8, i32* %B, align 4
%9 = load i32* %1, align 4
%10 = load i32* %B, align 4
%11 = call i32 (i8*, ...)* @​printf(i8* getelementptr inbounds ([13 x i8]* @.str, i32 0, i32 0), i32 %9, i32 %10)
store i8* null, i8** %L, align 4
ret void
}
...

[timurrrr]

Please disregard the "#" comments in c#1, I don't quite understand the llvm assembly yet.

Still, the reproducer gives a wrong result.

[timurrrr]

FTR, the va_arg on VS is a macro defined as:
#define _crt_va_arg(ap,t) ( *(t *)((ap += _INTSIZEOF(t)) - _INTSIZEOF(t)) )
#define va_arg _crt_va_arg

[nico]

Which stdarg.h include is this picking up? clang's or msvc's? clang doesn't support msvc's stdarg.h and doesn't plan to do so (see http://llvm.org/bugs/show_bug.cgi?id=12116#c1 ), so make sure you're it's picking up the one in llvm-build\bin\lib\include\clang\3.1\stdarg.h (you need to build the "clang-headers" target in msvc for that file to exist, then clang will use it automatically unless you build with -nobuiltininc).

[timurrrr]

Building clang-headers did help, thanks!

A few questions:

1. Why "clang" build target doesn't depend on "clang-headers" ?

2. Why have I got no error / warning about the missing headers?

3. Why don't we want to support MS's stdarg.h in the "-fms-compatibility" mode?

[nico]

Cool. Can this bug be closed then?

1. No idea, I'm not very familiar with the CMake-based build. Maybe just a bug?

2. Because clang finds msvc's headers and uses those instead. (If you pass -nobuiltininc, it will still do that.)

3. I don't know, but if Eli says that we don't want to do it, I'm sure there's a good reason. Eli, if you have time: Why don't we want to support msvc's stdarg.h?

[efriedma-quic]

3. I don't know, but if Eli says that we don't want to do it, I'm sure there's
   a good reason. Eli, if you have time: Why don't we want to support msvc's
   stdarg.h?

Without some specialized pattern recognition, msvc's va_start macro looks like pointer math with undefined behavior. We have to override it somehow, and just using our version seems much easier than pattern-matching the MSVC version back to our version.

[timurrrr]

Cool. Can this bug be closed then?

1. No idea, I'm not very familiar with the CMake-based build. Maybe just a bug?
   Looks like that's pretty much what this particular bug is all about :)

2. Because clang finds msvc's headers and uses those instead. (If you pass
   -nobuiltininc, it will still do that.)
   I'd like to have a warning / error message if possible.
   "Successfully" compiling and crashing at run-time on a simple C program with the default clang flags is a bad behavior IMO

[timurrrr]

FTR, calls between CL-compiled code and Clang-compiled code with ellipsis work fine for me if the correct stdarg.h is used.

[tritao]

Should a warning be added then if including the MS std*.h headers?

[AaronBallman]

When I run the clang example against ToT, I get the expected results:

c:\llvm>a.out
foo(1, 2)

So it seems we're picking up clang's stdarg.h instead of MSVC's (and using -### seems to bear this out). So the issue may be "resolved" at this point?

[AaronBallman]

*** Bug #12290 has been marked as a duplicate of this bug. ***

[rnk]

I think this bug still shows up, because CRT headers directly call _crt_va_start, which is defined like:
#define _crt_va_start(ap,v) ( ap = (va_list)_ADDRESSOF(v) + _INTSIZEOF(v) )

This test case still fails for me:

#include <stdarg.h>
#include <stdio.h>
void foo(int A, ...) {
int B;
va_list L;
_crt_va_start(L, A);
B = _crt_va_arg(L, int);
printf("foo(%d, %d)\n", A, B);
_crt_va_end(L);
}
int main(void) { foo(1, 2); }

[rnk]

Interestingly, we could "solve" this with inalloca, since it lets you truly take the address of incoming arguments.

[rnk]

Probably the best way to solve this is to shadow vadefs.h from Visual Studio.

Here's a program we fail to compile correctly with optimizations:

$ cat t.cpp
#include <stdio.h>
int main() {
wchar_t buffer[16];
_snwprintf_s(buffer, sizeof(buffer) / sizeof(wchar_t), L"0x%X", 0xdeadbeef);
printf("%S\n", buffer);
}

$ clang-cl t.cpp -O2 && ./t.exe
0x19FD44

Strangely it works w/o optimizations? That doesn't make sense.

$ clang-cl t.cpp && ./t.exe
0xDEADBEEF

[nico]

I think r219740 fixed this for bug 21247.

*** This bug has been marked as a duplicate of bug llvm/llvm-bugzilla-archive#21247 ***

[tritao]

mentioned in issue llvm/llvm-bugzilla-archive#13707

[nico]

mentioned in issue llvm/llvm-bugzilla-archive#21247

[rnk]

mentioned in issue llvm/llvm-bugzilla-archive#24320

[nico]

(This was duped into #21621)