[PATCH] gold: Add -z noreloc-overflow option

[Hang Deng (handeng)]

Please see reply marked with [hunter].



On 2020/11/3, 3:02 PM, "Fangrui Song" <i@maskray.me> wrote:



    On 2020-11-03, Hang Deng (handeng) via Binutils wrote:

    >HJ, Cary,

    >

    >Thanks, we tried your patch for X86_64, it works well.

    >

    >And the patch below also works with arm64,  but I am not familiar with  the test case and review/commit process, can you please help to commit this patch as well?

    >

    >Thanks,

    >Hunter

    >

    >diff --git a/gold/aarch64.cc b/gold/aarch64.cc

    >index b207dcdc..29da1e35 100644

    >--- a/gold/aarch64.cc

    >+++ b/gold/aarch64.cc

    >@@ -5972,7 +5972,6 @@ Target_aarch64<size, big_endian>::Scan::local(

    >     case elfcpp::R_AARCH64_NONE:

    >       break;

    >

    >-    case elfcpp::R_AARCH64_ABS32:

    >     case elfcpp::R_AARCH64_ABS16:

    >       if (parameters->options().output_is_position_independent())

    >        {

    >@@ -5981,6 +5980,13 @@ Target_aarch64<size, big_endian>::Scan::local(

    >        }

    >       break;

    >

    >+    case elfcpp::R_AARCH64_ABS32:

    >+      if ( parameters->options().output_is_position_independent() &&

    >+          !parameters->options().noreloc_overflow()) {

    >+              gold_error(_("%s: unsupported reloc %u in pos independent link."),

    >+                       object->name().c_str(), r_type);

    >+           break;

    >+       }



    Cary raised the point that -z noreloc-overflow should be related to the

    link-time value, rather than the possibility of runtime overflow.



    In -shared/-pie links, R_AARCH64_ABS32 referencing a non-preemptible SHN_ABS symbol should be allowed.



[Hunter]: Yes, this is related to link time check. ' -z noreloc-overflow' already supported in ld, which allow 32bit relocation in linker time. This request  is to get the same support in ld.gold.



                The runtime relocation overflow check is in glibc such as "./sysdeps/x86_64/dl-machine.h".



    >     case elfcpp::R_AARCH64_ABS64:

    >       // If building a shared library or pie, we need to mark this as a dynmic

    >       // reloction, so that the dynamic loader can relocate it.

    >

    >

    >On 2020/11/3, 2:53 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote:

    >

    >    On Mon, Nov 2, 2020 at 10:05 AM Cary Coutant <ccoutant@gmail.com> wrote:

    >    >

    >    > > +    case elfcpp::R_X86_64_32S:

    >    > > +      // -z noreloc-overflow

    >    > > +      if (parameters->options().noreloc_overflow())

    >    > > +       return;

    >    > > +      // Fall through.

    >    > > +

    >    > >      default:

    >    > >        // This prevents us from issuing more than one error per reloc

    >    > >        // section.  But we can still wind up issuing more than one

    >    >

    >    > This will issue the wrong error message in the normal (check overflow)

    >    > case, won't it? You'll get "requires unsupported dynamic reloc"

    >    > instead of "requires R_X86_64_32S reloc which may overflow at

    >    > runtime".

    >

    >    Since R_X86_64_32S is handled by the default label, the error

    >    message without -z noreloc-overflow is unchanged.



    Probably unrelated but many other relocation types e.g.  R_X86_64_8

    R_X86_64_16 lack checks as they do in GNU ld.



[Hunter]: This is to allow 32bit relocation in 64bit process to be compatible with some 32b legacy code.

                 8/16 bit pointer won't really happen, and no need to allow such relocation.



    >    > Also, shouldn't -z noreloc-overflow turn off link-time overflow

    >    > checking? All this patch does is disable the warning about a possible

    >    > overflow at runtime. If that's all you want, I think the option is

    >    > misnamed.

    >    >

    >

    >    This patch disables overflow checks for R_X86_64_32S and

   >    R_X86_64_32 as the tests show.

    >

    >    --

    >    H.J.

    >