Bug 5570 – 64 bit C ABI not followed for passing structs and complex numbers as function parameters

Status
RESOLVED
Resolution
FIXED
Severity
blocker
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2011-02-13T15:08:20Z
Last change time
2020-08-31T20:21:20Z
Keywords
backend, bounty, C++, pull, wrong-code
Assigned to
No Owner
Creator
Walter Bright
Depends on
9239
See also
https://issues.dlang.org/show_bug.cgi?id=12862, https://issues.dlang.org/show_bug.cgi?id=12343, https://issues.dlang.org/show_bug.cgi?id=13957, https://issues.dlang.org/show_bug.cgi?id=19179

Comments

Comment #0 by bugzilla — 2011-02-13T15:08:20Z
They are always pushed on the stack rather than sometimes being passed in registers. Haven't got to fixing this yet.
Comment #1 by leandro.lucarella — 2011-12-16T04:20:00Z
Any ETA for a fix?
Comment #2 by mathias.baumann — 2011-12-16T06:13:08Z
This bug prevents us from switching to 64bit. We really would appreciate a fix.
Comment #3 by nazriel6969 — 2011-12-16T06:45:46Z
Because of this bug, you can not use wxD with DMD64.
Comment #4 by siegelords_abode — 2011-12-17T11:49:28Z
See also http://d.puremagic.com/issues/show_bug.cgi?id=6348. It's plain impossible to use C libraries with value semantics for structs on 64 bits.
Comment #5 by hoganmeier — 2012-02-01T08:08:14Z
So should we raise this to 'blocker'?
Comment #6 by grahamc001uk — 2012-02-01T14:42:02Z
Issue 6772 http://d.puremagic.com/issues/show_bug.cgi?id=6772 "Cannot pass cfloat argument type to a function on x86_64" is related or possible duplicate.
Comment #7 by bus_dbugzilla — 2012-04-09T22:12:13Z
"So should we raise this to 'blocker'?" I would argue "yes" since the *only* way to work around it if you can't modify the C side is to just not use 64-bit D. So it is a blocker for 64-bit D. (And AIUI, not all 64-bit systems are multilib, so even switching to 32 isn't always going to be an option.) Plus, there's...how many people *right here* have identified it as blocking them? Fuck, I'm just going to go ahead and raise it to blocker. If anyone has a problem with that they can change it back. Either way, the important thing is for this to get fixed, not to worry about its proper classification. So I don't know why I even wrote all this...Oh well... ;)
Comment #8 by public — 2012-04-10T06:04:56Z
If bug preventing almost any x64 D code interconnecting with C libs is not a blocker, I can hardly imagine what is.
Comment #9 by issues.dlang — 2012-04-10T10:11:53Z
> If bug preventing almost any x64 D code interconnecting with C libs is not a > blocker, I can hardly imagine what is. I don't know how Walter decides that sort of thing or how he treats blockers. I believe that the worse that we normally see is critical. But this is something which has _never_ worked, and it's in a newer feature - 64-bit code generation - and it's something that not everyone is using. So, something which was in D itself (as opposed to how it talks with C code) which used to work but doesn't now could certainly be more of a blocker than this, depending on what it was. But in general, 64-bit specific stuff is less critical in that it only affects those using 64-bit rather than _everyone_ like many bugs do. I would actually expect Walter to consider this critical rather than a blocker. Certainly, if blocker indicates that something should block a release, this isn't it, since the bug has been around for as long as dmd's 64-bit code generation has been. But that's up to him. Regardless, this is a huge issue, and it should probably be treated as a higher priority than it has been, but much of what _has_ been being worked on is high priority stuff which affects much more D code than this does, so I suppose that it's not all that surprising that Walter hasn't gotten around to this yet. Still, I'd hope that this would be taken care of sooner rather than later.
Comment #10 by public — 2012-04-11T04:23:31Z
(In reply to comment #9) > So, something which was in D > itself (as opposed to how it talks with C code) which used to work but doesn't > now could certainly be more of a blocker than this, My point exactly is that interfacing to C broken at ABI level is much more important than any inside language issue. Regressions are more important, yes, but, well, there is an importance level "regression" before "blocker" exactly for them. I'd say it is in the same category as "wrong code generation" for basic language constructs. It is something you rarely expect from even the new language when facing problems, something rather hard to identify and something almost any desktop D user is doomed to meet when try to build something usable ( as most usable programs tend to involve C bindings at this stage of std lib ). Issue a warning that it can happen when compiling extern(C) stuff of x64 at least. tl;dr : it is really bad for marketing
Comment #11 by issues.dlang — 2012-04-11T09:51:04Z
In general, something which has never worked is going to be treated as less of a priority than something which worked before and doesn't now - _especially_ when it's part of a newer feature. And this has _never_ worked correctly. And I don't know why you would think that a C ABI issue would be more important than an issue in D itself. As bad as this is, most D code is completely unaffected by this. It's only once you start dealing with C and structs that it matters. Now, why Walter hasn't this considered a high enough priority to fix it yet, I don't know. This is clearly a serious bug, and I would have thought that he would have made it a fairly high priority once he figured out that it was a problem, but for whatever reason, he hasn't. There are plenty of other high priority issues that he's been working on though - many of which have been around much longer than this. Still, one would hope that this wouldn't languish much longer. It's a major impedement to using 64-bit on D projects. By the way, for those that this is actually preventing from using 64-bit dmd and who really need it, gdc probably doesn't have this problem, since it relates to code generation. So, you could try that while waiting for Walter to get around to this.
Comment #12 by leandro.lucarella — 2012-04-16T03:27:08Z
(In reply to comment #11) > In general, something which has never worked is going to be treated as less of > a priority than something which worked before and doesn't now - _especially_ > when it's part of a newer feature. Yeah, and you know that something that worked before and doesn't work now is exactly what's called a *regression*, which have higher priority than *blocker*. So we all agree on that :) > And this has _never_ worked correctly. And I > don't know why you would think that a C ABI issue would be more important than > an issue in D itself. As bad as this is, most D code is completely unaffected > by this. It's only once you start dealing with C and structs that it matters. And that is a lot of codebase, you know? For example wxD, as Damian Ziemba pointed out can't work without this fixed. Also, and probably much less important than wxD, is making life at the company I work a little miserable trying to migrate to 64bit with this bug around. AFAIK Walter cares about big projects as wxD and making D usable for companies to consider this bug a blocker when it really blocks that kind of uses.
Comment #13 by thepumpkin1979 — 2012-04-25T11:19:32Z
This is definitely a blocker, please fix this, this is a huge issue on OSX too.
Comment #14 by james — 2012-05-03T01:13:16Z
Throwing my vote in for this one, this is pretty much stopping me from using DMD64 for my C bindings. For the record, LDC works in this case.
Comment #15 by github-bugzilla — 2012-05-07T12:29:35Z
Comment #16 by github-bugzilla — 2012-05-07T12:29:45Z
Comment #17 by thepumpkin1979 — 2012-05-07T12:40:48Z
I'm very happy to see some commits on this issue, this is really important!!! Walter++!!!
Comment #18 by clugdbug — 2012-05-23T01:18:18Z
Comment #19 by leandro.lucarella — 2012-05-31T04:29:21Z
Just a simple example (I've used to see if the progress on the bug fixed some of the problems I had :): --- extern (C) { struct lldiv_t { long quot, rem; } lldiv_t lldiv(long numer, long denom); int printf(char* fmt, ...); } void main(char[][] args) { auto r = lldiv(94, 82); printf("%lld %lld\n", r.quot, r.rem); } --- For me, returns a high, randomish (changes on each run) number and 3 or 1. Examples: 20967488 3 32686144 3 38604832 1 20029472 1
Comment #20 by bugzilla — 2012-05-31T12:39:49Z
(In reply to comment #19) > Just a simple example (I've used to see if the progress on the bug fixed some > of the problems I had :): > > --- > extern (C) > { > struct lldiv_t > { > long quot, > rem; > } The current state only fixes things that fit in one register. The lldiv_t is a two register struct.
Comment #21 by code — 2012-06-17T14:50:19Z
When fixing the remaining issues, please also consider treating dynamic D arrays as »struct Array(T) { T* ptr; size_t length; }« on x86_64, i.e. passing them in two integer registers (if available). This is what GDC and LDC are doing right now, since always passing them on the stack, like DMD does right now, would require quite a lot of extra effort in resp. additions to the respective backend code. There currently is a »pass on the stack for efficiency« comment in TypeDArray::toArgTypes(), but I can't quite see why this should be true in the general case, to be honest.
Comment #22 by code — 2012-06-20T09:25:24Z
(In reply to comment #21) > When fixing the remaining issues, please also consider treating dynamic D > arrays as »struct Array(T) { T* ptr; size_t length; }« on x86_64, i.e. passing > them in two integer registers (if available). The fields of the struct should obviously have been swapped. In any case, this has been addressed in https://github.com/D-Programming-Language/dmd/commit/f50a339b86d9d2c141061d38f4f682211c3c07c3 and related commits – whether this was a coincidence or not, thanks a lot for the quick fix!
Comment #23 by code — 2012-06-20T09:25:25Z
(In reply to comment #21) > When fixing the remaining issues, please also consider treating dynamic D > arrays as »struct Array(T) { T* ptr; size_t length; }« on x86_64, i.e. passing > them in two integer registers (if available). The fields of the struct should obviously have been swapped. In any case, this has been addressed in https://github.com/D-Programming-Language/dmd/commit/f50a339b86d9d2c141061d38f4f682211c3c07c3 and related commits – whether this was a coincidence or not, thanks a lot for the quick fix!
Comment #24 by fawzi — 2012-06-25T05:03:48Z
see Bug 8294, for something probably related with the work being done here...
Comment #25 by code — 2012-06-26T04:48:42Z
Sigh – seems like I was not exactly right about how GDC and LDC are handling arrays. Instead of treating them like the equivalent struct, they are treated as if length and pointer were two separate arguments, with the important difference of course being that with the x86_64 ABI, structs are never only partly passed in registers (i.e., if there is only one register left, they would still pass the length in it and only push the pointer on the stack). The LDC x86_64 ABI implementation has had the following explanatory comment since it was originally written in 2009: > This helps make things like printf("%.*s", o.toString()) work as expected; if we didn't do this that wouldn't work if there were 4 other integer/pointer arguments before the toString() call because the string got bumped to memory with one integer register still free. Changing the implementation to treat dynamic arrays/delegates the same as two-element structs for a potentially easier implementation of va_arg, etc. wouldn't be a problem, but we need to make a decision and, most importantly, properly document the expected behavior on dlang.org.
Comment #26 by ibuclaw — 2012-06-26T07:16:09Z
(In reply to comment #25) > Sigh – seems like I was not exactly right about how GDC and LDC are handling > arrays. Instead of treating them like the equivalent struct, they are treated > as if length and pointer were two separate arguments, with the important > difference of course being that with the x86_64 ABI, structs are never only > partly passed in registers (i.e., if there is only one register left, they > would still pass the length in it and only push the pointer on the stack). > They are created as a two field struct in GDC. eg, delegates (D arrays are a little above) https://github.com/D-Programming-GDC/GDC/blob/master/gcc/d/d-glue.cc#L3801 which calls: https://github.com/D-Programming-GDC/GDC/blob/master/gcc/d/d-codegen.cc#L1514 > The LDC x86_64 ABI implementation has had the following explanatory comment > since it was originally written in 2009: > > > This helps make things like printf("%.*s", o.toString()) work as expected; if we didn't do this that wouldn't work if there were 4 other integer/pointer arguments before the toString() call because the string got bumped to memory with one integer register still free. > "%*.s" works purely out of coincidence. You should not rely on it working at all - and if you are, you should really instead be fixing your program. Regards Iain
Comment #27 by code — 2012-06-26T09:33:02Z
(In reply to comment #26) > (In reply to comment #25) > > Sigh – seems like I was not exactly right about how GDC and LDC are handling > > arrays. Instead of treating them like the equivalent struct, they are treated > > as if length and pointer were two separate arguments […] > They are created as a two field struct in GDC. Oh well, apparently GDC handles dynamic arrays like structs in most cases, but as size_t/void* pairs for variadic arguments, ABI-wise – I discovered this behavior looking at the generated assembly while working on the LDC vararg ABI, and didn't expect formal arguments to be treated differently. Maybe the behavior should be unified? > "%*.s" works purely out of coincidence. You should not rely on it working at > all - and if you are, you should really instead be fixing your program. It does _not_ work only out of coincidence with LDC, as the ABI it is using was apparently explicitly designed by Frits to support this, judging from the comment I quoted before. It's platform-dependent, yes, but guaranteed to work – with GDC/LDC, that is, as this official ABI docs don't specify any details for passing array arguments. I suppose this was done to support code which assumes x86 behavior. In any case, I can't see much value in having it like this, and would certainly find just treating dynamic arrays as structs more natural. I just wanted to highlight that this needs to be discussed and probably documented.
Comment #28 by bugzilla — 2012-06-26T11:19:22Z
My intention is to have the following all behave the same way: delegate struct S { void*, void* } void*[2] for parameter passing. The same goes for dynamic arrays. That means if you wrap a type in a struct, it will still pass the same way. The idea of making %.*s work with strings (without using .length and .ptr) has been deprecated for a long time, it isn't worth it.
Comment #29 by ibuclaw — 2012-06-27T10:02:08Z
(In reply to comment #27) > (In reply to comment #26) > > (In reply to comment #25) > > > Sigh – seems like I was not exactly right about how GDC and LDC are handling > > > arrays. Instead of treating them like the equivalent struct, they are treated > > > as if length and pointer were two separate arguments […] > > They are created as a two field struct in GDC. > > Oh well, apparently GDC handles dynamic arrays like structs in most cases, but > as size_t/void* pairs for variadic arguments, ABI-wise – I discovered this > behavior looking at the generated assembly while working on the LDC vararg ABI, > and didn't expect formal arguments to be treated differently. Maybe the > behavior should be unified? > Yes, that is correct for dynamic arrays. :~) It does it on a condition that's pre-set as 'true' and can't be altered by the user. Of course, if all two field types in D (D arrays; delegates) should *always* be passed in the same way as two field struct should be, then I can turn it off (and add a switch to toggle it), or remove this code.
Comment #30 by clugdbug — 2012-07-30T02:01:12Z
Progress at DMD1.075/2.060 beta2: This now works for structs which contain integral types only, or which contain floating point types only. It doesn't work if you have an int member and a float member.
Comment #31 by adrian — 2012-10-12T23:32:04Z
*** Issue 8810 has been marked as a duplicate of this issue. ***
Comment #32 by andrej.mitrovich — 2012-10-22T12:47:53Z
(In reply to comment #30) > Progress at DMD1.075/2.060 beta2: > This now works for structs which contain integral types only, or which contain > floating point types only. It doesn't work if you have an int member and a > float member. http://dpaste.dzfl.pl/f1ac00d2 Output is Test(5, 6, 5, 6) instead of Test(5, 6, 7, 8). It works OK if the function is extern(C).
Comment #33 by code — 2012-12-29T18:04:07Z
I added issue 9239 as a blocker of this, as it also concerns the System V x86-64 ABI.
Comment #34 by Marco.Leise — 2013-02-06T03:59:36Z
Is this why my XCB calls on Linux fail? I already accused XCB people of writing incorrect tutorials until I realized that the exact same code does work in C. After probably two hours of trial and error to fix my code and reading articles on the web the word x86-64 showed up there and I had a dim memory of something not working in DMD 64-bit and C structs. So I tried GDC again, which I stopped because it produced another error somewhere else. There it works. The struct I try to get returned by a C library function looks like this: struct { void*, int, int } Most notably the int fields in my case should be 116 and 1, but are 0 and 0, while the pointer does have some value (not sure if the correct one as they change from executable to executable).
Comment #35 by github-bugzilla — 2013-02-23T15:10:03Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/8fbb9279d51605d74824789d4954477b714bef52 64 bit ABI struct returns - Issue 5570 again https://github.com/D-Programming-Language/dmd/commit/a39da1070a5dbc84ecc1cd509ea85634ffdd7081 Merge pull request #1630 from Govelius/8fbb9279d51605d74824789d4954477b714bef52 64 bit ABI 16byte struct returns - Issue 5570 again
Comment #36 by github-bugzilla — 2013-02-25T10:38:47Z
Commit pushed to dmd-1.x at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/b2da13f39701796a6934e200f7c024305bd0c9e8 Merge pull request #1630 from Govelius/8fbb9279d51605d74824789d4954477b714bef52 64 bit ABI 16byte struct returns - Issue 5570 again
Comment #37 by code — 2013-06-06T13:18:21Z
Is this finally finished?
Comment #38 by code — 2013-06-06T13:20:26Z
I don't think the three byte struct issue (see linked bug) has been fixed yet, but I haven't checked in a while.
Comment #39 by thelastmammoth — 2013-09-21T19:48:09Z
(In reply to comment #38) > I don't think the three byte struct issue (see linked bug) has been fixed yet, > but I haven't checked in a while. has not been fixed. was going to submit a bug report until I saw this thread. problem with: struct VideoMode{ uint width; uint height; uint bitsPerPixel; } sfWindow* sfWindow_create(sfVideoMode mode, const(char)* title, uint style, const(ContextSettings)* settings); This is used in SFML-D https://github.com/krzat/SFML-D, making it unsable on osx 64bit: the sample program crashes with auto screen = new RenderWindow(VideoMode(800, 600), "Game", WindowStyle.Default, null); which I believe is caused by this. This leads to a lot of duplication, for example the authors had to duplicate c bindings just to address this: https://github.com/Jebbs/DSFML-C where they point to this bug.
Comment #40 by andrej.mitrovich — 2013-09-22T02:47:30Z
(In reply to comment #39) > This leads to a lot of duplication, for example the authors had to duplicate c > bindings just to address this: > https://github.com/Jebbs/DSFML-C where they point to this bug. I wonder if as a workaround you could type the prototypes in D as: // note the .tupleof sfWindow* sfWindow_create(VideoMode.tupleof mode, const(char)* title, uint style, const(ContextSettings)* settings); And then call it via: VideoMode vm; sfWindow_create(vm.tupleof, ...); I'd assume this would then properly use the stack? It's worth trying out to avoid any new code duplication, and then when 5570 is finally fixed all you have to do in user code is to remove ".tupleof" in the calls.
Comment #41 by thelastmammoth — 2013-09-22T20:07:53Z
(In reply to comment #40) > (In reply to comment #39) > > This leads to a lot of duplication, for example the authors had to duplicate c > > bindings just to address this: > > https://github.com/Jebbs/DSFML-C where they point to this bug. > > I wonder if as a workaround you could type the prototypes in D as: > > // note the .tupleof > sfWindow* sfWindow_create(VideoMode.tupleof mode, const(char)* title, uint > style, const(ContextSettings)* settings); corrected that to sfWindow_create(typeof(VideoMode.tupleof), ...) > > And then call it via: > > VideoMode vm; > sfWindow_create(vm.tupleof, ...); still segfaults > > I'd assume this would then properly use the stack? It's worth trying out to > avoid any new code duplication, and then when 5570 is finally fixed all you > have to do in user code is to remove ".tupleof" in the calls. so far my (sad) woraround is to add a new C function that takes a pointer to the struct, and link against it.
Comment #42 by camille — 2014-02-11T17:13:52Z
There is a $180 bounty on this issue: https://www.bountysource.com/issues/1325901. The bounty will get paid out to the developer who solves the issue.
Comment #43 by etienne — 2014-02-12T12:26:24Z
Could this be b/c struct size isn't calculated properly? ie. struct.c : ln 266 if (v->storage_class & (STCstatic | STCextern | STCtls | STCgshared | STCmanifest | STCctfe | STCtemplateparameter)) return 0; where 0 => this member doesn't need further processing to determine struct size
Comment #44 by bugzilla — 2014-03-10T02:26:47Z
Comment #45 by bugzilla — 2014-03-10T16:30:15Z
Comment #46 by github-bugzilla — 2014-03-10T17:01:14Z
Comment #47 by code — 2014-08-11T15:02:35Z
What's still left for this issue and why is there so much ABI dependent code in the frontend?
Comment #48 by andrei — 2014-08-29T21:40:30Z
From Walter's comment in the pull: "Partial in that it doesn't solve the problem for returning structs of sizes 3,5-7,9-15" Finalizing this is necessary for getting all geared up for core.stdcpp.
Comment #49 by yebblies — 2015-01-15T11:38:57Z
These other bug reports are part of this: Issue 13955 - 64 bit C ABI not followed for passing structs with mixed floating types Issue 13956 - 64 bit C ABI not followed for passing empty structs Issue 13957 - 64 bit C ABI not followed for passing structs with floating+integer types And issue 13955 and issue 13956 have been fixed in master. (In reply to Martin Nowak from comment #47) > What's still left for this issue and why is there so much ABI dependent code > in the frontend? Most of the remaining issues are in the code generator, and quite difficult to fix.
Comment #50 by timothee.cour2 — 2018-02-24T21:47:21Z
is this still being worked on ? IIRC calypso handles return by value of C/C++ correctly (via llvm), maybe some of implementation can be reused?
Comment #51 by slavo5150 — 2018-09-07T05:07:17Z
An potential additional $500 has been offered for a fix to this issue: https://forum.dlang.org/post/[email protected]
Comment #52 by iamthewilsonator — 2018-12-24T03:03:41Z
Comment #53 by dlang-bot — 2019-07-21T00:10:14Z
@SSoulaimane updated dlang/dmd pull request #10200 "Posix x64 ABI fixes 2" fixing this issue: - Fix issues 5570, 13957 - Testsuite: enable asserts on tests https://github.com/dlang/dmd/pull/10200
Comment #54 by acehreli — 2019-12-05T00:10:14Z
This may have fixed issue 6348 as well.
Comment #55 by dlang-bot — 2020-08-31T20:21:20Z
dlang/dmd pull request #10200 "Posix x64 ABI fixes 2" was merged into master: - 8a75689be02725741b2af939d63df432ab7c20f2 by سليمان السهمي (Suleyman Sahmi): Fix issues 5570, 13957 - Testsuite: enable asserts on tests https://github.com/dlang/dmd/pull/10200