1209930 - Update Mac clang to match the version in use everywhere else

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

bug 1205242 updated clang for basically everything *except* normal Mac builds. Ehsan and I talked about this briefly last week, he expected that updating would be a headache due to dealing with Talos fallout etc.

We should do this because we're going to hit this headache if we try to switch to the cross-mac builds (which are using the newer clang) anyway, so we might as well at least isolate it.

Comment 1

[(not currently active) Ted Mielczarek]

Try push to see how our tests and talos like it:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2247576feca

Comment 2

[(not currently active) Ted Mielczarek]

Lots of orange there, but it looks to be all the same crash: [@ jsimd_ycc_extbgrx_convert_sse2.rowloop + 0x6]

Comment 3

[(not currently active) Ted Mielczarek]

We're crashing here:
https://dxr.mozilla.org/mozilla-central/source/media/libjpeg/simd/jdcolext-sse2-64.asm#84

Comment 4

[(not currently active) Ted Mielczarek]

I can reliably reproduce this by running this build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-c2247576feca/try-macosx64-debug/firefox-44.0a1.en-US.mac64.dmg

and loading this jpg:
https://dxr.mozilla.org/mozilla-central/raw/dom/tests/mochitest/ajax/offline/jupiter.jpg

Comment 5

[(not currently active) Ted Mielczarek]

I did a local build with that version of clang and it doesn't crash, so it must have something to do with the version of cctools in use or maybe some other build options (I didn't use the exact mozconfig).

Comment 6

[(not currently active) Ted Mielczarek]

Uh, whoops, my local build was --disable-optimize, that's probably important. I'll try again with optimization enabled.

Comment 7

[(not currently active) Ted Mielczarek]

Ok, I can reproduce in a local optimized debug build, so that's good.

Comment 8

[(not currently active) Ted Mielczarek]

RyanVM points out that this issue is likely fixed in upstream libjpeg-turbo:
https://github.com/libjpeg-turbo/libjpeg-turbo/blob/1.4.x/ChangeLog.txt#L14

Comment 9

[(not currently active) Ted Mielczarek]

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

Comment 10

[(not currently active) Ted Mielczarek]

Pushed again to try on top of the patch from bug 1176196, which ought to fix that crash.

Comment 11

[(not currently active) Ted Mielczarek]

Try results are looking pretty great so far, need to get Talos results too.

Comment 12

[(not currently active) Ted Mielczarek]

Attachment: MozReview Request: bug 1209930 - update Mac clang version to match everywhere else. r?glandium

bug 1209930 - update Mac clang version to match everywhere else. r?glandium

Comment 13

[(not currently active) Ted Mielczarek]

We still need to wait on the full set of Talos data, and on the Talos data from bug 1176196, but modulo that this looks good.

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8669857 [details]
MozReview Request: bug 1209930 - update Mac clang version to match everywhere else. r?glandium

https://reviewboard.mozilla.org/r/21269/#review19203

::: browser/config/tooltool-manifests/macosx64/releng.manifest:6
(Diff revision 1)
> -"size": 59602619, 
> -"digest": "86662ebc0ef650490559005948c4f0cb015dad72c7cac43732c2bf2995247081e30c139cf8008d19670a0009fc302c4eee2676981ee3f9ff4a15c01af22b783b", 
> -"algorithm": "sha512", 
> -"filename": "clang.tar.bz2",
> +"size": 97314461,
> +"digest": "9a74670fa917f760a4767923485d5166bbd258a8023c8aeb899b8c4d22f2847be76508ac5f26d7d2193318a2bb368a71bc62888d1bfe9d81eb45329a60451aa4",
> +"algorithm": "sha512",
> +"filename": "clang.tar.xz",

Considering this syncs releng.manifest and clang.manifest, maybe it's the right time to remove clang.manifest and have testing/mozharness/configs/builds/releng_sub_mac_configs/64_stat_and_debug.py point to releng.manifest?

Comment 15

[(not currently active) Ted Mielczarek]

I talked to decoder about that and he was OK with making ASAN builds point to releng.manifest (not that we have Mac ASAN builds in automation anyway). If Ehsan is OK with making static analysis builds point to the same manifest that'd be a nice simplification. It's not particularly difficult to change back nowadays with everything in tree.

Comment 16

[(no longer active)]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #15)
> I talked to decoder about that and he was OK with making ASAN builds point
> to releng.manifest (not that we have Mac ASAN builds in automation anyway).
> If Ehsan is OK with making static analysis builds point to the same manifest
> that'd be a nice simplification. It's not particularly difficult to change
> back nowadays with everything in tree.

I prefer to not do that yet.  As things stand, we can't use clang 3.7 for static analysis on OSX yet.

Comment 17

[(not currently active) Ted Mielczarek]

The Talos results show a potential regression in Tp5o:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=6e1fa8593bbc&newProject=try&newRevision=348098145706&showExcludedPlatforms=1&hideMinorChanges=1

I'll try spinning a few more test runs of that test to see if it persists.

Comment 18

[(not currently active) Ted Mielczarek]

Looks like Tp5o is just noisy:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=6e1fa8593bbc&newProject=try&newRevision=348098145706&showExcludedPlatforms=1&hideMinorChanges=1

With 10 test runs for each changeset there's no regression showing. I'll land this.

Comment 19

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a43f6bb86588c323f5fa36b78b53adfd0f161c9e
bug 1209930 - update Mac clang version to match everywhere else. r=glandium

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a43f6bb86588

Comment 21

[(no longer active)]

\o/