Closed Bug 615212 Opened 14 years ago Closed 13 years ago

GCC's win64 ABI change broke compilation using mingw-w64

Categories

(Core :: XPCOM, defect)

x86_64
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jacek, Assigned: jacek)

Details

Attachments

(2 files, 1 obsolete file)

GCC (and binutils) has recently changes its win64 ABI to be more compatible with msvc. After the change, symbols are no longer underscored. Given that it was never possible to compile clean mozilla-central with mingw-w64 (although all required patches are on their way to the tree), I don't think we want to support backward compatibility. It will be cleaner to require GCC compatible with recent ABI. This change affected Mozilla compilation in three places:

- libffi
I've fixed it in mainstream:
https://github.com/atgreen/libffi/commit/5b9cd52784339a42e417174a55e310e214d435f9

- xptcall
It was partially supporting different underscores, but not in all places, which makes it useless. I've removed all SYMBOL_UNDERSCORE uses.

- MethodJIT.cpp
It already has macros for different symbol decorations, so the change is trivial.
Attached patch xptcall part v1.0 (obsolete) — Splinter Review
xptcall part
Assignee: nobody → jacek
Attachment #493679 - Flags: review?(timeless)
Attachment #493679 - Flags: feedback?(mook.moz+mozbz)
methodjit part
Attachment #493681 - Flags: review?(dvander)
Comment on attachment 493679 [details] [diff] [review]
xptcall part v1.0

>diff --git a/xpcom/reflect/xptcall/src/md/win32/Makefile.in b/xpcom/reflect/xptcall/src/md/win32/Makefile.in

> LOCAL_INCLUDES	= -I$(srcdir)/../unix
>+ifneq ($(TARGET_CPU),x86_64)
> DEFINES		+= -DMOZ_USE_STDCALL -DMOZ_NEED_LEADING_UNDERSCORE
>+endif

Is this right for MSVC?

>diff --git a/xpcom/reflect/xptcall/src/md/win32/xptcstubs_x86_64_gnu.cpp b/xpcom/reflect/xptcall/src/md/win32/xptcstubs_x86_64_gnu.cpp

>-  ".intel_syntax\n"                     /* switch to Intel syntax to look like the MSVC assembly */
>+  ".intel_syntax noprefix\n"                     /* switch to Intel syntax to look like the MSVC assembly */

I don't think the comment should change its column.
Thanks for review.

(In reply to comment #3)
> Comment on attachment 493679 [details] [diff] [review]
> xptcall part v1.0
> 
> >diff --git a/xpcom/reflect/xptcall/src/md/win32/Makefile.in b/xpcom/reflect/xptcall/src/md/win32/Makefile.in
> 
> > LOCAL_INCLUDES	= -I$(srcdir)/../unix
> >+ifneq ($(TARGET_CPU),x86_64)
> > DEFINES		+= -DMOZ_USE_STDCALL -DMOZ_NEED_LEADING_UNDERSCORE
> >+endif
> 
> Is this right for MSVC?

It's already in |ifdef GNU_CXX| so it doesn't really matter, but yes, MSVC also doesn't use leading underscore on win64 and has only one calling convention, so MOZ_USE_STDCALL is noop there.

> >diff --git a/xpcom/reflect/xptcall/src/md/win32/xptcstubs_x86_64_gnu.cpp b/xpcom/reflect/xptcall/src/md/win32/xptcstubs_x86_64_gnu.cpp
> 
> >-  ".intel_syntax\n"                     /* switch to Intel syntax to look like the MSVC assembly */
> >+  ".intel_syntax noprefix\n"                     /* switch to Intel syntax to look like the MSVC assembly */
> 
> I don't think the comment should change its column.

I will attach a fixed version.
fixed comment
Attachment #493679 - Attachment is obsolete: true
Attachment #493689 - Flags: review?(timeless)
Attachment #493679 - Flags: review?(timeless)
Attachment #493679 - Flags: feedback?(mook.moz+mozbz)
Attachment #493681 - Flags: review?(dvander) → review+
Attachment #493689 - Flags: review?(timeless) → review+
Attachment #493681 - Flags: approval2.0?
Attachment #493689 - Flags: approval2.0?
Attachment #493689 - Flags: approval2.0?
methodjit part on TM:
http://hg.mozilla.org/tracemonkey/rev/28ce3b08ea8f
Whiteboard: fixed-in-tracemonkey
Attachment #493681 - Flags: approval2.0?
merged to m-c:
http://hg.mozilla.org/mozilla-central/rev/28ce3b08ea8f
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-tracemonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: