Re: v2 [C PATCH] Fix regression causing ICE for structs with VLAs [PR 112488]

2023-12-11 Thread Joseph Myers
On Sat, 9 Dec 2023, Martin Uecker wrote:

> Fix regression causing ICE for structs with VLAs [PR 112488]
> 
> A previous patch the fixed several ICEs related to size expressions
> of VM types (PR c/70418, ...) caused a regression for structs where
> a DECL_EXPR is not generated anymore although reqired.  We now call
> add_decl_expr introduced by the previous patch from finish_struct.
> The function is revised with a new argument to not set the TYPE_NAME
> for the type to the DECL_EXPR in this specific case.

OK.

-- 
Joseph S. Myers
jos...@codesourcery.com


v2 [C PATCH] Fix regression causing ICE for structs with VLAs [PR 112488]

2023-12-09 Thread Martin Uecker


I revised version which fixes a problem with breaking other
callers of finish_rust. Please ignore the previous one.

Bootstrapped and regression tested on x86_64


Fix regression causing ICE for structs with VLAs [PR 112488]

A previous patch the fixed several ICEs related to size expressions
of VM types (PR c/70418, ...) caused a regression for structs where
a DECL_EXPR is not generated anymore although reqired.  We now call
add_decl_expr introduced by the previous patch from finish_struct.
The function is revised with a new argument to not set the TYPE_NAME
for the type to the DECL_EXPR in this specific case.

PR c/112488

gcc/c
* c-decl.cc (add_decl_expr): Revise.
(finish_struct): Create DECL_EXPR.
* c-parser.cc (c_parser_struct_or_union_specifier): Call
finish_struct with expression for VLA sizes.
* c-tree.h (finish_struct): Add argument.

gcc/testsuite
* gcc.dg/pr112488-1.c: New test.
* gcc.dg/pr112488-2.c: New test.
* gcc.dg/pr112898.c: New test.
* gcc.misc-tests/gcov-pr85350.c: Adapt.
---
 gcc/c/c-decl.cc | 33 -
 gcc/c/c-parser.cc   |  2 +-
 gcc/c/c-tree.h  |  3 +-
 gcc/testsuite/gcc.dg/pr112488-1.c   | 14 +
 gcc/testsuite/gcc.dg/pr112488-2.c   | 13 
 gcc/testsuite/gcc.dg/pr112898.c |  9 ++
 gcc/testsuite/gcc.misc-tests/gcov-pr85350.c |  2 +-
 7 files changed, 65 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr112488-1.c
 create mode 100644 gcc/testsuite/gcc.dg/pr112488-2.c
 create mode 100644 gcc/testsuite/gcc.dg/pr112898.c

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 92c83e1bf10..039a66fef09 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -6618,12 +6618,10 @@ smallest_type_quals_location (const location_t 
*locations,
the size evaluation prior to the side effects.  We therefore
use BIND_EXPRs in TYPENAME contexts too.  */
 static void
-add_decl_expr (location_t loc, enum decl_context decl_context, tree type,
-  tree *expr)
+add_decl_expr (location_t loc, tree type, tree *expr, bool set_name_p)
 {
   tree bind = NULL_TREE;
-  if (decl_context == TYPENAME || decl_context == PARM
-  || decl_context == FIELD)
+  if (expr)
 {
   bind = build3 (BIND_EXPR, void_type_node, NULL_TREE, NULL_TREE,
 NULL_TREE);
@@ -6636,7 +6634,8 @@ add_decl_expr (location_t loc, enum decl_context 
decl_context, tree type,
   pushdecl (decl);
   DECL_ARTIFICIAL (decl) = 1;
   add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl));
-  TYPE_NAME (type) = decl;
+  if (set_name_p)
+TYPE_NAME (type) = decl;
 
   if (bind)
 {
@@ -7635,7 +7634,12 @@ grokdeclarator (const struct c_declarator *declarator,
   type has a name/declaration of it's own, but special attention
   is required if the type is anonymous. */
if (!TYPE_NAME (type) && c_type_variably_modified_p (type))
- add_decl_expr (loc, decl_context, type, expr);
+ {
+   bool bind_p = decl_context == TYPENAME
+ || decl_context == FIELD
+ || decl_context == PARM;
+   add_decl_expr (loc, type, bind_p ? expr : NULL, true);
+ }
 
type = c_build_pointer_type (type);
 
@@ -7900,7 +7904,12 @@ grokdeclarator (const struct c_declarator *declarator,
 
/* The pointed-to type may need a decl expr (see above).  */
if (!TYPE_NAME (type) && c_type_variably_modified_p (type))
- add_decl_expr (loc, decl_context, type, expr);
+ {
+   bool bind_p = decl_context == TYPENAME
+ || decl_context == FIELD
+ || decl_context == PARM;
+   add_decl_expr (loc, type, bind_p ? expr : NULL, true);
+ }
 
type = c_build_pointer_type (type);
type_quals = array_ptr_quals;
@@ -9257,7 +9266,8 @@ is_flexible_array_member_p (bool is_last_field,
 
 tree
 finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
-  class c_struct_parse_info *enclosing_struct_parse_info)
+  class c_struct_parse_info *enclosing_struct_parse_info,
+  tree *expr)
 {
   tree x;
   bool toplevel = file_scope == current_scope;
@@ -9595,6 +9605,13 @@ finish_struct (location_t loc, tree t, tree fieldlist, 
tree attributes,
 
   finish_incomplete_vars (incomplete_vars, toplevel);
 
+  /* Make sure a DECL_EXPR is created for structs with VLA members.
+ Because we do not know the context, we always pass expr
+ to force creation of a BIND_EXPR which is required in some
+ contexts.  */
+  if (c_type_variably_modified_p (t))
+add_decl_expr (loc, t, expr, false);
+
   if (warn_cxx_compat)
 warn_cxx_compat_finish_struct 

[C PATCH] Fix regression causing ICE for structs with VLAs [PR 112488]

2023-12-08 Thread Martin Uecker


This fixes a regression caused by my previous VM fixes.


Fix regression causing ICE for structs with VLAs [PR 112488]

A previous patch the fixed several ICEs related to size expressions
of VM types (PR c/70418, ...) caused a regression for structs where
a DECL_EXPR is not generated anymore although reqired.  We now call
add_decl_expr introduced by the previous patch from finish_struct.
The function gets a new argument to not set the TYPE_NAME for the
type to the DECL_EXPR in this spicitic case.

PR c/112488

gcc/c
* c-decl.cc (add_decl_expr): Add argument.
(finish_struct): Create DECL_EXPR.
(c_simulate_record_decl): Adapt.
* c-parser.cc (c_parser_struct_or_union_specifier): Call
finish_struct with expression for VLA sizes.
* c-tree.h (finish_struct): Add argument.

gcc/testsuite
* gcc.dg/pr112488-1.c: New test.
* gcc.dg/pr112488-2.c: New test.
* gcc.dg/pr112898.c: New test.
* gcc.misc-tests/gcov-pr85350.c: Adapt.
---
 gcc/c/c-decl.cc | 22 +++--
 gcc/c/c-parser.cc   |  2 +-
 gcc/c/c-tree.h  |  3 ++-
 gcc/testsuite/gcc.dg/pr112488-1.c   | 14 +
 gcc/testsuite/gcc.dg/pr112488-2.c   | 13 
 gcc/testsuite/gcc.dg/pr112898.c |  9 +
 gcc/testsuite/gcc.misc-tests/gcov-pr85350.c |  2 +-
 7 files changed, 56 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr112488-1.c
 create mode 100644 gcc/testsuite/gcc.dg/pr112488-2.c
 create mode 100644 gcc/testsuite/gcc.dg/pr112898.c

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 92c83e1bf10..0b500c19e70 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -6619,7 +6619,7 @@ smallest_type_quals_location (const location_t *locations,
use BIND_EXPRs in TYPENAME contexts too.  */
 static void
 add_decl_expr (location_t loc, enum decl_context decl_context, tree type,
-  tree *expr)
+  tree *expr, bool set_name_p)
 {
   tree bind = NULL_TREE;
   if (decl_context == TYPENAME || decl_context == PARM
@@ -6636,7 +6636,8 @@ add_decl_expr (location_t loc, enum decl_context 
decl_context, tree type,
   pushdecl (decl);
   DECL_ARTIFICIAL (decl) = 1;
   add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl));
-  TYPE_NAME (type) = decl;
+  if (set_name_p)
+TYPE_NAME (type) = decl;
 
   if (bind)
 {
@@ -7635,7 +7636,7 @@ grokdeclarator (const struct c_declarator *declarator,
   type has a name/declaration of it's own, but special attention
   is required if the type is anonymous. */
if (!TYPE_NAME (type) && c_type_variably_modified_p (type))
- add_decl_expr (loc, decl_context, type, expr);
+ add_decl_expr (loc, decl_context, type, expr, true);
 
type = c_build_pointer_type (type);
 
@@ -7900,7 +7901,7 @@ grokdeclarator (const struct c_declarator *declarator,
 
/* The pointed-to type may need a decl expr (see above).  */
if (!TYPE_NAME (type) && c_type_variably_modified_p (type))
- add_decl_expr (loc, decl_context, type, expr);
+ add_decl_expr (loc, decl_context, type, expr, true);
 
type = c_build_pointer_type (type);
type_quals = array_ptr_quals;
@@ -9257,7 +9258,8 @@ is_flexible_array_member_p (bool is_last_field,
 
 tree
 finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
-  class c_struct_parse_info *enclosing_struct_parse_info)
+  class c_struct_parse_info *enclosing_struct_parse_info,
+  tree *expr)
 {
   tree x;
   bool toplevel = file_scope == current_scope;
@@ -9595,6 +9597,13 @@ finish_struct (location_t loc, tree t, tree fieldlist, 
tree attributes,
 
   finish_incomplete_vars (incomplete_vars, toplevel);
 
+  /* Make sure a DECL_EXPR is created for structs with VLA members.
+ Because we do not know the context, we use decl_context TYPENAME
+ here to force creation of a BIND_EXPR which is required in some
+ contexts.  */
+  if (c_type_variably_modified_p (t))
+add_decl_expr (loc, TYPENAME, t, expr, false);
+
   if (warn_cxx_compat)
 warn_cxx_compat_finish_struct (fieldlist, TREE_CODE (t), loc);
 
@@ -10191,7 +10200,8 @@ c_simulate_record_decl (location_t loc, const char 
*name,
DECL_CHAIN (fields[i - 1]) = fields[i];
 }
 
-  finish_struct (loc, type, fields[0], NULL_TREE, struct_info);
+  tree expr = NULL_TREE;
+  finish_struct (loc, type, fields[0], NULL_TREE, struct_info, );
 
   tree decl = build_decl (loc, TYPE_DECL, ident, type);
   set_underlying_type (decl);
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index df9a07928b5..dcb6c21da41 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -4087,7 +4087,7 @@ c_parser_struct_or_union_specifier (c_parser *parser)
   ret.spec = finish_struct (struct_loc, type, nreverse