[PATCH] Skip convert lui to GPREL_L for .LANCHOR

Nelson Chu nelson.chu@sifive.com
Fri Mar 19 08:31:55 GMT 2021


Hi Jim and MaskRay,

Thanks for your feedback and clarification.  The lld's behavior looks
interesting, I will find some time to see the details if possible.

Hi Lifang,

Thanks for your reporting.  I may have time and can come back to see
this problem next week.  I prefer to keep your name in the ChangeLog
and as the co-author in my following fixes, to credit your
contribution.  But unfortunately we notice that Alibaba only have the
copyright assignments for gcc and glibc, you might don't have the ones
for binutils and gdb, so we cannot commit your patches or keep your
name/company.  Do you have any plan to sign the copyrights for
binutils and gdb?  It's should be helpful and convenience if you are
willing to doing contributions for binutils and gdb in the future.
The process may need some times, so it would be great if you can start
to run the it recently.

Thanks
Nelson

On Tue, Mar 16, 2021 at 2:49 PM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Fri, Mar 12, 2021 at 5:51 AM Nelson Chu <nelson.chu@sifive.com> wrote:
> >
> > Thanks!
> >
> > I have given my thoughts on the bugzilla, but I think it would be good
> > to get more suggestions from other maintainers, including the experts
> > from llvm and lld.
> > What do you think?  Please feel free to share your thoughts if you are
> > interested and have times :)
> >
> > Thank you very much
> > Nelson
> >
> > On Fri, Mar 12, 2021 at 11:22 AM Lifang Xia
> > <lifang_xia@linux.alibaba.com> wrote:
> > >
> > > Hi Nelson,
> > >
> > > I report a bug here: https://sourceware.org/bugzilla/show_bug.cgi?id=27566
> > >
> > > BRs,
> > > Lifang
> > >
> > > 在 2021/3/12 10:38, Nelson Chu 写道:
> > > > Hi Lifang,
> > > >
> > > > Could you add the ld testcase for this?  It's better to report the
> > > > details somewhere, like sourceware bugzilla or riscv github.  I find a
> > > > similar fix in PR25258,
> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=25258.  The patch
> > > > works only for c.lui relaxation, so I think we might need a similar
> > > > fix for the regular lui to gp relaxation, rather than just disable the
> > > > relaxation.  Anyway, we need a testcase or at least a reduced case can
> > > > be reproduced.
> > > >
> > > > Thanks
> > > > Nelson
> > > >
> > > > On Thu, Mar 11, 2021 at 9:05 PM Lifang Xia via Binutils
> > > > <binutils@sourceware.org> wrote:
> > > >> If the .LANCHOR defined in .rodata, the gp may not reach the symbol
> > > >> after lang_size_relro_segment while linking with "-z relro".
> > > >>
> > > >> bfd/
> > > >>          * elfnn-riscv.c (_bfd_riscv_relax_lui): Skip that the symbol
> > > >>            is defined in rodata and linking with "-z relro".
> > > >> ---
> > > >>   bfd/elfnn-riscv.c | 12 ++++++++----
> > > >>   1 file changed, 8 insertions(+), 4 deletions(-)
> > > >>
> > > >> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> > > >> index 364d67b..74dce3c 100644
> > > >> --- a/bfd/elfnn-riscv.c
> > > >> +++ b/bfd/elfnn-riscv.c
> > > >> @@ -4251,10 +4251,14 @@ _bfd_riscv_relax_lui (bfd *abfd,
> > > >>        Valid gp range conservatively because of alignment issue.  */
> > > >>     if (undefined_weak
> > > >>         || (VALID_ITYPE_IMM (symval)
> > > >> -         || (symval >= gp
> > > >> -             && VALID_ITYPE_IMM (symval - gp + max_alignment + reserve_size))
> > > >> -         || (symval < gp
> > > >> -             && VALID_ITYPE_IMM (symval - gp - max_alignment - reserve_size))))
> > > >> +         /* Skip that the symbol defined in rodata by .set, such as .LANCHOR,
> > > >> +            and link with -z relro. The symbol may not be accessed with gp.  */
> > > >> +         || (!(sym_sec->flags & SEC_READONLY
> > > >> +               && link_info->relro)
> > > >> +             && ((symval >= gp
> > > >> +                  && VALID_ITYPE_IMM (symval - gp + max_alignment + reserve_size))
> > > >> +                 || (symval < gp
> > > >> +                     && VALID_ITYPE_IMM (symval - gp - max_alignment - reserve_size))))))
> > > >>       {
> > > >>         unsigned sym = ELFNN_R_SYM (rel->r_info);
> > > >>         switch (ELFNN_R_TYPE (rel->r_info))
> > > >> --
> > > >> 2.6.0-rc0
> > > >>
>
> I haven't looked into the details.
> .text < DATA_SEGMENT_ALIGN < .sdata < DATA_SEGMENT_RELRO_END
>
> If gp relaxation requires that ld estimates the distance between the
> relocated location and .sdata, being conservative and assuming
> DATA_SEGMENT_ALIGN+DATA_SEGMENT_RELRO_END may take
> maxpagesize+commonpagesize bytes looks good to me.
>
> LLD avoids the DATA_SEGMENT_ALIGN/DATA_SEGMENT_RELRO_END complexity by
> having 2 RW segments and making the first RW overlap with PT_GNU_RELRO
> (https://reviews.llvm.org/D58892).


More information about the Binutils mailing list