[Bug target/114698] New: dcfldd produces wrong sha256 sum on ppc64le and -O3

[Bug target/114698] New: dcfldd produces wrong sha256 sum on ppc64le and -O3

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114698

            Bug ID: 114698
           Summary: dcfldd produces wrong sha256 sum on ppc64le and -O3
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: bergner at gcc dot gnu.org
  Target Milestone: ---

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

[Bug target/114698] dcfldd produces wrong sha256 sum on ppc64le and -O3

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114698

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |11.0
                 CC|                            |dje at gcc dot gnu.org,
                   |                            |linkw at gcc dot gnu.org,
                   |                            |segher at gcc dot gnu.org
             Target|                            |powerpc64le-linux
   Last reconfirmed|                            |2024-04-11
      Known to fail|                            |12.0, 13.0, 14.0
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |wrong-code

--- Comment #1 from Peter Bergner <bergner at gcc dot gnu.org> ---
Confirmed.

[Bug ipa/114698] dcfldd produces wrong sha256 sum on ppc64le and -O3

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114698

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |ipa
                 CC|                            |hubicka at gcc dot gnu.org

--- Comment #2 from Peter Bergner <bergner at gcc dot gnu.org> ---
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(-)

[Bug ipa/114698] [12/13/14 regression] dcfldd produces wrong sha256 sum on ppc64le and -O3 since r12-5244-g64f3e71c302b4a

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114698

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
        uint8_t buffer[SHA256_BLOCK_LENGTH];

        W256 = (sha2_word32*)context->buffer;

This is starting to smell like the code is violating strict aliasing rules ...

[Bug ipa/114698] [12/13/14 regression] dcfldd produces wrong sha256 sum on ppc64le and -O3 since r12-5244-g64f3e71c302b4a

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114698

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(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.

[Bug ipa/114698] [12/13/14 regression] dcfldd produces wrong sha256 sum on ppc64le and -O3 since r12-5244-g64f3e71c302b4a

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114698

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Specifically
https://github.com/archiecobbs/libnbcompat/commit/864c1cf42c2c605636008626f171caf6410421cb
.

[Bug ipa/114698] [12/13/14 regression] dcfldd produces wrong sha256 sum on ppc64le and -O3 since r12-5244-g64f3e71c302b4a

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114698

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note this implementation of sha2.c is shared all over the place it seems and
has this known issue ...

[Bug ipa/114698] [12/13/14 regression] dcfldd produces wrong sha256 sum on ppc64le and -O3 since r12-5244-g64f3e71c302b4a

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114698

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
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.

[Bug ipa/114698] [12/13/14 regression] dcfldd produces wrong sha256 sum on ppc64le and -O3 since r12-5244-g64f3e71c302b4a

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114698

--- Comment #8 from Peter Bergner <bergner at gcc dot gnu.org> ---
(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.

[Bug ipa/114698] [12/13/14 regression] dcfldd produces wrong sha256 sum on ppc64le and -O3 since r12-5244-g64f3e71c302b4a

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114698

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|---                         |INVALID

--- Comment #9 from Peter Bergner <bergner at gcc dot gnu.org> ---
(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.