[PATCH] elf/riscv: Fix relaxation with aliases [PR28021]

[Nelson Chu]

Hi,

On Tue, Jun 29, 2021 at 12:36 AM Michael Matz <matz@suse.de> wrote:
> On Mon, 28 Jun 2021, Michael Matz wrote:
>
> > the fix for PR22756 only changed behaviour for hidden aliases,
> > but the same situation exists for non-hidden aliases: sym_hashes[]
> > can contain multiple entries pointing to the same symbol structure
> > leading to relaxation adjustment to be applied twice.
> >
> > Fix this by testing for duplicates for everything that looks like it
> > has a version.

Thanks, it looks reasonable and good.

> > [tested by a testsuite run on a x86-64->riscv64 cross binutils]

I get the errors when running the riscv32 regression, since the ld/sd
are unrecognized by riscv32 toolchain.  There are two solutions we
usually use, one is to support two testcases for riscv32 (use lw/sw)
and riscv64 (use ld/sw), or you can merge them into one source by
using .ifdef and .endif, and then pass --defsym when assembling them.
The other solution is to fix the march, for example, -march=rv64gc.
Both of the two solutions are good to me, so it depends on what you
think is better.

> Oh, and, I'm unhappy with how I test for the situation, but couldn't find
> a different way.  Ideally I want to test that both symbols have the same
> value in the resulting shared lib.  But that's outside the power of
> regexps, and our test infrastructure doesn't seem to have an easy way for
> such more complete matching.  A runtime test would be easy, but I don't
> want that either.
>
> So, if there are ideas how to test for equality of two symbol addresses
> within our test framework: I'm all ears.

Thanks Andreas' suggestion, but I'm not familiar with the ld_nm.  I
usually give a linker script to fix the section addresses and symbols,
then the comparison will be easier.  The addresses of the above test
cases are simple enough to be fixed.  There is only "call sscanf@plt"
will be relaxed to jal, before foobar_new.  So if we force the start
address of the text section to 0xXXX, then the values of foobar_new
and foobar@@new will always be XXX + 0x2c.  It would be more safe if
we add the .option norvc/rvc with the .option pic, but that's fine
since it doesn't affect your example.


However, except for the minor failures of riscv32 toolchain
regressions, the patch looks reasonable and good to me.  Thanks for
the fix, you could commit it after the riscv32 problem is resolved.

Thank you very much
Nelson