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

[Jim Wilson <jimw at sifive dot com>]

On Wed, Jul 31, 2019 at 4:19 PM Ilia Diachkov
<ilia.diachkov@optimitech.com> wrote:
> 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.

I think it is a little more complicated than this.  I think the linker
is getting confused by some padding alignment.  The linker is trying
to align the data sections to start at a page boundary.  And it is
trying to align the end of the relro sections to a page boundary.  In
theory this should just be one alignment as they are adjacent.  But
along the way some alignment padding bytes end up between the relro
sections and the data sections, and this seems to confuse the linker
into doing two alignments.  In one pass, it aligns the start of the
data section.  Then in the next pass, it notices that the relro
section ends 0xf4 bytes before a page boundary because of padding
bytes and aligns it, putting the data sections 0xf4 bytes after a page
boundary.  Then it has to align the data sections again in the same
pass.  When we come around the next time, it seems to properly handle
the padding bytes and correctly only does one alignment.

relro is linux specific, and it is the embedded developers that care
the most about code size, so I think increasing the alignment check to
two pages with relro is fine.

> 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.

On second thought, I don't think that this is right.  c.lui can load
0xfffe0000 to 0xfffff000 and 0x00001000 to 0x0001f000.  So there is a
problem if the symval is in the zero page and adding the offset places
it after 0x00001000.  And there is a problem if symval is before
0xfffe0000 and adding the offset puts if after.  So I think we still
need the two tests.

Otherwise this looks OK to me.  I committed a slightly modified form
of the patch that keeps the two symval checks.

Jim