Re: g++ problem with order of evaluation of arguments of delete.

[Georg-Johann Lay <avr@gjlay.de>]

Am 04.05.23 um 12:12 schrieb Jonathan Wakely:
> On Thu, 4 May 2023 at 11:06, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>>
>> On Thu, 4 May 2023 at 10:46, Georg-Johann Lay <avr@gjlay.de> wrote:
>>>
>>> Given the following C++ code:
>>>
>>> struct Lexer;
>>>
>>> struct Token
>>> {
>>>       Lexer* const lexer_;
>>>       Token (Lexer *l) : lexer_(l) {}
>>>       ~Token() = default;
>>>
>>>       Token() = delete;
>>>       Token (const Token&) = delete;
>>>       Token (Token&&) = delete;
>>>       void operator= (const Token&) = delete;
>>>       void operator= (Token&&) = delete;
>>> };
>>>
>>> struct Lexer
>>> {
>>>       Token *token_;
>>>       Lexer() = default;
>>>       ~Lexer() { delete token_; }
>>>
>>>       Lexer (const Lexer&) = delete;
>>>       Lexer (Lexer&&) = delete;
>>>       void operator= (const Lexer&) = delete;
>>>       void operator= (Lexer&&) = delete;
>>> };
>>>
>>> int main()
>>> {
>>>       Lexer *lexer = new Lexer();
>>>       Token *token = new Token (lexer);
>>>       lexer->token_ = token;
>>>       delete token->lexer_;
>>>       // delete lexer; // is OK
>>> }
>>>
>>> When I compile this with g++ v11.3 (same with g++ from master from
>>> 2023-04-20) and run
>>>
>>> $ g++ main-3.cpp -Os -W -Wall -Wextra -dumpbase "" -save-temps -dp  &&
>>> ./a.out
>>>
>>> Segmentation fault (core dumped)
>>>
>>> The assembly shows that the generated code does two calls to "delete"
>>> but just one call to "new", so it's clear something is going wrong.
>>>
>>> As far as I understand, the "delete token_" in ~Lexer is a sequence
>>> point, so that dereferencing token in "delete->lexer_" must be sequenced
>>> before calling ~Token ?
>>>
>>> Segmentation fault also occurs with -O0, but goes away when removing the
>>> "const" in "Lexer* const lexer_;".
>>>
>>> My question: Is this a GCC problem, or a problem with the code and
>>> sequence points?
>>
>> It's definitely a GCC bug.
>>
>> The code is compiled to something like:
>>
>> token->lexer_->~Lexer();
>> operator delete(token->lexer_);
>>
>> But that means that we evaluate 'token' twice, even though it's been
>> invalidated by the destructor. It should be compiled to something more
>> like:
>>
>> auto* p = token->lexer_;
>> p->~Lexer();
>> operator delete(p);
> 
> The C++ standard is clear, see [expr.delete] p4:
> 
> "The cast-expression in a delete-expression shall be evaluated exactly once."
> 
> That wording has been present since C++98.
> 
> Please file a bug.

Thank you. I came across that clause, but why is "token->lexer_" in 
delete's argument a cast?