[Bug c++/109731] New: g++ eveluates delete's expressio more than once

[Bug c++/109731] New: g++ eveluates delete's expressio more than once

[gjl at gcc dot gnu.org]

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

            Bug ID: 109731
           Summary: g++ eveluates delete's expressio more than once
           Product: gcc
           Version: 11.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gjl at gcc dot gnu.org
  Target Milestone: ---

Created attachment 54992
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54992&action=edit
C++ test case

The attached test case evaluates the last delete expression more than once,
which leads to a segmentation fault when the code is executed:

> g++ main-3.cpp -O2 && ./a.out

but it also occurs with -O0. Seen on:

g++ (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0

g++ (GCC) 14.0.0 20230420 (experimental)

https://eel.is/c++draft/expr.delete#4

[Bug c++/109731] g++ evaluates delete expressions more than once

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-05-04
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
      Known to fail|                            |13.1.0, 14.0, 4.1.0

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Confirmed. GCC does not follow [expr.delete] p4:

 "The cast-expression in a delete-expression shall be evaluated exactly once."

[Bug c++/109731] g++ evaluates delete expressions more than once

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
ASan shows the use-after-free:

=================================================================
==46643==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000000010
at pc 0x0000004010c4 bp 0x7ffff0f09830 sp 0x7ffff0f09828
READ of size 8 at 0x602000000010 thread T0
    #0 0x4010c3 in main /tmp/del.cc:33
    #1 0x7fc85844a50f in __libc_start_call_main (/lib64/libc.so.6+0x2750f)
(BuildId: 81daba31ee66dbd63efdc4252a872949d874d136)
    #2 0x7fc85844a5c8 in __libc_start_main@GLIBC_2.2.5
(/lib64/libc.so.6+0x275c8) (BuildId: 81daba31ee66dbd63efdc4252a872949d874d136)
    #3 0x401114 in _start (/tmp/a.out+0x401114)

0x602000000010 is located 0 bytes inside of 8-byte region
[0x602000000010,0x602000000018)
freed by thread T0 here:
    #0 0x7fc858addd48 in operator delete(void*, unsigned long)
/home/jwakely/src/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:164
    #1 0x40109a in Lexer::~Lexer() /tmp/del.cc:20
    #2 0x40109a in main /tmp/del.cc:33

previously allocated by thread T0 here:
    #0 0x7fc858adce48 in operator new(unsigned long)
/home/jwakely/src/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:95
    #1 0x40108a in main /tmp/del.cc:31

SUMMARY: AddressSanitizer: heap-use-after-free /tmp/del.cc:33 in main


valgrind agrees:

==46706== Invalid read of size 8
==46706==    at 0x40106B: main (del.cc:33)
==46706==  Address 0x4dca080 is 0 bytes inside a block of size 8 free'd
==46706==    at 0x48468DD: operator delete(void*, unsigned long)
(vg_replace_malloc.c:947)
==46706==    by 0x40106A: ~Lexer (del.cc:20)
==46706==    by 0x40106A: main (del.cc:33)
==46706==  Block was alloc'd at
==46706==    at 0x4843FF5: operator new(unsigned long)
(vg_replace_malloc.c:434)
==46706==    by 0x40105A: main (del.cc:31)



Marc Glisse pointed out that we're not using a SAVE_EXPR for the operand of the
delete-expression: https://gcc.gnu.org/pipermail/gcc-help/2023-May/142512.html

"""
It normally uses a SAVE_EXPR, but seems to consider token->lexer_ as "safe".

         try
           {
             Lexer::~Lexer ((struct Lexer *) token->lexer_);
           }
         finally
           {
             operator delete ((void *) token->lexer_, 8);
           }

whereas if I write delete (token->lexer_ + i); where i is 0, I get

         try
           {
             Lexer::~Lexer (SAVE_EXPR <(struct Lexer *) token->lexer_ + 
(sizetype) (i * 8)>);
           }
         finally
           {
             operator delete ((void *) (SAVE_EXPR <(struct Lexer *) 
token->lexer_ + (sizetype) (i * 8)>), 8);
           }

which works.
"""

[Bug c++/109731] g++ evaluates delete expressions more than once

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It seems that it's considered safe because Token::lexer_ is const, so GCC
thinks the value of token->lexer_ can't be changed by the ~Lexer() destructor.
That's true, but 'token' can become an invalid pointer.

[Bug c++/109731] g++ evaluates delete expressions more than once

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=52339

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I think this is a dup of bug 52339 .

[Bug c++/109731] g++ evaluates delete expressions more than once

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |DUPLICATE
             Status|NEW                         |RESOLVED

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Ah yes, it's exactly the same, even noting that it works without a const
variable. Let's close this one.

*** This bug has been marked as a duplicate of bug 52339 ***