guix: fix GCC 10.3.0 + mingw-w64 setjmp/longjmp issues #24842
Merged
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 commit backports a patch to the GCC 10.3.0 we build for Windows cross-compilation in Guix. The commit has been backported to the GCC releases/gcc-10 branch, but hasn't yet made it into a release. The patch corrects a regression from an earlier GCC commit, see: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=357c4350680bf29f0c7a115424e3da11c53b5582 and https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=074226d5aa86cd3de517014acfe34c7f69a2ccc7, related to the way newer versions of mingw-w64 implement setjmp/longjmp. Ultimately this was causing a crash for us when Windows users were viewing the network traffic tab inside the GUI. After some period, long enough that a buffer would need reallocating, a call into FreeTypes gray_record_cell() would result in a call to ft_longjmp (longjmp), which would then trigger a crash. Fixes: bitcoin-core/gui#582. See also: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e8d1ca7d2c344a411779892616c423e157f4aea8. https://bugreports.qt.io/browse/QTBUG-93476.
Member
|
Approach and review-only ACK 457148a |
fanquake
added a commit
to fanquake/bitcoin
that referenced
this pull request
Apr 13, 2022
This commit backports a patch to the GCC 10.3.0 we build for Windows cross-compilation in Guix. The commit has been backported to the GCC releases/gcc-10 branch, but hasn't yet made it into a release. The patch corrects a regression from an earlier GCC commit, see: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=357c4350680bf29f0c7a115424e3da11c53b5582 and https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=074226d5aa86cd3de517014acfe34c7f69a2ccc7, related to the way newer versions of mingw-w64 implement setjmp/longjmp. Ultimately this was causing a crash for us when Windows users were viewing the network traffic tab inside the GUI. After some period, long enough that a buffer would need reallocating, a call into FreeTypes gray_record_cell() would result in a call to ft_longjmp (longjmp), which would then trigger a crash. Fixes: bitcoin-core/gui#582. See also: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e8d1ca7d2c344a411779892616c423e157f4aea8. https://bugreports.qt.io/browse/QTBUG-93476. Github-Pull: bitcoin#24842 Rebased-From: 457148a
Merged
Member
|
Concept and code review ACK 457148a |
Member
Guix build on
|
Contributor
|
ACK 457148a |
hebasto
approved these changes
Apr 13, 2022
Member
hebasto
left a comment
hebasto
left a comment
There was a problem hiding this comment.
ACK 457148a, tested bitcoin-457148a803ce-win64.zip on Windows 11 Pro 21H2. Confirming that bitcoin-core/gui#582 is fixed.
Also checked that this PR patch fixes bitcoin-core/gui#582 for the 23.x branch.
Contributor
|
GUIX hashes on x86, mine match hebasto and fanquake: |
jarolrod
approved these changes
Apr 13, 2022
Contributor
jarolrod
left a comment
jarolrod
left a comment
There was a problem hiding this comment.
ACK 457148a
I guix built both master and this PR and tested the resulting binary on Windows 11. Confirming this fixes the linked issue.
Obviously, this should have been caught earlier through testing. When I perform manual testing of the GUI, it's never a standard set of routines and actions. And I assume that for other reviewers, it's a similar situation. What this means is that all actions the GUI can perform aren't reviewed. I'm going to put together a checklist of actions that should be performed when manually reviewing the GUI, and hopefully it will catch issues like this much earlier in the release process.
Member
We should be especially careful with testing on compiler updates, especially the Windows one. It's a low tier platform for GCC and time after time it's clear that it cannot be assumed even basic things keep working with a new version. |
Member
Author
|
Being backported in #24843. |
Member
Author
|
cc @skitt. This patch might be something to backport for Debian mingw-w64, which currently builds with 10.3.0? |
|
Thanks for the heads-up @fanquake; |
laanwj
added a commit
that referenced
this pull request
Apr 14, 2022
a75b8ec guix: fix GCC 10.3.0 + mingw-w64 setjmp/longjmp issues (fanquake) Pull request description: Currently backports: * #24842 ACKs for top commit: gruve-p: ACK a75b8ec hebasto: ACK a75b8ec jonatack: Code review and commit meta-data ACK a75b8ec jarolrod: ACK a75b8ec Tree-SHA512: 1071b675647ed990ffda9c84391538b6c2e741b423a64e534754322250ca7f95cbc948e005cb976bf070099038f86997bfa1946c543a109e2086d2aaeaacb189
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 14, 2022
…issues 457148a guix: fix GCC 10.3.0 + mingw-w64 setjmp/longjmp issues (fanquake) Pull request description: This commit backports [a patch](https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e8d1ca7d2c344a411779892616c423e157f4aea8) to the GCC 10.3.0 we build for Windows cross-compilation in Guix. The commit has been [backported to the GCC releases/gcc-10 branch](https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e3abcc56d2604b9d2652b615ff9e68981cb7f79e), but hasn't yet made it into a 10.x release. The patch corrects a regression from an earlier GCC commit, see: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=357c4350680bf29f0c7a115424e3da11c53b5582 and https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=074226d5aa86cd3de517014acfe34c7f69a2ccc7, related to the way newer versions of mingw-w64 implement setjmp/longjmp. Ultimately this was causing a crash for us when Windows users were viewing the network traffic tab inside the GUI. After some period, long enough that a buffer would need reallocating, a call into FreeTypes [`gray_record_cell()`](https://github.com/ImageMagick/freetype/blob/a18906091cd17c623a6819661589df5566958918/src/smooth/ftgrays.c#L526) would result in a call to [`ft_longjmp` (longjmp)](https://github.com/ImageMagick/freetype/blob/a18906091cd17c623a6819661589df5566958918/src/smooth/ftgrays.c#L165), which would then trigger a crash. Fixes: bitcoin-core/gui#582. See also: https://bugreports.qt.io/browse/QTBUG-93476 - very similar issue reported to Qt. Guix Build (on x86_64): ```bash 62172df3089e7bca3fd00f63acc9c8d3678a35bfb2bb5a0af905e61e9d8def52 guix-build-457148a803ce/output/dist-archive/bitcoin-457148a803ce.tar.gz f8318d16d0418e0e790efd94527a5be374ac50f51df53e05a6d54cc8c08a8633 guix-build-457148a803ce/output/x86_64-w64-mingw32/SHA256SUMS.part 72076e6896297a36beec6c62065b3d8aeeeb87fed407df947261cefdc81cdb93 guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-debug.zip c617d2347f50d2706bbdcc2b3b97f2ecaf59243747f4c81d7747a22e64cb9d76 guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-setup-unsigned.exe 8b1e7821e495121bea8a70f09ea6a0b703503b054d831b0dd86a0fe29cece457 guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-unsigned.tar.gz c8d2c0e68e3bf21ed7cfe08df64925bfa54ce6225c6d29bb710f9d9d4474caee guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64.zip ``` ACKs for top commit: jonatack: Approach and review-only ACK 457148a laanwj: Concept and code review ACK 457148a gruve-p: ACK bitcoin@457148a hebasto: ACK 457148a, tested `bitcoin-457148a803ce-win64.zip` on Windows 11 Pro 21H2. Confirming that bitcoin-core/gui#582 is fixed. jarolrod: ACK 457148a Tree-SHA512: dfb832ce93d72827009458cebbbdd408175b90b98d3eb546f9bbd21efe7bdd4ceca6ed13f5f6ce8e8e15d1c5d613f3a10399847a3589e4e7cc113ac0196d4010
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
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 commit backports a patch to the GCC 10.3.0 we build for Windows
cross-compilation in Guix. The commit has been backported to the GCC
releases/gcc-10 branch, but hasn't yet made it into a 10.x release.
The patch corrects a regression from an earlier GCC commit, see:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=357c4350680bf29f0c7a115424e3da11c53b5582
and
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=074226d5aa86cd3de517014acfe34c7f69a2ccc7,
related to the way newer versions of mingw-w64 implement setjmp/longjmp.
Ultimately this was causing a crash for us when Windows users were
viewing the network traffic tab inside the GUI. After some period, long
enough that a buffer would need reallocating, a call into FreeTypes
gray_record_cell()would result in a call toft_longjmp(longjmp), whichwould then trigger a crash.
Fixes: bitcoin-core/gui#582.
See also:
https://bugreports.qt.io/browse/QTBUG-93476 - very similar issue reported to Qt.
Guix Build (on x86_64):