Skip to content

Conversation

@knst
Copy link
Collaborator

@knst knst commented Feb 9, 2023

Issue being fixed or feature implemented

This PR enables building dash in Guix containers. Guix is a transactional package manager much like Nix, but unlike Nix, it has more of a focus on bootstrappability and reproducibility which are attractive for security-sensitive projects like dash, bitcoin.

Furthermore seems as Guix will replace gitian for binary builds.

What was done?

For this first PR backported minimum amount of PR to make guix run successful in my environment.
Currently it can build these targets:

  • aarch64-linux-gnu
  • arm-linux-gnueabihf
  • riscv64-linux-gnu
  • x86_64-linux-gnu
  • x86_64-w64-mingw32

There is NO DASHification yet
Please, help me to test guix on your local host, review this pull request and give me your feedback.
I'd like to do DASHification later, bitcoin has ~50 other backports that are easier to merge without dashification at this early stage.

How Has This Been Tested?

Were built all 5 targets on my kubuntu 22.xx and tried to run some binaries

Breaking Changes

No breaking changes

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

@knst
Copy link
Collaborator Author

knst commented Feb 9, 2023

To run guix build, please, run:

./contrib/guix/guix-build.sh clean

To run guix build for specific target, run:

HOSTS=x86_64-linux-gnu ./contrib/guix/guix-build.sh clean

To clean up working directory for new build, this command will help:

rm -rf distsrc-* outputs/ windeploy/unsigned/

@knst
Copy link
Collaborator Author

knst commented Feb 10, 2023

I got these hashes:

$ for i in $(cat my_list) ; do sha1sum $i ; done
b087f2488184a2495bae45cd12c076b4544836f4  distsrc-aarch64-linux-gnu/src/dashd
9cd67801799b14a9b20c0803684c1267f55a54f9  distsrc-arm-linux-gnueabihf/src/dashd
e6ead65c23c05d02ce906560741136d96c73f4bc  distsrc-riscv64-linux-gnu/src/dashd
9de809968908176b3c90edc342d9e11e4230ca6c  distsrc-x86_64-linux-gnu/src/dashd
6e8fd5055868b90fb94c221c73fef74bd8181f63  distsrc-x86_64-w64-mingw32/src/dashd.exe

@UdjinM6
Copy link

UdjinM6 commented Feb 10, 2023

$ sha1sum ./distsrc-*/src/dashd{,.exe}
57e0bc2c47ac702c397a530768140e789c3b1f8a  ./distsrc-aarch64-linux-gnu/src/dashd
b28ecdaebdd333c8cc4c9789b0fc753afd526b75  ./distsrc-arm-linux-gnueabihf/src/dashd
8e0d6d093686eb9e57168da72d509f3786cfbd10  ./distsrc-riscv64-linux-gnu/src/dashd
780c610f4b22805bbece416047e54ff8a42d7f75  ./distsrc-x86_64-linux-gnu/src/dashd
43a42bd48c4917c52173198d22666be3730fdb04  ./distsrc-x86_64-w64-mingw32/src/dashd.exe

🤷‍♂️

@UdjinM6
Copy link

UdjinM6 commented Feb 10, 2023

gitian for linux (tested it because of changes in contrib/gitian-descriptors/gitian-linux.yml)

$ ./dash/contrib/gitian-build.py --detach-sign --no-commit -b -p -j 4 -o l UdjinM6 5194
...
c4095f852653daf82f3ae3a3daf7c6b19ab283b2f2b00bf75ef31f6d28787a05  dashcore-e830d6b6def6-aarch64-linux-gnu-debug.tar.gz
1bec764a94fc18a8813100f21a2daf671996b97deb05cf02f2fb24890a6c9a79  dashcore-e830d6b6def6-aarch64-linux-gnu.tar.gz
02c74ae4726ae357ec5cfeaa32b2133c4c058ac3f5ff91b8cddca48d88cd0116  dashcore-e830d6b6def6-riscv64-linux-gnu-debug.tar.gz
ae5373523784c5e75a25ae30a8c52cee4786e2ee820d71561150f9794da148b1  dashcore-e830d6b6def6-riscv64-linux-gnu.tar.gz
e4a2a010ae7d9825067435745a7701c37614cba15e90dd439a2d8c71ed50bb72  dashcore-e830d6b6def6-x86_64-linux-gnu-debug.tar.gz
18387439ca495831e931342ed5eb56adcb8a14163def8e374e651abd4df0945e  dashcore-e830d6b6def6-x86_64-linux-gnu.tar.gz
77038d37adc5abadaaa4a54dbe46e301fa59dab494051103aba757c1fffc569b  src/dashcore-e830d6b6def6.tar.gz
19371d3c6db7d93a3c9fda63251b187cabe92e1507fead7a4ce0c4f306907a49  dash-linux-18-res.yml

@thephez
Copy link
Collaborator

thephez commented Feb 13, 2023

I got this for hashes so far.

57e0bc2c47ac702c397a530768140e789c3b1f8a  ./distsrc-aarch64-linux-gnu/src/dashd
b28ecdaebdd333c8cc4c9789b0fc753afd526b75  ./distsrc-arm-linux-gnueabihf/src/dashd
780c610f4b22805bbece416047e54ff8a42d7f75  ./distsrc-x86_64-linux-gnu/src/dashd

Mine seems to have choked on the riscv64 build.

make[4]: Leaving directory '/bitcoin/depends/work/build/riscv64-linux-gnu/qt/5.12.11-14dca4cda29/qtbase/src/widgets'
make[3]: Leaving directory '/bitcoin/depends/work/build/riscv64-linux-gnu/qt/5.12.11-14dca4cda29/qtbase/src'
make[2]: *** [Makefile:51: sub-src-make_first] Error 2
make[2]: Leaving directory '/bitcoin/depends/work/build/riscv64-linux-gnu/qt/5.12.11-14dca4cda29/qtbase'
make[1]: *** [Makefile:49: sub-qtbase-make_first] Error 2
make[1]: Leaving directory '/bitcoin/depends/work/build/riscv64-linux-gnu/qt/5.12.11-14dca4cda29'
make: *** [funcs.mk:262: /bitcoin/depends/work/build/riscv64-linux-gnu/qt/5.12.11-14dca4cda29/./.stamp_built] Error 2

@knst knst marked this pull request as draft February 13, 2023 17:03
@PastaPastaPasta
Copy link
Member

My hash matches with KNST

sha1sum ./distsrc-x86_64-linux-gnu/src/dashd             
9de809968908176b3c90edc342d9e11e4230ca6c  ./distsrc-x86_64-linux-gnu/src/dashd

@PastaPastaPasta
Copy link
Member

On a brand new ubuntu server I got the following:

root@pasta3:~/dash# sha1sum ./distsrc-*/src/dashd
780c610f4b22805bbece416047e54ff8a42d7f75  ./distsrc-x86_64-linux-gnu/src/dashd

@thephez
Copy link
Collaborator

thephez commented Feb 16, 2023

On a brand new ubuntu server I got the following:

root@pasta3:~/dash# sha1sum ./distsrc-*/src/dashd
780c610f4b22805bbece416047e54ff8a42d7f75  ./distsrc-x86_64-linux-gnu/src/dashd

Welcome to team 780c 👋
Curious - is this a different OS than your previous build?

@PastaPastaPasta
Copy link
Member

Both are

No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 20.04.4 LTS
Release:        20.04
Codename:       focal

@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Feb 17, 2023

I've reproduced 780c inside of a docker container

3e98697c2f35:/dash# sha1sum ./distsrc-*/src/dashd
57e0bc2c47ac702c397a530768140e789c3b1f8a  ./distsrc-aarch64-linux-gnu/src/dashd
8e0d6d093686eb9e57168da72d509f3786cfbd10  ./distsrc-riscv64-linux-gnu/src/dashd
780c610f4b22805bbece416047e54ff8a42d7f75  ./distsrc-x86_64-linux-gnu/src/dashd

Steps to reproduce

# cd to a good working directory
# download the Dockerfile I used
wget https://gist.githubusercontent.com/PastaPastaPasta/a7c9574ab10e84040638088d17cd6142/raw/a97b3c6b9ec8a11065eea83a7ffc48d3f9f3e6d9/Dockerfile
# Build the docker image
DOCKER_BUILDKIT=1 docker build --pull --no-cache -t alpine-guix - < Dockerfile
# start the container (note this will run in the foreground, consider appending "&" to run in the background
docker run -it --name alpine-guix --privileged alpine-guix
# get a  bash terminal
docker exec -it alpine-guix /bin/bash

# checkout 5194
git fetch origin pull/5194/head:pr_5194
git checkout pr_5194

# start the build (only for x86 here) and then get the hash
time BASE_CACHE="/base_cache" SOURCE_PATH="/sources" SDK_PATH="/SDKs" HOSTS="x86_64-linux-gnu" ./contrib/guix/guix-build.sh
sha1sum ./distsrc-*/src/dashd

@PastaPastaPasta
Copy link
Member

Soo, imo we can move forward with these backports and other guix backports. I think especially if we can ensure determinism via a docker image then there's no issue

@UdjinM6
Copy link

UdjinM6 commented Feb 20, 2023

Agree. At least it's always deterministic and reproducible when building on the same machine :)

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@UdjinM6 UdjinM6 added this to the 19 milestone Feb 20, 2023
@UdjinM6 UdjinM6 marked this pull request as ready for review February 20, 2023 10:15
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK for merging via merge commit

laanwj and others added 10 commits February 20, 2023 09:09
751549b contrib: guix: Additional clarifications re: substitutes (Carl Dong)
cd3e947 contrib: guix: Various improvements. (Carl Dong)
8dff3e4 contrib: guix: Clarify SOURCE_DATE_EPOCH. (Carl Dong)
3e80ec3 contrib: Add deterministic Guix builds. (Carl Dong)

Pull request description:

  ~~**This post is kept updated as this project progresses. Use this [latest update link](bitcoin#15277 (comment)) to see what's new.**~~

  Please read the `README.md`.

  -----

  ### Guix Introduction

  This PR enables building bitcoin in Guix containers. [Guix](https://www.gnu.org/software/guix/manual/en/html_node/Features.html) is a transactional package manager much like Nix, but unlike Nix, it has more of a focus on [bootstrappability](https://www.gnu.org/software/guix/manual/en/html_node/Bootstrapping.html) and [reproducibility](https://www.gnu.org/software/guix/blog/tags/reproducible-builds/) which are attractive for security-sensitive projects like bitcoin.

  ### Guix Build Walkthrough

  Please read the `README.md`.

  [Old instructions no. 4](bitcoin#15277 (comment))

  [Old instructions no. 3](bitcoin#15277 (comment))

  [Old instructions no. 2](bitcoin#15277 (comment))

  <details>
  <summary>Old instructions no. 1</summary>
  In this PR, we define a Guix [manifest](https://www.gnu.org/software/guix/manual/en/html_node/Invoking-guix-package.html#profile_002dmanifest) in `contrib/guix/manifest.scm`, which declares what packages we want in our environment.

  We can then invoke
  ```
  guix environment --manifest=contrib/guix/manifest.scm --container --pure --no-grafts --no-substitutes
  ```
  To have Guix:
  1. Build an environment containing the packages we defined in our `contrib/guix/manifest.scm` manifest from the Guix bootstrap binaries (see [bootstrappability](https://www.gnu.org/software/guix/manual/en/html_node/Bootstrapping.html) for more details).
  2. Start a container with that environment that has no network access, and no access to the host's filesystem except to the `pwd` that it was started in.
  3. Drop you into a shell in that container.

  > Note: if you don't want to wait hours for Guix to build the entire world from scratch, you can eliminate the `--no-substitutes` option to have Guix download from available binary sources. Note that this convenience doesn't necessarily compromise your security, as you can check that a package was built correctly after the fact using `guix build --check <packagename>`

  Therefore, we can perform a build of bitcoin much like in Gitian by invoking the following:

  ```
  make -C depends -j"$(nproc)" download && \
      cat contrib/guix/build.sh | guix environment --manifest=contrib/guix/manifest.scm --container --pure --no-grafts --no-substitutes
  ```

  We don't include `make -C depends -j"$(nproc)" download` inside `contrib/guix/build.sh` because `contrib/guix/build.sh` is run inside the container, which has no network access (which is a good thing).
  </details>

  ### Rationale

  I believe that this represents a substantial improvement for the "supply chain security" of bitcoin because:

  1. We no longer have to rely on Ubuntu for our build environment for our releases ([oh the horror](https://github.com/bitcoin/bitcoin/blob/72bd4ab867e3be0d8410403d9641c08288d343e3/contrib/gitian-descriptors/gitian-linux.yml#L10)), because Guix builds everything about the container, we can perform this on almost any Linux distro/system.
  2. It is now much easier to determine what trusted binaries are in our supply chain, and even make a nice visualization! (see [bootstrappability](https://www.gnu.org/software/guix/manual/en/html_node/Bootstrapping.html)).
  3. There is active effort among Guix folks to minimize the number of trusted binaries even further. OriansJ's [stage0](https://github.com/oriansj/stage0), and janneke's [Mes](https://www.gnu.org/software/mes/) all aim to achieve [reduced binary boostrap](http://joyofsource.com/reduced-binary-seed-bootstrap.html) for Guix. In fact, I believe if OriansJ gets his way, we will end up some day with only a single trusted binary: hex0 (a ~500 byte self-hosting hex assembler).

  ### Steps to Completion

  - [x] Successfully build bitcoin inside the Guix environment
  - [x] Make `check-symbols` pass
  - [x] Do the above but without nasty hacks
  - [x] Solve some of the more innocuous hacks
  - [ ] Make it cross-compile (HELP WANTED HERE)
    - [x] Linux
      - [x] x86_64-linux-gnu
      - [x] i686-linux-gnu
      - [x] aarch64-linux-gnu
      - [x] arm-linux-gnueabihf
      - [x] riscv64-linux-gnu
    - [ ] OS X
      - [ ] x86_64-apple-darwin14
    - [ ] Windows
      - [ ] x86_64-w64-mingw32
  - [ ] Maybe make importer for depends syntax
  - [ ] Document build process for future releases
  - [ ] Extra: Pin the revision of Guix that we build with with Guix [inferiors](https://www.gnu.org/software/guix/manual/en/html_node/Inferiors.html)

  ### Help Wanted

  [Old content no. 3](bitcoin#15277 (comment))

  [Old content no. 2](bitcoin#15277 (comment))

  <details>
  <summary>Old content no. 1</summary>
  As of now, the command described above to perform a build of bitcoin a lot like Gitian works, but fails at the `check-symbols` stage. This is because a few dynamic libraries are linked in that shouldn't be.

  Here's what `ldd src/bitcoind` looks like when built in a Guix container:
  ```
  	linux-vdso.so.1 (0x00007ffcc2d90000)
  	libdl.so.2 => /gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/libdl.so.2 (0x00007fb7eda09000)
  	librt.so.1 => /gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/librt.so.1 (0x00007fb7ed9ff000)
  	libstdc++.so.6 => /gnu/store/4sqps8dczv3g7rwbdibfz6rf5jlk7w90-gcc-5.5.0-lib/lib/libstdc++.so.6 (0x00007fb7ed87c000)
  	libpthread.so.0 => /gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/libpthread.so.0 (0x00007fb7ed85b000)
  	libm.so.6 => /gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/libm.so.6 (0x00007fb7ed6da000)
  	libgcc_s.so.1 => /gnu/store/4sqps8dczv3g7rwbdibfz6rf5jlk7w90-gcc-5.5.0-lib/lib/libgcc_s.so.1 (0x00007fb7ed6bf000)
  	libc.so.6 => /gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/libc.so.6 (0x00007fb7ed506000)
  	/gnu/store/h90vnqw0nwd0hhm1l5dgxsdrigddfmq4-glibc-2.28/lib/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007fb7ee3a0000)
  ```

  And here's what it looks in one of our releases:
  ```
  	linux-vdso.so.1 (0x00007ffff52cd000)
  	libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007f87726b4000)
  	librt.so.1 => /usr/lib/librt.so.1 (0x00007f87726aa000)
  	libm.so.6 => /usr/lib/libm.so.6 (0x00007f8772525000)
  	libgcc_s.so.1 => /usr/lib/libgcc_s.so.1 (0x00007f877250b000)
  	libc.so.6 => /usr/lib/libc.so.6 (0x00007f8772347000)
  	/lib64/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007f8773392000)
  ```

  ~~I suspect it is because my script does not apply the gitian-input patches [described in the release process](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#fetch-and-create-inputs-first-time-or-when-dependency-versions-change) but there is no description as to how these patches are applied.~~ It might also be something else entirely.

  Edit: It is something else. It appears that the gitian inputs are only used by [`gitian-win-signer.yml`](https://github.com/bitcoin/bitcoin/blob/d6e700e40f861ddd6743f4d13f0d6f6bc19093c2/contrib/gitian-descriptors/gitian-win-signer.yml#L14)
  </details>

  ### How to Help

  1. Install Guix on your distro either [from source](https://www.gnu.org/software/guix/manual/en/html_node/Requirements.html) or perform a [binary installation](https://www.gnu.org/software/guix/manual/en/html_node/Binary-Installation.html#Binary-Installation)
  2. Try out my branch and the command described above!

ACKs for top commit:
  MarcoFalke:
    Thanks for the replies. ACK 751549b
  laanwj:
    ACK 751549b

Tree-SHA512: 50e6ab58c6bda9a67125b6271daf7eff0ca57d0efa8941ed3cd951e5bf78b31552fc5e537b1e1bcf2d3cc918c63adf19d685aa117a0f851024dc67e697890a8d
0065ead contrib: guix: Remove ssp spec file hack (Carl Dong)
0093a58 contrib: guix: More robust search paths, add checks (Carl Dong)

Pull request description:

  See commit messages for more details

ACKs for top commit:
  fanquake:
    ACK 0065ead

Tree-SHA512: fde04005fb31cd4b75b80da4936a7c394f63f0b3bdcc33c20c99e05604a63efd9c850a8d097030ff0bf4b4e83f1a9997fc4621ce291ebcecd8397893447600a7
ac83133 doc: Fix some misspellings (randymcmillan)

Pull request description:

  Here is a more thorough lint-spelling update.
  This PR takes care of easy to fix spelling errors to clean up the linting stages.
  There are misspellings coded into the functional tests.
  That is a whole separate job within itself.

ACKs for top commit:
  practicalswift:
    ACK ac83133 -- diff looks correct

Tree-SHA512: d8fad83fed083715655f148263ddeffc6752c8007d568fcf3dc2c418ccd5db70089ce3ccfd3994fcbd78043171402eb9cca5bdd5125287e22c42ea305aaa6e9d
88c8363 guix: Update documentation for time-machine (Carl Dong)
e605088 guix: Pin Guix using `guix time-machine` (Carl Dong)

Pull request description:

  An alternative to bitcoin#16519, pinning our version of Guix and eliminating a `guix pull` and changing the default Guix profile of builders.

  I think this method might be superior, as it:
  - Eliminates the possibility of future changes to the `guix environment` command line interface breaking our builds
  - Eliminates the need to set up a separate channel repo

  It is a more general pinning solution than bitcoin#16519.

  -----

  The reason why I didn't originally propose this is because `guix time-machine` is a recent addition to Guix, only available since `f675f8dec73d02e319e607559ed2316c299ae8c7`

ACKs for top commit:
  fanquake:
    ACK 88c8363

Tree-SHA512: 85e03b0987ffa86da73e02801e1cd8b7622698d70c4ba4e60561611be1e9717d661c2811a59b3e137b1b8eef2d0ba37c313867d035ebc89c3bd06a23a078064a
…tian

fae9084 build: Skip i686 build by default in guix and gitian (MarcoFalke)
fa55a25 depends: Remove reference to win32 (MarcoFalke)

Pull request description:

  Closes bitcoin#17504

  Now that we no longer provide downloads for i686 on our website (https://bitcoincore.org/en/download/), there is no need to build them by default.

  i686 can still be built in depends (tested by ci/travis) and in guix/gitian by setting the appropriate `HOSTS`.

ACKs for top commit:
  practicalswift:
    ACK fae9084 -- patch looks correct
  dongcarl:
    ACK fae9084 patch looks correct
  laanwj:
    Code review ACK fae9084
  hebasto:
    ACK fae9084, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: b000c19a2cd2a596a52028fa298c4022c24cfdfc1bdb3795a90916d0a00a32e4dd22278db93790b6a11724e08ea8451f4f05c77bc40d1664518e11a8c82d6e29
0ae42a1 guix: Remove now-unnecessary gcc make flag (Carl Dong)

Pull request description:

  ```
  Previously, Guix would produce a gcc which did not know to use the SSP
  function from glibc, and required a gcc make flag for it to do so, in my
  attempt to fix it upstream I realized that this is no longer the case.

  This can be verified by performing a Guix build and doing

    readelf -s ... | grep __stack_chk

  to check that symbols are coming from glibc, and doing

    readelf -d ... | grep NEEDED | grep ssp

  to see that libssp.so is not being depended on
  ```

ACKs for top commit:
  fanquake:
    ACK 0ae42a1 - ran a Guix build (hashes below) and checked all the linux binaries:

Tree-SHA512: 701b91e7c323b12a29af9539cb2656d10ce0a93af573a02e57f0b7fea05a6e1819798536eadb24d0a17e7f35b503f5e863fee5e7409db1b8a3973c4375e49d4e
…arget

a35e323 guix: Appease travis. (Carl Dong)
0b66d22 guix: Use gcc-9 for mingw-w64 instead of 8 (Carl Dong)
ba0b99b guix: Don't set MINGW_HAS_SECURE_API CFLAG in depends (Carl Dong)
93439a7 guix: Bump to upstream commit with mingw-w64 changes (Carl Dong)
35a9679 guix: Check mingw symbols, improve SSP fix docs (Carl Dong)
449d8fe guix: Expand on INT trap message (Carl Dong)
3f1f03c guix: Spelling fixes (Carl Dong)
ff821dd guix: Reinstate make-ssp-fixed-gcc (Carl Dong)
360a9e0 guix: Bump time-machine for mingw-w64 patches (Carl Dong)
93e41b7 guix: Use gcc-8 for mingw-w64 instead of 7 (Carl Dong)
ef4f7e4 guix: Set the well-known timezone env var (Carl Dong)
acf4b3b guix: Make x86_64-w64-mingw32 builds reproducible (Carl Dong)
c4cce00 guix: Remove dead links from README. (Carl Dong)
df953a4 guix: Appease shellcheck. (Carl Dong)
91897c9 guix: Improve guix-build.sh documentation (Carl Dong)
570d769 guix: Build support for Windows (Carl Dong)

Pull request description:

  ~~Based on: bitcoin#16519
  Based on: bitcoin#17933 (Time Machines are... shall we say... superior 😁)

  This PR allows us to perform Guix builds for the `x86_64-w64-mingw32` target. We do this _without_ splitting up the build script like we do in Gitian by using this newfangled alien technology called `case` statements. (This is WIP and might be changed to `if` statements soon)

ACKs for top commit:
  fanquake:
    ACK a35e323 2/3

Tree-SHA512: c471951c23eb2cda919a71285d8b8f2580cb20f09d5db17b53e13dbd8813e01b3e7a83ea848e4913fd0f2bc12c6c133c5f76b54e65c0d89fed4dfd2e0be19875
… guix (Linux)

f2b5b0a build: add linker optimization flags to guix (fanquake)
b8b050a build: add linker optimization flags to gitian descriptors (fanquake)

Pull request description:

  This PR adds `-Wl,O2` to our gitian and guix LDFLAGS. This makes the linker perform certain optimisations (and is different from LTO).

  Any -O argument will enable optimizations in GNU ld. We can use -O2 here, as this matches our compile flags. Note that this would also enable additional optimizations if using the lld or gold linkers, when compared to -O0.

  A nice writeup + diagrams of some of these optimizations is  available here: http://lwn.net/Articles/192624/.

  #### master
  ```bash
  # bitcoind
  Histogram for `.gnu.hash' bucket list length (total of 3 buckets)
   Length  Number     % of total  Coverage
        0  1          ( 33.3%)       0.0%
        1  0          (  0.0%)       0.0%
        2  1          ( 33.3%)      40.0%
        3  1          ( 33.3%)     100.0%
  ```
  ```bash
  # bitcoin-qt
  Histogram for `.gnu.hash' bucket list length (total of 3 buckets)
   Length  Number     % of total  Coverage
        0  0          (  0.0%)       0.0%
        1  1          ( 33.3%)      10.0%
        2  0          (  0.0%)      10.0%
        3  0          (  0.0%)      10.0%
        4  1          ( 33.3%)      50.0%
        5  1          ( 33.3%)     100.0%
  ```

  #### this PR:
  ```bash
  # bitcoind
  Histogram for `.gnu.hash' bucket list length (total of 8 buckets)
   Length  Number     % of total  Coverage
        0  3          ( 37.5%)       0.0%
        1  5          ( 62.5%)     100.0%
  ```
  ```bash
  # bitcoin-qt
  Histogram for `.gnu.hash' bucket list length (total of 19 buckets)
   Length  Number     % of total  Coverage
        0  9          ( 47.4%)       0.0%
        1  10         ( 52.6%)     100.0%
  ```

  #### GNU ld -O

  > If level is a numeric values greater than zero ld optimizes the output. This might take significantly longer and therefore probably should only be enabled for the final binary. At the moment this option only affects ELF shared library generation. Future releases of the linker may make more use of this option. Also currently there is no difference in the linker’s behaviour for different non-zero values of this option. Again this may change with future releases.

  #### lld -O

  > Optimize output file size

ACKs for top commit:
  dongcarl:
    ACK f2b5b0a
  laanwj:
    ACK f2b5b0a

Tree-SHA512: e53f3a4338317dbec65d3a93b57b5a6204aabdf9ac82d99447847a3c8627facc53c58c2cf947376f13edd979fc8129a80f18d9ebeccd191a576c83f1dad5c513
bfe1ba2 rel-builds: Specify core.abbrev for git-rev-parse (Carl Dong)
27e63e0 build: Accomodate makensis v2.x (Carl Dong)
1f2c39a guix: Remove logical cores requirement (Carl Dong)
a4f6ffa lint: Also enable source statements for non-gitian (Carl Dong)
d256f91 rel-builds: Directly deploy win installer to OUTDIR (Carl Dong)
fa791da nsis: Specify OutFile path only once (Carl Dong)
1470160 guix: Expose GIT_COMMON_DIR in container as readonly (Carl Dong)
f5a6ac4 guix: Make source tarball using git-archive (Carl Dong)
395c113 gitian: Limit sourced script to just assignments (Carl Dong)

Pull request description:

  Based on: bitcoin#18556
  Related: bitcoin#17595 (comment)

ACKs for top commit:
  fanquake:
    ACK bfe1ba2 - I agree with Carl, and am going to merge this. I'd like for Linux Guix builds to be working again, and we can rebase bitcoin#18818.

Tree-SHA512: c87ada7e3de17ca0b692a91029b86573442ded5780fc081c214773f6b374a0cdbeaf6f6898c36669c2e247ee32aa7f82defb1180f8decac52c65f0c140f18674
315a4d3 build: fix ASLR for bitcoin-cli on Windows (fanquake)

Pull request description:

  ASLR is not currently working for the `bitcoin-cli.exe` binary. This is
  due to it not having a .reloc section, which is stripped by default by
  the mingw-w64 ld we use for gitian builds. A good summary of issues with
  ld and mingw-w64 is available in this thread:
  https://sourceware.org/bugzilla/show_bug.cgi?id=19011.

  All other Windows binaries that we distribute (bitcoind, bitcoin-qt,
  bitcoin-wallet, bitcoin-tx and test_bitcoin) do not suffer this issue,
  and currently having working ASLR. This is due to them exporting
  (inadvertent or not) libsecp256k1 symbols, and, as a result, the .reloc
  section is not stripped by ld.

  This change is a temporary workaround, also the same one described here:
  https://www.kb.cert.org/vuls/id/307144/, that causes main() to be
  exported. Exporting a symbol will mean that the .reloc section is not
  stripped, and ASLR will function correctly.

  Ultimately, this will be fixed by using a newer version of binutils (that has this [change](https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=dc9bd8c92af67947db44b3cb428c050259b15cd0)). Whether that's through bumping our gitian distro, or Guix.

  Related to bitcoin#18629, which has a bunch of additional information in the PR description. If you would like to verify whether or not ASLR is indeed working, with or without this change. One easy way to check is using a tool like [VMMap](https://docs.microsoft.com/en-us/sysinternals/downloads/vmmap).

  Here are the memory mappings for the 0.20.0rc1 `bitcoind.exe` and `bitcoin-cli.exe` binaries. You'll notice that over machine restarts, even though the image is marked `(ASLR)` (which I assume may be due to the header bit being set), no ASLR is actually occuring for `bitcoin-cli.exe`:

  #### bitcoind.exe

  ![bitcoind-1](https://user-images.githubusercontent.com/863730/79678203-74065c80-822b-11ea-90bc-9c883d0aeefa.png)

  ![bitcoind-2](https://user-images.githubusercontent.com/863730/79678204-7668b680-822b-11ea-9263-3e7ba22f904c.png)

  ![bitcoind-3](https://user-images.githubusercontent.com/863730/79678206-7963a700-822b-11ea-972f-af31a514b9b4.png)

  #### bitcoin-cli.exe

  ![bitcoin-cli-1](https://user-images.githubusercontent.com/863730/79678208-7ec0f180-822b-11ea-8480-a4b5d1762945.png)

  ![bitcoin-cli-2](https://user-images.githubusercontent.com/863730/79678213-81bbe200-822b-11ea-964d-994f58ff12b0.png)

  ![bitcoin-cli-3](https://user-images.githubusercontent.com/863730/79678215-84b6d280-822b-11ea-9cd6-fee2e239c003.png)

ACKs for top commit:
  dongcarl:
    ACK 315a4d3
  laanwj:
    ACK 315a4d3

Tree-SHA512: 95f4dc15420ed9bcdeacb763e11c3c7e563eec594a172746fa0346c13f97db3a8769357dffc89fea1e57ae67133f337b1013a73b584662f5b6c4d251ca20a2b1
@knst
Copy link
Collaborator Author

knst commented Feb 21, 2023

just for history: builds are different in one byte:
image
Looks like binary on the right is more correct because contains one extra byte of git revision to build. b3f242da1466a6fc676e46e3836557846be7b04c

@knst
Copy link
Collaborator Author

knst commented Feb 21, 2023

Builds are different, because git command output are different:

$ git rev-parse --short HEAD
b3f242da14 - on my localhost Kubuntu 22.10
b3f242da1 - inside pastapastapasta's docker

I will fix it in further PR.

UdjinM6 pushed a commit that referenced this pull request Feb 27, 2023
…#5222)

Builds are different, because git command output are different:




## Issue being fixed or feature implemented
in pr #5194 different OS produced different hashes of binary.
That's due to different behaviour of this command:
```sh
$ git rev-parse --short HEAD
b3f242d - on my localhost Kubuntu 22.10
b3f242d - inside pastapastapasta's docker
```

## What was done?
This fix forces to git print always 12 character


## How Has This Been Tested?
Run in different environment

## Breaking Changes
No breaking changes


## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have assigned this pull request to a milestone
@knst knst mentioned this pull request Mar 6, 2023
3 tasks
@knst knst mentioned this pull request Jun 8, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants