1289847 - Pass -Wl,-no_compact_unwind on mac builds with --enable-rust

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Ralph Giles (:rillian)]

Work around bug 1188030 by passing -no_compact_unwind (the typical work-around) to the linker on Mac. We need this to unblock updates of the rust toolchain on that platform so we can move forward with other work.

Comment 1

[Ralph Giles (:rillian)]

I tried a local build with rust 1.10.0-stable and the following lines in mozconfig:

  ac_add_options LDFLAGS="-Wl,-no_compact_unwind"
  mk_add_options LDFLAGS="-Wl,-no_compact_unwind"

It really seems to require it be set in the parent mach process environment. I'm not sure how to do that. Maybe with mozharness for integration, and punt on making local builds work?

Rail, any suggestions how we can set an environment variable for mac builds in automation?

Comment 2

[Nathan Froyd [:froydnj]]

(In reply to Ralph Giles (:rillian) needinfo me from comment #1)
> Rail, any suggestions how we can set an environment variable for mac builds
> in automation?

Use export?

http://dxr.mozilla.org/mozilla-central/source/build/unix/mozconfig.asan#5

Alternatively, set it in configury, where everybody will benefit, and nobody needs to add weird hacks.

Comment 3

[Nathan Froyd [:froydnj]]

I tried this with the rust repack that was uploaded in bug 1285323 for Macs (the bug is for Linux, but it looks like Rust repacks for every platform were uploaded?):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b44e2c0f042

Looks like we run into linker errors:

 08:16:28     INFO -  Assertion failed: (_mode == modeFinalAddress), function finalAddress, file /SourceCache/ld64/ld64-123.2.1/src/ld/ld.hpp, line 573.
 08:16:28     INFO -  0  0x10b0d371c  __assert_rtn + 76
 08:16:28     INFO -  1  0x10b14c01c  ld::tool::OutputFile::addressOf(ld::Internal const&, ld::Fixup const*, ld::Atom const**) + 172
 08:16:28     INFO -  2  0x10b14ea25  ld::tool::OutputFile::applyFixUps(ld::Internal&, unsigned long long, ld::Atom const*, unsigned char*) + 3909
 08:16:28     INFO -  3  0x10b14af70  ld::tool::OutputFile::writeOutputFile(ld::Internal&) + 816
 08:16:28     INFO -  4  0x10b143ab9  ld::tool::OutputFile::write(ld::Internal&) + 153
 08:16:28     INFO -  5  0x10b0d3caa  main + 1178
08:16:28 INFO - clang-3.8: error: linker command failed with exit code 1 (use -v to see invocation)

ISTR this linker error coming up in a different context; Ralph, does it ring a bell?  I thought it had something to do with our need to upgrade the Mac toolchain we were using...

Comment 4

[Nathan Froyd [:froydnj]]

Ah, it's similar to bug 1165528...but upgrading our Rust version resolved things there. :(

Comment 5

[Nathan Froyd [:froydnj]]

...and the linker error only turns up in debug builds. :(

Comment 6

[Ralph Giles (:rillian)]

Yeah, it sounds familiar. I'd been assuming the various errors were different manifestations of the same issue. I had similar problems with the the 1.12-nightly build on try which works on my local MacOS X 10.10.5.

Can we package a newer linker through tooltool?

Comment 7

[Ralph Giles (:rillian)]

BTW, your commit message doesn't match my understanding of https://github.com/rust-lang/rust/pull/34832

I thought the issue was that prior to 1.12, rust was _reusing_ the C++ eh_personality routines in a way that presented confusing different manifestations, and the fix was to provide entirely separate, rust-specific implementations. Am I confused about the underlying cause?

Comment 8

[Nathan Froyd [:froydnj]]

(In reply to Ralph Giles (:rillian) needinfo me from comment #6)
> Yeah, it sounds familiar. I'd been assuming the various errors were
> different manifestations of the same issue. I had similar problems with the
> the 1.12-nightly build on try which works on my local MacOS X 10.10.5.
> 
> Can we package a newer linker through tooltool?

We don't get the linker through tooltool, AIUI; we get the linker through whatever XCode is installed on the machines.  Compiling the Mac linker is...not straightforward, but possibly doable since we compile our own clang?

Comment 9

[Ralph Giles (:rillian)]

Yes, I meant package up the linker from a newer xcode binary, or do a darwin build of whatever the mac-cross build uses. I suppose it might be hard to find one new enough to avoid the error but old enough to run on 10.7.

Comment 10

[Ralph Giles (:rillian)]

For the record, using export in mozconfig worked on my local mac, where ac_add_options + mk_add_options didn't.

Comment 11

[(not currently active) Ted Mielczarek]

We build our own linker for the cross-mac builds, it's probably not any harder to build it for our native mac builds.

Comment 12

[Mike Hommey [:glandium]]

Relevant information about building ld64 on osx:
https://bugzilla.mozilla.org/show_bug.cgi?id=1262781#c2

Comment 13

[Nathan Froyd [:froydnj]]

Attachment: use a newer ld for Mac builds

The ld that we use for Mac builds is old (Xcode circa OS X 10.7), and
also crashes in various ways when we try to use newer Rust versions
and/or pass options to make the linker work with newer Rust versions.
To mitigate this, let's build with a newer linker, compiled from:

https://github.com/tpoechtrager/cctools-port

We use this port, rather than the packages from opensource.apple.com,
because the packages from Apple have decidely non-intuitive build
systems, and require some hacking to get to build.  This port, in
contrast, is simply built with:

CFLAGS='-mcpu=generic -mtune=generic' ./configure --target=x86_64-apple-darwin11
env MACOSX_DEPLOYMENT_TARGET=10.7 make

and the resulting x86_64-apple-darwin11-ld is renamed as 'ld' and
packaged up for automation's purposes.

However, since this linker is newer, it also produces bits of Mach-O
that our older build tools don't understand.  Fortunately, we can pass
appropriate options to the linker to turn off generation of those Mach-O
bits.

Comment 14

[Nathan Froyd [:froydnj]]

Attachment: work around Mac linking failures when Rust is enabled

Current stable versions of Rust use two Rust-specific personality
routines to perform exception handling, which empirically does not play
well with the Mac linker's optimizations for using compact unwind
formats. Nightly Rust has solved this issue, but for now, we'll have to
use -no_compact_unwind to disable the linker optimization.  The size
impact is negligible (0.02%) and will be going away once nightly Rust
becomes stable.

Comment 15

[Michael Shal [:mshal]]

Comment on attachment 8776110 [details] [diff] [review]
work around Mac linking failures when Rust is enabled

>Current stable versions of Rust use two Rust-specific personality
>routines to perform exception handling, which empirically does not play
>well with the Mac linker's optimizations for using compact unwind
>formats. Nightly Rust has solved this issue, but for now, we'll have to
>use -no_compact_unwind to disable the linker optimization.  The size
>impact is negligible (0.02%) and will be going away once nightly Rust
>becomes stable.

Are you saying this moz.build change will go away? Or just the performance impact of using this flag? If the former, can you file a follow-up and put the bug number in a comment in the moz.build file?

Comment 16

[(not currently active) Ted Mielczarek]

Even if it's trivial, it'd be great to have a script in-tree somewhere to reproduce the ld64 build.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f22763859aae
part 1 - use a newer ld for Mac builds; r=mshal
https://hg.mozilla.org/integration/mozilla-inbound/rev/74922f9ce5c6
part 2 - work around Mac linking failures when Rust is enabled; r=mshal

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for static build bustage in https://hg.mozilla.org/integration/mozilla-inbound/rev/a000a5db67aa

https://treeherder.mozilla.org/logviewer.html#?job_id=33032550&repo=mozilla-inbound

Comment 19

[Nathan Froyd [:froydnj]]

(In reply to Wes Kocher (:KWierso) from comment #18)
> Backed out for static build bustage in
> https://hg.mozilla.org/integration/mozilla-inbound/rev/a000a5db67aa
> 
> https://treeherder.mozilla.org/logviewer.html#?job_id=33032550&repo=mozilla-
> inbound

Sigh, I *really* wish that when you pushed to try -p $PLATFORM, it would run *all* the builds for that platform, so you didn't have to screw around with -p $PLATFORM_WITH_MAGIC_BUILD_TYPE.

I'm unsure of why this is failing...this is just building on a Mac like our other Mac builds!

Comment 20

[Nathan Froyd [:froydnj]]

(In reply to Nathan Froyd [:froydnj] from comment #19)
> I'm unsure of why this is failing...this is just building on a Mac like our
> other Mac builds!

Our static analysis builds use a different manifest than our regular builds, so the static analysis build was not downloading the new linker...but it was passing flags that only the new linker understood.

The fix is straightforward: add the cctools tarball to the static analysis tooltool manifest.  Assuming that's really the only problem, everything here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c653626358b

should come back green and I will reland.  I don't think I need to ask for re-review of the manifest update.

Comment 21

[Nathan Froyd [:froydnj]]

If I could pick the right git revisions to push to try, that'd be good:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eac8f15a6148

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4cac318f42d
part 1 - use a newer ld for Mac builds; r=mshal
https://hg.mozilla.org/integration/mozilla-inbound/rev/3cda786e6bb6
part 2 - work around Mac linking failures when Rust is enabled; r=mshal

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e4cac318f42d
https://hg.mozilla.org/mozilla-central/rev/3cda786e6bb6

Comment 24

[aleth [:aleth]]

Attachment: Followup to fix linker path for comm-* builds

This broke builds on comm-central as the paths there are slightly different due to the unfortunate split build system.

Comment 25

[aleth [:aleth]]

c-c try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2535a0ec68fba9271acdfc74c9cdbae7fb5932f2
(m-c is not affected by the followup patch.)

Comment 26

[Nathan Froyd [:froydnj]]

Comment on attachment 8777618 [details] [diff] [review]
Followup to fix linker path for comm-* builds

Review of attachment 8777618 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch!

Comment 27

[aleth [:aleth]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/175e41ede356f1978eabf4631950e6efd82cf8e8
Bug 1289847 - Followup to fix linker path for comm-* builds. r=mshal

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/175e41ede356
Followup to fix linker path for comm-* builds. r=mshal

Comment 29

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/43b0a8b09a294030ab799734e5c2a41d5c8e727a
Bug 1289847 - Followup to fix linker path for comm-* builds. r=mshal a=aleth CLOSED TREE DONTBUILD

Comment 30

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/175e41ede356