[PATCH] D26327: Convert MIPS .reginfo section to synthetic input section.

[Rui Ueyama via llvm-commits]

This looks good to me. Please take over this patch.

On Mon, Nov 7, 2016 at 6:58 AM, Simon Atanasyan <simon at atanasyan.com> wrote:

> Hi,
>
> Here is the patch fixes all issues mentioned below. It also converts
> .MIPS.options section to synthetic one. The only failed test is the
> mips-options-r.test. The linker creates .MIPS.options section, but
> does not create a corresponding section symbol. I'm going to
> investigate it.
>
> Rui, what do you think about the patch?
>
> On Mon, Nov 7, 2016 at 12:40 AM, Simon Atanasyan <simon at atanasyan.com>
> wrote:
> > atanasyan added inline comments.
> >
> > ================
> > Comment at: ELF/InputFiles.cpp:166
> >      return MipsOptions->Reginfo->ri_gp_value;
> > -  if (!ELFT::Is64Bits && MipsReginfo && MipsReginfo->Reginfo)
> > -    return MipsReginfo->Reginfo->ri_gp_value;
> > ----------------
> > When we calculate R_MIPS_GPREL16/32 relocations we need to know `GP0`
> value stored in the .MIPS.options or .reginfo sections (take a look at the
> `computeAddend` routine). Each file might have it's own value. Usually
> `GP0` is equal to zero but it is not always true. After this change
> `getMipsGp0()` does not take in account `GP0` value stored in the .reginfo
> section and the `mips-gprel32-relocs-gp0.s` test case starts to fail.
> >
> > To solve the problem we can add `MipsGp0` field to the `ObjectFile`
> class and retrieve and save `GP0` value to this field in the say
> `ObjectFile::initializeSections` function.
> >
> > If you want I can work on that and convert MipsOptionsInputSection to
> synthetic input section too. It looks like it is better to convert .reginfo
> and .MIPS.options sections in a single patch.
> >
> >
> > ================
> > Comment at: ELF/SyntheticSections.cpp:110
> > +    if (Config->Relocatable && RI->ri_gp_value)
> > +      error("supported non-zero ri_gp_value");
> > +    Reginfo.ri_gprmask |= RI->ri_gprmask;
> > ----------------
> > - s/supported/unsupported/
> > - it is not very important, but it would be nice to continue to show a
> file's name in this error message. maybe add an argument to the
> `getSectionContents` routine to be able to pass a //validate// function.
> >
> >
> > https://reviews.llvm.org/D26327
>
> --
> Simon Atanasyan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161108/e44ae888/attachment.html>