Bug 105860 - [10/11/12/13 Regression] Miscompilation causing clobbered union contents since r10-918-gc56c86024f8fba0c
Summary: [10/11/12/13 Regression] Miscompilation causing clobbered union contents sinc...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 10.3.0
: P3 normal
Target Milestone: 10.5
Assignee: Martin Jambor
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2022-06-06 12:51 UTC by John Hodge
Modified: 2022-07-13 11:19 UTC (History)
5 users (show)

See Also:
Host: x86_64-linux-gnu
Target: x86_64-linux-gnu
Build:
Known to work: 9.4.0
Known to fail: 10.3.0, 11.1.0
Last reconfirmed: 2022-06-15 00:00:00


Attachments
Reproduction source file, compile with `gcc -O1` (719 bytes, text/x-csrc)
2022-06-06 12:51 UTC, John Hodge
Details
Reproduction, partially minimised and including assertion (554 bytes, text/plain)
2022-06-15 12:05 UTC, John Hodge
Details

Note You need to log in before you can comment on or make changes to this bug.
Description John Hodge 2022-06-06 12:51:23 UTC
Created attachment 53092 [details]
Reproduction source file, compile with `gcc -O1`

Found while debugging this issue with auto-generated code: https://github.com/thepowersgang/mrustc/issues/266#issuecomment-1147389581

gcc generates code that reads 32-bits from offset 8 of a union, and then writes that value back to offset 4 before copying 64-bits from offset 4, causing data corruption.
This issue is present on gcc 10.3.0 (`gcc (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0`) and on gcc 11.1 (`gcc-11 (Ubuntu 11.1.0-1ubuntu1~20.04) 11.1.0`), but not on gcc 9.4.0 (`gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0`)

Compiler flags required: `-O1`


Disassembly with comments pointing to the faulty instructions.
```
0000000000000000 <ZRQG3cM17rustc_middle0_0_02ty3sty20ExistentialPredicate0g3c_A2ty4fold12TypeFoldable0g15super_fold_with1gG3c_A2ty_E16Bou$c6411ae8e3203bad>:
   0:   f3 0f 1e fa             endbr64 
   4:   41 56                   push   %r14
   6:   41 55                   push   %r13
   8:   41 54                   push   %r12
   a:   49 89 fc                mov    %rdi,%r12
   d:   55                      push   %rbp
   e:   53                      push   %rbx
   f:   48 83 ec 10             sub    $0x10,%rsp
  13:   8b 5c 24 40             mov    0x40(%rsp),%ebx
  17:   8b 6c 24 48             mov    0x48(%rsp),%ebp   ; Read word 3
  1b:   81 fb 01 ff ff ff       cmp    $0xffffff01,%ebx
  21:   74 4d                   je     70 <ZRQG3cM17rustc_middle0_0_02ty3sty20ExistentialPredicate0g3c_A2ty4fold12TypeFoldable0g15super_fold_with1gG3c_A2ty_E16Bou$c6411ae8e3203bad+0x70>
  23:   81 fb 03 ff ff ff       cmp    $0xffffff03,%ebx
  29:   74 5d                   je     88 <ZRQG3cM17rustc_middle0_0_02ty3sty20ExistentialPredicate0g3c_A2ty4fold12TypeFoldable0g15super_fold_with1gG3c_A2ty_E16Bou$c6411ae8e3203bad+0x88>
-- SNIP ---
  88:   89 6c 24 44             mov    %ebp,0x44(%rsp)   ; Write word 3 over word 2
  8c:   48 8b 44 24 44          mov    0x44(%rsp),%rax   ; Read words 2/3 (values from 3/3)
  91:   48 89 47 04             mov    %rax,0x4(%rdi)    ; Write to words 2/3 of output
  95:   eb af                   jmp    46 <ZRQG3cM17rustc_middle0_0_02ty3sty20ExistentialPredicate0g3c_A2ty4fold12TypeFoldable0g15super_fold_with1gG3c_A2ty_E16Bou$c6411ae8e3203bad+0x46>
```
Comment 1 Martin Liška 2022-06-15 11:43:47 UTC
Can you please contact a self-contained test case (that contains an assert or so) that I can run?
Comment 2 John Hodge 2022-06-15 12:05:41 UTC
Created attachment 53144 [details]
Reproduction, partially minimised and including assertion

Attached is an updated reproduction, that includes asserts.
This fails the assert when compiled with gcc 10.3.0-1ubuntu1~20.04 when -O1 is passed, and passes when no optimisation is enabled.

(Platform is x86_64, in case that wasn't already obvious from the disassembly)
Comment 3 Martin Liška 2022-06-15 12:50:47 UTC
Oh, great, thanks for it! It started with Martin's r10-918-gc56c86024f8fba0c.
Comment 4 Jakub Jelinek 2022-06-28 10:49:38 UTC
GCC 10.4 is being released, retargeting bugs to GCC 10.5.
Comment 5 Martin Jambor 2022-07-01 18:12:17 UTC
Mine.
Comment 6 Martin Jambor 2022-07-01 20:49:11 UTC
I proposed a fix on the mailing list:

https://gcc.gnu.org/pipermail/gcc-patches/2022-July/597674.html
Comment 7 GCC Commits 2022-07-04 15:08:34 UTC
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:b110e5283e368b5377e04766e4ff82cd52634208

commit r13-1460-gb110e5283e368b5377e04766e4ff82cd52634208
Author: Martin Jambor <mjambor@suse.cz>
Date:   Fri Jul 1 20:57:18 2022 +0200

    tree-sra: Fix union handling in build_reconstructed_reference
    
    As the testcase in PR 105860 shows, the code that tries to re-use the
    handled_component chains in SRA can be horribly confused by unions,
    where it thinks it has found a compatible structure under which it can
    chain the references, but in fact it found the type it was looking
    for elsewhere in a union and generated a write to a completely wrong
    part of an aggregate.
    
    I don't remember whether the plan was to support unions at all in
    build_reconstructed_reference but it can work, to an extent, if we
    make sure that we start the search only outside the outermost union,
    which is what the patch does (and the extra testcase verifies).
    
    gcc/ChangeLog:
    
    2022-07-01  Martin Jambor  <mjambor@suse.cz>
    
            PR tree-optimization/105860
            * tree-sra.cc (build_reconstructed_reference): Start expr
            traversal only just below the outermost union.
    
    gcc/testsuite/ChangeLog:
    
    2022-07-01  Martin Jambor  <mjambor@suse.cz>
    
            PR tree-optimization/105860
            * gcc.dg/tree-ssa/alias-access-path-13.c: New test.
            * gcc.dg/tree-ssa/pr105860.c: Likewise.
Comment 8 GCC Commits 2022-07-11 16:47:40 UTC
The releases/gcc-12 branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:1a78fffb3845f879e36ba33b4d15e75b3df8d5a7

commit r12-8564-g1a78fffb3845f879e36ba33b4d15e75b3df8d5a7
Author: Martin Jambor <mjambor@suse.cz>
Date:   Mon Jul 11 18:43:23 2022 +0200

    tree-sra: Fix union handling in build_reconstructed_reference
    
    As the testcase in PR 105860 shows, the code that tries to re-use the
    handled_component chains in SRA can be horribly confused by unions,
    where it thinks it has found a compatible structure under which it can
    chain the references, but in fact it found the type it was looking
    for elsewhere in a union and generated a write to a completely wrong
    part of an aggregate.
    
    I don't remember whether the plan was to support unions at all in
    build_reconstructed_reference but it can work, to an extent, if we
    make sure that we start the search only outside the outermost union,
    which is what the patch does (and the extra testcase verifies).
    
    Additionally, this commit also contains sqashed in it a backport of
    b984b84cbe4bf026edef2ba37685f3958a1dc1cf which fixes the testcase
    gcc.dg/tree-ssa/alias-access-path-13.c for many 32-bit targets.
    
    gcc/ChangeLog:
    
    2022-07-01  Martin Jambor  <mjambor@suse.cz>
    
            PR tree-optimization/105860
            * tree-sra.cc (build_reconstructed_reference): Start expr
            traversal only just below the outermost union.
    
    gcc/testsuite/ChangeLog:
    
    2022-07-01  Martin Jambor  <mjambor@suse.cz>
    
            PR tree-optimization/105860
            * gcc.dg/tree-ssa/alias-access-path-13.c: New test.
            * gcc.dg/tree-ssa/pr105860.c: Likewise.
    
    (cherry picked from commit b110e5283e368b5377e04766e4ff82cd52634208)
Comment 9 GCC Commits 2022-07-12 11:18:23 UTC
The releases/gcc-11 branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:16afe2e2862f3dd93c711d7f8d436dee23c6c34d

commit r11-10144-g16afe2e2862f3dd93c711d7f8d436dee23c6c34d
Author: Martin Jambor <mjambor@suse.cz>
Date:   Tue Jul 12 13:16:35 2022 +0200

    tree-sra: Fix union handling in build_reconstructed_reference
    
    As the testcase in PR 105860 shows, the code that tries to re-use the
    handled_component chains in SRA can be horribly confused by unions,
    where it thinks it has found a compatible structure under which it can
    chain the references, but in fact it found the type it was looking
    for elsewhere in a union and generated a write to a completely wrong
    part of an aggregate.
    
    I don't remember whether the plan was to support unions at all in
    build_reconstructed_reference but it can work, to an extent, if we
    make sure that we start the search only outside the outermost union,
    which is what the patch does (and the extra testcase verifies).
    
    Additionally, this commit also contains sqashed in it a backport of
    b984b84cbe4bf026edef2ba37685f3958a1dc1cf which fixes the testcase
    gcc.dg/tree-ssa/alias-access-path-13.c for many 32-bit targets.
    
    gcc/ChangeLog:
    
    2022-07-01  Martin Jambor  <mjambor@suse.cz>
    
            PR tree-optimization/105860
            * tree-sra.c (build_reconstructed_reference): Start expr
            traversal only just below the outermost union.
    
    gcc/testsuite/ChangeLog:
    
    2022-07-01  Martin Jambor  <mjambor@suse.cz>
    
            PR tree-optimization/105860
            * gcc.dg/tree-ssa/alias-access-path-13.c: New test.
            * gcc.dg/tree-ssa/pr105860.c: Likewise.
    
    (cherry picked from commit b110e5283e368b5377e04766e4ff82cd52634208)
Comment 10 GCC Commits 2022-07-13 11:19:13 UTC
The releases/gcc-10 branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:2691107f973770bafc84b9e8aae7c791053f411e

commit r10-10894-g2691107f973770bafc84b9e8aae7c791053f411e
Author: Martin Jambor <mjambor@suse.cz>
Date:   Wed Jul 13 13:17:25 2022 +0200

    tree-sra: Fix union handling in build_reconstructed_reference
    
    As the testcase in PR 105860 shows, the code that tries to re-use the
    handled_component chains in SRA can be horribly confused by unions,
    where it thinks it has found a compatible structure under which it can
    chain the references, but in fact it found the type it was looking
    for elsewhere in a union and generated a write to a completely wrong
    part of an aggregate.
    
    I don't remember whether the plan was to support unions at all in
    build_reconstructed_reference but it can work, to an extent, if we
    make sure that we start the search only outside the outermost union,
    which is what the patch does (and the extra testcase verifies).
    
    Additionally, this commit also contains sqashed in it a backport of
    b984b84cbe4bf026edef2ba37685f3958a1dc1cf which fixes the testcase
    gcc.dg/tree-ssa/alias-access-path-13.c for many 32-bit targets.
    
    gcc/ChangeLog:
    
    2022-07-01  Martin Jambor  <mjambor@suse.cz>
    
            PR tree-optimization/105860
            * tree-sra.c (build_reconstructed_reference): Start expr
            traversal only just below the outermost union.
    
    gcc/testsuite/ChangeLog:
    
    2022-07-01  Martin Jambor  <mjambor@suse.cz>
    
            PR tree-optimization/105860
            * gcc.dg/tree-ssa/alias-access-path-13.c: New test.
            * gcc.dg/tree-ssa/pr105860.c: Likewise.
    
    (cherry picked from commit b110e5283e368b5377e04766e4ff82cd52634208)
Comment 11 Martin Jambor 2022-07-13 11:19:58 UTC
Fixed.  Thanks for reporting.