On 10/23/2018 03:53 PM, Joseph Myers wrote:
> On Mon, 22 Oct 2018, Martin Sebor wrote:
>
>> 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
>
> What do the macro expansions look like there?  All the places where you're
> adding "copy" attributes are for extern declarations, not static ones,
> whereas your list of warnings seems to indicate this is appearing for
> ifunc resolvers (which are static, but should not be copying attributes
> from anywhere).

These must have been caused by the bug in the patch (below).
They have cleared up with it fixed.  I'm down to just 18
instances of a -Wmissing-attributes warning, all for string
functions.  The cause of those is described below.

>
>> 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’
>
> That looks like a bug in the GCC patch to me; you appear to be adding copy
> attributes in the correct place.  Note that __EI_* gets declared twice
> (first with __asm__, second with an alias attribute), so anything related
> to handling of such duplicate declarations might be a cause for such a
> bug (and an indication of what you need to add a test for when fixing such
> a bug).

There was a bug in the patch, but there is also an issue in Glibc
that made it tricky to see the problem.

The tests I had in place were too simple to catch the GCC bug:
the problem there was that when the decl didn't have an attribute
the type of the "template" did the check would fail without also
considering the decl's type.  Tricky stuff!  I've added tests to
exercise this.

The Glibc issue has to do with the use of __hidden_ver1 macro
to declare string functions.  sysdeps/x86_64/multiarch/strcmp.c
for instance has:

   __hidden_ver1 (strcmp, __GI_strcmp, __redirect_strcmp)
     __attribute__ ((visibility ("hidden")));

and __redirect_strcmp is missing the nonnull attribute because
it's #undefined in include/sys/cdefs.h.  An example of one of
these warnings is attached.

Using strcmp instead of __redirect_strcmp would solve this but
__redirect_strcmp should have all the same attributes as strcmp.
But nonnull is removed from the declaration because the __nonnull
macro that controls it is undefined in include/sys/cdefs.h.  There
is a comment above the #undef in the header that reads:

/* The compiler will optimize based on the knowledge the parameter is
    not NULL.  This will omit tests.  A robust implementation cannot allow
    this so when compiling glibc itself we ignore this attribute.  */
# undef __nonnull
# define __nonnull(params)

I don't think this is actually true for recent versions of GCC.
The nonnull optimization is controlled by
-fisolate-erroneous-paths-attribute and according to the manual
and common.opt the option is disabled by default.

But if you do want to avoid the attribute on declarations of
these functions regardless it should be safe to add it after
the declaration in the .c file, like so:

__hidden_ver1 (strcmp, __GI_strcmp, __redirect_strcmp)
   __attribute__ ((visibility ("hidden"), copy (strcmp)));

That should make it straightforward to adopt the enhancement
and experiment with -Wattribute-alias=2 to see if it does what
you had  in mind.

The latest GCC patch with the fix mentioned above is attached.

Martin