On Thu, 4 May 2023 at 11:06, Jonathan Wakely <jwakely.gcc@xxxxxxxxx> wrote:
>
> On Thu, 4 May 2023 at 10:46, Georg-Johann Lay <avr@xxxxxxxx> 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.
