Bug 16243 – wrong C++ argument passing with empty struct when interfacing with Clang

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P3
Component
dmd
Product
D
Version
D2
Platform
x86
OS
Mac OS X
Creation time
2016-07-06T18:38:41Z
Last change time
2018-09-05T21:40:29Z
Keywords
C++, wrong-code
Assigned to
No Owner
Creator
Martin Nowak

Comments

Comment #0 by code — 2016-07-06T18:38:41Z
The bug is OSX-32 only, so not that important (P3), but looks easy to fix. cat > bug.d << CODE struct S13956 { } extern(C++) void func13956(S13956 arg0, int arg1, int arg2, int arg3, int arg4, int arg5, int arg6); extern(C++) void check13956(S13956 arg0, int arg1, int arg2, int arg3, int arg4, int arg5, int arg6) { assert(arg0 == S13956()); assert(arg1 == 1); assert(arg2 == 2); assert(arg3 == 3); assert(arg4 == 4); assert(arg5 == 5); assert(arg6 == 6); // fails on OSX 32-bit } void main() { func13956(S13956(), 1, 2, 3, 4, 5, 6); } CODE dmd -m32 -run bug ---- The arg6 parameter is uninitialized (or maybe a pointer). See disabled test case here https://github.com/dlang/dmd/pull/5915/commits/6e6c6ed9db5935fb36e9b06e309cea6be7afa9b1.
Comment #1 by code — 2016-07-06T18:55:12Z
Complete test case, also requires a C++ roundtrip. C++ compiler is. ---- Apple LLVM version 5.0 (clang-500.2.79) (based on LLVM 3.3svn) Target: x86_64-apple-darwin12.2.0 Thread model: posix ---- cat > cpp.cc << CODE struct S13956 { }; void check13956(S13956 arg0, int arg1, int arg2, int arg3, int arg4, int arg5, int arg6); void func13956(S13956 arg0, int arg1, int arg2, int arg3, int arg4, int arg5, int arg6) { check13956(arg0, arg1, arg2, arg3, arg4, arg5, arg6); } CODE cat > bug.d << CODE struct S13956 { } extern(C++) void func13956(S13956 arg0, int arg1, int arg2, int arg3, int arg4, int arg5, int arg6) { check13956(arg0, arg1, arg2, arg3, arg4, arg5, arg6); } extern(C++) void check13956(S13956 arg0, int arg1, int arg2, int arg3, int arg4, int arg5, int arg6) { assert(arg0 == S13956()); assert(arg1 == 1); assert(arg2 == 2); assert(arg3 == 3); assert(arg4 == 4); assert(arg5 == 5); assert(arg6 == 6); // fails on OSX 32-bit } void main() { func13956(S13956(), 1, 2, 3, 4, 5, 6); } CODE c++ -m32 -c cpp.cc dmd -m32 cpp.o -run bug
Comment #2 by code — 2016-07-07T12:32:11Z
Also fails with a single int argument. cat > cpp.cc << CODE #include <stdio.h> struct S13956 { }; void check13956(S13956 arg0, int arg1); void func13956(S13956 arg0, int arg1) { printf("C %d\n", arg1); check13956(arg0, arg1); } CODE cat > bug.d << CODE import core.stdc.stdio; struct S13956 { } extern(C++) void func13956(S13956 arg0, int arg1) { check13956(arg0, arg1); } extern(C++) void check13956(S13956 arg0, int arg1) { printf("C %d\n", arg1); assert(arg0 == S13956()); assert(arg1 == 1); } void main() { func13956(S13956(), 1); } CODE c++ -m32 -c cpp.cc dmd -m32 cpp.o -run bug
Comment #3 by code — 2016-07-24T14:09:16Z
Now this also fails on the OSX-64/32 test on the auto-tester, so this might actually be a relevant regression.
Comment #4 by kinke — 2016-07-24T17:47:42Z
clang doesn't pass empty structs at all for 32-bit, while GCC does. We have such a special case in LDC too for 32-bit OSX, where we assume clang, but not for 32-bit Linux, where we assume GCC. See https://github.com/ldc-developers/ldc/blob/master/gen/abi-x86.cpp#L214.
Comment #5 by bugzilla — 2016-07-27T05:56:23Z
So the regression is that Apple switched to clang, and clang does things differently?
Comment #6 by doob — 2016-07-27T06:25:50Z
(In reply to Walter Bright from comment #5) > So the regression is that Apple switched to clang, and clang does things > differently? There's a long time since Apple switched to Clang. GCC is dead on Apple platforms. I've mentioned this several times that the autotester should switch to Clang.
Comment #7 by code — 2016-09-12T04:24:24Z
(In reply to kinke from comment #4) > clang doesn't pass empty structs at all for 32-bit, while GCC does. We have > such a special case in LDC too for 32-bit OSX, where we assume clang, but > not for 32-bit Linux, where we assume GCC. See > https://github.com/ldc-developers/ldc/blob/master/gen/abi-x86.cpp#L214. Thanks, assuming clang by default on OSX indeed seems like a reasonable solution. We also have that issue with GCC5's C++ ABI change (http://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/), so detecting the actual compiler might be helpful as well. (In reply to Jacob Carlborg from comment #6) > There's a long time since Apple switched to Clang. GCC is dead on Apple > platforms. I've mentioned this several times that the autotester should > switch to Clang. Yes, would be good if we updated the auto-testers to use clang @Brad.
Comment #8 by doob — 2016-09-12T06:55:28Z
(In reply to Martin Nowak from comment #7) > We also have that issue with GCC5's C++ ABI change > (http://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/), so > detecting the actual compiler might be helpful as well. As far as I understand, it's configurable if that ABI should be used or not. So just detecting the compiler is not enough, unless we decide to only support one ABI.
Comment #9 by doob — 2016-09-12T07:59:04Z
(In reply to kinke from comment #4) > clang doesn't pass empty structs at all for 32-bit, while GCC does. We have > such a special case in LDC too for 32-bit OSX, where we assume clang, but > not for 32-bit Linux, where we assume GCC. See > https://github.com/ldc-developers/ldc/blob/master/gen/abi-x86.cpp#L214. Clang claims to be compatible with GCC. If there's a case when it's not compatible, it might be a bug in Clang.
Comment #10 by dbugz — 2016-09-14T05:36:56Z
I ran into this on Android/ARM too, with ldc. As the linked ldc comment and Jacob note, this is an incompatibility in the way clang and gcc work with empty structs on every platform, whether Linux/x86 or Android/ARM. This test and the auto-tester simply use gcc on every other tested platform, while clang is the system compiler on OS X now.
Comment #11 by braddr — 2016-09-19T04:55:39Z
I haven't tested building with clang on osx in ages. If it's at the point of passing tests then I'd be happy to convert any/all of the three testers back to their default compiler.
Comment #12 by doob — 2016-09-19T06:20:26Z
I've been using Clang for ages now and it passes the test suite, at least for 64bit. I'm running Apple LLVM version 7.3.0 (clang-703.0.31)
Comment #13 by dbugz — 2017-05-20T10:13:23Z
I spent a little time looking into this again. The issue now appears to only be reproducible if clang compiles without any optimizations, which is what the dmd testsuite does. When run without any optimizations, clang assumes the empty struct will not be passed in and only passes along the first six arguments, ie arg0 through arg5. Since arg6 is not passed back to check13956, it asserts. These issues only crop up on 32-bit arches, including ARM32, as kinke noted last summer, which explains why Jacob hasn't seen this on 64-bit OS X. Here is the disassembly for what clang produces on linux/x86 without optimization: 00000000 <_Z9func139566S13956iiiiii>: 0: 55 push %ebp 1: 89 e5 mov %esp,%ebp 3: 53 push %ebx 4: 57 push %edi 5: 56 push %esi 6: 83 ec 3c sub $0x3c,%esp 9: 8b 45 1c mov 0x1c(%ebp),%eax c: 8b 4d 18 mov 0x18(%ebp),%ecx f: 8b 55 14 mov 0x14(%ebp),%edx 12: 8b 75 10 mov 0x10(%ebp),%esi 15: 8b 7d 0c mov 0xc(%ebp),%edi 18: 8b 5d 08 mov 0x8(%ebp),%ebx 1b: 89 5d ec mov %ebx,-0x14(%ebp) 1e: 89 7d e8 mov %edi,-0x18(%ebp) 21: 89 75 e4 mov %esi,-0x1c(%ebp) 24: 89 55 e0 mov %edx,-0x20(%ebp) 27: 89 4d dc mov %ecx,-0x24(%ebp) 2a: 89 45 d8 mov %eax,-0x28(%ebp) 2d: 8b 45 ec mov -0x14(%ebp),%eax 30: 8b 4d e8 mov -0x18(%ebp),%ecx 33: 8b 55 e4 mov -0x1c(%ebp),%edx 36: 8b 75 e0 mov -0x20(%ebp),%esi 39: 8b 7d dc mov -0x24(%ebp),%edi 3c: 8b 5d d8 mov -0x28(%ebp),%ebx 3f: 89 04 24 mov %eax,(%esp) 42: 89 4c 24 04 mov %ecx,0x4(%esp) 46: 89 54 24 08 mov %edx,0x8(%esp) 4a: 89 74 24 0c mov %esi,0xc(%esp) 4e: 89 7c 24 10 mov %edi,0x10(%esp) 52: 89 5c 24 14 mov %ebx,0x14(%esp) 56: e8 fc ff ff ff call 57 <_Z9func139566S13956iiiiii+0x57> 57: R_386_PC32 _Z10check139566S13956iiiiii 5b: 83 c4 3c add $0x3c,%esp 5e: 5e pop %esi 5f: 5f pop %edi 60: 5b pop %ebx 61: 5d pop %ebp 62: c3 ret Conversely, here's what gcc produces: 00000000 <_Z9func139566S13956iiiiii>: 0: 55 push %ebp 1: 89 e5 mov %esp,%ebp 3: 83 ec 08 sub $0x8,%esp 6: 83 ec 04 sub $0x4,%esp 9: ff 75 20 pushl 0x20(%ebp) c: ff 75 1c pushl 0x1c(%ebp) f: ff 75 18 pushl 0x18(%ebp) 12: ff 75 14 pushl 0x14(%ebp) 15: ff 75 10 pushl 0x10(%ebp) 18: ff 75 0c pushl 0xc(%ebp) 1b: 50 push %eax 1c: e8 fc ff ff ff call 1d <_Z9func139566S13956iiiiii+0x1d> 1d: R_386_PC32 _Z10check139566S13956iiiiii 21: 83 c4 20 add $0x20,%esp 24: 90 nop 25: c9 leave 26: c3 ret When clang is run with optimization, it just calls the D function directly, which happens to avoid the problem, though likely the underlying assumption is still there: 00000000 <_Z9func139566S13956iiiiii>: 0: e9 fc ff ff ff jmp 1 <_Z9func139566S13956iiiiii+0x1> 1: R_386_PC32 _Z10check139566S13956iiiiii I'm not sure if clang with optimization always did this: just don't remember if I tried it before, maybe not. As kinke said, clang and gcc assume different argument passing for empty structs on 32-bit arches, confirmed for both x86 and ARM32. dmd and ldc work with the gcc approach, but not clang's.
Comment #14 by doob — 2017-05-20T12:29:16Z
(In reply to Joakim from comment #13) > dmd and ldc work with the gcc approach, but not clang's. Clang is the native compiler on macOS, so I would say that we should go with whatever Clang is doing.
Comment #15 by bugzilla — 2018-02-24T10:42:23Z
*** Issue 18514 has been marked as a duplicate of this issue. ***
Comment #16 by bugzilla — 2018-02-24T10:43:58Z
With the upgrade to FreeBSD 11, this is now happening on 32 bit FreeBSD.
Comment #17 by bugzilla — 2018-02-24T10:57:10Z
On Linux x86, given the code: struct S { }; int test(S s, int a) { return a; } g++ produces: mov EAX,8[ESP] ret while clang++ produces: mov EAX,4[ESP] ret This is with optimization on. With it off, the same problem.
Comment #18 by bugzilla — 2018-02-24T20:37:57Z
Temporary workaround
Comment #19 by bugzilla — 2018-02-24T20:40:02Z
(In reply to Walter Bright from comment #18) > Temporary workaround https://github.com/dlang/dmd/pull/7952
Comment #20 by github-bugzilla — 2018-02-24T21:12:38Z
Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/6966e6e0a9bf4ddf2760a46619e0c247ae8405ff Temporary workaround for Issue 16243 https://github.com/dlang/dmd/commit/7969814467287d19726c81886c27e0692581092f Merge pull request #7952 from WalterBright/workaround16243 Temporary workaround for Issue 16243
Comment #21 by bugzilla — 2018-02-28T06:52:00Z
Comment #22 by github-bugzilla — 2018-03-03T00:17:28Z
Commit pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/3ec7537f76142b23f5eed062f0420a20abcc0f9b Temporary workaround for Issue 16243 (#7970)
Comment #23 by github-bugzilla — 2018-03-03T15:17:37Z
Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/cc2969695518e87d3be9a21d18689db46a8a0fae fix Issue 16243 - wrong C++ argument passing with empty struct when interfacing with Clang https://github.com/dlang/dmd/commit/5fa9df9ebbfabd109dd20895d86b48eee636e1c6 Merge pull request #7963 from WalterBright/fix16243 fix Issue 16243 - wrong C++ argument passing with empty struct when i…
Comment #24 by code — 2018-09-05T17:04:11Z
Comment #25 by ibuclaw — 2018-09-05T21:40:29Z
(In reply to David Nadlinger from comment #24) > cf. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60336 (per @ibuclaw) Thanks. To summarize the above GCC bug. They have recognized the difference in empty struct sizes as a bug, and this is now fixed in GCC 8. So C, C++, GCC, and Clang should all pass empty structs in the same way - rather than two different ways. How this affects dmd? I guess you will need to revert whatever workarounds you've applied in order to support the double behaviour.