LLVM miscompiles dlls/oleaut32/tmarshal.c from Wine

[llvmbot]

Bugzilla Link	9702
Resolution	INVALID
Resolved on	Dec 21, 2012 18:40
Version	trunk
OS	Linux
Blocks	llvm/llvm-bugzilla-archive#10638 llvm/llvm-bugzilla-archive#10682
Reporter	LLVM Bugzilla Contributor
CC	@asl,@efriedma-quic

Extended Description

Part 1 of a long series filed on behalf of Austin English.

Lots of Wine code is miscompiled. (Seriously, you should consider pulling Wine into LLVM's test suite. It's a huge compiler stress test.) One such source file is dlls/oleaut32/tmarshal.c. A .ll file will be attached (I don't have one on hand at the moment). This miscompilation results in yet another AV within Wine, breaking not one but two of Wine's regression tests (see http://bugs.winehq.org/show_bug.cgi?id=25808 and http://bugs.winehq.org/show_bug.cgi?id=25828).

This happens at all optimization levels. This probably happens without the integrated assembler as well.

[efriedma-quic]

Please attach preprocessed C, not .ll files, for reports against clang.

[llvmbot]

If I compile dlls/oleaut32/tmarshal.c with gcc, the original crash is gone, but I still get a crash:
Backtrace:
=>0 0x00000001 (0x6837108c)

compiling all of dlls/oleaut32/ with gcc and the rest of wine with clang has the same result. I'll attach tmarshal.i, which is the preprocessed C file for this crash:
Backtrace:
=>0 0x685697a3 serialize_param+0xd53(tinfo=0x126d48, writeit=0, debugout=0, dealloc=0, tdesc=0x1249e8, arg=0x3, buf=0x33f680) [/home/austin/src/wine-clang/dlls/oleaut32/tmarshal.c:785] in oleaut32 (0x0033f160)
1 0x685698e7 serialize_param+0xe96(tinfo=0x126d48, writeit=0, debugout=0, dealloc=0, tdesc=0x126de0, arg=0x33f6ac, buf=0x33f680) [/home/austin/src/wine-clang/dlls/oleaut32/tmarshal.c:793] in oleaut32 (0x0033f460)
2 0x6856d83c xCall+0x9fb(retptr=0x683715e7, method=0x3, tpinfo=0x129838) [/home/austin/src/wine-clang/dlls/oleaut32/tmarshal.c:1395] in oleaut32 (0x0033f6d0)
3 0x0055003c (0x0033fc68)
4 0x6837108c func_tmarshal+0xcb() [/home/austin/src/wine-clang/dlls/oleaut32/tests/tmarshal.c:1549] in oleaut32_test (0x0033fcb8)
5 0x684f6bbd run_test+0xcc(name="tmarshal.c") [/home/austin/src/wine-clang/dlls/oleaut32/tests/../../../include/wine/test.h:556] in oleaut32_test (0x0033fd28)
6 0x684f69ed main+0x35c(argc=0x2, argv=0x1103f8) [/home/austin/src/wine-clang/dlls/oleaut32/tests/../../../include/wine/test.h:624] in oleaut32_test (0x0033fe08)
7 0x684f7061 __wine_spec_exe_entry+0xa0(peb=(nil)) [/home/austin/src/wine-clang/dlls/winecrt0/exe_entry.c:36] in oleaut32_test (0x0033fe50)
8 0x7b87abac call_process_entry+0xb() in kernel32 (0x0033fe68)
9 0x7b87ef76 start_process+0x1d5(peb=0xffffffff) in kernel32 (0x0033fec8)
10 0x7bca5e54 call_thread_func+0xb() in ntdll (0x0033fed8)
11 0x7bca99bd call_thread_entry_point+0x5c(entry=0x7b87eda0, arg=0x7ffdf000) in ntdll (0x0033ffc8)
12 0x7bc6731a start_process+0x39(kernel_start=0x7b87eda0) in ntdll (0x0033ffe8)
13 0x6802ae7d wine_call_on_stack+0x1c() in libwine.so.1 (0x00000000)

[llvmbot]

tmarshal.i, bzip2 -9'ed

[llvmbot]

This also fails under Dragonegg SVN (r137784) (breaks dlls/ieframe/tests/ie.c test).

[llvmbot]

Still present, as of
git-svn-id: http://llvm.org/svn/llvm-project/llvm/trunk@150433 91177308-0d34-0410-b5e6-96231b3b80d8

and wine-1.4-rc3-23-gd548af4. Not a stack alignment issue, the wine patch from bug 9830 does not help here.

[llvmbot]

Still present in:
wine-1.5.19-186-g1cd0c4a
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@170295 91177308-0d3
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@170294 91177308-0d34

[llvmbot]

Still present in:
wine-1.5.19-186-g1cd0c4a
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@170295 91177308-0d3
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@170294 91177308-0d34

Follow up,

I've noticed that this is the only wine test failure I that I don't have with gcc on my machine.

Exact versions:
austin@debian-home:~$ clang --version
clang version 3.3 (git://github.com/llvm-mirror/clang.git 4f35f74017aa581986927bb3602d98a1ce969093) (git://github.com/llvm-mirror/llvm.git e4e0089)
Target: i386-pc-linux-gnu
Thread model: posix

austin@debian-home:~$ gcc --version
gcc (Debian 4.7.2-4) 4.7.2
Copyright (C) 2012 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[llvmbot]

(Fixing title to be more specific)

Still present in:
wine-1.5.19-186-g1cd0c4a
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@170295 91177308-0d3
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@170294 91177308-0d34

Follow up,

I've noticed that this is the only wine test failure I that I don't have with
gcc on my machine.
Yeah, I've noticed this is still present on Mac OS, too. This one looks like a real miscompilation (unlike most of the others, which were either stack alignment mismatches or in one case UB in Wine). Then again, this one might also be UB in Wine.

Seriously, this is some crazy stuff in here. This particular file will actually emit x86 machine code to be executed later (i.e. in a COM type-library marshaller stub-less proxy call; cf. init_proxy_entry_point() starting on line 1735 in the original source). Then, when someone makes a COM v-call on the proxy object, one of these machine-code stubs will actually be executed.

More investigation is required. I'll get back to you if I figure out exactly how LLVM is miscompiling this (if at all).

Exact versions:
austin@debian-home:~$ clang --version
clang version 3.3 (git://github.com/llvm-mirror/clang.git
4f35f74017aa581986927bb3602d98a1ce969093)
(git://github.com/llvm-mirror/llvm.git
e4e0089)
Target: i386-pc-linux-gnu
Thread model: posix

austin@debian-home:~$ gcc --version
gcc (Debian 4.7.2-4) 4.7.2
Copyright (C) 2012 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[llvmbot]

There's actually two crashes here. The first is another stack-alignment issue (the xCall() function needs to force-align its stack). But the second is another matter entirely. I don't even know if you'll see it.

The second crash I'm seeing is in test_typelibmarshal() (running the oleaut32:tmarshal test), after the RPC to KindaEnum::Next() returns. The method succeeded, and yet it returns a null pointer. I have no idea why this happens, but I'm going to go out on a limb here and say that somehow, the pointer isn't getting marshaled properly on return.

Another, possibly related issue is that this particular code only works at all at -O2. Below that, and I get strange segfaults in memset(3) when the stubless proxy marshals pointers to the callee--probably related to the way it grabs the arguments to the target method off the stack (it actually takes the address of its last parameter, then walks up the stack, assuming right-to-left argument passing order). Could this actually "work" at -O2 by accident, getting the wrong pointer off the stack and writing to who knows where? Could this be why we're crashing and burning after the RPC completes?

I wish I knew more about how COM RPCs work in Wine. Maybe someone over there knows more.

[llvmbot]

Another, possibly related issue is that this particular code only works at all
at -O2. Below that, and I get strange segfaults in memset(3) when the stubless
proxy marshals pointers to the callee--probably related to the way it grabs the
arguments to the target method off the stack (it actually takes the address of
its last parameter, then walks up the stack, assuming right-to-left argument
passing order). Could this actually "work" at -O2 by accident, getting the
wrong pointer off the stack and writing to who knows where? Could this be why
we're crashing and burning after the RPC completes?
Yeah, it's definitely this. The ieframe:ie test crashes the same way at -O2. So it was both stack misalignment and UB :). (At least, that's what I'm getting on my system.)

I'll wait for Austin to confirm that this is indeed the problem, and then decide what to do about this bug.

[llvmbot]

Another, possibly related issue is that this particular code only works at all
at -O2. Below that, and I get strange segfaults in memset(3) when the stubless
proxy marshals pointers to the callee--probably related to the way it grabs the
arguments to the target method off the stack (it actually takes the address of
its last parameter, then walks up the stack, assuming right-to-left argument
passing order). Could this actually "work" at -O2 by accident, getting the
wrong pointer off the stack and writing to who knows where? Could this be why
we're crashing and burning after the RPC completes?
Yeah, it's definitely this. The ieframe:ie test crashes the same way at -O2. So
it was both stack misalignment and UB :). (At least, that's what I'm getting
on my system.)

I'll wait for Austin to confirm that this is indeed the problem, and then
decide what to do about this bug.

Yes, I can confirm this.

However, it was fixed upstream in Wine today, by:
http://source.winehq.org/git/wine.git/commitdiff/2915e47979e31a8911a4f95d2d90264e6d6a0f40

the previous code works on gcc, though, so may still be a valid (albeit less important) bug.

[efriedma-quic]

static DWORD
xCall(LPVOID retptr, int method, TMProxyImpl tpinfo /, args */)
{
DWORD args = ((DWORD)&tpinfo)+1, *xargs;

The subsequent use of "args" is undefined behavior. tpinfo is a single pointer-sized variable; we make no guarantees about the layout of other variables/arguments/etc. relative to it. The only supported way to do varargs in C is to use stdarg.h.