On 10/01/2018 05:00 PM, Joseph Myers wrote:
> On Mon, 1 Oct 2018, Martin Sebor wrote:
>
>> Testing the patch with Glibc triggers thousands of warnings of
>> both kinds.  After reviewing a small subset it became apparent
>
> Thousands of warnings suggests initially having the warning outside -Wall
> (though one might hope to move it into -Wall at some point, depending on
> how hard the warnings are to address and to what extent they appear at all
> for other packages - most don't make heavy use of aliases like that - or
> failing that, to enable it explicitly for glibc once all the warnings are
> fixed, since this is certainly a useful warning for glibc showing issues
> we want to fix) - it's not like the typical case of a new warning where
> you can quickly and easily fix all the instances in glibc, for all
> architectures, to keep it building with mainline GCC.

I have improved the Glibc patch to avoid many of the warnings.
To avoid most of the rest I have adjusted the GCC patch to make
-Wattribute-alias a two-level warning, and to disregard mismatches
between aliases and ifunc resolvers.  With -Wattribute-alias=1
that reduced the number of unique instances of the warnings for
a Glibc build to just 27.  Of those, all but one of
the -Wattributes instances are of the form:

   warning: ‘leaf’ attribute has no effect on unit local functions

All the -Wmissing-attributes instances are due to a missing
nonnull attribute on the __EI__ kinds of functions, like:

   warning: ‘__EI_vfprintf’ specifies less restrictive attribute than 
its target ‘vfprintf’: ‘nonnull’

I think both sets might be caused either by a bug/missing handling
in my Glibc patch, or by a bug in the GCC patch, but I'm a little
lost in the maze of Glibc ifunc macro to tell which just yet.
I can keep looking into it but it would make it easier if you
could apply it and see if the Glibc macros need tweaking.

Enabling -Wattribute-alias=2 shows the remaining attribute
mismatches (those highlighting "potential bugs," although
I suspect few, if any, are real bugs; more than likely they
are benign.)

>
>> attribute called copy.  The attribute copies attributes from
>> one declaration (or type) to another.  The attribute doesn't
>> resolve all the warnings but it helps.
>
> (For actual use in glibc that use would of course need to be conditional
> on a GCC version supporting the attribute.)

Sure.  The patch is just to show what I did to get the warnings.
(Attached is an update with the changes I mentioned above and
the Glibc warning breakdown with it.)

>
>> The class of warnings I noticed that can't be so easily handled
>> are due to inconsistencies between ifuncs and their resolvers.
>> One way to solve it might be to have resolvers automatically
>> "inherit" all the attributes of their targets (and enhance
>> GCC to warn for violations).  Another way might be to expect
>> resolvers to be explicitly declared with attribute copy to copy
>> the attributes of all the targets (and also warn for violations).
>
> I'm not sure we should care about the attributes on IFUNC resolvers at
> all; no normal code will see a declaration of the resolver and also call
> the function, whereas lots of code calls functions under internal alias
> names that currently lack the same attributes as the public declaration
> has.  It's also not obvious whether there might be more cases of
> attributes for a function that are inapplicable to IFUNC resolvers than
> just the attributes relating to a symbol rather than the function itself
> which are hardcoded as excluded.

I agree that checking attributes on ifunc resolvers probably doesn't
make much sense.  It would be helpful to check their targets against
the aliases, but that's a whole other project.

Attached is the latest update along with the Glibc warning breakdown.
Tested on x86_64-linux.

Martin