[C/C++] Fix up build of GCC 4.6 and earlier with GCC 9+ (PR c/90677)
diff mbox series

Message ID 20191119232943.GN4650@tucnak
State New
Headers show
Series
  • [C/C++] Fix up build of GCC 4.6 and earlier with GCC 9+ (PR c/90677)
Related show

Commit Message

Jakub Jelinek Nov. 19, 2019, 11:29 p.m. UTC
Hi!

The following patch fixes build of older GCC releases with newer ones.
In GCC 4.6 and earlier, we had:
struct cgraph_node;
struct cgraph_node *cgraph_node (tree);
and that is something on which GCC 9+ code errors on if it sees any
__gcc_diag__ and similar attributes, because cgraph_node it wants to find is
not a type.

As older GCC releases don't have the __gcc_diag__ etc. attributes guarded on
no newer GCC releases, only on minimum GCC version that does support it,
I think we need to make sure we don't reject what older GCCs used to have.

The following patch does that.  In addition, get_pointer_to_named_type
looked misnamed, because we actually aren't interested in getting gimple *
etc. type, but gimple.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
and eventually 9 too?

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

	PR c/90677
	* c-format.c (get_pointer_to_named_type): Renamed to ...
	(get_named_type): ... this.  If result is FUNCTION_DECL for
	cgraph_node, return cgraph_node from pointee of return type if
	possible instead of emitting an error.
	(init_dynamic_diag_info): Adjust get_pointer_to_named_type callers
	to call get_named_type instead.  Formatting fixes.

	* c-c++-common/pr90677.c: New test.


	Jakub

Comments

Richard Biener Nov. 20, 2019, 10:25 a.m. UTC | #1
On Wed, Nov 20, 2019 at 12:30 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> The following patch fixes build of older GCC releases with newer ones.
> In GCC 4.6 and earlier, we had:
> struct cgraph_node;
> struct cgraph_node *cgraph_node (tree);
> and that is something on which GCC 9+ code errors on if it sees any
> __gcc_diag__ and similar attributes, because cgraph_node it wants to find is
> not a type.
>
> As older GCC releases don't have the __gcc_diag__ etc. attributes guarded on
> no newer GCC releases, only on minimum GCC version that does support it,
> I think we need to make sure we don't reject what older GCCs used to have.
>
> The following patch does that.  In addition, get_pointer_to_named_type
> looked misnamed, because we actually aren't interested in getting gimple *
> etc. type, but gimple.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
> and eventually 9 too?

Nothing on the specific patch but I wonder if we should change the
error ()s in init_dynamic_diag_info to warnings to make stage1 uses
fine?  That probably then boils down to ignoring __gcc_diag__ formats
when initializing wasn't suceessful.

It also looks like we repeatedly do work in init_dynamic_diag_info ...

Richard.

> 2019-11-19  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c/90677
>         * c-format.c (get_pointer_to_named_type): Renamed to ...
>         (get_named_type): ... this.  If result is FUNCTION_DECL for
>         cgraph_node, return cgraph_node from pointee of return type if
>         possible instead of emitting an error.
>         (init_dynamic_diag_info): Adjust get_pointer_to_named_type callers
>         to call get_named_type instead.  Formatting fixes.
>
>         * c-c++-common/pr90677.c: New test.
>
> --- gcc/c-family/c-format.c.jj  2019-10-05 09:35:12.917997709 +0200
> +++ gcc/c-family/c-format.c     2019-11-19 13:05:10.113308578 +0100
> @@ -4899,21 +4899,39 @@ init_dynamic_gfc_info (void)
>      }
>  }
>
> -/* Lookup the type named NAME and return a pointer-to-NAME type if found.
> -   Otherwise, return void_type_node if NAME has not been used yet, or NULL_TREE if
> -   NAME is not a type (issuing an error).  */
> +/* Lookup the type named NAME and return a NAME type if found.
> +   Otherwise, return void_type_node if NAME has not been used yet,
> +   or NULL_TREE if NAME is not a type (issuing an error).  */
>
>  static tree
> -get_pointer_to_named_type (const char *name)
> +get_named_type (const char *name)
>  {
>    tree result;
> -  if ((result = maybe_get_identifier (name)))
> +  if (tree id = maybe_get_identifier (name))
>      {
> -      result = identifier_global_value (result);
> +      result = identifier_global_value (id);
>        if (result)
>         {
>           if (TREE_CODE (result) != TYPE_DECL)
>             {
> +             if (TREE_CODE (result) == FUNCTION_DECL
> +                 && !strcmp (name, "cgraph_node")
> +                 && TREE_CODE (TREE_TYPE (result)) == FUNCTION_TYPE)
> +               {
> +                 /* Before GCC 4.7, there used to be
> +                    struct cgraph_node;
> +                    struct cgraph_node *cgraph_node (tree);
> +                    Don't error in this case, so that GCC 9+ can still
> +                    compile GCC 4.6 and earlier.  */
> +                 tree res = TREE_TYPE (TREE_TYPE (result));
> +                 if (TREE_CODE (res) == POINTER_TYPE
> +                     && (TYPE_NAME (TREE_TYPE (res)) == id
> +                         || (TREE_CODE (TYPE_NAME (TREE_TYPE (res)))
> +                             == TYPE_DECL
> +                             && (DECL_NAME (TYPE_NAME (TREE_TYPE (res)))
> +                                 == id))))
> +                   return TREE_TYPE (res);
> +               }
>               error ("%qs is not defined as a type", name);
>               result = NULL_TREE;
>             }
> @@ -4953,23 +4971,24 @@ init_dynamic_diag_info (void)
>          an extra type level.  */
>        if ((local_tree_type_node = maybe_get_identifier ("tree")))
>         {
> -         local_tree_type_node = identifier_global_value (local_tree_type_node);
> +         local_tree_type_node
> +           = identifier_global_value (local_tree_type_node);
>           if (local_tree_type_node)
>             {
>               if (TREE_CODE (local_tree_type_node) != TYPE_DECL)
>                 {
>                   error ("%<tree%> is not defined as a type");
> -                 local_tree_type_node = 0;
> +                 local_tree_type_node = NULL_TREE;
>                 }
>               else if (TREE_CODE (TREE_TYPE (local_tree_type_node))
>                        != POINTER_TYPE)
>                 {
>                   error ("%<tree%> is not defined as a pointer type");
> -                 local_tree_type_node = 0;
> +                 local_tree_type_node = NULL_TREE;
>                 }
>               else
> -               local_tree_type_node =
> -                 TREE_TYPE (TREE_TYPE (local_tree_type_node));
> +               local_tree_type_node
> +                 = TREE_TYPE (TREE_TYPE (local_tree_type_node));
>             }
>         }
>        else
> @@ -4979,12 +4998,12 @@ init_dynamic_diag_info (void)
>    /* Similar to the above but for gimple*.  */
>    if (!local_gimple_ptr_node
>        || local_gimple_ptr_node == void_type_node)
> -    local_gimple_ptr_node = get_pointer_to_named_type ("gimple");
> +    local_gimple_ptr_node = get_named_type ("gimple");
>
>    /* Similar to the above but for cgraph_node*.  */
>    if (!local_cgraph_node_ptr_node
>        || local_cgraph_node_ptr_node == void_type_node)
> -    local_cgraph_node_ptr_node = get_pointer_to_named_type ("cgraph_node");
> +    local_cgraph_node_ptr_node = get_named_type ("cgraph_node");
>
>    static tree hwi;
>
> --- gcc/testsuite/c-c++-common/pr90677.c.jj     2019-11-19 13:02:45.709465310 +0100
> +++ gcc/testsuite/c-c++-common/pr90677.c        2019-11-19 13:03:58.925371802 +0100
> @@ -0,0 +1,11 @@
> +/* PR c/90677 */
> +/* { dg-do compile } */
> +/* { dg-options "-W -Wall" } */
> +
> +struct cgraph_node;
> +union tree_node;
> +typedef union tree_node *tree;
> +union gimple_statement_d;
> +typedef union gimple_statement_d *gimple;
> +struct cgraph_node *cgraph_node (tree);
> +void foo (int, const char *, ...) __attribute__((__format__(__gcc_diag__, 2, 3)));
>
>         Jakub
>
Jakub Jelinek Nov. 20, 2019, 10:34 a.m. UTC | #2
On Wed, Nov 20, 2019 at 11:25:46AM +0100, Richard Biener wrote:
> Nothing on the specific patch but I wonder if we should change the
> error ()s in init_dynamic_diag_info to warnings to make stage1 uses
> fine?  That probably then boils down to ignoring __gcc_diag__ formats
> when initializing wasn't suceessful.

Turning the errors into warnings is fine with me, but I think it still
doesn't hurt to handle the common case of cgraph_node being a function
the way the patch does, without any warning, as otherwise it would warn on
every single GCC file.
There is another thing, gimple, in the past we had typedef union ... *gimple;
and now we have struct gimple instead.  Though, both %G and %C that use
gimple * or cgraph_node * aren't going to appear in GCC 4.6 or earlier
anyway, so as long as we don't error (or ideally warn) all the time
on that, we just don't care.

> It also looks like we repeatedly do work in init_dynamic_diag_info ...

Yes, but only as long as the types aren't found yet.  I bet it is for the
case that there is
prototype with __gcc_diag__
struct cgraph_node;
or similar ordering, but that is just a guess.

	Jakub
Jason Merrill Nov. 20, 2019, 7:01 p.m. UTC | #3
On 11/19/19 11:29 PM, Jakub Jelinek wrote:
> Hi!
> 
> The following patch fixes build of older GCC releases with newer ones.
> In GCC 4.6 and earlier, we had:
> struct cgraph_node;
> struct cgraph_node *cgraph_node (tree);
> and that is something on which GCC 9+ code errors on if it sees any
> __gcc_diag__ and similar attributes, because cgraph_node it wants to find is
> not a type.
> 
> As older GCC releases don't have the __gcc_diag__ etc. attributes guarded on
> no newer GCC releases, only on minimum GCC version that does support it,
> I think we need to make sure we don't reject what older GCCs used to have.
> 
> The following patch does that.  In addition, get_pointer_to_named_type
> looked misnamed, because we actually aren't interested in getting gimple *
> etc. type, but gimple.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
> and eventually 9 too?
> 
> 2019-11-19  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/90677
> 	* c-format.c (get_pointer_to_named_type): Renamed to ...
> 	(get_named_type): ... this.  If result is FUNCTION_DECL for
> 	cgraph_node, return cgraph_node from pointee of return type if
> 	possible instead of emitting an error.
> 	(init_dynamic_diag_info): Adjust get_pointer_to_named_type callers
> 	to call get_named_type instead.  Formatting fixes.
> 
> 	* c-c++-common/pr90677.c: New test.
> 
> --- gcc/c-family/c-format.c.jj	2019-10-05 09:35:12.917997709 +0200
> +++ gcc/c-family/c-format.c	2019-11-19 13:05:10.113308578 +0100
> @@ -4899,21 +4899,39 @@ init_dynamic_gfc_info (void)
>       }
>   }
>   
> -/* Lookup the type named NAME and return a pointer-to-NAME type if found.
> -   Otherwise, return void_type_node if NAME has not been used yet, or NULL_TREE if
> -   NAME is not a type (issuing an error).  */
> +/* Lookup the type named NAME and return a NAME type if found.
> +   Otherwise, return void_type_node if NAME has not been used yet,
> +   or NULL_TREE if NAME is not a type (issuing an error).  */
>   
>   static tree
> -get_pointer_to_named_type (const char *name)
> +get_named_type (const char *name)
>   {
>     tree result;
> -  if ((result = maybe_get_identifier (name)))
> +  if (tree id = maybe_get_identifier (name))
>       {
> -      result = identifier_global_value (result);
> +      result = identifier_global_value (id);

I would think that get_named_type should find struct or enum names that 
have been hidden by another declaration; that would fix this without 
special-casing cgraph_node.  For the C++ front-end, that would be

lookup_qualified_name (global_namespace, id, /*prefer_type*/2, 
/*complain*/false)

Jason

Patch
diff mbox series

--- gcc/c-family/c-format.c.jj	2019-10-05 09:35:12.917997709 +0200
+++ gcc/c-family/c-format.c	2019-11-19 13:05:10.113308578 +0100
@@ -4899,21 +4899,39 @@  init_dynamic_gfc_info (void)
     }
 }
 
-/* Lookup the type named NAME and return a pointer-to-NAME type if found.
-   Otherwise, return void_type_node if NAME has not been used yet, or NULL_TREE if
-   NAME is not a type (issuing an error).  */
+/* Lookup the type named NAME and return a NAME type if found.
+   Otherwise, return void_type_node if NAME has not been used yet,
+   or NULL_TREE if NAME is not a type (issuing an error).  */
 
 static tree
-get_pointer_to_named_type (const char *name)
+get_named_type (const char *name)
 {
   tree result;
-  if ((result = maybe_get_identifier (name)))
+  if (tree id = maybe_get_identifier (name))
     {
-      result = identifier_global_value (result);
+      result = identifier_global_value (id);
       if (result)
 	{
 	  if (TREE_CODE (result) != TYPE_DECL)
 	    {
+	      if (TREE_CODE (result) == FUNCTION_DECL
+		  && !strcmp (name, "cgraph_node")
+		  && TREE_CODE (TREE_TYPE (result)) == FUNCTION_TYPE)
+		{
+		  /* Before GCC 4.7, there used to be
+		     struct cgraph_node;
+		     struct cgraph_node *cgraph_node (tree);
+		     Don't error in this case, so that GCC 9+ can still
+		     compile GCC 4.6 and earlier.  */
+		  tree res = TREE_TYPE (TREE_TYPE (result));
+		  if (TREE_CODE (res) == POINTER_TYPE
+		      && (TYPE_NAME (TREE_TYPE (res)) == id
+			  || (TREE_CODE (TYPE_NAME (TREE_TYPE (res)))
+			      == TYPE_DECL
+			      && (DECL_NAME (TYPE_NAME (TREE_TYPE (res)))
+				  == id))))
+		    return TREE_TYPE (res);
+		}
 	      error ("%qs is not defined as a type", name);
 	      result = NULL_TREE;
 	    }
@@ -4953,23 +4971,24 @@  init_dynamic_diag_info (void)
 	 an extra type level.  */
       if ((local_tree_type_node = maybe_get_identifier ("tree")))
 	{
-	  local_tree_type_node = identifier_global_value (local_tree_type_node);
+	  local_tree_type_node
+	    = identifier_global_value (local_tree_type_node);
 	  if (local_tree_type_node)
 	    {
 	      if (TREE_CODE (local_tree_type_node) != TYPE_DECL)
 		{
 		  error ("%<tree%> is not defined as a type");
-		  local_tree_type_node = 0;
+		  local_tree_type_node = NULL_TREE;
 		}
 	      else if (TREE_CODE (TREE_TYPE (local_tree_type_node))
 		       != POINTER_TYPE)
 		{
 		  error ("%<tree%> is not defined as a pointer type");
-		  local_tree_type_node = 0;
+		  local_tree_type_node = NULL_TREE;
 		}
 	      else
-		local_tree_type_node =
-		  TREE_TYPE (TREE_TYPE (local_tree_type_node));
+		local_tree_type_node
+		  = TREE_TYPE (TREE_TYPE (local_tree_type_node));
 	    }
 	}
       else
@@ -4979,12 +4998,12 @@  init_dynamic_diag_info (void)
   /* Similar to the above but for gimple*.  */
   if (!local_gimple_ptr_node
       || local_gimple_ptr_node == void_type_node)
-    local_gimple_ptr_node = get_pointer_to_named_type ("gimple");
+    local_gimple_ptr_node = get_named_type ("gimple");
 
   /* Similar to the above but for cgraph_node*.  */
   if (!local_cgraph_node_ptr_node
       || local_cgraph_node_ptr_node == void_type_node)
-    local_cgraph_node_ptr_node = get_pointer_to_named_type ("cgraph_node");
+    local_cgraph_node_ptr_node = get_named_type ("cgraph_node");
 
   static tree hwi;
 
--- gcc/testsuite/c-c++-common/pr90677.c.jj	2019-11-19 13:02:45.709465310 +0100
+++ gcc/testsuite/c-c++-common/pr90677.c	2019-11-19 13:03:58.925371802 +0100
@@ -0,0 +1,11 @@ 
+/* PR c/90677 */
+/* { dg-do compile } */
+/* { dg-options "-W -Wall" } */
+
+struct cgraph_node;
+union tree_node;
+typedef union tree_node *tree;
+union gimple_statement_d;
+typedef union gimple_statement_d *gimple;
+struct cgraph_node *cgraph_node (tree);
+void foo (int, const char *, ...) __attribute__((__format__(__gcc_diag__, 2, 3)));