445294 - Enable XPCOM to use non-stdcall calling convention when building with Mingw

Status: VERIFIED WONTFIX

(Core :: XPCOM, enhancement)

Description

[Bo Yang]

Attachment: Enhanced Patch

We can't declare __stdcall__ function pointers to member functions in some version of gcc such as 3.4.0, 4.3.* . And this cause many problems when user tries to build Mozilla on Mingw. The problems include some linkage problems and the result FF crashed when launched because of stack corrupt.
And this patch add a new configure option, which is used to make the XPCOM use the C default calling convention instead of the __stdcall__ to make the code base finally Mingw-buildable.

Comment 1

[Bo Yang]

See Gcc bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=9381 for details of the gcc problem.

Comment 2

[Benjamin Smedberg]

Chris, do you see any reason we couldn't disable stdcall on mingw completely? The only reason we used stdcall with MSVC was for COM compatibility, which is not really an issue for mingw.

Comment 3

[timeless]

bsmedberg: that's wrong. the reason is so that a random addon built with a random compiler can be dropped into the platform.

this should still be the case. mingw should not change the windows abi.

unless someone has a reason to support these very broken compilers, i suggest a configure test that simply breaks them "sorry, your compiler is broken. upgrade."

Comment 4

[Benjamin Smedberg]

The mingw vtable layout is already different from the Windows ABI. Since the ABI is already different, what's the point of maintaining the stdcall convention?

Comment 5

[timeless]

mingw should be fixed to the point where they implement the abi properly... don't encourage divergence.

everyone else implements stdcall (which is a platform standard) correctly. and we have people who write xpcom code using borland delphi (among other interesting platforms).

the reason for mingw is to be able to make a compatible cross compile from linux, not having the proper abi breaks this.

Comment 6

[Benjamin Smedberg]

mingw implements the "standard GCC ABI". It does not and will not implement the MSVC ABI. There is no use pretending that it may someday in the future, or that compatibility with the MSVC ABI is a goal.

GCC 3.4.0 and 4.2+ are broken with __stdcall__. Bo is not yet sure why, but it is more important to me that we get a running mingw build with static analysis than that we maintain theoretical and nonworking purity.

Comment 7

[cls]

From reading the gcc bug, the current behavior is a bug, not a feature.  You should not change the calling conventions, much less adding a configure option, because of a bug in a specific version of a compiler.  Fix your compiler or use a different version.  FYI, the build I created from the CVS trunk a few weeks ago using mingw gcc 3.4.5 had no problems launching FF.  Maybe something was added since then or is in the hg repo that exposes the problem but it's not universal.  I'll download the 3.0 source and test a build tonight.

I tend to agree with timeless on this.  We implemented the mingw port to be as compatible to the msvc win32 port as possible.  The c++ abi is different but the underlying c abi should not be. 

Comment 8

[cls]

After a very painful vnc-over-ssh session, I verified that ff 3.0 works fine when built with mingw gcc 3.4.5.

Comment 9

[Bo Yang]

(In reply to comment #7)
> From reading the gcc bug, the current behavior is a bug, not a feature.  You
> should not change the calling conventions, much less adding a configure option,
> because of a bug in a specific version of a compiler.  Fix your compiler or use
> a different version.  FYI, the build I created from the CVS trunk a few weeks
> ago using mingw gcc 3.4.5 had no problems launching FF.  Maybe something was
> added since then or is in the hg repo that exposes the problem but it's not
> universal.  I'll download the 3.0 source and test a build tonight.

I am sure it is not Mozilla code problem. It is the operator typeof() failed in gcc 4.3.*. For detail, one can't declare __stdcall__ a pointer to a member function in gcc 4.3.*. The testcase attached will make this problem clear. I guess you can compile the test.cpp and get a workable program with gcc-3.4.5. But with gcc 4.3.*, I bet you will fail. 
 
> I tend to agree with timeless on this.  We implemented the mingw port to be as
> compatible to the msvc win32 port as possible.  The c++ abi is different but
> the underlying c abi should not be. 

The reason why we insist on gcc 4.3.0 is that the now dehydra gcc version is designed for gcc 4.3.0. And we want to use that dehydra to analyze Mozilla windows-specific code.

Comment 10

[Bo Yang]

Attachment: The gcc bug testcase

Comment 11

[Bo Yang]

The most important point here is that we want to do statical analysis on Windows code with gcc dehydra. And of course, we want the final FF is workable. 
And the now Dehydra is designed for Gcc 4.3.0, so we must make Mozilla Mingw/gcc 4.3.0 workable. But the now Gcc 4.3.* have a bug, I have post here.(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=9381) 
We don't know how long Gcc community to fix that bug... So, the best way to archive our target is to add a new configure option, which let us disable __stdcall__ when neccessary. So, I post this patch.
Users can do nothing to compile Mozilla with mingw and gcc 3.4.1, 3.4.2...3.4.5. For users who use gcc 4.3.*, he should add this option in the .mozconfig file and get Mozilla buildable too. And we do not need to convert Mingw to drop __stdcall__ completely...

Comment 12

[cls]

Having no vested interest in this statistical analysis testing, I think it's a Bad Idea(tm) to intentionally remove compatibility by changing the calling conventions so that you can use the new-ish(?) debugging tool of the season with a 2 month old *alpha* release of a compiler.  That said, if you must make the changes to accommodate a broken compiler, don't add a public configure option but have a hidden check.  There's no reason to support this change other than this very specific testing scenario and it should go away once a non-broken compiler is available.  That's a lot easier to do when you don't have a public configure option that people wrongly start using.

Comment 13

[Benjamin Smedberg]

Wait, what? gcc 4.2 and 4.3 are not alpha compilers...

In any case, I can see skipping the configure check and making it require a makefile override.

Comment 14

[cls]

I meant the gcc 4.3.0 mingw alpha release specifically.  The proposed changes aren't for all gcc 4.x builds, just the mingw ones.  There aren't any other official mingw releases of 4.x that I know of.

http://www.mingw.org/MinGWiki/index.php/GccStatus

Comment 15

[Benjamin Smedberg]

Comment on attachment 329639 [details] [diff] [review]
Enhanced Patch

Please re-submit this patch without the configure change. Therefore you'd have to configure with 'CPPFLAGS=-DMOZ_DISABLE_XPCOM_STDCALL' to build with this option.

Comment 16

[Bo Yang]

Attachment: the revised patch

This patch detect whether the gcc version is 4.*.*, if so, it add the -DMOZ_DISABLE_XPCOM_STDCALL.

Comment 17

[Benjamin Smedberg]

Comment on attachment 331288 [details] [diff] [review]
the revised patch

There should be no configure changes.

Comment 18

[Bo Yang]

Attachment: the third version of patch

Ah-oh, I misunderstood what you said, now I remove the configure option change.

Comment 19

[Benjamin Smedberg]

Bo, since you fixed the bug with stdcall function pointers and we can work around the linkage errors with --enable-libxul, do we still need this patch? I'd like to avoid the complexity if we don't actually need it.

Comment 20

[Bo Yang]

(In reply to comment #19)
> Bo, since you fixed the bug with stdcall function pointers and we can work
> around the linkage errors with --enable-libxul, do we still need this patch?
> I'd like to avoid the complexity if we don't actually need it.
> 

Ah, if the gcc bug is accepted, I think we can discard this patch. But without this patch, I think the .mozconfig file's options should be carefully set. Because many options may cause the build system not generate a libxul and then the linkage errors...

Comment 21

[Bo Yang]

(In reply to comment #19)
> Bo, since you fixed the bug with stdcall function pointers and we can work
> around the linkage errors with --enable-libxul, do we still need this patch?
> I'd like to avoid the complexity if we don't actually need it.

I am now hoping this patch can be landed... 

Comment 22

[Jacek Caban]

GCC has been fixed, so we can use stdcall on mingw.