Re: [musl] Initial xtensa/fdpic port review

[Max Filippov <jcmvbkbc@gmail.com>]

On Wed, Feb 28, 2024 at 10:30 AM Rich Felker <dalias@libc.org> wrote:
> On Wed, Feb 28, 2024 at 09:20:33AM -0800, Max Filippov wrote:
> > On Tue, Feb 27, 2024 at 4:12 PM Rich Felker <dalias@libc.org> wrote:
> > > > diff --git a/ldso/dlstart.c b/ldso/dlstart.c
> > > > index 259f5e18..beca953f 100644
> > > > --- a/ldso/dlstart.c
> > > > +++ b/ldso/dlstart.c
> > > > @@ -90,12 +90,19 @@ hidden void _dlstart_c(size_t *sp, size_t *dynv)
> > > >                               - segs[rel_addr[1]].p_vaddr
> > > >                               + syms[R_SYM(rel[1])].st_value;
> > > >                       rel_addr[1] = dyn[DT_PLTGOT];
> > > > +             } else if (R_TYPE(rel[1]) == REL_RELATIVE) {
> > > > +                     size_t val = *rel_addr;
> > > > +                     for (j=0; val-segs[j].p_vaddr >= segs[j].p_memsz; j++);
> > > > +                     *rel_addr += segs[j].addr - segs[j].p_vaddr;
> > >
> > > So xtensa has a "relative" reloc type that's just adjusted by the load
> > > offset of the segment the relocation lives in, rather than needing to
> > > use a symbolic relocation referencing a section symbol like other
> > > fdpic archs do?
> >
> > I was looking at the ARM BFD code while doing that and
> > my impression was that they do the same.
> > Regardless, I wonder why either relocation form might be preferable?
>
> I think the relative type here is perfectly acceptable. At first I
> thought it was weaker (less capable of representing addresses of
> objects in different segments), but looking again, I don't think
> that's the case.

I found that this treatment of the REL_RELATIVE is not consistent
with the REL_RELATIVE treatment in do_relocs(), where RELA type
relocation is expected to have an addend, and only the addend
matters, not the initial value of the field being relocated.
I just realized that looking at what ARM does might not be a good
idea for the xtensa port, as ARM uses the REL type relocations and
xtensa uses RELA.

Also do_relocs() does not use the DSO load map, so the following
change is required for it to work in the FDPIC case:
               case REL_RELATIVE:
-                       *reloc_addr = (size_t)base + addend;
+                       *reloc_addr = (size_t)laddr(dso, addend);
                       break;

-- 
Thanks.
-- Max