Bug 115187 - [14 Regression] ICE when deleting temporary array
Summary: [14 Regression] ICE when deleting temporary array
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 15.0
: P2 normal
Target Milestone: 14.2
Assignee: Jason Merrill
URL:
Keywords: ice-on-invalid-code
Depends on:
Blocks:
 
Reported: 2024-05-22 11:09 UTC by Mital Ashok
Modified: 2024-05-24 15:25 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work: 13.2.0
Known to fail:
Last reconfirmed: 2024-05-22 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mital Ashok 2024-05-22 11:09:35 UTC
A delete-expression where the operand comes from a temporary array causes an internal compiler error

https://godbolt.org/z/7a9sM9KqT

    void f() {
      using T = int[2];
      delete T{};
    }


test.cpp: In function ‘void f()’:
test.cpp:3:10: warning: deleting array ‘T()’
    3 |   delete T{};
      |          ^~~
test.cpp:1:6: internal compiler error: in verify_gimple_stmt, at tree-cfg.cc:5169
    1 | void f() {
      |      ^
0x9069f1 verify_gimple_stmt
        ./gcc/gcc/tree-cfg.cc:5169
0x13ed54f verify_gimple_in_seq_2
        ./gcc/gcc/tree-cfg.cc:5288
0x13ed518 verify_gimple_in_seq_2
        ./gcc/gcc/tree-cfg.cc:5257
0x13ed588 verify_gimple_in_seq_2
        ./gcc/gcc/tree-cfg.cc:5252
0x13ed5ed verify_gimple_in_seq(gimple*, bool)
        ./gcc/gcc/tree-cfg.cc:5327
0x107426b gimplify_body(tree_node*, bool)
        ./gcc/gcc/gimplify.cc:19237
0x10743f9 gimplify_function_tree(tree_node*)
        ./gcc/gcc/gimplify.cc:19355
0xe873c7 cgraph_node::analyze()
        ./gcc/gcc/cgraphunit.cc:687
0xe899d7 analyze_functions
        ./gcc/gcc/cgraphunit.cc:1251
0xe8a721 symbol_table::finalize_compilation_unit()
        ./gcc/gcc/cgraphunit.cc:2560
Please submit a full bug report, with preprocessed source.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

This also happens when the array is a subobject of a temporary:

    struct X { int x[2]; };
    void f() {
      delete X{}.x;
    }

This also happens if the operand is a pointer derived from that array, `delete +T{};`, `delete (T{} + 1);`, `delete +X{}.x;`, `delete (X{}.x + 1)`
Comment 1 Richard Biener 2024-05-22 13:37:20 UTC
Confirmed.  Missed gimplification:

#1  0x0000000001c12d7e in verify_gimple_stmt (
    stmt=<gimple_with_cleanup_expr 0x7ffff71cea80>)
    at /space/rguenther/src/gcc/gcc/tree-cfg.cc:5169

try
  {
    <<< Unknown GIMPLE statement: gimple_with_cleanup_expr >>>

    D.2795 = {};
    D.2796 = &D.2795;
    MEM[(int *)D.2796] = {CLOBBER(eob)};
  }
finally
  {
    operator delete (D.2796, 4);
  }


clang complains:

t.ii:3:7: error: cannot delete expression of type 'T' (aka 'int[2]')
    3 |       delete T{};
      |       ^      ~~~
Comment 2 Richard Biener 2024-05-22 13:38:41 UTC
And GCC 13 complains:

t.ii: In function ‘void f()’:
t.ii:3:14: warning: deleting array ‘T()’
    3 |       delete T{};
      |              ^~~
t.ii:3:14: error: taking address of temporary array
t.ii:3:14: error: type ‘using T = int [2]’ {aka ‘int [2]’} argument given to ‘delete’, expected pointer
Comment 3 Drea Pinski 2024-05-22 20:49:17 UTC
(In reply to Richard Biener from comment #1) 
> clang complains:
> 
> t.ii:3:7: error: cannot delete expression of type 'T' (aka 'int[2]')
>     3 |       delete T{};
>       |       ^      ~~~

But that might be due to not doing `prvalue array decay`. See PR 	
94264  which I think introduced the ICE here.
Comment 4 Drea Pinski 2024-05-22 20:51:11 UTC
I am not 100% sure this is invalid code either. It is definitely undefined code if it is semantically valid code though.
Comment 5 Mital Ashok 2024-05-23 06:52:18 UTC
PR94264 prevented the first version from being an issue in GCC13, but the second version

    struct X { int x[2]; };
    void f() {
      delete X{}.x;
    }

still crashed in older GCC versions. This isn't technically invalid code since `f()` should just be like `std::unreachable()`. Or it also crashes when it appears in `if (false) delete X{}.x;` or `false ? delete X{}.x : (void) 0;`

A "valid" array delete (like `delete[] *__builtin_launder(reinterpret_cast<int(*)[2]>(new int[2]))`) doesn't involve an array temporary (since the array must have been `new`d), so this does seem to only happen in code that can't be executed.
Comment 6 GCC Commits 2024-05-23 20:24:11 UTC
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

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

commit r15-796-ged63cd2aa5b114565fe5499c3a6bf8da5e8e48ba
Author: Jason Merrill <jason@redhat.com>
Date:   Wed May 22 18:41:27 2024 -0400

    c++: deleting array temporary [PR115187]
    
    Decaying the array temporary to a pointer and then deleting that crashes in
    verify_gimple_stmt, because the TARGET_EXPR is first evaluated inside the
    TRY_FINALLY_EXPR, but the cleanup point is outside.  Fixed by using
    get_target_expr instead of save_expr.
    
    I also adjust the stabilize_expr comment to prevent me from again thinking
    it's a suitable replacement.
    
            PR c++/115187
    
    gcc/cp/ChangeLog:
    
            * init.cc (build_delete): Use get_target_expr instead of save_expr.
            * tree.cc (stabilize_expr): Update comment.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp1z/array-prvalue3.C: New test.
Comment 7 GCC Commits 2024-05-24 15:15:22 UTC
The releases/gcc-14 branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:9031c027827bff44e1b55c366fc7034c43501b4c

commit r14-10242-g9031c027827bff44e1b55c366fc7034c43501b4c
Author: Jason Merrill <jason@redhat.com>
Date:   Wed May 22 18:41:27 2024 -0400

    c++: deleting array temporary [PR115187]
    
    Decaying the array temporary to a pointer and then deleting that crashes in
    verify_gimple_stmt, because the TARGET_EXPR is first evaluated inside the
    TRY_FINALLY_EXPR, but the cleanup point is outside.  Fixed by using
    get_target_expr instead of save_expr.
    
    I also adjust the stabilize_expr comment to prevent me from again thinking
    it's a suitable replacement.
    
            PR c++/115187
    
    gcc/cp/ChangeLog:
    
            * init.cc (build_delete): Use get_target_expr instead of save_expr.
            * tree.cc (stabilize_expr): Update comment.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp1z/array-prvalue3.C: New test.
Comment 8 Jason Merrill 2024-05-24 15:25:11 UTC
Fixed for 14.2/15.