Re: Commit: readelf: Improve display of RELR relocations

[Fangrui Song <i@maskray.me>]

On Fri, Apr 19, 2024 at 3:55 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi Fangrui Song,
>
>
> > Relocation section '.relr.dyn' at offset 0x1f4 contains 2 entries:
> >
> > Considering REL/RELA output, users might misinterpret "entries" as
> > "relocation entries" instead of referring to RELR words.
> > Overall, the number of relocation entries seems more valuable than the
> > number of RELR words (users can infer the number from the section
> > size, or the new "Index:" column).
>
> A fair point.
>
>
> >>    The patch also checks for malformed RELR entries (such as an entry
> >>   with a value of just 1).
> >
> > Trailing 1 can be utilized by linkers to avoid convergence issues
> > (https://reviews.llvm.org/D67164):
>
> Interesting.  I had not realised that that could happen.
>
> So how about the attached patch ?  It changes the start of the RELR
> section display so that it looks something like this:
>
>    Relocation section '.relr.dyn' at offset 0x1a0 contains 3 entries which relocate 27 locations:
>
> It also removes the warning about an no-op bitmap entry.
>
> The patch is inefficient in that the section data is loaded twice, once
> to count the entries and a second time to display them.  But this can
> always be fixed up later if it proves to be a significant overhead.
> In my testing I did not encounter any delays because of the double
> load...
>
> Cheers
>    Nick

Hi Nick,

Thanks for the change.
I like the new header "Relocation section '.relr.dyn' at offset 0x1f4
contains 2 entries which relocate 24 locations:"

I've only got two nits:

Perhaps PRId64 should be changed to PRIu64 as num_rela is unsigned?

At /* It is theoretically possible for nentries to be 1.  */ , `if
(nentries == 0` does not free `relrs`.
Can `if (nentries == 0` be removed since the for loop handles the
zero-entry case?