Building the dcfldd v1.9.1 package on powerpc64le-linux when configured to use -O3 produces an incorrect sha256 sum for GCC trunk, 13 and 12. GCC 11 and earlier produces correct output. For example (-O3 trunk build): bergner@ltcden2-lp1:dcfldd [v1.9.1]$ echo TestInput | ./src/dcfldd hash=sha256 TestInput Total (sha256): d627605bdee37e388a5c232dc407cb5cd287d27187d6787999ad3bb59d383e9a 0+1 records in 0+1 records out ...versus expected output from an -O2 trunk build: bergner@ltcden2-lp1:dcfldd [v1.9.1]$ echo TestInput | ./src/dcfldd hash=sha256 TestInput Total (sha256): 8021973df8498a650e444fd84c705d9168639a246bc6024066e4091b2b450da6 0+1 records in 0+1 records out ...and from sha256sum: bergner@ltcden2-lp1:dcfldd-git [v1.9.1]$ echo TestInput | /usr/bin/sha256sum 8021973df8498a650e444fd84c705d9168639a246bc6024066e4091b2b450da6 - Current steps to recreate: git clone https://github.com/resurrecting-open-source-projects/dcfldd.git cd dcfldd/ git checkout v1.9.1 -b v1.9.1 ./autogen.sh ./configure CFLAGS="-O3" make echo TestInput | ./src/dcfldd hash=sha256
Confirmed.
I've confirmed that the function being miscompiled is src/sha2.c:SHA256_Transform() on line 440. I can add configure dcfldd with a normal -O2 and add a __attribute__((optimize (3))) to this function and I see bad output. I can also configure dcfldd with -O3 and add a __attribute__((optimize (2))) to this function and I see good output. Doing a git bisect, it identified the following GCC commit as causing the bug: 64f3e71c302b4a13e61656ee509e7050b9bce978 is the first bad commit commit 64f3e71c302b4a13e61656ee509e7050b9bce978 Author: Jan Hubicka <jh@suse.cz> Date: Sun Nov 14 18:49:15 2021 +0100 Extend modref to track kills This patch adds kill tracking to ipa-modref. This is representd by array of accesses to memory locations that are known to be overwritten by the function. gcc/ChangeLog: 2021-11-14 Jan Hubicka <hubicka@ucw.cz> * ipa-modref-tree.c (modref_access_node::update_for_kills): New member function. (modref_access_node::merge_for_kills): Likewise. (modref_access_node::insert_kill): Likewise. * ipa-modref-tree.h (modref_access_node::update_for_kills, modref_access_node::merge_for_kills, modref_access_node::insert_kill): Declare. (modref_access_node::useful_for_kill): New member function. * ipa-modref.c (modref_summary::useful_p): Release useless kills. (lto_modref_summary): Add kills. (modref_summary::dump): Dump kills. (record_access): Add mdoref_access_node parameter. (record_access_lto): Likewise. (merge_call_side_effects): Merge kills. (analyze_call): Add ALWAYS_EXECUTED param and pass it around. (struct summary_ptrs): Add always_executed filed. (analyze_load): Update. (analyze_store): Update; record kills. (analyze_stmt): Add always_executed; record kills in clobbers. (analyze_function): Track always_executed. (modref_summaries::duplicate): Duplicate kills. (update_signature): Release kills. * ipa-modref.h (struct modref_summary): Add kills. * tree-ssa-alias.c (alias_stats): Add kill stats. (dump_alias_stats): Dump kill stats. (store_kills_ref_p): Break out from ... (stmt_kills_ref_p): Use it; handle modref info based kills. gcc/testsuite/ChangeLog: 2021-11-14 Jan Hubicka <hubicka@ucw.cz> * gcc.dg/tree-ssa/modref-dse-3.c: New test. gcc/ipa-modref-tree.c | 179 +++++++++++++++++++++++ gcc/ipa-modref-tree.h | 15 ++ gcc/ipa-modref.c | 126 +++++++++++++--- gcc/ipa-modref.h | 1 + gcc/testsuite/gcc.dg/tree-ssa/modref-dse-3.c | 22 +++ gcc/tree-ssa-alias.c | 207 +++++++++++++++++++-------- 6 files changed, 471 insertions(+), 79 deletions(-)
uint8_t buffer[SHA256_BLOCK_LENGTH]; W256 = (sha2_word32*)context->buffer; This is starting to smell like the code is violating strict aliasing rules ...
(In reply to Andrew Pinski from comment #3) > uint8_t buffer[SHA256_BLOCK_LENGTH]; > > W256 = (sha2_word32*)context->buffer; > > This is starting to smell like the code is violating strict aliasing rules > ... The patch in https://github.com/NetBSD/pkgsrc/issues/122 applies directly here too.
Specifically https://github.com/archiecobbs/libnbcompat/commit/864c1cf42c2c605636008626f171caf6410421cb .
Note this implementation of sha2.c is shared all over the place it seems and has this known issue ...
See https://github.com/archiecobbs/libnbcompat/issues/4 for the full analysis of the issue with sha2.c and even mentions it is shared with many projects.
(In reply to Andrew Pinski from comment #6) > Note this implementation of sha2.c is shared all over the place it seems and > has this known issue ... (In reply to Andrew Pinski from comment #4) > (In reply to Andrew Pinski from comment #3) > > uint8_t buffer[SHA256_BLOCK_LENGTH]; > > > > W256 = (sha2_word32*)context->buffer; > > > > This is starting to smell like the code is violating strict aliasing rules > > ... > > The patch in https://github.com/NetBSD/pkgsrc/issues/122 applies directly > here too. Thanks for the pointer, I'll try the patch and report back. Jan's commit does seem to make a change in the alias handling, so it very well could have exposed that type of problem in the sha2 routine.
(In reply to Andrew Pinski from comment #6) > Note this implementation of sha2.c is shared all over the place it seems and > has this known issue ... Confirmed that the patch fixes the error. It's too bad the "fix" hasn't been as widely shared. :-( Closing this as INVALID.