guix: fix vmov alignment issues with gcc 10.3.0 & mingw-w64 #24736
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This introduces a patch to our GCC (10.3.0) mingw-w64 compiler, in Guix, to make it avoid using aligned vmov instructions. This works around a longstanding issue in GCC, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54412, which was recently discovered to be causing issues, see bitcoin#24726. Note that distros like Debian are also patching around this issue, and that is where this patch comes from. This would also explain why we haven't run into this problem earlier, in development builds. See: https://salsa.debian.org/mingw-w64-team/gcc-mingw-w64/-/blob/master/debian/patches/vmov-alignment.patch. Fixes bitcoin#24726. Alternative to bitcoin#24727. See also: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=939559
|
Concept ACK. Looks more preferable than #24727. |
|
Approach ACK on using the debian patch. |
|
I'd normally consider this too hacky to be worth it, but as the patch is used by debian (which we've indirectly, through Ubuntu, already built with) there's good precedent and I'm reasonably convinced this is safe, and think it's ok to do instead of disabling AVX2 for the Windows build. Edit: I've updated https://gist.github.com/laanwj/95d97cd218d99c68c1437115e0db9394/archive/9ba4b193a27e970b8866ba22389c89d198783bc7.zip to add the asm for Assembly-checked ACK d6fae98 |
|
Whoa, impressive and quick detective work @laanwj and @fanquake! This makes sense for release, but this leaves us with a known miscompile when building for win from source with a modern compiler, no? So the only realistic way to get a safe exe other than msvc I guess is the official binary or a guix self-build. If that's the case, I'd actually favor #24727. |
Or a Debian or Fedora packaged cross-compiler. It was guix with the "wrong" compiler. Not sure about mingw-w64 own (non-cross) binaries though. |
Right, sorry, I meant that vanilla gcc is known-broken. Or am I misunderstanding? I guess if we're confident enough that most users will encounter a patched version via their distro it's not a concern. |
hebasto
reviewed
Apr 1, 2022
There was a problem hiding this comment.
Approach ACK d6fae98.
Guix build on x86_64:
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
97076613fb9e9259b686a3e28f43f3c3efd0b717a3d13ff035657fd78b622e35 guix-build-d6fae988eff7/output/dist-archive/bitcoin-d6fae988eff7.tar.gz
29103515a199785d9f1de3ada9927f2bf9184e6d94edd46fdaf9750c9c74b598 guix-build-d6fae988eff7/output/x86_64-w64-mingw32/SHA256SUMS.part
7e2f18d5d4e9d1a2ad17848376b779be16566d3c5b0bffb850b4a00e084ea6b2 guix-build-d6fae988eff7/output/x86_64-w64-mingw32/bitcoin-d6fae988eff7-win64-debug.zip
1a3ee51f9e5bd1a0a08c222e52abb655481e8a40c6c3a7c5b5e3fa7d401dc241 guix-build-d6fae988eff7/output/x86_64-w64-mingw32/bitcoin-d6fae988eff7-win64-setup-unsigned.exe
8fe762415dcd3ac2106bd54611040ba815c4db105c8de508018c17510d409638 guix-build-d6fae988eff7/output/x86_64-w64-mingw32/bitcoin-d6fae988eff7-win64-unsigned.tar.gz
a2cba27dd7a0dcf6654c0d8c052b2885ec48e84cdccaa398bf1473d800a4440b guix-build-d6fae988eff7/output/x86_64-w64-mingw32/bitcoin-d6fae988eff7-win64.zip
Verified that the patch is the same as one in the Debian repo (module paths).
hebasto
approved these changes
Apr 1, 2022
There was a problem hiding this comment.
ACK d6fae98, tested Guix bitcoin-d6fae988eff7-win64.zip artifact on Windows 11 Pro 21H2:
C:\Users\hebasto\Desktop\pr24736-avx2\bitcoin-d6fae988eff7>bin\bitcoind.exe -signet
2022-04-01T14:37:59Z Bitcoin Core version v23.99.0-gd6fae988eff78e28756d9b6219ec0239c420f51b (release build)
2022-04-01T14:37:59Z Signet derived magic (message start): 0a03cf40
2022-04-01T14:37:59Z Assuming ancestors of block 00000112852484b5fe3451572368f93cfd2723279af3464e478aee35115256ef have valid signatures.
2022-04-01T14:37:59Z Setting nMinimumChainWork=000000000000000000000000000000000000000000000000000000de26b0e471
2022-04-01T14:37:59Z Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
...
#24726 is fixed for me.
|
Guix Build (on x86_64): bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
6c19da96916339afe15f3ce8fe98ca9187fdf0fdde6e98004314fca5493f38b7 guix-build-d6fae988eff7/output/aarch64-linux-gnu/SHA256SUMS.part
239299236943857cc9e6e487ab2a40ce7bda4051857c98a3b61eaf6c4aba8222 guix-build-d6fae988eff7/output/aarch64-linux-gnu/bitcoin-d6fae988eff7-aarch64-linux-gnu-debug.tar.gz
5e561074dcce8d1a00b034d14e4f9dce881a51f75d3cc7a3704309fa4cb8a2cc guix-build-d6fae988eff7/output/aarch64-linux-gnu/bitcoin-d6fae988eff7-aarch64-linux-gnu.tar.gz
4bfd173fa8dcc94eec541a3494816966807dcde1e34d642df60fd35af6077daa guix-build-d6fae988eff7/output/arm-linux-gnueabihf/SHA256SUMS.part
dd4ab4ab0624f6aa5b47ff781c012e73ce3f4111e9007d911d072063d32d531e guix-build-d6fae988eff7/output/arm-linux-gnueabihf/bitcoin-d6fae988eff7-arm-linux-gnueabihf-debug.tar.gz
511f6eefcdb7831c9beb4e7728df4c984e5814994796e6df91173d047cd34305 guix-build-d6fae988eff7/output/arm-linux-gnueabihf/bitcoin-d6fae988eff7-arm-linux-gnueabihf.tar.gz
7ebc138f5e9ac9cef8fbb171e33ed8f21c6b21ac74b63406018f891803bd779f guix-build-d6fae988eff7/output/arm64-apple-darwin/SHA256SUMS.part
06bd8fdc92881652d83204873fbaecc67b5e18532d7ef9e68e96b925c94f2e65 guix-build-d6fae988eff7/output/arm64-apple-darwin/bitcoin-d6fae988eff7-arm64-apple-darwin-unsigned.dmg
cc07af5d7f55fa836043a8f6d71c6ed2316abdda89fde84cb0ae1b4dbb256205 guix-build-d6fae988eff7/output/arm64-apple-darwin/bitcoin-d6fae988eff7-arm64-apple-darwin-unsigned.tar.gz
9a96dd87534b4e55c9945d74a008aaa27ff4bae2db20e710b8956063968d625f guix-build-d6fae988eff7/output/arm64-apple-darwin/bitcoin-d6fae988eff7-arm64-apple-darwin.tar.gz
97076613fb9e9259b686a3e28f43f3c3efd0b717a3d13ff035657fd78b622e35 guix-build-d6fae988eff7/output/dist-archive/bitcoin-d6fae988eff7.tar.gz
b292159601a3d44b31d4d4502fd353680766dc3e0e66625fa9f1814f9244164f guix-build-d6fae988eff7/output/powerpc64-linux-gnu/SHA256SUMS.part
33bdc80c32eb8d9560f132825a3ba87231372985abab3cbbd7cdb571891a28eb guix-build-d6fae988eff7/output/powerpc64-linux-gnu/bitcoin-d6fae988eff7-powerpc64-linux-gnu-debug.tar.gz
a9e3862bd2f2a1af2030c6982a0f3732154a62e4d7e0cce56777032b9d0d5ca6 guix-build-d6fae988eff7/output/powerpc64-linux-gnu/bitcoin-d6fae988eff7-powerpc64-linux-gnu.tar.gz
4436f0add80cee47b85d041e8ddd2dbe0771f940f05b832f1b7c8c3269cd12c2 guix-build-d6fae988eff7/output/powerpc64le-linux-gnu/SHA256SUMS.part
c6eb3e76f006204e8de0aafa591b130bf5333c6ace7454e602e35c218e82da23 guix-build-d6fae988eff7/output/powerpc64le-linux-gnu/bitcoin-d6fae988eff7-powerpc64le-linux-gnu-debug.tar.gz
3431480378ac9f1121c67d5bd6117039c1bfd871a4284f55f6cbe55efabcd15a guix-build-d6fae988eff7/output/powerpc64le-linux-gnu/bitcoin-d6fae988eff7-powerpc64le-linux-gnu.tar.gz
dab7d6be15295a5225c152b1061696074c26434ee70e4ba4a1008bf676fdf8cf guix-build-d6fae988eff7/output/riscv64-linux-gnu/SHA256SUMS.part
6d9e74c4b70a5fc60c988898beee30da0cf064d143b26db89f86d72683a88216 guix-build-d6fae988eff7/output/riscv64-linux-gnu/bitcoin-d6fae988eff7-riscv64-linux-gnu-debug.tar.gz
45a39d43854864815b1e5522e6aa7b793c9ddc8d85f02d6dfdf95207f5530ecb guix-build-d6fae988eff7/output/riscv64-linux-gnu/bitcoin-d6fae988eff7-riscv64-linux-gnu.tar.gz
c4aa832be0c7e354eaedc940c1c0657d3e8711120e2bc7631dcc702114ca409c guix-build-d6fae988eff7/output/x86_64-apple-darwin/SHA256SUMS.part
f5627d971be03f42d8b1cde0078d0a18a6f45ba49524e2f56f9d81f24a742323 guix-build-d6fae988eff7/output/x86_64-apple-darwin/bitcoin-d6fae988eff7-x86_64-apple-darwin-unsigned.dmg
8b6b47bd317e3e522e8e06e5c1b7802f3579f26ea563bcac0e42853192ba2d0b guix-build-d6fae988eff7/output/x86_64-apple-darwin/bitcoin-d6fae988eff7-x86_64-apple-darwin-unsigned.tar.gz
829bab1c4c9f660689cd7458cd43bab48e2ef1cca87fe40c88dcfa165ed2e264 guix-build-d6fae988eff7/output/x86_64-apple-darwin/bitcoin-d6fae988eff7-x86_64-apple-darwin.tar.gz
526bfc98275304745ef5e0fd777138f5c3424c8cf69fd609bc1d828b5209b5e8 guix-build-d6fae988eff7/output/x86_64-linux-gnu/SHA256SUMS.part
51340f607294006dd0c715b6928ab65edb0ee6f53b27799e253dd86bbc400a1e guix-build-d6fae988eff7/output/x86_64-linux-gnu/bitcoin-d6fae988eff7-x86_64-linux-gnu-debug.tar.gz
e4b78350b27b7773e7cd4f1fe95657b5feb82e322245888f21e2d48cbcd59752 guix-build-d6fae988eff7/output/x86_64-linux-gnu/bitcoin-d6fae988eff7-x86_64-linux-gnu.tar.gz
29103515a199785d9f1de3ada9927f2bf9184e6d94edd46fdaf9750c9c74b598 guix-build-d6fae988eff7/output/x86_64-w64-mingw32/SHA256SUMS.part
7e2f18d5d4e9d1a2ad17848376b779be16566d3c5b0bffb850b4a00e084ea6b2 guix-build-d6fae988eff7/output/x86_64-w64-mingw32/bitcoin-d6fae988eff7-win64-debug.zip
1a3ee51f9e5bd1a0a08c222e52abb655481e8a40c6c3a7c5b5e3fa7d401dc241 guix-build-d6fae988eff7/output/x86_64-w64-mingw32/bitcoin-d6fae988eff7-win64-setup-unsigned.exe
8fe762415dcd3ac2106bd54611040ba815c4db105c8de508018c17510d409638 guix-build-d6fae988eff7/output/x86_64-w64-mingw32/bitcoin-d6fae988eff7-win64-unsigned.tar.gz
a2cba27dd7a0dcf6654c0d8c052b2885ec48e84cdccaa398bf1473d800a4440b guix-build-d6fae988eff7/output/x86_64-w64-mingw32/bitcoin-d6fae988eff7-win64.zip |
|
GUIX hashes on x86, my hashes match fanquake's latest round |
|
Concept and code review ACK d6fae98 |
fanquake
added a commit
to fanquake/bitcoin
that referenced
this pull request
Apr 4, 2022
This introduces a patch to our GCC (10.3.0) mingw-w64 compiler, in Guix, to make it avoid using aligned vmov instructions. This works around a longstanding issue in GCC, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54412, which was recently discovered to be causing issues, see bitcoin#24726. Note that distros like Debian are also patching around this issue, and that is where this patch comes from. This would also explain why we haven't run into this problem earlier, in development builds. See: https://salsa.debian.org/mingw-w64-team/gcc-mingw-w64/-/blob/master/debian/patches/vmov-alignment.patch. Fixes bitcoin#24726. Alternative to bitcoin#24727. See also: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=939559 Github-Pull: bitcoin#24736 Rebased-From: d6fae98
|
Being backported in #24755. |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 4, 2022
…& mingw-w64 d6fae98 guix: fix vmov alignment issues with gcc 10.3.0 & mingw-w64 (fanquake) Pull request description: This introduces a patch to our GCC (10.3.0) mingw-w64 compiler, in Guix, to make it avoid using aligned vmov instructions. This works around a longstanding issue in GCC, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54412, which was recently discovered to be causing issues, see bitcoin#24726. Note that distros like Debian are also patching around this issue, and that is where this patch comes from. This would also explain why we haven't run into this problem earlier, in development builds. See: https://salsa.debian.org/mingw-w64-team/gcc-mingw-w64/-/blob/master/debian/patches/vmov-alignment.patch. Fixes bitcoin#24726. Alternative to bitcoin#24727. See also: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=939559 ACKs for top commit: laanwj: Concept and code review ACK d6fae98 hebasto: ACK d6fae98, tested Guix ` bitcoin-d6fae988eff7-win64.zip` artifact on Windows 11 Pro 21H2: Tree-SHA512: f522efd8e604ab1d9f9c385147f6f488767cfe66f08a1c8b4ff67d448e065f8f2334bf825d99e7fe9571ada9038002b08434585f639120cb29b2e314da7b556e
maflcko
pushed a commit
that referenced
this pull request
Apr 5, 2022
39396ab build: Fix "ERR: Unsigned tarballs do not exist" (Hennadii Stepanov) db8a5d6 guix: fix vmov alignment issues with gcc 10.3.0 & mingw-w64 (fanquake) Pull request description: Backports: * #24733 * #24736 ACKs for top commit: gruve-p: ACK 39396ab hebasto: ACK 39396ab, backported locally, got zero diff with the PR branch. jarolrod: ACK 39396ab Tree-SHA512: 3573870c48fbde538a490c8b7103779987d3cce1165ca639c164aaf8ef82290fb99fcd461d0fed0208b43174b21284b21eb032c00512b986ae824295cc7935a8
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduces a patch to our GCC (10.3.0) mingw-w64 compiler, in Guix, to make
it avoid using aligned vmov instructions. This works around a longstanding issue
in GCC, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54412, which was recently
discovered to be causing issues, see #24726.
Note that distros like Debian are also patching around this issue, and that is
where this patch comes from. This would also explain why we haven't run into this
problem earlier, in development builds. See:
https://salsa.debian.org/mingw-w64-team/gcc-mingw-w64/-/blob/master/debian/patches/vmov-alignment.patch.
Fixes #24726.
Alternative to #24727.
See also:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=939559