diff mbox series

[C] Evaluate argument of sizeof that are structs of variable size.

Message ID 50d17a1faa652493a0fe0c303f9541b2cca4e68a.camel@med.uni-goettingen.de
State New
Headers show
Series [C] Evaluate argument of sizeof that are structs of variable size. | expand

Commit Message

Uecker, Martin Aug. 9, 2021, 9:25 p.m. UTC
Here is a patch which changes the behavior of sizeof
when applied to structs of variable size (a GNU
extension) to evaluate its arguments as it does
for VLAs. This is a breaking change, but it seems
this is required if we want to fix [PR29970] (and
it is also more consistent). Together with the patch
I sent previously (which needs fixing as it breaks Ada)
and another small change, this would fix most
of the examples in PR29970.

Comments?




Evaluate arguments of sizeof that are structs of variable size.

Evaluate arguments of sizeof for all types of variable size
and not just for VLAs. This fixes some issues related to
[PR29970] where statement expressions need to be evaluated
so that the size is well defined.

2021-08-09  Martin Uecker  <muecker@gwdg.de>
    
    gcc/
        PR c/29970
        * c-typeck.c (c_expr_sizeof_expr): Evaluate
	size expressions for structs of variable size.

    gcc/testsuite/
        PR c/29970
        * gcc.dg/vla-stexp-1.c: New test.

Comments

Joseph Myers Aug. 9, 2021, 9:42 p.m. UTC | #1
On Mon, 9 Aug 2021, Uecker, Martin wrote:

> Evaluate arguments of sizeof that are structs of variable size.
> 
> Evaluate arguments of sizeof for all types of variable size
> and not just for VLAs. This fixes some issues related to
> [PR29970] where statement expressions need to be evaluated
> so that the size is well defined.

OK.  It might be a good idea to check if something like this is also 
needed in c_expr_sizeof_type, since that's also using c_vla_type_p at 
present.
Uecker, Martin Aug. 9, 2021, 10:47 p.m. UTC | #2
Am Montag, den 09.08.2021, 21:42 +0000 schrieb Joseph Myers:
> On Mon, 9 Aug 2021, Uecker, Martin wrote:
> 
> > Evaluate arguments of sizeof that are structs of variable size.
> > 
> > Evaluate arguments of sizeof for all types of variable size
> > and not just for VLAs. This fixes some issues related to
> > [PR29970] where statement expressions need to be evaluated
> > so that the size is well defined.
> 
> OK.  It might be a good idea to check if something like this is also 
> needed in c_expr_sizeof_type, since that's also using c_vla_type_p at 
> present.

I am still trying to figure out what the branch guarded
by c_vla_type_p is for in c_expr_sizeof_type. The comment
talks about [*], but how would you apply sizeof to
such an array?  One can form something like

int foo(int (*x)[*], int a[sizeof *x]);

but only in prototypes and then it is never evaluated.

I guess it is to avoid this being folded to:

int foo(int (*x)[*], int a[0]);

So the question is whether this can also happen with
struct types. This seems possible:

int foo(struct { int z[*]; } v, int (*y)[sizeof v]);

So maybe we need the change there too.

Martin
Joseph Myers Aug. 10, 2021, 5:19 p.m. UTC | #3
On Mon, 9 Aug 2021, Uecker, Martin wrote:

> I am still trying to figure out what the branch guarded
> by c_vla_type_p is for in c_expr_sizeof_type. The comment
> talks about [*], but how would you apply sizeof to
> such an array?  One can form something like

The comment describes a requirement on what happens for [*], but that 
branch in the code isn't restricted to cases involving [*]; it covers any 
case where (the type is a VLA and) groktypename has returned an expression 
to be evaluated before the type name.

E.g. take your vla-stexp-1.c test but with sizeof(typeof(...)) instead of 
sizeof(...) - that aborts, but I'd hope such a fix in c_expr_sizeof_type 
would make it pass.
diff mbox series

Patch

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 5d6565bdaa9..c5bf3372321 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -2992,7 +2992,7 @@  c_expr_sizeof_expr (location_t loc, struct c_expr expr)
       c_last_sizeof_loc = loc;
       ret.original_code = SIZEOF_EXPR;
       ret.original_type = NULL;
-      if (c_vla_type_p (TREE_TYPE (folded_expr)))
+      if (C_TYPE_VARIABLE_SIZE (TREE_TYPE (folded_expr)))
 	{
 	  /* sizeof is evaluated when given a vla (C99 6.5.3.4p2).  */
 	  ret.value = build2 (C_MAYBE_CONST_EXPR, TREE_TYPE (ret.value),
diff --git a/gcc/testsuite/gcc.dg/vla-stexp-01.c b/gcc/testsuite/gcc.dg/vla-stexp-01.c
new file mode 100644
index 00000000000..97d66937e9a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-stexp-01.c
@@ -0,0 +1,18 @@ 
+/* PR29970*/
+/* { dg-do run } */
+/* { dg-options "-Wall -O0" } */
+
+int foo(void)
+{
+	int n = 0;
+	return sizeof(*({ n = 10; struct foo { int x[n]; } x; &x; }));
+}
+
+
+int main()
+{
+	if (sizeof(struct foo { int x[10]; }) != foo())
+		__builtin_abort();
+
+	return 0;
+}