Add test for GCC bug 90348 #15985
Merged
Add test for GCC bug 90348 #15985
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
Member
|
Looks like this only fails for the no-wallet build and win64 (not sure what that wine32 warning means): |
Member
Author
|
I guess that's expected; sanitizers break the bug (I think they automatically enable stack-reuse=none). |
Member
|
What about win32, linux32, bionic qt5 and the trusty build without sanitizers? The trusty one could have a gcc that is old enough to not have this bug, but the others should fail, hmm |
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
Member
|
Needs update after #15983 which adds |
Member
|
@sipa 👀 ^ |
Member
|
utACK 9d269e1 (needs rebase) I did some investigating using Compiler Explorer and the c test case from the GCC bug report to better understand what's happening here. Using the flags produced by -fstack-reuse=none -Wstack-protector -fstack-protector-all -Wall -Wextra -Wformat -Wvla -Wredundant-decls -Wno-unused-parameter -Wno-implicit-fallthrough -g -O2I setup a test to compare the assembly produced with and without You can see my setup on Compiler Explorer here. The difference produced with and without < lea rdi, [rsp+3]
---
> lea rdi, [rsp+4]As mentioned in the original bug report, the issue is seen on GCC 7, 8 and 9. |
maflcko
force-pushed
the
201905_gccbug90348
branch
from
9d269e1 to
58e291c
Compare
Member
|
There hasn't been any activity in the last two weeks, so I went ahead to rebase and force push to the branch in your repo. Hope you don't mind. |
Member
|
Unless there are objections, this will be merged tomorrow. |
pull bot
pushed a commit
to uniibu/bitcoin
that referenced
this pull request
Jun 5, 2019
58e291c Add test for GCC bug 90348 (Pieter Wuille) Pull request description: This adds a test for GCC bug 90348 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348), using a test case extracted from our own `sha256d64` test in crypto_tests.cpp, which was failing on some platforms. This is based on top of bitcoin#15983 to make sure the bug doesn't trigger (it does in some Travis configurations without it). ACKs for commit 58e291: Tree-SHA512: 4dc9084e92dd143a53930e42bb68e33d922a2a2b891406b259d3a0bed4511dcc49e7447a7a8e4eb793a26e3eacb188ca293b71e0e061f9b3230f8e7fcfd29525
Member
|
Included for backport in #16035. |
fanquake
pushed a commit
to fanquake/bitcoin
that referenced
this pull request
Jun 7, 2019
Github-Pull: bitcoin#15985 Rebased-From: 58e291c
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Jun 7, 2019
Github-Pull: bitcoin#15985 Rebased-From: 58e291c
laanwj
added a commit
that referenced
this pull request
Jun 13, 2019
… case b5a4abe Add test for GCC bug 90348 (Pieter Wuille) 05fb9f7 build with -fstack-reuse=none (MarcoFalke) Pull request description: Backports: * [build with -fstack-reuse=none](#15983) * [Add test for GCC bug 90348](#15985) b5a4abe has been modified to replace the `setup_common.h` with `test_bitcoin.h` include. ACKs for commit b5a4ab: Empact: ACK b5a4abe by review of the linked PRs, the GCC bug and option, and visual inspection/comparison of the ported code laanwj: Code review + relevancy for backport ACK b5a4abe Tree-SHA512: cdfdc6e2f208e8dc6a8a86cd7a7ed0f2a6f96604a0663efc970f580f693c1975353341fa8434b23de3cb681e03c6918e3342178752ed595d16a0ec50db913266
HashUnlimited
pushed a commit
to HashUnlimited/chaincoin
that referenced
this pull request
Aug 23, 2019
Github-Pull: bitcoin#15985 Rebased-From: 58e291c
Bushstar
pushed a commit
to Bushstar/omnicore
that referenced
this pull request
Aug 24, 2019
Github-Pull: bitcoin#15985 Rebased-From: 58e291c
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Oct 9, 2020
Summary: Add a unit test for compiler bug fixed in D5040 This is a backport of Core [[bitcoin/bitcoin#15985 | PR15985]] Test Plan: ``` cmake .. -GNinja ninja && ninja check ``` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7817
Munkybooty
pushed a commit
to Munkybooty/dash
that referenced
this pull request
Nov 9, 2021
58e291c Add test for GCC bug 90348 (Pieter Wuille) Pull request description: This adds a test for GCC bug 90348 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348), using a test case extracted from our own `sha256d64` test in crypto_tests.cpp, which was failing on some platforms. This is based on top of bitcoin#15983 to make sure the bug doesn't trigger (it does in some Travis configurations without it). ACKs for commit 58e291: Tree-SHA512: 4dc9084e92dd143a53930e42bb68e33d922a2a2b891406b259d3a0bed4511dcc49e7447a7a8e4eb793a26e3eacb188ca293b71e0e061f9b3230f8e7fcfd29525
pravblockc
pushed a commit
to pravblockc/dash
that referenced
this pull request
Nov 18, 2021
58e291c Add test for GCC bug 90348 (Pieter Wuille) Pull request description: This adds a test for GCC bug 90348 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348), using a test case extracted from our own `sha256d64` test in crypto_tests.cpp, which was failing on some platforms. This is based on top of bitcoin#15983 to make sure the bug doesn't trigger (it does in some Travis configurations without it). ACKs for commit 58e291: Tree-SHA512: 4dc9084e92dd143a53930e42bb68e33d922a2a2b891406b259d3a0bed4511dcc49e7447a7a8e4eb793a26e3eacb188ca293b71e0e061f9b3230f8e7fcfd29525
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 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 adds a test for GCC bug 90348 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348), using a test case extracted from our own
sha256d64test in crypto_tests.cpp, which was failing on some platforms.This is based on top of #15983 to make sure the bug doesn't trigger (it does in some Travis configurations without it).