This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] add support for GCC 9 attribute copy


On 11/12/2018 04:10 PM, Joseph Myers wrote:
On Mon, 12 Nov 2018, Martin Sebor wrote:

Could you look at the powerpc soft-float, s390x and mips failures, which
look somewhat different (and thus could indicate issues with the details
of how this warning / attribute are specified, as opposed to simply
needing a few more copy attributes somewhere)?

Do you by chance have an easy way to get the preprocessed sources
for these files so I can easily verify that my quick test cases
match the real problems?

Attached.

Thanks!


Based on the warning alone I think the test case below shows
the problem:

  __thread int i __attribute__ ((tls_model ("local-exec")));
  extern __attribute__ ((alias ("i"), copy (i))) int j;

tls_model is one of the few attributes I didn't test.  The warning
suggests the copy attribute code might need to filter out some more
attributes.  Let me work on that.

Or maybe some variable of libc_hidden_data_def is needed that uses
__thread when defining aliases, in which case tls_model attributes might
not be considered to be ignored?  (The declarations in soft-supp.h use
libc_hidden_tls_proto which does use __thread when defining aliases.)

Attached is a small test case reduced from the TU.

Attribute copy calls decl_attributes to copy tls_model from
__sim_exceptions_thread to __EI___sim_exceptions_thread.  That
triggers a warning because
DECL_THREAD_LOCAL_P (__EI___sim_exceptions_thread) is false.
I think __EI___sim_exceptions_thread needs to be declared
thread-local (just like __sim_exceptions_thread) for this
to work.  Can you think of something better?

Does it look close to what's going on in the file?  If so, does
it make sense to define an alias target inline/always_inline?
(I can filter attribute always_inline out of copy but I wonder
if changing the target to avoid always_inline would be more
appropriate.)

I can imagine there might be uses for having some aliases that should be
inlined and others that shouldn't be (provided there is actually an
external definition for the function - if it's GNU extern inline with no
separate definition provided, aliases aren't exactly useful).

Okay, let me just exclude always_inline and gnu_inline from
the attributes to copy and not worry about this as a potential
problem for now.

So it sounds like the mips16 attribute is meaningful on definitions
but doesn't impact callers, correct?  I think it would be useful to

Strictly speaking I think it *can* impact callers by telling them whether
they might need to generate code compatible with interlinking mips16 and
non-mips16 for that call (in the absence of the attribute, the caller
needs to make safe assumptions about not knowing what instruction set the
function is built for).

The purpose of the alias, however, is definitely to avoid problems with
declarations of __longjmp being inconsistent as to whether they have the
attribute; see
<https://sourceware.org/ml/libc-ports/2013-01/msg00047.html>.

Hmm.  I confess I'm confused by this and by the referenced
discussions.  Most of it sounds like the alias in this case
suppresses a valid error: the mismatch between the caller and
the callee.  But then the conclusion seems to be that
declaring a function one way and defining it another is (or
can be, perhaps with some care) okay.  So either issuing
an error for the mismatch between declarations of the same
function is overly strict (in which case a warning for it
should be appropriate, as would be the attribute warning) or
the error does makes sense in which case so should the attribute
warning.

I'm open to filtering out these two attributes since it doesn't
seem that important one way or the other but unless I'm even
more confused than I think I am I would prefer to keep things
as they are and change Glibc to suppress the warning.  Partly
because of the rationale I just gave and in part also because
I don't want to hardcode a target attribute into c-attribs.c
and I'm not convinced yet we need to introduce some sort of
a hook for this (though ultimately we might end up going that
route).

Let me know if you see a problem with suppressing the warning
in Glibc.

Martin

Attachment: sim-full.c
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]