Closed Bug 1336569 Opened 3 years ago Closed 3 years ago

mingw link step fails with undefined references to ffi_...

Categories

(Core :: js-ctypes, defect)

52 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

28:20.64     INPUT("StaticXULComponentsEnd/StaticXULComponentsEnd.o")
28:20.64 
28:20.64 ../../js/src/libjs_static.a(ffi.o): In function `ffi_call':
28:20.64 /home/worker/workspace/mingw-w64-build/firefox/js/src/ctypes/libffi/src/x86/ffi.c:390: undefined reference to `ffi_call_win32'
28:20.64 /home/worker/workspace/mingw-w64-build/firefox/js/src/ctypes/libffi/src/x86/ffi.c:418: undefined reference to `ffi_call_win32'
28:20.64 ../../js/src/libjs_static.a(ffi.o): In function `ffi_prep_closure_loc':
28:20.64 /home/worker/workspace/mingw-w64-build/firefox/js/src/ctypes/libffi/src/x86/ffi.c:673: undefined reference to `ffi_closure_SYSV'
28:20.64 /home/worker/workspace/mingw-w64-build/firefox/js/src/ctypes/libffi/src/x86/ffi.c:679: undefined reference to `ffi_closure_FASTCALL'
28:20.64 /home/worker/workspace/mingw-w64-build/firefox/js/src/ctypes/libffi/src/x86/ffi.c:685: undefined reference to `ffi_closure_THISCALL'
28:20.64 /home/worker/workspace/mingw-w64-build/firefox/js/src/ctypes/libffi/src/x86/ffi.c:691: undefined reference to `ffi_closure_STDCALL'
28:20.64 /home/worker/workspace/mingw-w64-build/firefox/js/src/ctypes/libffi/src/x86/ffi.c:698: undefined reference to `ffi_closure_SYSV'
28:20.64 collect2: error: ld returned 1 exit status
The attached patch shows what needs to be fixed, but is not the correct way to do it. Looking at js/src/ctypes/libffi/configure it seems like I need to investigate the sys_symbol_underscore check.
This was broken by 1262155 and we need to set DEFINES['SYMBOL_UNDERSCORE'] = True for mingw builds in config/external/ffi/moz.build
Comment on attachment 8833459 [details]
Bug 1336569 Define SYMBOL_UNDERSCORE for ffi in the mingw build

https://reviewboard.mozilla.org/r/109706/#review111678

::: build/moz.configure/toolchain.configure:770
(Diff revision 2)
>      if language == 'C':
> +        set_config(
> +            '%s_TYPE' % var, delayed_getattr(valid_compiler, 'type'))
>          add_old_configure_assignment(
>              '%s_TYPE' % var, delayed_getattr(valid_compiler, 'type'))
>          add_old_configure_assignment(

CC_VERSION is aparently already defined somewhere, since when I added a set_config for it, I got an error saying so. (I couldn't figure out where though...)
Attachment #8833459 - Flags: review?(mh+mozilla)
Comment on attachment 8833459 [details]
Bug 1336569 Define SYMBOL_UNDERSCORE for ffi in the mingw build

https://reviewboard.mozilla.org/r/109706/#review111838

::: config/external/ffi/moz.build:88
(Diff revision 2)
> +        #MinGW Build for 32 bit
> +        if CONFIG['CC_TYPE'] == 'gcc':
> +            DEFINES['SYMBOL_UNDERSCORE'] = True

Isn't the same needed for 64-bits?

::: config/external/ffi/moz.build:88
(Diff revision 2)
>      elif CONFIG['FFI_TARGET'] == 'X86':
>          ffi_srcs = ('ffi.c', 'sysv.S', 'win32.S')
>      elif CONFIG['FFI_TARGET'] == 'X86_64':
>          ffi_srcs = ('ffi64.c', 'unix64.S', 'ffi.c', 'sysv.S')
>      elif CONFIG['FFI_TARGET'] == 'X86_WIN32':
> +        #MinGW Build for 32 bit

Space after the #.
Attachment #8833459 - Flags: review?(mh+mozilla) → review+
Blocks: 1262155
(In reply to Mike Hommey [:glandium] from comment #6)
> Isn't the same needed for 64-bits?

Probably. But we're pretty far from doing a 64 bit mingw build, so I don't just want to stick it in there assuming it's what's needed without being able to test it. When we get to the 64 bit build we can put it in.
Assignee: nobody → tom
Keywords: checkin-needed
Autoland can't push this until all pending issues are marked as resolved in MozReview.
Keywords: checkin-needed
Fixed, sorry! (I had fixed the nit but didn't mark it as fixed.)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9d83651da332
Define SYMBOL_UNDERSCORE for ffi in the mingw build r=glandium
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9d83651da332
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8833459 [details]
Bug 1336569 Define SYMBOL_UNDERSCORE for ffi in the mingw build

[Feature/Bug causing the regression]: mingw build failure in ESR52.

[User impact if declined]: Tor will need to carry another custom patch.

[Is this code covered by automated tests?]: N/A  (It's a compilation problem)

[Has the fix been verified in Nightly?]: N/A (It's a compilation problem)

[Needs manual test from QE? If yes, steps to reproduce]: No.

[List of other uplifts needed for the feature/fix]: None

[Is the change risky?]: No

[Why is the change risky/not risky?]: It only affects gcc compilations of Firefox for Windows.

[String changes made/needed]: None
Attachment #8833459 - Flags: approval-mozilla-beta?
Comment on attachment 8833459 [details]
Bug 1336569 Define SYMBOL_UNDERSCORE for ffi in the mingw build

mingw build fix for beta52.  And probably aurora as well.
Attachment #8833459 - Flags: approval-mozilla-beta?
Attachment #8833459 - Flags: approval-mozilla-beta+
Attachment #8833459 - Flags: approval-mozilla-aurora+
Setting qe-verify- per comment 13.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.