#23228 closed task (fixed)

Build a Windows 64 toolchain based on mingw-w64

Reported by: boklm Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm, TorBrowserTeam201711R
Cc: boklm Actual Points:
Parent ID: #20636 Points:
Reviewer: Sponsor:

Description

In tor-browser-build.git, we need to add support for building a mingw-w64 toolchain for creating Windows 64bit binaries.

Child Tickets

Attachments (1)

0001-64bit-fixups.patch (2.0 KB) - added by gk 20 months ago.

Download all attachments as: .zip

Change History (29)

comment:1 Changed 21 months ago by boklm

Keywords: TorBrowserTeam201708R added
Status: newneeds_review

This is done with commit 80026a6348856f161ef2ea70d06b0c93939e68b5 in branch bug_23228_v1:
https://oniongit.eu/boklm/tor-browser-build/commit/80026a6348856f161ef2ea70d06b0c93939e68b5

comment:2 Changed 21 months ago by boklm

A build of mingw-w64 for Windows 64 can be done with:

$ ./rbm/rbm build mingw-w64 --target torbrowser-windows-x86_64 --target alpha

comment:3 Changed 20 months ago by gk

Keywords: TorBrowserTeam201709R added; TorBrowserTeam201708R removed

Moving reviews to September.

comment:4 Changed 20 months ago by boklm

An updated version of the patch is in commit b9904226f49a7c5a15c31e8466d7af57f8239538 from branch bug_20636_v5:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_20636_v5&id=b9904226f49a7c5a15c31e8466d7af57f8239538

In this new version of the patch we remove the projects/mingw-w64/i686-w64-mingw32-* files (which are now created in projects/mingw-w64/build).

comment:5 in reply to:  4 Changed 20 months ago by gk

Keywords: TorBrowserTeam201709 added; TorBrowserTeam201709R removed
Status: needs_reviewneeds_revision

Replying to boklm:

An updated version of the patch is in commit b9904226f49a7c5a15c31e8466d7af57f8239538 from branch bug_20636_v5:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_20636_v5&id=b9904226f49a7c5a15c31e8466d7af57f8239538

In this new version of the patch we remove the projects/mingw-w64/i686-w64-mingw32-* files (which are now created in projects/mingw-w64/build).

I think this needs revision as we need the relocation patch for 64-bit builds as well. Having the DLL characteristics showing proper output as the script we use does is necessary but not sufficient. We need an relocation table as well. Otherwise we land in the same situation as the bitcoin people (https://github.com/bitcoin/bitcoin/issues/8248) (I guess we can point them to our stuff to fix their problem?)). (comment:description:ticket:10505 summarizes the requirements, too)

There is an old message on the mingw-w64 mailing list that summarizes all the pitfalls and requirements good: https://sourceforge.net/p/mingw-w64/mailman/message/31034877/

skruffy wrote the patch for Tor Browser on Windows 32 platforms (PE) but we need to fix it for 64-bit (PE+ or probably PEP in binutils lingo). I think the way to go is to look at the pep-dll.c to understand what it does and add the missing things (I guess we could think about patching pep.em as well to rename pe_dll_enable_reloc_section to pep_dll_enable_reloc_section to make things more straightforward, but maybe not. I have not looked that close).

Something for a different ticket I guess: Given that the script that checks the headers for ASLR is not enough to be sure we have ASLR enabled we should think about a better test.

comment:6 Changed 20 months ago by gk

Oh, I forgot to mention: https://sourceware.org/bugzilla/show_bug.cgi?id=19011 is a good read as well. The ffmpeg people seem to use a different workaround to avoid the crashing (mentioned in the mingw-w64 mailing list message above) when specifying -pie (see: https://github.com/TheRyuu/FFmpeg/commit/91b668acd6decec0a6f8d20bf56e2644f96adcb9).

comment:7 Changed 20 months ago by boklm

I opened #23458 to check if we could use the FFmpeg workaround.

comment:8 Changed 20 months ago by gk

Just looking at the patch skruffy wrote: what about the attached patch trying to take the 64bit case into account? The compile issue is gone. Not sure if ASLR is working, though. If there are some binaries with and without the patch I could test I guess. Maybe tor binaries would already be enough.

Changed 20 months ago by gk

Attachment: 0001-64bit-fixups.patch added

comment:10 Changed 20 months ago by boklm

Ah, I just noticed that I forgot re-adding the --enable-reloc-section flag to the x86_64 builds, so the builds with the patch are probably wrong. I will redo them.

comment:11 in reply to:  10 Changed 20 months ago by gk

Replying to boklm:

Ah, I just noticed that I forgot re-adding the --enable-reloc-section flag to the x86_64 builds, so the builds with the patch are probably wrong. I will redo them.

I can confirm that they match the no-patch-behavior. :)

comment:14 Changed 20 months ago by cypherpunks

You are building Firefox without adapted config https://dxr.mozilla.org/mozilla-esr52/source/old-configure.in#1216, so there could be surprises.

comment:15 in reply to:  12 Changed 20 months ago by gk

Replying to boklm:

The patch adding the flag back:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_20636_v6&id=90c730d131d3e003ff626ed613f37c43ada0cb7e

And the build with the patch and the flag:
https://people.torproject.org/~boklm/tmp/bug_23228/with-patch/tor-0.3.1.5-alpha-windows-x86_64-3eb3d4/tor-win32-0.3.1.5-alpha.zip.asc
https://people.torproject.org/~boklm/tmp/bug_23228/with-patch/tor-0.3.1.5-alpha-windows-x86_64-3eb3d4/tor-win32-0.3.1.5-alpha.zip

Looks better. The ImageBase address changes for me on different restarts, so, yay! You can test that yourself with e.g. ProcessExplorer (opening the lower pane and there should be a column for the addresses). The .exe without the patch has 0x400000 as ImageBase address.

This might need more testing, I guess, and someone familiar with binutils to look over the changes.
Not finding anyone to do so should at least not block nightly builds with the new toolchain.

comment:16 Changed 20 months ago by cypherpunks

One dllexport function, and you don't need all that patches (like Firefox without --disable-sandbox).

comment:17 Changed 19 months ago by boklm

I can confirm that both tor.exe and firefox.exe have 0x400000 as ImageBase in the version without patch, and something different in the version with the patch.

comment:18 in reply to:  17 ; Changed 19 months ago by gk

Replying to boklm:

I can confirm that both tor.exe and firefox.exe have 0x400000 as ImageBase in the version without patch, and something different in the version with the patch.

You mean something different with every restart, right? If so, I guess we can start using that idea and close #23458? If you agree could you provide an updated patch for this ticket for review?

comment:19 Changed 19 months ago by cypherpunks

We could fix ld, if you wish.

comment:20 in reply to:  19 ; Changed 19 months ago by gk

Replying to cypherpunks:

We could fix ld, if you wish.

Yes, I wish but aren't we doing that? If not, how would a fix look like?

comment:21 in reply to:  20 Changed 19 months ago by cypherpunks

Replying to gk:

Replying to cypherpunks:

We could fix ld, if you wish.

Yes, I wish but aren't we doing that? If not, how would a fix look like?

That patches are hacks and are not upstreamable. The fix would change the behavior of ld back to normal and would fulfill the requirements of https://sourceware.org/bugzilla/show_bug.cgi?id=19011.

comment:22 Changed 19 months ago by cypherpunks

So, if you're ready to get it going till merged upstream, then I can provide the required info.

comment:23 in reply to:  22 Changed 19 months ago by gk

Replying to cypherpunks:

So, if you're ready to get it going till merged upstream, then I can provide the required info.

Not sure what exactly you want from us. Of course, we would like to upstream things. What special info is it that you have and we may lack?

comment:24 Changed 19 months ago by cypherpunks

I can write what to fix and how, if you agree to do all other things of this process.

comment:25 Changed 19 months ago by gk

Keywords: TorBrowserTeam201710 added; TorBrowserTeam201709 removed

Items for October 2017

comment:26 in reply to:  18 Changed 18 months ago by boklm

Keywords: TorBrowserTeam201710R added; TorBrowserTeam201710 removed
Status: needs_revisionneeds_review

Replying to gk:

Replying to boklm:

I can confirm that both tor.exe and firefox.exe have 0x400000 as ImageBase in the version without patch, and something different in the version with the patch.

You mean something different with every restart, right? If so, I guess we can start using that idea and close #23458? If you agree could you provide an updated patch for this ticket for review?

Yes. I checked again with a new build that ImageBase is changing on each restart.

My branch bug_23228_v2 has a patch for review:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_23228_v2&id=1a3203baca12e9dcd8b5aa22ee1cdeb8b80fb727

comment:27 Changed 18 months ago by gk

Keywords: TorBrowserTeam201711R added; TorBrowserTeam201710R removed

Moving review to November

comment:28 Changed 18 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Looks good to me. This is commit b82d82d9cf9cb751e088085fb1093026b20a0e4d on master, thanks. boklm: Could you update the other patches for the win64 series accordingly so that we get nightlies built asap?

Note: See TracTickets for help on using tickets.