Thanks Joseph!
Revised version attached. Ok?
But I wonder whether we generally need to do something
about
sizeof *x
when x is NULL or not initialized. This is quite commonly
used in C code and if the type is not of variable size,
it is also unproblematic. So the UB for variable size is
unfortunate and certainly also affects existing code in
the wild. In practice it does not seem to cause
problems because there is no lvalue conversion and this
then seems to work. Maybe we document this as an
extension? (and make sure in the C FE that it
works) This would also make this idiom valid:
char (*buf)[n] = malloc(sizeof *buf);
Or if we do not want to do this, then I think we should
add some warnings (and UBSan check for null pointer)
which currently do not exist:
https://godbolt.org/z/fhWMKvYc8
Martin
Am Donnerstag, dem 18.05.2023 um 21:46 + schrieb Joseph Myers:
> On Thu, 18 May 2023, Martin Uecker via Gcc-patches wrote:
>
> > + /* we still have to evaluate size expressions */
>
> Comments should start with a capital letter and end with ". ".
>
> > diff --git a/gcc/testsuite/gcc.dg/nested-vla-1.c
> > b/gcc/testsuite/gcc.dg/nested-vla-1.c
> > new file mode 100644
> > index 000..408a68524d8
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/nested-vla-1.c
> > @@ -0,0 +1,37 @@
> > +/* { dg-do run } */
> > +/* { dg-options "-std=gnu99" } */
>
> I'm concerned with various undefined behavior in this and other tests;
> they look very fragile, relying on some optimizations and not others
> taking place. I think they should be adjusted to avoid undefined behavior
> if all the evaluations from the abstract machine (in particular, of sizeof
> operands with variable size) take place, and other undefined behavior from
> calling functions through function pointers with incompatible type.
>
> > + struct bar { char x[++n]; } (*bar2)(void) = bar;/* { dg-warning
> > "incompatible pointer type" } */
> > +
> > + if (2 != n)
> > + __builtin_abort();
> > +
> > + if (2 != sizeof((*bar2)()))
> > + __builtin_abort();
>
> You're relying on the compiler not noticing that a function is being
> called through an incompatible type and thus not turning the call (which
> should be evaluated, because the operand of sizeof has a type with
> variable size) into a call to abort.
>
> > diff --git a/gcc/testsuite/gcc.dg/nested-vla-2.c
> > b/gcc/testsuite/gcc.dg/nested-vla-2.c
> > new file mode 100644
> > index 000..504eec48c80
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/nested-vla-2.c
> > @@ -0,0 +1,33 @@
> > +/* { dg-do run } */
> > +/* { dg-options "-std=gnu99" } */
> > +
> > +
> > +int main()
> > +{
> > + int n = 1;
> > +
> > + typeof(char (*)[++n]) bar(void) { }
> > +
> > + if (2 != n)
> > + __builtin_abort();
> > +
> > + if (2 != sizeof(*bar()))
> > + __builtin_abort();
>
> In this test, *bar() is evaluated, i.e. an undefined pointer is
> dereferenced; it would be better to return a valid pointer to a
> sufficiently large array to avoid that undefined behavior.
>
> > diff --git a/gcc/testsuite/gcc.dg/pr106465.c
> > b/gcc/testsuite/gcc.dg/pr106465.c
> > new file mode 100644
> > index 000..b03e2442f12
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr106465.c
> > @@ -0,0 +1,86 @@
> > +/* PR c/106465
> > + * { dg-do run }
> > + * { dg-options "-std=gnu99" }
> > + * */
> > +
> > +int main()
> > +{
> > + int n = 3;
> > +
> > + void g1(int m, struct { char p[++m]; }* b) /* { dg-warning
> > "anonymous struct" } */
> > + {
> > + if (3 != m)
> > + __builtin_abort();
> > +
> > + if (3 != sizeof(b->p))
> > + __builtin_abort();
> > + }
>
> > + g1(2, (void*)0);
>
> Similarly, this is dereferencing a null pointer in the evaluated operand
> of sizeof.
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 90d7cd27cd5..f63c1108ab5 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5378,7 +5378,8 @@ start_decl (struct c_declarator *declarator, struct
c_declspecs *declspecs,
if (lastdecl != error_mark_node)
*lastloc = DECL_SOURCE_LOCATION (lastdecl);
- if (expr)
+ /* Make sure the size expression is evaluated at this point. */
+ if (expr && !current_scope->parm_flag)
add_stmt (fold_convert (void_type_node, expr));
if (TREE_CODE (decl) != FUNCTION_DECL && MAIN_NAME_P (DECL_NAME (decl))
@@ -7510,7 +7511,8 @@ grokdeclarator (const struct c_declarator *declarator,
&& c_type_variably_modified_p (type))
{
tree bind = NULL_TREE;
- if (decl_context == TYPENAME || decl_context == PARM)
+ if (decl_context == TYPENAME || decl_context == PARM
+ || decl_context == FIELD)
{
bind = build3 (BIND_EXPR, void_type_node, NULL_TREE,
NULL_TREE, NULL_TREE);
@@ -7519,10
