[PATCH] RISC-V: enable lui relaxation for CODE and MERGE sections.

[Ilia Diachkov]

Hi Kito,

I have investigated the fail. It is triggered by my fix but the fix is 
definitely correct and the problem lays in the general relaxation 
mechanism and its implementation for riscv.

If you look to the end of _bfd_riscv_relax_lui, you'll find comments 
"Alignment might move the section forward;     account for this assuming 
page alignment at worst." and check "VALID_RVC_LUI_IMM 
(RISCV_CONST_HIGH_PART (symval + ELF_MAXPAGESIZE))". I.e. the range of 
addresses accepted by lui is reduced to avoid errors during relaxation. 
It's reasonable since current implementation of relaxation runs several 
passes and the first/second ones may give to a symbol the smallest 
address while the next/last one may increase it.

Briefly saying my investigation shown that the check must be more 
conservative in the case when we link objects with RELRO segment. That 
is because the code in lang_size_relro_segment_1 (ld/ldlang.c) tries to 
make the end of the segment aligned by page size. It achieves this 
(later) by increasing the addresses of sections belonging to the 
segment. So if you have a symbol which address is bigger then the 
address of the first section of RELRO segment, the symbol's address may 
be shifted forward by 1 page size by lang_size_relro_segment which is 
called during relaxation.

In the failed test case, address of .tm_clone_table section is loaded by 
a lui insn in .text section. At one moment during relaxation (it's pass 
0, see comments for _bfd_riscv_relax_section) the section gets address 
0x1e104. So it's Ok to convert that lui insn to c.lui. However later on 
pass 2 all sections are moved forward and the address becomes 0x1f0f4. 
It would be correct address for c.lui instruction. However RELRO segment 
is located before tm_clone_table section and later 
lang_size_relro_segment makes additional alignment of the segment's end 
by page size so tm_clone_table gets 0x20000 address which is not 
suitable for c.lui and breaks the relocation processing.

Thus we have to change the condition of c.lui->lui conversion. The 
simplest way I see is just to harden the worst assumption in the case we 
have RELRO segment. This patch is attached. It heals the failure. I have 
tested it by dejagnu with both eabi and linux toolchains and have 
checked freedom-u-sdk build. Also I have verified on tests (EEMBC 
automotive 1.1/2.0, SPEC2006 C/C++, riscv-beebs benchmarks) that the fix 
does not affect code size with eabi toolchain.

Since the symval is positive I'm not sure whether we really need to 
check this condition in elfnn-riscv.c twice:
       && VALID_RVC_LUI_IMM (RISCV_CONST_HIGH_PART (symval))
       && VALID_RVC_LUI_IMM (RISCV_CONST_HIGH_PART (symval + 
ELF_MAXPAGESIZE))
so I leaved the stricter check.

The more complex approach may include address comparison of RELRO 
segment (its first section) and relocatable symbol and if the symbol 
address is bigger then use stricter conditions on the symbol address.

Best regards,
Ilia.

bfd/ChangeLog:
     * elfnn-riscv.c (_bfd_riscv_relax_lui): set lui relax safety area to 
two pages in relro presence.

Kito Cheng wrote 2019-07-23 09:45:
> Hi Ilia:
> 
> Forgot to attend my binutils configuration and git hash:
> ../binutils-gdb/configure  --target=riscv32-unknown-linux-gnu
> 
> commit 89356123a17c27548c7e71f4f000b1f74e551c31
> Refs: {origin/master}, {origin/HEAD},
> users/ARM/embedded-gdb-master-2018q4-2078-g89356123a1
> Author:     GDB Administrator <gdbadmin@sourceware.org>
> AuthorDate: Tue Jul 23 00:00:36 2019 +0000
> Commit:     GDB Admin <brobecker@adacore.com>
> CommitDate: Tue Jul 23 00:00:36 2019 +0000
> 
>    Automatic date update in version.in
> 
> On Tue, Jul 23, 2019 at 2:40 PM Kito Cheng <kito.cheng@gmail.com> 
> wrote:
>> 
>> Hi Illa:
>> 
>> I just found a case will got `relocation truncated to fit` error on
>> rv32 linux with this patch.
>> 
>> Here is the tarball for reproduce, included minimized sysroot and
>> necessary objects/libraries.
>> 
>> `make relax` can reproduce the problem and `make norelax` for disable
>> relax build.
>> 
>> https://drive.google.com/file/d/1l-IOiK3Ay7s2mnFWYXOkHYyPFNsIV58H/view?usp=sharing
>> 
>> 
>> On Tue, Jun 25, 2019 at 4:55 AM Jim Wilson <jimw@sifive.com> wrote:
>> >
>> > On Fri, Jun 21, 2019 at 11:59 AM Ilia Diachkov
>> > <ilia.diachkov@optimitech.com> wrote:
>> > > This patch enables lui relaxation for CODE and MERGE sections on RISC-V.
>> > > It helps to reduce code size.
>> >
>> > I spotted two minor style issues.  An extra blank line where we had
>> > the bfd_assert call during testing.  And a missing space before an
>> > open paren.
>> >
>> > I rewrote the ChangeLog entry.  It should describe the actual changes
>> > that are being made, and not the effect of the change.  Also, it
>> > should be sentences that start with capital letter.
>> >
>> > I did a quick test to make sure I didn't accidentally break it and
>> > then committed and pushed it.
>> >
>> > Jim
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-RISC-V-fix-degradation-after-enabling-lui-relaxation.patch
Type: text/x-diff
Size: 1144 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20190731/59679c67/attachment.bin>