Opened 3 years ago

Closed 2 years ago

#21328 closed task (fixed)

Move to clang 3.8.0 for Tor Browser's clang-based macOS toolchain

Reported by: gk Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-gitian, ff52-esr, GeorgKoppen201703, tbb-7.0-must-nightly, TorBrowserTeam201704R
Cc: boklm Actual Points:
Parent ID: #21147 Points:
Reviewer: Sponsor: Sponsor4

Description

Mozilla switched to clang 3.8.0 for their cross-compilation toolchain a while ago. We should do the same. This has the benefit that we can get away from using some random commit having signed tarballs instead.

Child Tickets

TicketStatusOwnerSummaryComponent
#10369closedgkBuilding the Utils component in OS X TBBs is broken with the new cross-compilerApplications/Tor Browser
#19783closedtbb-teamTypo in build helpers: `MAXOSX_DEPLOYEMENT_TARGET`Applications/Tor Browser
#21753closedtbb-teamGet rid of old GCC toolchain in pluggable transports macOS descriptorApplications/Tor Browser
#21754closedtbb-teamSwitch to OS X 10.7 SDK everywhere in our OS X descriptorsApplications/Tor Browser

Attachments (1)

tor.diff (84.8 KB) - added by gk 2 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 Changed 3 years ago by gk

Just bumping the version is not enough, see https://bugzilla.mozilla.org/show_bug.cgi?id=1273981.

comment:2 Changed 3 years ago by cypherpunks

Just bumping the version is not enough

now or for ff52-esr too?

comment:3 Changed 3 years ago by gk

Parent ID: #21147

Just for ESR52.

comment:4 Changed 3 years ago by gk

Keywords: TorBrowserTeam201702 added; TorBrowserTeam201701 removed

Moving our tickets to Feb 2017.

comment:5 Changed 3 years ago by gk

Sponsor: Sponsor4

This is Sponsor4 work

comment:6 Changed 2 years ago by gk

Keywords: tbb-7.0-must added

comment:7 Changed 2 years ago by gk

Keywords: TorBrowserTeam201703 added; TorBrowserTeam201702 removed

Moving tickets to March

comment:8 Changed 2 years ago by gk

Keywords: GeorgKoppen201703 added

Adding to my plate.

comment:9 Changed 2 years ago by gk

Okay, here is my plan. clang 3.8.0 requires GCC 4.8 which our current host system does not have. So, we actually have two options to fix that:

1) We build a recent enough GCC (or clang for that matter) first and then build our clang. That could be tricky and time-consuming given a non-standard location of that GCC and its related libs (https://btorpey.github.io/blog/2015/01/02/building-clang/ has a fancy tutorial for that).

2) We could just use Debian Jessie (which comes with GCC 4.9) instead of Debian Wheezy. I tried that today and it worked as expected.

I think I'll do 2) and spend a bit time on getting rid of the old macOS toolchain which allows us to get rid of i386 VMs in the macOS build process. I am almost done with a respective patch for #10369 which is the bulk of that work. Then we only need to adapt the PT descriptor (i.e. we need to switch to using clang instead of GCC).

comment:10 Changed 2 years ago by gk

It seems moving to the new toolchain has reproducibility problems as a result. :( I just bumped the Debian distro to Jessie and put clang together according to https://bugzilla.mozilla.org/show_bug.cgi?id=1273981 and the resulting dmgs don't match anymore. Using the old clang compiler does not have the same problems.

comment:11 Changed 2 years ago by cypherpunks

comment:12 in reply to:  10 ; Changed 2 years ago by gk

Cc: boklm added

Replying to gk:

It seems moving to the new toolchain has reproducibility problems as a result. :( I just bumped the Debian distro to Jessie and put clang together according to https://bugzilla.mozilla.org/show_bug.cgi?id=1273981 and the resulting dmgs don't match anymore. Using the old clang compiler does not have the same problems.

Okay, attached is the diff. I used https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/log/?h=bug_21328_v3. The problematic commit is: https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_21328_v3&id=868433db05b3dc57926e32b79213383483f15f59. Testing with the ones before that one gives me matching builds on the same build machine. Some things we could do to find the problem:

1) We could try to pinpoint where exactly the issue is. Is it in tor code or in openssl code, or...?
2) We could test whether the update to Jessie is causing this (by taking https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_21328_v3&id=6ff54b812b08f3b64fb277f0541dfd9562bf3d6e and just bump everything to Jessie)
3) We could just use the clang part for compiling the remaining utils and tor (and not include libcxx nor libcxxabi which should not be needed anyway) to make sure the issue is inside clang.
4) Assuming this is indeed in clang, because the investigation done in 1) - 3) shows that, we could do some bisecting finding the problematic commit.

boklm: Can you take that one from here?

Changed 2 years ago by gk

Attachment: tor.diff added

comment:13 Changed 2 years ago by gk

Keywords: tbb-7.0-must-nightly added

We want those tickets for our first ESR52 nightlies.

comment:14 in reply to:  12 ; Changed 2 years ago by boklm

Replying to gk:

boklm: Can you take that one from here?

Ok, I'm currently doing some test builds to find which change is causing the diff.

In your tests, was the tor binary the only file that was not reproducible, or did it affect other parts too (such as firefox)?

comment:15 in reply to:  14 Changed 2 years ago by gk

Replying to boklm:

Replying to gk:

boklm: Can you take that one from here?

Ok, I'm currently doing some test builds to find which change is causing the diff.

In your tests, was the tor binary the only file that was not reproducible, or did it affect other parts too (such as firefox)?

Just the tor binary.

comment:16 Changed 2 years ago by boklm

It looks like the update to jessie is causing the diff.

I did some test rebuilds of tor, openssl and libevent using rbm (the current master branch is reproducible on OSX).

I used the following patch to update to jessie:

diff --git a/rbm.conf b/rbm.conf
index e206193..0ffdb22 100644
--- a/rbm.conf
+++ b/rbm.conf
@@ -166,7 +166,7 @@ targets:
   torbrowser-osx-x86_64:
     - osx-x86_64
   osx-x86_64:
-    distribution: Debian-7.11
+    distribution: Debian-8.7
     arch: x86_64
     var:
       osx: 1

And I built the tor compenent using this command:

./rbm/rbm build tor --target alpha --target torbrowser-osx-x86_64

After doing two builds, cleaning tor, openssl and libevent between the two builds, I get a non-matching tor binary:
https://people.torproject.org/~boklm/tmp/bug_21328/move-jessie/build1/
https://people.torproject.org/~boklm/tmp/bug_21328/move-jessie/build2/

In build3 and build4, I did two builds of the tor component only (without cleaning openssl and libevent between them), and still get a non-matching tor binary:
https://people.torproject.org/~boklm/tmp/bug_21328/move-jessie/build3/
https://people.torproject.org/~boklm/tmp/bug_21328/move-jessie/build4/

comment:17 Changed 2 years ago by boklm

The reason for this diff seems to be that the path to libfaketime.so.1 changed between wheezy and jessie.

Debian wheezy: /usr/lib/faketime/libfaketime.so.1
Debian jessie: /usr/lib/x86_64-linux-gnu/faketime/libfaketime.so.1

So we need to fix the libfaketime path there:
https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/tree/gitian/descriptors/mac/gitian-tor.yml?h=bug_21328_v3#n73

Fixing this path removed the diff in my test with rbm (although I did not try with the other changes yet).

comment:18 Changed 2 years ago by gk

Keywords: TorBrowserTeam201703R added; TorBrowserTeam201703 removed
Status: newneeds_review

This makes sense, thanks. I just pushed bug_21328_v4 (the new commit https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_21328_v4&id=b9ee73abc959b930186138ea203152d1773b24f8) with small changes and the one mentioned in comment:17.

comment:19 in reply to:  18 Changed 2 years ago by boklm

Replying to gk:

This makes sense, thanks. I just pushed bug_21328_v4 (the new commit https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_21328_v4&id=b9ee73abc959b930186138ea203152d1773b24f8) with small changes and the one mentioned in comment:17.

I have been building this branch twice and got the same result.

comment:20 in reply to:  18 Changed 2 years ago by boklm

Replying to gk:

This makes sense, thanks. I just pushed bug_21328_v4 (the new commit https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_21328_v4&id=b9ee73abc959b930186138ea203152d1773b24f8) with small changes and the one mentioned in comment:17.

The changes in this branch look good to me.

I also pushed branch bug_21328 in my tor-browser-build.git user repo, doing corresponding changes to the rbm based build:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/log/?h=bug_21328

comment:21 Changed 2 years ago by gk

Keywords: TorBrowserTeam201704R added; TorBrowserTeam201703R removed

Moving review tickets to April.

comment:22 Changed 2 years ago by gk

Keywords: tbb-7.0-must removed

No need to use somewhat duplicated keywords.

comment:23 Changed 2 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

This is commit cd4465477ac10d2b740d5a32505cdab5a197e405 on master.

Note: See TracTickets for help on using tickets.