This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)
- From: Martin Sebor <msebor at gmail dot com>
- To: Jakub Jelinek <jakub at redhat dot com>, "Joseph S. Myers" <joseph at codesourcery dot com>, Marek Polacek <polacek at redhat dot com>, Jason Merrill <jason at redhat dot com>, Richard Biener <rguenther at suse dot de>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 21 Dec 2018 09:03:04 -0700
- Subject: Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)
- References: <6ff6565b-51d8-8620-f345-a0082747297b@gmail.com> <20181221100509.GY23305@tucnak>
On 12/21/18 3:05 AM, Jakub Jelinek wrote:
Hi!
I think the main question is whether we should accept leaf attribute
on weakrefs, despite them being marked as !TREE_PUBLIC.
I know we haven't allowed that until now, but weakrefs are weirdo things
which have both static and external effects, static for that they are a
local alias and external for being actually aliases to (usually) external
functions. If we add a weakref for some function declared as leaf,
it is unnecessarily pessimizing when we don't allow the leaf attribute on
the weakref.
Your patch looks reasonable to me to revert to previous state, but if we
decide to change the above, it would need to change.
Yes. I confess I don't fully understand the rationale for disallowing
attribute leaf on extern functions, or the implications of allowing it
again. It's accepted by GCC 4.1 but in r108074 it was made an error.
(GCC 4.1 also requires the declaration to be extern and gives an error
for static.) The description in the patch submission says it's because:
- requires that 'weakref' is attached only to a static object,
because that's all that the object file formats support; and
- makes it work on Darwin, or at least makes the testcases pass.
https://gcc.gnu.org/ml/gcc-patches/2005-12/msg00375.html
The text added to the manual makes it sounds like it was thought to be
a limitation that could be removed in the future:
At present, a declaration to which @code{weakref} is attached can
only be @code{static}.
I leave it up to you and others to decide if it's possible to accept
it on externs again.
That said, I'm also not sure the warning is necessarily the best way
to deal with the attribute mismatches in these cases (declarations
of aliases in .c files). Wouldn't it make more sense to copy
the attributes from targets to their aliases unconditionally?
Joseph, any thoughts based on your experience with the warning (and
attribute copy) in Glibc?
On Thu, Dec 20, 2018 at 08:45:03PM -0700, Martin Sebor wrote:
--- gcc/c-family/c-attribs.c (revision 267282)
+++ gcc/c-family/c-attribs.c (working copy)
@@ -2455,6 +2455,12 @@ handle_copy_attribute (tree *node, tree name, tree
|| is_attribute_p ("weakref", atname))
continue;
+ /* Aattribute leaf only applies to extern functions.
+ Avoid copying it to static ones. */
s/Aatribute/Attribute/
Fixed, thanks.
Martin