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;
}
