[Bug c/97860] New: [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244

[Bug c/97860] New: [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244

[gscfq@t-online.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

            Bug ID: 97860
           Summary: [11 Regression] ICE in handle_argspec_attribute, at
                    c-family/c-attribs.c:3244
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gscfq@t-online.de
  Target Milestone: ---

Changed somewhere between 20200823 and 20201004 :


$ cat z1.c
void f (int n)
{
  typedef int T[0];
  typedef T a[n];
  void *g (a);
}


$ gcc-10-20201114 -c z1.c
$
$ gcc-11-20201115 -c z1.c
z1.c: In function 'f':
z1.c:5:3: internal compiler error: in handle_argspec_attribute, at
c-family/c-attribs.c:3244
    5 |   void *g (a);
      |   ^~~~
0x6f34c9 handle_argspec_attribute
        ../../gcc/c-family/c-attribs.c:3244
0x6299f1 decl_attributes(tree_node**, tree_node*, int, tree_node*)
        ../../gcc/attribs.c:724
0x642595 push_parm_decl(c_parm const*, tree_node**)
        ../../gcc/c/c-decl.c:5871
0x67788c c_parser_parms_list_declarator
        ../../gcc/c/c-parser.c:4300
0x677be8 c_parser_parms_declarator
        ../../gcc/c/c-parser.c:4226
0x671163 c_parser_direct_declarator_inner
        ../../gcc/c/c-parser.c:4139
0x671583 c_parser_declarator(c_parser*, bool, c_dtr_syn, bool*)
        ../../gcc/c/c-parser.c:3889
0x6882b7 c_parser_declaration_or_fndef
        ../../gcc/c/c-parser.c:2148
0x6879e9 c_parser_compound_statement_nostart
        ../../gcc/c/c-parser.c:5709
0x687e23 c_parser_compound_statement
        ../../gcc/c/c-parser.c:5606
0x6896b1 c_parser_declaration_or_fndef
        ../../gcc/c/c-parser.c:2543
0x690297 c_parser_external_declaration
        ../../gcc/c/c-parser.c:1777
0x690db9 c_parser_translation_unit
        ../../gcc/c/c-parser.c:1650
0x690db9 c_parse_file()
        ../../gcc/c/c-parser.c:21882
0x6dfb82 c_common_parse_file()
        ../../gcc/c-family/c-opts.c:1196

[Bug c/97860] [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244 since r11-3303-g6450f07388f9fe57

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |10.2.0
   Target Milestone|---                         |11.0
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |msebor at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
      Known to fail|                            |11.0
            Summary|[11 Regression] ICE in      |[11 Regression] ICE in
                   |handle_argspec_attribute,   |handle_argspec_attribute,
                   |at                          |at
                   |c-family/c-attribs.c:3244   |c-family/c-attribs.c:3244
                   |                            |since
                   |                            |r11-3303-g6450f07388f9fe57
   Last reconfirmed|                            |2020-11-16
     Ever confirmed|0                           |1

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Thank you for the report.
Started with r11-3303-g6450f07388f9fe57.

[Bug c/97860] [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244 since r11-3303-g6450f07388f9fe57

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug c/97860] [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244 since r11-3303-g6450f07388f9fe57

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The problem is that in the C FE TYPE_MAX_VALUE (TYPE_DOMAIN (type)) isn't set
for the [0] types (yet they are complete), while the middle-end expects for
them to have TYPE_MAX_VALUE of -1 or so, so array_type_nelts returns
error_mark_node.
We could do:
              /* array_type_nelts assumes the middle-end TYPE_DOMAINs, while
                 GNU [0] array in the FE don't have TYPE_MAX_VALUE of the
                 domain set, yet they are complete types with no elements.  */
              if (nelts == error_mark_node
                  && TYPE_DOMAIN (type)
                  && !TYPE_MAX_VALUE (TYPE_DOMAIN (type))
                  && COMPLETE_P (type)
                  && integer_zerop (TYPE_SIZE (type)))
                continue;
but then it doesn't make sense to queue up error_mark_nodes in the attributes
when we ICE on them later, so perhaps better:
2020-11-18  Jakub Jelinek  <jakub@redhat.com>

        PR c/97860
        * c-decl.c (get_parm_array_spec): Don't chain error_mark_node as
        VLA bounds.

        * gcc.dg/pr97860.c: New test.

--- gcc/c/c-decl.c.jj   2020-11-11 01:46:03.245697697 +0100
+++ gcc/c/c-decl.c      2020-11-18 15:11:14.613565216 +0100
@@ -5775,7 +5775,7 @@ get_parm_array_spec (const struct c_parm
               type = TREE_TYPE (type))
            {
              tree nelts = array_type_nelts (type);
-             if (TREE_CODE (nelts) != INTEGER_CST)
+             if (TREE_CODE (nelts) != INTEGER_CST && nelts != error_mark_node)
                {
                  /* Each variable VLA bound is represented by the dollar
                     sign.  */
--- gcc/testsuite/gcc.dg/pr97860.c.jj   2020-11-18 15:15:08.858931877 +0100
+++ gcc/testsuite/gcc.dg/pr97860.c      2020-11-18 15:14:50.751135430 +0100
@@ -0,0 +1,11 @@
+/* PR c/97860 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+void
+foo (int n)
+{
+  typedef int T[0];
+  typedef T V[n];
+  void bar (V);
+}

[Bug c/97860] [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244 since r11-3303-g6450f07388f9fe57

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
I was going to commit the following but I'll leave it to you.

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index d348e39c27a..95cf9e4cb00 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -5775,6 +5775,10 @@ get_parm_array_spec (const struct c_parm *parm, tree
attrs)
               type = TREE_TYPE (type))
            {
              tree nelts = array_type_nelts (type);
+             if (error_operand_p (nelts))
+               /* Avoid erroneous expressions.  */
+               return attrs;
+
              if (TREE_CODE (nelts) != INTEGER_CST)
                {
                  /* Each variable VLA bound is represented by the dollar

[Bug c/97860] [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244 since r11-3303-g6450f07388f9fe57

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #3)
> I was going to commit the following but I'll leave it to you.
> 
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index d348e39c27a..95cf9e4cb00 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -5775,6 +5775,10 @@ get_parm_array_spec (const struct c_parm *parm, tree
> attrs)
>                type = TREE_TYPE (type))
>             {
>               tree nelts = array_type_nelts (type);
> +             if (error_operand_p (nelts))
> +               /* Avoid erroneous expressions.  */
> +               return attrs;
> +
>               if (TREE_CODE (nelts) != INTEGER_CST)
>                 {
>                   /* Each variable VLA bound is represented by the dollar

For errors why not, but typedef int T[0]; really is not an error actually.

[Bug c/97860] [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244 since r11-3303-g6450f07388f9fe57

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

--- Comment #5 from Martin Sebor <msebor at gcc dot gnu.org> ---
Actually, I missed that your patch just skips over the error node.  That will
leave the attribute spec out of sync with the argument (it contains a '$' for
each VLA bound).  Rather than continuing to the next bound I think we should
bail.  The alternative is to change the rest of the code (the attribute handler
and the whole attribute access machinery) to skip these erroneous bounds.

[Bug c/97860] [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244 since r11-3303-g6450f07388f9fe57

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
As I said, [0] is not a VLA bound.
And we don't record anything for constant bounds (even if they are in the
middle).
So perhaps:
              /* array_type_nelts assumes the middle-end TYPE_DOMAINs, while
                 GNU [0] array in the FE don't have TYPE_MAX_VALUE of the
                 domain set, yet they are complete types with no elements.  */
              if (nelts == error_mark_node
                  && TYPE_DOMAIN (type)
                  && !TYPE_MAX_VALUE (TYPE_DOMAIN (type))
                  && COMPLETE_P (type)
                  && integer_zerop (TYPE_SIZE (type)))
                continue;
              if (error_mark_operand (nelts))
                return attrs;
which would bail for errors, but would handle [0] complete arrays as
non-errors.
After all, even for those it might be meaningful to complain about out of
bounds accesses.
Sure, for [0] arrays all dereferences are invalid, but taking address of
element [0] is ok.

[Bug c/97860] [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244 since r11-3303-g6450f07388f9fe57

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

--- Comment #7 from Martin Sebor <msebor at gcc dot gnu.org> ---
What I mean is that unless error_mark_node necessarily implies (and guarantees)
the bound is a constant zero (as opposed to a similarly "broken" VLA bound),
simply bailing is safer than skipping it.  I have no idea if the front end
guarantees it but I don't see it mentioned in array_type_nelts().  If it is
guaranteed then continuing is fine but it should also be documented in the
comment above array_type_nelts(), and I would suggest to also mention it in the
change to get_parm_array_spec.

[Bug c/97860] [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244 since r11-3303-g6450f07388f9fe57

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:8156cfaa4c45f1249bbdda29d04b4fef84b7eafe

commit r11-5180-g8156cfaa4c45f1249bbdda29d04b4fef84b7eafe
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Nov 19 20:02:41 2020 +0100

    c, tree: Fix ICE from get_parm_array_spec [PR97860]

    The C and C++ FEs handle zero sized arrays differently, C uses
    NULL TYPE_MAX_VALUE on non-NULL TYPE_DOMAIN on complete ARRAY_TYPEs
    with bitsize_zero_node TYPE_SIZE, while C++ FE likes to set
    TYPE_MAX_VALUE to the largest value (and min to the lowest).

    Martin has used array_type_nelts in get_parm_array_spec where the
    function on the C form of [0] arrays returns error_mark_node and the code
    crashes soon afterwards.  The following patch teaches array_type_nelts
about
    this (e.g. dwarf2out already handles that as [0]).  While it will change
    what is_empty_type returns for certain types (e.g. struct S { int a[0];
};),
    as those types occupy zero bits in C, it should make an ABI difference.

    So, the tree.c change makes the c-decl.c code handle the [0] arrays
    like any other constant extents, and the c-decl.c change just makes sure
    that if we'd run into error_mark_node e.g. from the VLA expressions, we
    don't crash on those.

    2020-11-19  Jakub Jelinek  <jakub@redhat.com>

            PR c/97860
            * tree.c (array_type_nelts): For complete arrays with zero min
            and NULL max and zero size return -1.

            * c-decl.c (get_parm_array_spec): Bail out of nelts is
            error_operand_p.

            * gcc.dg/pr97860.c: New test.

[Bug c/97860] [11 Regression] ICE in handle_argspec_attribute, at c-family/c-attribs.c:3244 since r11-3303-g6450f07388f9fe57

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97860

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.