public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
@ 2012-02-22 10:37 rombust at hotmail dot co.uk
2012-02-22 11:22 ` [Bug c++/52339] " redi at gcc dot gnu.org
` (10 more replies)
0 siblings, 11 replies; 12+ messages in thread
From: rombust at hotmail dot co.uk @ 2012-02-22 10:37 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
Bug #: 52339
Summary: using delete ptr1->ptr2 where ptr2 destructor deletes
a const ptr1 fails
Classification: Unclassified
Product: gcc
Version: 4.6.2
Status: UNCONFIRMED
Severity: major
Priority: P3
Component: c++
AssignedTo: unassigned@gcc.gnu.org
ReportedBy: rombust@hotmail.co.uk
The following code crashes and triggers the valgrind error with gcc 4.4 and
4.6. With clang it works just fine. And it also works fine if you remove
"const" on line 15
*******
struct B;
struct A {
B* b;
};
struct B {
A* a;
B() : a(new A{this}) {}
~B() { delete a; }
};
int main() {
B* b = new B;
// A* a = b->a;
const A* a = b->a;
delete a->b;
}
******
I have no idea if is it undefined behaviour to delete a->b if b deletes a.
Expressions should be evaluated only once, but for some reason this line:
delete a->b;
is basically acting as if was written like:
a->b->~B();
operator delete(a->b); // a is already deleted
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
@ 2012-02-22 11:22 ` redi at gcc dot gnu.org
2012-02-22 13:07 ` redi at gcc dot gnu.org
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: redi at gcc dot gnu.org @ 2012-02-22 11:22 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |wrong-code
Status|UNCONFIRMED |NEW
Last reconfirmed| |2012-02-22
Ever Confirmed|0 |1
--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-02-22 11:09:53 UTC ---
Confirmed, [expr.delete] p4 says "The cast-expression in a delete-expression
shall be evaluated exactly once."
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
2012-02-22 11:22 ` [Bug c++/52339] " redi at gcc dot gnu.org
@ 2012-02-22 13:07 ` redi at gcc dot gnu.org
2023-05-04 11:34 ` redi at gcc dot gnu.org
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: redi at gcc dot gnu.org @ 2012-02-22 13:07 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-02-22 12:48:04 UTC ---
b = b.0;
a = b->a;
D.2361 = a->b;
if (D.2361 != 0B) goto <D.2362>; else goto <D.2363>;
<D.2362>:
D.2364 = a->b;
B::~B (D.2364);
D.2365 = a->b;
operator delete (D.2365);
which is indeed as if written like:
a->b->~B();
operator delete(a->b); // a is already deleted
if 'a' is not a pointer-to-const then instead you get
b = b.0;
a = b->a;
D.2361 = a->b;
if (D.2361 != 0B) goto <D.2362>; else goto <D.2363>;
<D.2362>:
B::~B (D.2361);
operator delete (D.2361);
calling the destructor and operator delete directly on the value of a->b
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
2012-02-22 11:22 ` [Bug c++/52339] " redi at gcc dot gnu.org
2012-02-22 13:07 ` redi at gcc dot gnu.org
@ 2023-05-04 11:34 ` redi at gcc dot gnu.org
2023-05-04 11:35 ` redi at gcc dot gnu.org
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: redi at gcc dot gnu.org @ 2023-05-04 11:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |gjl at gcc dot gnu.org
--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
*** Bug 109731 has been marked as a duplicate of this bug. ***
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
` (2 preceding siblings ...)
2023-05-04 11:34 ` redi at gcc dot gnu.org
@ 2023-05-04 11:35 ` redi at gcc dot gnu.org
2023-05-04 12:23 ` jakub at gcc dot gnu.org
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: redi at gcc dot gnu.org @ 2023-05-04 11:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Known to fail| |13.1.0, 14.0, 4.1.0, 4.6.2
Last reconfirmed|2021-08-11 00:00:00 |2023-5-4
--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
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.
"""
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
` (3 preceding siblings ...)
2023-05-04 11:35 ` redi at gcc dot gnu.org
@ 2023-05-04 12:23 ` jakub at gcc dot gnu.org
2023-05-04 12:45 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-05-04 12:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org,
| |jason at gcc dot gnu.org,
| |jsm28 at gcc dot gnu.org
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
struct L;
struct T
{
L *const l;
T (L *x) : l(x) {}
~T () = default;
T () = delete;
T (const T &) = delete;
T (T &&) = delete;
void operator= (const T &) = delete;
void operator= (T &&) = delete;
};
struct L
{
T *t;
L () = default;
~L () { delete t; }
L (const L&) = delete;
L (L&&) = delete;
void operator= (const L &) = delete;
void operator= (L &&) = delete;
};
int
main ()
{
L *l = new L ();
T *t = new T (l);
l->t = t;
delete t->l;
}
build_delete uses
5174 if (deleting)
5175 /* We will use ADDR multiple times so we must save it. */
5176 addr = save_expr (addr);
and for
((struct L *) this)->t
in ~L destructor that wraps it in a SAVE_EXPR, for
(struct L *) VIEW_CONVERT_EXPR<struct T *>(t)->l
in main it doesn't, as the nop is stripped and the COMPONENT_REF is
TREE_READONLY and !TREE_SIDE_EFFECTS.
tree.cc (save_expr) documents that:
Constants, and certain read-only nodes, are returned with no
SAVE_EXPR because that is safe. Expressions containing placeholders
are not touched; see tree.def for an explanation of what these
are used for.
So, the big question is whether it is just this use of save_expr in
build_delete that needs stronger treatment (e.g. use addr = get_target_expr
(addr, complain); instead of
addr = save_expr (addr)) or if we should reconsider the save_expr
implementation and wrap with SAVE_EXPR if the expression contains any
INDIRECT_REFs/MEM_REFs (the latter perhaps with exception of ADDR_EXPR decl as
address) despite it currently just returning the passed in expression, for fear
that what the pointer points to could be deleted/reallocated in between.
Is the following testcase valid C99+? If yes, I think we want to change
SAVE_EXPR rather than the C++ FE. clang compiles it into something that
passes, gcc evaluates p->a multiple times (to find out the size of the VLA and
then again in the sizeof evaluation) and so it is a heap use after free
(diagnosed e.g. by -fsanitize=address).
#include <stdlib.h>
struct S { int a; };
void
bar (int *p, struct S *q)
{
free (q);
}
size_t
foo (const struct S *p, struct S *q)
{
int b[p->a];
bar (b, q);
return sizeof (b);
}
int
main ()
{
struct S *p = malloc (sizeof (struct S));
if (!p)
return 0;
p->a = 42;
if (foo (p, p) != 42 * sizeof (int))
abort ();
return 0;
}
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
` (4 preceding siblings ...)
2023-05-04 12:23 ` jakub at gcc dot gnu.org
@ 2023-05-04 12:45 ` jakub at gcc dot gnu.org
2023-05-04 13:45 ` jakub at gcc dot gnu.org
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-05-04 12:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Though, walking the whole tree to find them in tree_invariant_p_1 would result
in bad compile time complexity, because e.g. skip_simple_arithmetic can call
that on both operands of binary expression etc.
So, perhaps check it in save_expr?
- if (tree_invariant_p_1 (inner))
+ if (tree_invariant_p_1 (inner)
+ && !walk_tree_without_duplicates (&expr, contains_indirect_refs, NULL))
return expr;
?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
` (5 preceding siblings ...)
2023-05-04 12:45 ` jakub at gcc dot gnu.org
@ 2023-05-04 13:45 ` jakub at gcc dot gnu.org
2023-05-04 16:58 ` joseph at codesourcery dot com
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-05-04 13:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 54994
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54994&action=edit
gcc14-pr52339.patch
Untested fix.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
` (6 preceding siblings ...)
2023-05-04 13:45 ` jakub at gcc dot gnu.org
@ 2023-05-04 16:58 ` joseph at codesourcery dot com
2023-05-04 22:12 ` jason at gcc dot gnu.org
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: joseph at codesourcery dot com @ 2023-05-04 16:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
--- Comment #8 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
I think it's valid C99, yes: the VLA size should be evaluated exactly
once, when the declaration is passed in the order of execution.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
` (7 preceding siblings ...)
2023-05-04 16:58 ` joseph at codesourcery dot com
@ 2023-05-04 22:12 ` jason at gcc dot gnu.org
2023-11-09 6:35 ` pinskia at gcc dot gnu.org
2023-12-21 3:18 ` pinskia at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: jason at gcc dot gnu.org @ 2023-05-04 22:12 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
--- Comment #9 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> in main it doesn't, as the nop is stripped and the COMPONENT_REF is
> TREE_READONLY and !TREE_SIDE_EFFECTS.
> tree.cc (save_expr) documents that:
> Constants, and certain read-only nodes, are returned with no
> SAVE_EXPR because that is safe.
Yeah, assuming that TREE_READONLY (t) && !TREE_SIDE_EFFECTS (t) is enough to
assume that T won't change seems bogus to me.
We could remove that line from tree_invariant_p_1, or restrict it to only apply
if DECL_P (t) is also true?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
` (8 preceding siblings ...)
2023-05-04 22:12 ` jason at gcc dot gnu.org
@ 2023-11-09 6:35 ` pinskia at gcc dot gnu.org
2023-12-21 3:18 ` pinskia at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-11-09 6:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #7)
> Created attachment 54994 [details]
> gcc14-pr52339.patch
>
> Untested fix.
I think this might fix PR 108789 too ...
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug c++/52339] using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
` (9 preceding siblings ...)
2023-11-09 6:35 ` pinskia at gcc dot gnu.org
@ 2023-12-21 3:18 ` pinskia at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-12-21 3:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52339
--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #10)
> (In reply to Jakub Jelinek from comment #7)
> > Created attachment 54994 [details]
> > gcc14-pr52339.patch
> >
> > Untested fix.
>
> I think this might fix PR 108789 too ...
And PR 53424 .
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-12-21 3:18 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-22 10:37 [Bug c++/52339] New: using delete ptr1->ptr2 where ptr2 destructor deletes a const ptr1 fails rombust at hotmail dot co.uk
2012-02-22 11:22 ` [Bug c++/52339] " redi at gcc dot gnu.org
2012-02-22 13:07 ` redi at gcc dot gnu.org
2023-05-04 11:34 ` redi at gcc dot gnu.org
2023-05-04 11:35 ` redi at gcc dot gnu.org
2023-05-04 12:23 ` jakub at gcc dot gnu.org
2023-05-04 12:45 ` jakub at gcc dot gnu.org
2023-05-04 13:45 ` jakub at gcc dot gnu.org
2023-05-04 16:58 ` joseph at codesourcery dot com
2023-05-04 22:12 ` jason at gcc dot gnu.org
2023-11-09 6:35 ` pinskia at gcc dot gnu.org
2023-12-21 3:18 ` pinskia at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).