Bug 36834 - structure return ABI for windows targets differs from native MSVC
Summary: structure return ABI for windows targets differs from native MSVC
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.4.0
: P3 normal
Target Milestone: ---
Assignee: Danny Smith
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-15 09:27 UTC by Danny Smith
Modified: 2014-02-17 12:31 UTC (History)
5 users (show)

See Also:
Host: i686-pc-mingw32
Target: i686-pc-mingw32
Build: i686-pc-mingw32
Known to work:
Known to fail: 4.2.0, 4.3.0, 4.4.0
Last reconfirmed: 2008-07-15 09:29:57


Attachments
Functions in libstdc++ returning aggregates > 8 bytes (899 bytes, text/plain)
2009-03-21 16:45 UTC, Mattias Engdegård
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Danny Smith 2008-07-15 09:27:22 UTC
Like i386-netware, the native MS Windows compiler assumes that the CALLER pops the stack for the implicit arguments pointing to aggregate return value.  This differs from the default i386 ABI which assumes the CALLEE pops the stack.

This is documented at http://www.angelcode.com/dev/callconv/callconv.html in the section  on __cdecl calling convention.

The bug was reported to mingw users list by Magnus Christensson at:
http://www.nabble.com/Problem-returning-C-struct-from-MinGW-to-MSVC-td18444899.html
This report contains a testcase demostrating the problem.
 
Danny
Comment 1 Danny Smith 2008-07-15 09:29:57 UTC
Testing  a patch to add a new switch for windows targets.
Comment 2 Danny Smith 2008-07-17 08:16:02 UTC
Patch submitted
http://gcc.gnu.org/ml/gcc-patches/2008-07/msg01250.html
Danny
Comment 3 Mattias Engdegård 2008-07-18 09:43:52 UTC
Thank you. Should the option be enabled by default on i686-pc-mingw32? Perhaps not, but it does make good on a previously broken ABI promise.

The test case in that patch only covers the callee, not the caller. The difference in the calling code is perhaps less easily recognisable.
Comment 4 Roger Pack 2009-03-11 11:35:16 UTC
I'd be willing to offer a small bounty for this one.  Maybe $50 for an accepted patch that handles both the caller and callee sides?

The problem seems to be common [1].

Thanks!
-=r
[1] http://209.85.173.132/search?q=cache:e7XCjhLwHacJ:luabinaries.luaforge.net/manual.html+http://luabinaries.luaforge.net/manual.html%23LuaBinariesCompatible&hl=en&ct=clnk&cd=1&gl=us&client=opera is it the same bug [they mention incompats when passing CRT objects among libraries]? dunno.
Comment 5 Mattias Engdegård 2009-03-11 12:42:48 UTC
(In reply to comment #4)

>http://209.85.173.132/search?q=cache:e7XCjhLwHacJ:luabinaries.luaforge.net/manual.html+http://luabinaries.luaforge.net/manual.html%23LuaBinariesCompatible&hl=en&ct=clnk&cd=1&gl=us&client=opera
> is it the same bug [they mention incompats when passing CRT objects among
> libraries]? dunno.

No, that page is concerned with multiple CRTs in the same process and passing around values between them (such as using fopen() in one CRT and fclose() in another).

This bug is about calling conventions for functions returning structs by value.
Comment 6 Roger Pack 2009-03-14 21:14:30 UTC
anybody know if this bug is related to this post? http://www.gamedev.net/community/forums/topic.asp?topic_id=482230
Thanks!
-=roger
Comment 7 Mattias Engdegård 2009-03-20 17:16:39 UTC
The proposed patch works for plain C code, but also affects C++. Since libstdc++ contains code that returns aggregates or calls code that does, -mms-aggregate-return will make the generated code incompatible with the C++ library. This makes it impossible to write C++ code that links (as a DLL) to code compiled with Microsoft or Intel compilers, even if the interface is in plain C.

We are exactly in this situation. We will have to build libstdc++ with -mms-aggregate-return, but that option would then become mandatory for all C++ code. Things would be easier if the option were on by default - we can do this for our build locally, of course, but we'd rather not diverge too much from the standard behaviour.

An alternative would be to make the option only affect C (including extern "C" in C++), as other languages are not subject to compatibility with MS compilers.
Comment 8 Danny Smith 2009-03-21 01:03:55 UTC
(In reply to comment #7)
> The proposed patch works for plain C code, but also affects C++. Since
> libstdc++ contains code that returns aggregates or calls code that does,

An example, please.  The patch only affects functions that return large (> 8 bytes) aggregates by value.

Danny

> -mms-aggregate-return will make the generated code incompatible with the C++
> library. 

Comment 9 Mattias Engdegård 2009-03-21 16:45:58 UTC
Created attachment 17508 [details]
Functions in libstdc++ returning aggregates > 8 bytes

These are the functions in libstdc++ that contain the instruction "ret 0x4", assuming that they correspond exactly to those returning aggregates larger than 8 bytes. There are 127 of them.
Comment 10 Mattias Engdegård 2009-03-21 21:49:08 UTC
Note that C++ objects need not be larger than 8 bytes to qualify for returning on the stack (and thus subject to this cleanup problem). Any class with a copy constructor, for example, seems to be affected.
Try compiling:

struct A {
  A();
  A(const A&);
  int x;
};
A f() { return A(); }

This is probably why methods returning string::reverse_iterator make the list.
(http://www.agner.org/optimize/calling_conventions.pdf was useful for understanding this.)
Comment 11 Danny Smith 2009-03-23 22:10:44 UTC
(In reply to comment #10)
> Note that C++ objects need not be larger than 8 bytes to qualify for returning
> on the stack (and thus subject to this cleanup problem). Any class with a copy
> constructor, for example, seems to be affected.

Thanks.  I understand your concern now.  
Do you think that a function  __attibute__((ms-aggregate-return)) would be  useful to specify individual problematic functions.
I expect that this would be equivalent to  (__attribute__((target("ms-aggregate-return"))) which should work now, but I haven't tested,  Will do so soon.
Comment 12 Kai Tietz 2009-06-24 12:05:16 UTC
As I read this. Would it make sense to use for x86-mingw the callabi feature (as we do for the x64 variant)? This would be useful for 32-bit based multilib version, too (but this is more a side-note for this).

Kai
Comment 13 Kai Tietz 2010-12-18 10:16:16 UTC
Author: ktietz
Date: Sat Dec 18 10:16:13 2010
New Revision: 168019

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168019
Log:
2010-12-18  Kai Tietz  <kai.tietz@onevision.com>

	PR target/36834
	* config/i386/i386.c (ix86_keep_aggregate_return_pointer):
	New local function.
	(ix86_return_pops_args): Use ix86_keep_aggregate_return_pointer
	function instead of KEEP_AGGREGATE_RETURN_POINTER.
	(ix86_handle_callee_pop_aggregate_return): New handler.
	(ix86_attribute_table): Add new attribute
	callee_pop_aggregate_return.
	* doc/extend.texi (callee_pop_aggregate_return): Add
	attribute documentation.

2010-12-18  Kai Tietz  <kai.tietz@onevision.com>

	PR target/36834
	* gcc.target/i386/aggregate-ret1.c: New.
	* gcc.target/i386/aggregate-ret2.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/i386/aggregate-ret1.c
    trunk/gcc/testsuite/gcc.target/i386/aggregate-ret2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/doc/extend.texi
    trunk/gcc/testsuite/ChangeLog
Comment 14 Kai Tietz 2010-12-18 10:38:11 UTC
By this new attribute functions can be explicit marked to use VC compatible aggregate return pointer handling.
I don't plan to backmerge this fix to 4.5.x tree, so I close this bug as fixed for 4.6 tree. For 4.7 there is planned to introduce for mingw 32-bit target the callabi MS_ABI/SYSV_ABI (as it is already present for 64-bit mingw target). This will then automatically switch on MS specific calling convention changes - eg ms-bitfield, caller-pops-aggregate-return, and co.
Comment 15 Jackie Rosen 2014-02-16 13:12:39 UTC Comment hidden (spam)