[PATCH i-g-t] lib: Inline igt_x86_features() into ifunc resolvers

[Zbigniew Kempczyński]

On Wed, Mar 20, 2024 at 11:09:11PM -0400, Matt Turner wrote:
> On Wed, Mar 13, 2024 at 5:09 AM Zbigniew Kempczyński
> <zbigniew.kempczynski at intel.com> wrote:
> >
> > On Mon, Mar 04, 2024 at 05:16:40PM -0500, Matt Turner wrote:
> > > Quoting https://sourceware.org/glibc/wiki/GNU_IFUNC
> > >
> > > > When LD_BIND_NOW=1 or -Wl,z,now is in effect symbols must be
> > > > immediately resolved at startup. In cases where an external function
> > > > call depends needs to be made that may fail if such a call has not
> > > > been initialized yet (PLT-based relocation which is processed later).
> > > > For example calling strlen in an IFUNC resolver built with -Wl,z,now
> > > > may lead to a segfault because the PLT is not yet resolved.
> > >
> > > We cannot rely on function calls through the PLT in ifunc resolvers as
> > > the PLT may not have been initialized yet.
> > >
> > > In practice, this causes crashes when igt is linked with -Wl,-z,now or
> > > when linked with the mold linker.
> > >
> > > To avoid this problem, we do two things:
> > >     1. move igt_x86_features() to igt_x86.h so its definition is
> > >        available to compilation units that call the function.
> > >     2. mark the ifunc resolvers with __attribute__((flatten)) to ensure
> > >        igt_x86_features() is inlined.
> > >
> > > Bug: https://bugs.gentoo.org/788625
> > > Bug: https://bugs.gentoo.org/925348
> > > Signed-off-by: Matt Turner <mattst88 at gmail.com>
> >
> > Hi.
> >
> > I started review of your code, but this this touches some linking intrisics
> > I'm not familiar with yet so I need more time to explore this. I hope
> > this is not a problem for you.
> 
> Do you have any questions I can answer? I tried my best to explain the
> how and the why of the change in the commit message.

I wanted to prepare better for the discussion - I extracted few
functions on which I'm reproducing segv issue when LD_BIND_NOW=1 is set.
I confirm, GOT doesn't contain valid function addresses so jump goes
to nowhere. I'm pretty sure intel_vbd_decode with LD_BIND_NOW=1 only
reveals the problem, but it lays within linker logic:

# LD_BIND_NOW=1 LD_PRELOAD=./build/lib/libigt.so.0 LD_DEBUG=all ps

ps definitely doesn't call igt_half_to_float() which in turn calls
unresolved igt_x86_features at plt. It looks linker first tries to
call all ifunc's to establish addresses, unfortunately it is doing
this before filling GOT. Backtrace shows we're still in _start
from /lib64/ld-linux-x86-64.so.2 so we even didn't get a chance
to get to the main().

Before I'll give you my ack (I still want to experiment with this
one more day) - may you explain what for you need to bind relocs
immediately? This is slower than lazy binding as it resolves all
PLTs instead resolving only required symbols.

I'm sorry this review takes so much time. Due to work duties other
tasks have higher prio.

--
Zbigniew