Re: [musl] [RFC v2 0/2] xtensa FDPIC port

[Max Filippov <jcmvbkbc@gmail.com>]

On Wed, Apr 3, 2024 at 2:45 PM Rich Felker <dalias@libc.org> wrote:
>
> On Wed, Apr 03, 2024 at 04:55:56PM -0400, Rich Felker wrote:
> > On Tue, Apr 02, 2024 at 07:30:57PM -0700, Max Filippov wrote:
> > > On Thu, Mar 28, 2024 at 6:48 PM Rich Felker <dalias@libc.org> wrote:
> > > > On Thu, Mar 28, 2024 at 05:48:50PM -0700, Max Filippov wrote:
> > > > > On Thu, Mar 28, 2024 at 4:00 PM Rich Felker <dalias@libc.org> wrote:
> > > > > > On Thu, Mar 28, 2024 at 01:03:17PM -0700, Max Filippov wrote:
> > > > > > > functional/dlopen fails with the
> > > > > > >   src/functional/dlopen.c:39: dlsym main failed: (null)
> > > > > > > There's no failure in the dlsym call, but the pointers don't match.
> > > > > >
> > > > > > Is this something related to canonical function descriptors? Is it
> > > > > > musl's fault or a bug in the tooling? I suspect the latter.
> > > > >
> > > > > Yes, dlsym() returns the pointer into def.dso->funcdescs,
> > > > > but (void *)main returns the pointer to the canonical function
> > > > > descriptor. I understand that the linker must use the
> > > > > R_XTENSA_FUNCDESC relocation for the locally defined
> > > > > global symbol instead of the .rofixup entries.
> > > >
> > > > If the xtensa FDPIC ABI is going to be that the linker makes canonical
> > > > function descriptors, I think that's workable, but the dynamic linker
> > > > would need a way to find and usee them. I'm not sure how that would
> > > > work.
> > > >
> > > > The simple (but probably less efficient) way is to copy what SH did
> > > > and have the dynamic linker always be responsible for them (load
> > > > descriptor address from GOT).
> > >
> > > I've built and tested SH FDPIC toolchain, it fails this test in exactly
> > > the same way: pointer loaded directly does not match the pointer
> > > returned by dlsym().
> >
> > Yes, I've been able to reproduce this and it's a clear bug. There does
> > not seem to be any way the dynamic linker could find these GOTFUNCDESC
> > slots to use them as a canonical address for the function, and
> > moreover, they're not even unique; there would be one per library.
> >
> > The code path for legitimize_pic_address in sh.c that emits
> > GOTFUNCDESC has the wrong logic. A simple fix would be just making
> > that path never be taken, but I'm not sure if that would break use of
> > GOTFUNCDESC for pure-call purposes.
> >
> > The condition should probably be something like: if it's just used for
> > a call (if this is even needed; pure call is probably handled
> > elsewhere) or if the function is static or hidden, use GOTFUNCDESC;
> > otherwise, use GOT.
> >
> > I might try patching it and see what happens.
>
> Attached patch seems to fix it. I'm not sure if this is the most
> idiomatic way to write the predicate in gcc sources, but it should be
> correct.

It's not what I observe. On my side it doesn't change the result of the
dlopen test, and it also breaks building of all statically-linked tests.

There are no relocations against the symbol 'main' neither in the test
built with the original gcc nor in the test built with the patched one.
dlopen.o built with the original gcc had R_SH_GOTOFFFUNCDESC
relocation against the symbol 'main', dlopen.o built with the patched
gcc has R_SH_GOTOFF instead. The code generated with the patched
gcc:

       if (dlsym(g, "main") != (void*)main) {
28c:   50 d1           mov.l   3d0 <main+0x3d0>,r1     ! 0 <main>
28e:   8c 31           add     r8,r1
290:   12 61           mov.l   @r1,r1
292:   13 62           mov     r1,r2
294:   8f 91           mov.w   3b6 <main+0x3b6>,r1     ! 1e0
296:   ec 31           add     r14,r1
298:   23 65           mov     r2,r5
29a:   1c 54           mov.l   @(48,r1),r4
29c:   83 6c           mov     r8,r12
29e:   4d d6           mov.l   3d4 <main+0x3d4>,r6     ! 130
2a0:   03 06           bsrf    r6
2a2:   09 00           nop
2a4:   03 61           mov     r0,r1
2a6:   4c d2           mov.l   3d8 <main+0x3d8>,r2     ! 0 <main>
2a8:   8c 32           add     r8,r2
2aa:   20 31           cmp/eq  r2,r1
2ac:   27 89           bt      2fe <main+0x2fe>
...
3d8:   00 00           .word 0x0000
                       3d8: R_SH_GOTOFF        main

doesn't look right to me at all. Using R_SH_GOTOFF
for the symbol in text doesn't make sense. Using R_SH_GOT
(AFAIU that's what you meant it to be) doesn't make sense
to me as well, as the value stored in the GOT would be the
address of the main() entry point, not of its descriptor.

I believe that gcc need to generate R_SH_GOTFUNCDESC
instead of R_SH_GOTOFFFUNCDESC for this test to work
correctly, and that the linker need to put R_SH_FUNCDESC
relocation against that GOT entry, so that the dynamic linker
could put there the address of the function descriptor associated
with the symbol.

-- 
Thanks.
-- Max