| 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) | expand |
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 >
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
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
--- 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)));