public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: "Carlos O'Donell" <carlos@redhat.com>
Cc: Joseph Myers <joseph@codesourcery.com>,
	GNU C Library <libc-alpha@sourceware.org>
Subject: Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]
Date: Wed, 12 Oct 2016 22:19:00 -0000	[thread overview]
Message-ID: <CAMe9rOr=LKK6XVHGh2uehYgkRhLmEXMcEsdP25zd0ny8GPGfBA@mail.gmail.com> (raw)
In-Reply-To: <bc2016fe-c194-582b-dc80-a6392d51aa17@redhat.com>

On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>
>>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
>>
>> I changed it to use __builtin_memset.
>>
>>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>>
>>> You could make the test pass on either of those results (while failing if
>>> ld.so crashes).
>>>
>>
>> I moved the test to elf.  It passes if the test runs or ld.so issues an
>> error.  Please try it on arm, powerpc and s390.
>
> This is the wrong way to test this.
>
> The point of this test is this:
>
> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
>   on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
>   DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
>   resolver is called, because DSO B's resolver might need global data to make
>   the IFUNC decision e.g. GOT setup.
>
> The invariant we want to hold true for IFUNC is that to call the resolver
> function you must have relocated the DSO which contains the resolver. This _should_
> have been done by a symbol reocation dependency analysis, but that isn't working
> correctly IMO or needs deeper analysis in the dynamic loader.
>
> The solution we want in place today is to issue some kind of diagnostic until we
> fix the real problem.
>
> The test should look like this:
>
> - DSO A with an unversioned symbol reference to 'foo'.
> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
>   resolver function which references global data from DSO C to decide which of
>   two functions to return.
> - DSO C with global data set to a value.
>
> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
> relocated first, then B, such that B's GOT is setup to access C's global data.
>
> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
> get the error about needing to relink DSO A so it depends on DSO B, to form
> the initialization order of C->B->A.
>
> I expect this test case will now crash the other arches, rather than just
> avoiding the crash by relying on internal libc.so details about which ifuncs
> you're using.
>
> This is one step towards a better definition of IFUNC semantics, which need to
> be more clearly defined (something I wish I had time to define and fix so
> more projects could use them).

IFUNC resolver can fail for various reasons.  My goal is to make sure
that IFUNC inside of glibc works correctly or an error message is given
when glibc isn't used properly.  In case of x86,  CPU feature info is
retrieved and stored in ld.so very early at startup, which is used by IFUNC
and only accessible in libc.so and libm.so after they have been relocated.
My change in x86 ld.so checks it and my test verifies the check.  My fix
won't cover other possible IFUNC failures.

-- 
H.J.

  reply	other threads:[~2016-10-12 22:19 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-04 18:46 H.J. Lu
2016-10-04 21:27 ` Joseph Myers
2016-10-04 22:21   ` H.J. Lu
2016-10-04 22:55     ` Joseph Myers
2016-10-05  0:05       ` H.J. Lu
2016-10-05  0:10         ` Joseph Myers
2016-10-05 18:16           ` H.J. Lu
2016-10-06 21:28             ` Adhemerval Zanella
2016-10-12  5:45             ` Carlos O'Donell
2016-10-12 22:19               ` H.J. Lu [this message]
2017-01-13 19:03                 ` H.J. Lu
2017-01-13 19:30                   ` Khem Raj
2017-01-13 20:16                     ` H.J. Lu
2017-01-19 18:43                       ` Khem Raj
2017-01-20 17:00                         ` H.J. Lu
2017-01-20 18:36                           ` Khem Raj
2017-01-20 19:02                             ` H.J. Lu
2017-01-20 20:41                               ` Khem Raj

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMe9rOr=LKK6XVHGh2uehYgkRhLmEXMcEsdP25zd0ny8GPGfBA@mail.gmail.com' \
    --to=hjl.tools@gmail.com \
    --cc=carlos@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).