There are couple of unsolved problems and missing features
related to -Wvla-parameter.  I found it difficult to address these
with the current implementation.   Here is a patch to improve theĀ 
implementation of the `arg spec' attribute.  This patch would also
give us access to the type of the unadjusted array for array
parameters in the C FE, which can be used for improved warnings
(or the _Countof extension Alex has proposed).


Bootstrapped and regression tested on x86.


    c: rewrite implementation of `arg spec' attribute
    
    Rewrite the implementation of the `arg spec' attribute to pass the the
    original type of an array parameter instead of passing a string description
    and a list of bounds.  The string and list are then created
    from this type during the integration of the information of `arg spec'
    into the access attribute because it still needed later for various
    warnings.   This change makes the implementation simpler and more robust
    as the declarator is not processed and information that is already encoded
    in the type is not duplicated.  A similar change to the access attribute
    could then completely remove the string processing.  With this change, the
    original type is now available and can be used for other warnings or for
    _Countof.  The new implementation tries to be faithful to the original,
    but this is not entirely true as it fixes a bug in the old implementation.
    
    gcc/c-family/ChangeLog:
            * c-attribs.cc (handle_argspec_attribute): Update.
            (build_arg_spec): New function.
            (build_attr_access_from_parms): Rewrite `arg spec' handling.
    
    gcc/c/ChangeLog:
            * c/c-decl.cc (get_parm_array_spec): Remove.
            (push_parm_decl): Do not add `arg spec` attribute.
            (build_arg_spec_attribute): New function.
            (grokdeklarator): Add `arg spec` attribute.
    
    gcc/testsuite/ChangeLog:
            * gcc.dg/Warray-parameter-11.c: Change Warray-parameter to
            -Wvla-parameter as these are VLAs.
            * gcc.dg/Warray-parameter.c: Remove xfail.

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 1f4a0df1205..a0d832b5e05 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -4120,10 +4120,11 @@ handle_argspec_attribute (tree *, tree, tree args, int, 
bool *)
 {
   /* Verify the attribute has one or two arguments and their kind.  */
   gcc_assert (args && TREE_CODE (TREE_VALUE (args)) == STRING_CST);
-  for (tree next = TREE_CHAIN (args); next; next = TREE_CHAIN (next))
+  if (TREE_CHAIN (args))
     {
-      tree val = TREE_VALUE (next);
-      gcc_assert (DECL_P (val) || EXPR_P (val));
+      tree val = TREE_VALUE (TREE_CHAIN (args));
+      gcc_assert (!TREE_CHAIN (TREE_CHAIN (args)));
+      gcc_assert (TYPE_P (val));
     }
   return NULL_TREE;
 }
@@ -5736,6 +5737,71 @@ handle_access_attribute (tree node[3], tree name, tree 
args, int flags,
   return NULL_TREE;
 }
 
+
+/* This function builds a string which is concatenated to SPEC and returns
+   list of variably bounds corresponding to an array/VLA parameter with
+   type TYPE.  The string consists of one dollar symbol for each specified
+   variable bound, one asterisk for each unspecified variable bound,
+   a space for an array of unknown size (only possibly for the outermost),
+   and a zero for a zero-sized array.
+
+   The chainof variable bounds starts with the most significant bound.
+   For example, the TYPE T[2][m][3][n] will produce "$$" and (m, (n, nil)).  */
+
+static tree
+build_arg_spec (tree type, std::string *spec)
+{
+  while (POINTER_TYPE_P (type))
+    type = TREE_TYPE (type);
+
+  if (TREE_CODE (type) != ARRAY_TYPE)
+    return NULL_TREE;
+
+  tree list = build_arg_spec (TREE_TYPE (type), spec);
+
+  if (!COMPLETE_TYPE_P (type))
+    {
+      (*spec) += ' ';
+      return list;
+    }
+
+  tree mval = TYPE_MAX_VALUE (TYPE_DOMAIN (type));
+
+  if (!mval)
+    {
+     (*spec) += '0';
+     return list;
+    }
+
+  if (TREE_CODE (mval) == COMPOUND_EXPR
+      && integer_zerop (TREE_OPERAND (mval, 0))
+      && integer_zerop (TREE_OPERAND (mval, 1)))
+    {
+      (*spec) += '*';
+      return list;
+    }
+
+  if (TREE_CODE (mval) == INTEGER_CST)
+    return list;
+
+  /* A variable bound.  */
+  (*spec) += '$';
+
+  mval = array_type_nelts_top (type);
+
+  /* Remove NOP_EXPR and SAVE_EXPR to uncover possible PARM_DECLS.  */
+  if (TREE_CODE (mval) == NOP_EXPR)
+    mval = TREE_OPERAND (mval, 0);
+   if (TREE_CODE (mval) == SAVE_EXPR)
+    {
+      mval = TREE_OPERAND (mval, 0);
+      if (TREE_CODE (mval) == NOP_EXPR)
+       mval = TREE_OPERAND (mval, 0);
+    }
+
+  return tree_cons (NULL_TREE, mval, list);
+}
+
 /* Extract attribute "arg spec" from each FNDECL argument that has it,
    build a single attribute access corresponding to all the arguments,
    and return the result.  SKIP_VOIDPTR set to ignore void* parameters
@@ -5812,15 +5878,16 @@ build_attr_access_from_parms (tree parms, bool 
skip_voidptr)
       argspec = TREE_VALUE (argspec);
 
       /* The attribute arg spec string.  */
-      tree str = TREE_VALUE (argspec);
-      const char *s = TREE_STRING_POINTER (str);
+      const char *s = TREE_STRING_POINTER (TREE_VALUE (argspec));
+      bool static_p = s && (0 == strcmp("static", s));
 
       /* Collect the list of nonnull arguments which use "[static ..]".  */
-      if (s != NULL && s[0] == '[' && s[1] == 's')
+      if (static_p)
        nnlist = tree_cons (NULL_TREE, build_int_cst (integer_type_node,
                                                      argpos + 1), nnlist);
 
-      /* Create the attribute access string from the arg spec string,
+      tree argvbs;
+      /* Create the attribute access string from the arg spec data,
         optionally followed by position of the VLA bound argument if
         it is one.  */
       {
@@ -5831,21 +5898,52 @@ build_attr_access_from_parms (tree parms, bool 
skip_voidptr)
            specend = 1;
          }
 
-       /* Format the access string in place.  */
-       int len = snprintf (NULL, 0, "%c%u%s",
-                           attr_access::mode_chars[access_deferred],
-                           argpos, s);
-       spec.resize (specend + len + 1);
-       sprintf (&spec[specend], "%c%u%s",
-                attr_access::mode_chars[access_deferred],
-                argpos, s);
+       spec += attr_access::mode_chars[access_deferred];
+       spec += std::to_string (argpos);
+       spec += '[';
+       tree type = TREE_VALUE (TREE_CHAIN (argspec));
+       argvbs = build_arg_spec (type, &spec);
+
+       /* Postprocess the string to bring it in the format expected
+          by the code handling the access attribute.  First, we
+          add 's' if the array was declared as [static ...].  */
+       if (static_p)
+         {
+           size_t send = spec.length();
+
+           if (spec[send - 1] == '[')
+             {
+               spec += 's';
+             }
+           else
+             {
+               /* If there is a symbol, we need to swap the order.  */
+               spec += spec[send - 1];
+               spec[send - 1] = 's';
+             }
+         }
+
+       /* If the outermost bound is an integer constant, we need to write
+          the size  if it is constant.  */
+       if (type && TYPE_DOMAIN (type))
+         {
+           tree mval = TYPE_MAX_VALUE (TYPE_DOMAIN (type));
+           if (mval && TREE_CODE (mval) == INTEGER_CST)
+             {
+               char buf[40];
+               unsigned HOST_WIDE_INT n = tree_to_uhwi (mval) + 1;
+               sprintf (buf, HOST_WIDE_INT_PRINT_UNSIGNED, n);
+               spec += buf;
+             }
+         }
+       spec += ']';
+
        /* Trim the trailing NUL.  */
-       spec.resize (specend + len);
+       spec.resize (spec.length ());
       }
 
       /* The (optional) list of expressions denoting the VLA bounds
         N in ARGTYPE <arg>[Ni]...[Nj]...[Nk].  */
-      tree argvbs = TREE_CHAIN (argspec);
       if (argvbs)
        {
          spec += ',';
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 4bef43864a1..7850365f35c 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -6208,184 +6208,7 @@ grokparm (const struct c_parm *parm, tree *expr)
   return decl;
 }
 
-/* Return attribute "arg spec" corresponding to an array/VLA parameter
-   described by PARM, concatenated onto attributes ATTRS.
-   The spec consists of one dollar symbol for each specified variable
-   bound, one asterisk for each unspecified variable bound, followed
-   by at most one specification of the most significant bound of
-   an ordinary array parameter.  For ordinary arrays the specification
-   is either the constant bound itself, or the space character for
-   an array with an unspecified bound (the [] form).  Finally, a chain
-   of specified variable bounds is appended to the spec, starting with
-   the most significant bound.  For example, the PARM T a[2][m][3][n]
-   will produce __attribute__((arg spec ("[$$2]", m, n)).
-   For T a typedef for an array with variable bounds, the bounds are
-   included in the specification in the expected order.
-   No "arg spec"  is created for parameters of pointer types, making
-   a distinction between T(*)[N] (or, equivalently, T[][N]) and
-   the T[M][N] form, all of which have the same type and are represented
-   the same, but only the last of which gets an "arg spec" describing
-   the most significant bound M.  */
 
-static tree
-get_parm_array_spec (const struct c_parm *parm, tree attrs)
-{
-  /* The attribute specification string, minor bound first.  */
-  std::string spec;
-
-  /* A list of VLA variable bounds, major first, or null if unspecified
-     or not a VLA.  */
-  tree vbchain = NULL_TREE;
-  /* True for a pointer parameter.  */
-  bool pointer = false;
-  /* True for an ordinary array with an unpecified bound.  */
-  bool nobound = false;
-
-  /* Create a string representation for the bounds of the array/VLA.  */
-  for (c_declarator *pd = parm->declarator, *next; pd; pd = next)
-    {
-      next = pd->declarator;
-      while (next && next->kind == cdk_attrs)
-       next = next->declarator;
-
-      /* Remember if a pointer has been seen to avoid storing the constant
-        bound.  */
-      if (pd->kind == cdk_pointer)
-       pointer = true;
-
-      if ((pd->kind == cdk_pointer || pd->kind == cdk_function)
-         && (!next || next->kind == cdk_id))
-       {
-         /* Do nothing for the common case of a pointer.  The fact that
-            the parameter is one can be deduced from the absence of
-            an arg spec for it.  */
-         return attrs;
-       }
-
-      if (pd->kind == cdk_id)
-       {
-         if (pointer
-             || !parm->specs->type
-             || TREE_CODE (parm->specs->type) != ARRAY_TYPE
-             || !TYPE_DOMAIN (parm->specs->type)
-             || !TYPE_MAX_VALUE (TYPE_DOMAIN (parm->specs->type)))
-           continue;
-
-         tree max = TYPE_MAX_VALUE (TYPE_DOMAIN (parm->specs->type));
-         if (!vbchain
-             && TREE_CODE (max) == INTEGER_CST)
-           {
-             /* Extract the upper bound from a parameter of an array type
-                unless the parameter is an ordinary array of unspecified
-                bound in which case a next iteration of the loop will
-                exit.  */
-             if (spec.empty () || spec.end ()[-1] != ' ')
-               {
-                 if (!tree_fits_shwi_p (max))
-                   continue;
-
-                 /* The upper bound is the value of the largest valid
-                    index.  */
-                 HOST_WIDE_INT n = tree_to_shwi (max) + 1;
-                 char buf[40];
-                 sprintf (buf, HOST_WIDE_INT_PRINT_UNSIGNED, n);
-                 spec += buf;
-               }
-             continue;
-           }
-
-         /* For a VLA typedef, create a list of its variable bounds and
-            append it in the expected order to VBCHAIN.  */
-         tree tpbnds = NULL_TREE;
-         for (tree type = parm->specs->type; TREE_CODE (type) == ARRAY_TYPE;
-              type = TREE_TYPE (type))
-           {
-             tree nelts_minus_one = array_type_nelts_minus_one (type);
-             if (error_operand_p (nelts_minus_one))
-               return attrs;
-             if (TREE_CODE (nelts_minus_one) != INTEGER_CST)
-               {
-                 /* Each variable VLA bound is represented by the dollar
-                    sign.  */
-                 spec += "$";
-                 tpbnds = tree_cons (NULL_TREE, nelts_minus_one, tpbnds);
-               }
-           }
-         tpbnds = nreverse (tpbnds);
-         vbchain = chainon (vbchain, tpbnds);
-         continue;
-       }
-
-      if (pd->kind != cdk_array)
-       continue;
-
-      if (pd->u.array.vla_unspec_p)
-       {
-         /* Each unspecified bound is represented by a star.  There
-            can be any number of these in a declaration (but none in
-            a definition).  */
-         spec += '*';
-         continue;
-       }
-
-      tree nelts = pd->u.array.dimen;
-      if (!nelts)
-       {
-         /* Ordinary array of unspecified size.  There can be at most
-            one for the most significant bound.  Exit on the next
-            iteration which determines whether or not PARM is declared
-            as a pointer or an array.  */
-         nobound = true;
-         continue;
-       }
-
-      if (pd->u.array.static_p)
-       spec += 's';
-
-      if (!INTEGRAL_TYPE_P (TREE_TYPE (nelts)))
-       /* Avoid invalid NELTS.  */
-       return attrs;
-
-      STRIP_NOPS (nelts);
-      nelts = c_fully_fold (nelts, false, nullptr);
-      if (TREE_CODE (nelts) == INTEGER_CST)
-       {
-         /* Skip all constant bounds except the most significant one.
-            The interior ones are included in the array type.  */
-         if (next && (next->kind == cdk_array || next->kind == cdk_pointer))
-           continue;
-
-         if (!tree_fits_uhwi_p (nelts))
-           /* Bail completely on invalid bounds.  */
-           return attrs;
-
-         char buf[40];
-         unsigned HOST_WIDE_INT n = tree_to_uhwi (nelts);
-         sprintf (buf, HOST_WIDE_INT_PRINT_UNSIGNED, n);
-         spec += buf;
-         break;
-       }
-
-      /* Each variable VLA bound is represented by a dollar sign.  */
-      spec += "$";
-      vbchain = tree_cons (NULL_TREE, nelts, vbchain);
-    }
-
-  if (spec.empty () && !nobound)
-    return attrs;
-
-  spec.insert (0, "[");
-  if (nobound)
-    /* Ordinary array of unspecified bound is represented by a space.
-       It must be last in the spec.  */
-    spec += ' ';
-  spec += ']';
-
-  tree acsstr = build_string (spec.length () + 1, spec.c_str ());
-  tree args = tree_cons (NULL_TREE, acsstr, vbchain);
-  tree name = get_identifier ("arg spec");
-  return tree_cons (name, args, attrs);
-}
 
 /* Given a parsed parameter declaration, decode it into a PARM_DECL
    and push that on the current scope.  EXPR is a pointer to an
@@ -6401,7 +6224,6 @@ push_parm_decl (const struct c_parm *parm, tree *expr)
   if (decl && DECL_P (decl))
     DECL_SOURCE_LOCATION (decl) = parm->loc;
 
-  attrs = get_parm_array_spec (parm, attrs);
   decl_attributes (&decl, attrs, 0);
 
   decl = pushdecl (decl);
@@ -6775,6 +6597,25 @@ add_decl_expr (location_t loc, tree type, tree *expr, 
bool set_name_p)
     }
 }
 
+
+/* Add attribute "arg spec" to ATTRS corresponding to an array/VLA parameter
+   declared with type TYPE.  The attribute has two arguments.  The first is
+   a string that encodes the presence of the static keyword.  The second is
+   the declared type of the array before adjustment, i.e. as an array type
+   including the outermost bound.  */
+
+static tree
+build_arg_spec_attribute (tree type, bool static_p, tree attrs)
+{
+  tree vbchain = tree_cons (NULL_TREE, type, NULL_TREE);
+  tree acsstr = static_p ? build_string (7, "static") :
+                          build_string (1, "");
+  tree args = tree_cons (NULL_TREE, acsstr, vbchain);
+  tree name = get_identifier ("arg spec");
+  return tree_cons (name, args, attrs);
+}
+
+
 /* Given declspecs and a declarator,
    determine the name and type of the object declared
    and construct a ..._DECL node for it.
@@ -6834,6 +6675,7 @@ grokdeclarator (const struct c_declarator *declarator,
   bool funcdef_flag = false;
   bool funcdef_syntax = false;
   bool size_varies = false;
+  bool size_error = false;
   tree decl_attr = declspecs->decl_attr;
   int array_ptr_quals = TYPE_UNQUALIFIED;
   tree array_ptr_attrs = NULL_TREE;
@@ -7326,6 +7168,7 @@ grokdeclarator (const struct c_declarator *declarator,
                                "size of unnamed array has non-integer type");
                    size = integer_one_node;
                    size_int_const = true;
+                   size_error = true;
                  }
                /* This can happen with enum forward declaration.  */
                else if (!COMPLETE_TYPE_P (TREE_TYPE (size)))
@@ -7338,6 +7181,7 @@ grokdeclarator (const struct c_declarator *declarator,
                                "type");
                    size = integer_one_node;
                    size_int_const = true;
+                   size_error = true;
                  }
 
                size = c_fully_fold (size, false, &size_maybe_const);
@@ -7363,6 +7207,7 @@ grokdeclarator (const struct c_declarator *declarator,
                          error_at (loc, "size of unnamed array is negative");
                        size = integer_one_node;
                        size_int_const = true;
+                       size_error = true;
                      }
                    /* Handle a size folded to an integer constant but
                       not an integer constant expression.  */
@@ -7978,6 +7823,10 @@ grokdeclarator (const struct c_declarator *declarator,
 
        if (TREE_CODE (type) == ARRAY_TYPE)
          {
+           if (!size_error)
+             *decl_attrs = build_arg_spec_attribute (type, array_parm_static,
+                                                     *decl_attrs);
+
            /* Transfer const-ness of array into that of type pointed to.  */
            type = TREE_TYPE (type);
            if (orig_qual_type != NULL_TREE)
diff --git a/gcc/testsuite/gcc.dg/Warray-parameter-11.c 
b/gcc/testsuite/gcc.dg/Warray-parameter-11.c
index 8ca1b55bd28..e05835c902e 100644
--- a/gcc/testsuite/gcc.dg/Warray-parameter-11.c
+++ b/gcc/testsuite/gcc.dg/Warray-parameter-11.c
@@ -9,7 +9,7 @@ typedef __INTPTR_TYPE__ intptr_t;
 void f0 (double[!copysign (~2, 3)]);
 
 void f1 (double[!copysign (~2, 3)]);
-void f1 (double[1]);                    // { dg-warning "-Warray-parameter" }
+void f1 (double[1]);                    // { dg-warning "-Wvla-parameter" }
 
 void f2 (int[(int)+1.0]);
 void f2 (int[(int)+1.1]);
@@ -21,4 +21,4 @@ extern struct S *sp;
 
 void f3 (int[(intptr_t)((char*)sp->a - (char*)sp)]);
 void f3 (int[(intptr_t)((char*)&sp->a[0] - (char*)sp)]);
-void f3 (int[(intptr_t)((char*)&sp->a[1] - (char*)sp)]);   // { dg-warning 
"-Warray-parameter" }
+void f3 (int[(intptr_t)((char*)&sp->a[1] - (char*)sp)]);   // { dg-warning 
"-Wvla-parameter" }
diff --git a/gcc/testsuite/gcc.dg/Warray-parameter.c 
b/gcc/testsuite/gcc.dg/Warray-parameter.c
index 6c5195a31be..31879a869fc 100644
--- a/gcc/testsuite/gcc.dg/Warray-parameter.c
+++ b/gcc/testsuite/gcc.dg/Warray-parameter.c
@@ -118,8 +118,7 @@ typedef int IA2[2];
 typedef int IA3[3];
 
 // The message should differentiate between the [] form and *.
-void f1IAx_A1 (IAx);            // { dg-message "previously declared as 
'int\\\[]'" "pr?????" { xfail *-*-* } }
-                                // { dg-message "previously declared as 'int 
*\\\*'" "note" { target *-*-* } .-1 }
+void f1IAx_A1 (IAx);            // { dg-message "previously declared as 
'int\\\[]'" }
 void f1IAx_A1 (IA1);            // { dg-message "argument 1 of type 
'int\\\[1]' with mismatched bound" }
 
 void f1IA1_A2 (IA1);            // { dg-message "previously declared as 
'int\\\[1]'" }

Reply via email to