Closed
Bug 590996
Opened 14 years ago
Closed 7 years ago
Build of libffi broken by valid linker switch
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
INCOMPLETE
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: swsnyder, Assigned: Mitch)
References
Details
(Whiteboard: [patchlove])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
glandium
:
review-
|
Details | Diff | Splinter Review |
The build of libffi is broken if LDFLAGS contains the VS2005 linker switch -LTCG.
The reason is that libtool believes that -L is always followed by a path to required libraries, and chokes when it sees those trailing "TCG" characters. It aborts compilation, complaining that path TCG can't be found.
I marked this bug as OS == "Windows XP" but it is really specific to the Visual Studio compiler.
Seen on Firefox 4.0b4 release tarball, built with VS2005 on WinXP.
Reporter | ||
Updated•14 years ago
|
Attachment #469552 -
Attachment is patch: true
Attachment #469552 -
Attachment mime type: application/octet-stream → text/plain
Comment 1•14 years ago
|
||
I've just hit the same build error, and was pleased to find your patch attached. Thanks!
Did you mean to request review?
Reporter | ||
Comment 2•14 years ago
|
||
(In reply to comment #1)
> Did you mean to request review?
Maybe this patch is the best course, but it seems kind of clunky to me. (What other valid switches will be inappropriately matched in the libtool gen?) There's probably a better way, but I'm not very familiar with the details of libtool.
Given my uncertainty it seemed best to create this bug just to get it on the record, and to provided this patch to at least work around the problem.
Reporter | ||
Comment 3•14 years ago
|
||
See also: Bug 600237
Comment 4•14 years ago
|
||
I think this should block 2.0 as it breaks the build in a configuration (MS VS 2005) that, I think, is still supported. And if it doesn't block, it probably still deserves some attention to at least determine the exact set of build configurations that no longer work.
blocking2.0: --- → ?
Our tinderboxes run VS2005, I believe, so I'm not quite sure what's different about this setup. If you're saying "there exist ways to change your setup and make configurations that don't work", then that's true and I could see taking patches for them, but I can't see us blocking.
blocking2.0: ? → -
Comment 6•14 years ago
|
||
I was under the, apparently false, assumption that tinderboxes had VS2005 installed along newer versions and used VS2008 or 2010 to build on the mozilla-central tree. Sorry about that.
I'm certainly not looking for ways to change my setup to make configurations that don't work. Both my local Windows virtual machine and the Windows virtual machine used for Instantbird's buildbot slave couldn't compile the code from mozilla-central without applying the patch attached here.
These 2 virtual machines have Windows XP (service pack 2 I think) with Visual Studo 2005 Pro SP1.
Assignee | ||
Comment 7•14 years ago
|
||
This is based on Steve's patch and works around -LARGEADDRESSAWARE too, which is apparently a problem: https://bugzilla.mozilla.org/show_bug.cgi?id=556382#c30
Completely untested.
Attachment #491822 -
Flags: feedback?(florian)
Comment 8•14 years ago
|
||
Comment on attachment 491822 [details] [diff] [review]
Patch
A Windows build of Instantbird has just completed successfully with this patch.
Attachment #491822 -
Flags: feedback?(florian) → feedback+
Assignee | ||
Updated•14 years ago
|
Attachment #491822 -
Flags: review?(shaver)
Comment on attachment 491822 [details] [diff] [review]
Patch
Lets get this into the tree.
Attachment #491822 -
Flags: review?(shaver) → review+
Comment 10•14 years ago
|
||
Patch 491822 works fine. But I also got it to work just by disabling LARGEADDRESSAWARE. Disabling LARGEADDRESSAWARE is enough and disabling LTCG is not required.
# Avoid matching MSVC linker option.
-LARGEADDRESSAWARE)
continue
;;
The following is not required since the issue fundamentally is because of LARGEADDRESSAWARE:
# Avoid matching MSVC linker option.
-LTCG)
continue
;;
Comment 11•14 years ago
|
||
(In reply to comment #10)
> Patch 491822 works fine. But I also got it to work just by disabling
> LARGEADDRESSAWARE. Disabling LARGEADDRESSAWARE is enough and disabling LTCG is
> not required.
It was required when this bug was filed, and probably still is in some configurations. The LARGEADDRESSAWARE issue appeared later when bug 556382 landed.
Comment 12•14 years ago
|
||
ok. So just as a note, I was building ff4b8 on Win7 with vc9 and just avoiding LARGEADDRESSAWARE worked for me.
Updated•14 years ago
|
Assignee: general → mitchell.field
Mitch, is there a reason this hasn't landed yet?
Comment 15•13 years ago
|
||
Just got bit by this for -LARGEADDRESSAWARE on Windows 7.
Comment 16•13 years ago
|
||
I was hit by this last month, and was easily able to workaround the problem by changing -LARGEADDRESSAWARE and -LTCG to -largeaddressaware and -ltcg respectively in config files and LDFLAGS (if applicable). libiff is case-sensitive with its switches, but the MSVC linker doesn't care. When using -largeaddressaware and -ltcg, libiff will ignore them (only cares about capital '-L') and they will get passed to the MSVC linker as normal.
Comment 17•13 years ago
|
||
khuey, any reason this can't land?
Updated•13 years ago
|
Whiteboard: [patchlove]
Comment 18•11 years ago
|
||
khuey, do you know the status of this patch, and especially why it hasn't landed?
Flags: needinfo?(khuey)
Comment 19•11 years ago
|
||
Similar hilarity from current builds:
cl : Command line warning D9002 : ignoring unknown option '-O3'
cl : Command line warning D9002 : ignoring unknown option '-OP'
cl : Command line warning D9002 : ignoring unknown option '-OT'
cl : Command line warning D9002 : ignoring unknown option '-O:'
cl : Command line warning D9002 : ignoring unknown option '-OR'
cl : Command line warning D9002 : ignoring unknown option '-OE'
cl : Command line warning D9002 : ignoring unknown option '-OF'
cl : Command line warning D9002 : ignoring unknown option '-OP'
cl : Command line warning D9002 : ignoring unknown option '-OT'
cl : Command line warning D9002 : ignoring unknown option '-O:'
cl : Command line warning D9002 : ignoring unknown option '-OI'
cl : Command line warning D9002 : ignoring unknown option '-OC'
cl : Command line warning D9002 : ignoring unknown option '-OF'
cl : Command line warning D9002 : ignoring unknown option '-warn'
cl : Command line warning D9024 : unrecognized source file type 'all', object file assumed
We should probably ignore -O3 and -warn as well. Honestly, I'm currently updating the in-tree libffi from upstream in bug 810631. Maybe we should just try to get these fixed there and get this for "free" that way.
Flags: needinfo?(khuey)
I don't know anything beyond what's in the bug. I suspect the reason it never landed is that Mitch has been inactive for a few years.
Comment 21•11 years ago
|
||
Mike, does this look sensible to you? If so, my intent is to just get this upstreamed rather than landing it in-tree. We'll then pick it up in bug 810631.
Attachment #469552 -
Attachment is obsolete: true
Attachment #491822 -
Attachment is obsolete: true
Attachment #8384176 -
Flags: review?(mh+mozilla)
Comment 22•11 years ago
|
||
Comment on attachment 8384176 [details] [diff] [review]
rebased
Review of attachment 8384176 [details] [diff] [review]:
-----------------------------------------------------------------
The biggest problem with this patch is that ltmain.sh comes from another third party, libtool, and I don't think they would accept this patch. Even if they would, you'd need to wait for another libtool release which then would then need to make it to another libffi release using it.
On top of that, the assumption in the libffi build system is that the given flags really are for gcc: the compiler we use for ffi is really msvcc.sh, which is a gcc-command-line-like wrapper around msvc. If there really needs to be (upstreamable) ldflags filtering, we should then have a similar wrapper around link.exe (or reuse msvcc.sh for that)
Attachment #8384176 -
Flags: review?(mh+mozilla) → review-
Comment 23•7 years ago
|
||
Is this bug still actual, taking into account that VS2005 is no longer supported for building?
Flags: needinfo?(ryanvm)
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(florian)
Whiteboard: [patchlove] → [patchlove][closeme 2017-05-15]
Comment 24•7 years ago
|
||
(In reply to Phoenix from comment #23)
> Is this bug still actual, taking into account that VS2005 is no longer
> supported for building?
Most likely not.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(florian)
Resolution: --- → INCOMPLETE
Flags: needinfo?(ryanvm)
Flags: needinfo?(mh+mozilla)
Whiteboard: [patchlove][closeme 2017-05-15] → [patchlove]
Comment 25•7 years ago
|
||
We're not using the libffi build system anyways, so this is now irrelevant.
Flags: needinfo?(jorendorff)
Comment 26•7 years ago
|
||
Sorry, I don't know where this needinfo came from.
Flags: needinfo?(jorendorff)
You need to log in
before you can comment on or make changes to this bug.
Description
•