Ping: https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01481.html
There was quite a bit of discussion between Joseph and me about
the Glibc changes needed to take advantage of the solution but
the GCC patch itself (above) still needs reviewing/approval.
Other than some (minor) changes to the C++ front end the bulk
of the changes for review are to the attribute machinery in
c-family and in the middle-end (attribs.c).
With the Glibc patch below applied, the GCC patch builds Glibc
with no warnings by default. To find the alias attribute
mismatches as requested, Glibc would enable -Wattribute-alias=2.
Once the GCC patch is committed I will also submit the Glibc
patch.
Glibc patch for reference:
https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01538.html
On 10/24/2018 02:38 PM, Martin Sebor wrote:
On 10/24/2018 11:27 AM, Joseph Myers wrote:
On Wed, 24 Oct 2018, Martin Sebor wrote:
On 10/24/2018 06:22 AM, Joseph Myers wrote:
On Wed, 24 Oct 2018, Martin Sebor wrote:
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)));
The obvious question there is whether the glibc patch should use copy
(local) as well as copy (name) in the definition of __hidden_ver1.
I tried copy(local) but it breaks where local isn't declared.
I think errno_location was the first one I saw where it referred
to __GI__something_or_other that was previously only defined via
an asm.
In that case maybe it should go in the .c files (the patch should define
some common attribute_copy macro in some internal header, to avoid
lots of
places needing to duplicate conditionals for whether it's supported).
I defined __attribute_copy__ in cdefs.h like other attributes
and used it in libc-symbols.h and in the string .c files but
it turns out that cdefs.h isn't always included when the
libc-symbols.h macros are used. For instance,
sysdeps/unix/sysv/linux/umount.c is compiled without it which
results in errors. So the way I worked around it pretty hacky.
Attached is what I have. I'm sure there's a better way that
you will want to adopt for Glibc but this works as a proof of
concept and results in no warnings by default. With
-Wattribute-alias=2 it gives the attached warnings for aliases
with more restrictive attributes than their targets (alloc_size,
const, leaf, malloc, nonnull, noreturn, nothrow, pure, and
returns_nonnull).
The GCC patch is the same so please let me know if there's
something to change there.
Whether nonnull attributes should be disabled when building glibc is a
separate question which would involve reviewing lots of functions with
such attributes against
<https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers>
to see whether there are checks for NULL that are actually appropriate
under current glibc conventions but might be optimized away given such
attributes. So it should clearly be kept separate from fixes to use copy
attributes to get better attribute consistency fot aliases.
Agreed. I certainly don't plan to undertake this project as
part of the GCC attribute enhancement.
Martin
