Re: [GOLD][PATCH] Fix -z relro bugs

[Doug Kwan (éæå) <dougkwan at google dot com>]

Ian,

   I used ORDER_NON_RELRO_FIRST so that gold places .got section in
the same place as ld.  That is not very important.  I changed that to
ORDER_DATA and checked in the following.

Thanks

-Doug


2012-03-15  Doug Kwan  <dougkwan@google.com>

        * arm.cc (Target_arm::got_section): Make .got section read-only
        if -z now is given.


On Thu, Mar 15, 2012 at 10:25 AM, Ian Lance Taylor <iant@google.com> wrote:
> "Doug Kwan (éæå)" <dougkwan@google.com> writes:
>
>> Â ÂThis patch fixes two problems related to the -z relro option. ÂThe
>> first one is that .init_array, .fini_array and .preinit_array are not
>> handle properly in Layout::make_output_seciton. ÂThe function only
>> force the relro flag if section type if SHT_PROGBITS. ÂThe second
>> problem is that the ARM backend ignores the -z now option. ÂThis patch
>> fixes this problem and also places the .got section at the RELRO
>> boundary. ÂThis is tested on both ARM and x86_64.
>>
>> -Doug
>>
>>
>> 2012-03-15 ÂDoug Kwan Â<dougkwan@google.com>
>>
>> Â Â Â Â * arm.cc (Target_arm::got_section): Make .got section read-only
>> Â Â Â Â if -z now is given. Also place .got section at RELRO boundary.
>> Â Â Â Â * layout.cc (Layout::make_output_section): Fix a bug that causes
>> Â Â Â Â .init_array, .fini_array and .preinit_array to be not included
>> Â Â Â Â in the RELRO segment when -z now is given.
>
>
> (Please send to binutils@sourceware.org, not
> binutils@sources.redhat.com. ÂThanks.)
>
> Coincidentally somebody reported the layout.cc change thing as a PR and
> I fixed it this morning.
>
>> + Â Â Â// When using -z now, we can treat .got as a relro section.
>> + Â Â Â// Without -z now, it is modified after program startup by lazy
>> + Â Â Â// PLT relocations.
>> + Â Â Âbool is_got_relro = parameters->options().now();
>> + Â Â ÂOutput_section_order got_order = (is_got_relro
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ? ORDER_RELRO_LAST
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â : ORDER_NON_RELRO_FIRST);
>
> As far as I can see this hsould be ORDER_DATA rather than
> ORDER_NON_RELRO_FIRST. ÂThe x86 support uses ORDER_NON_RELRO_FIRST
> because it calls layout->increase_relro. ÂIf you don't use that, you
> might as well use ORDER_DATA. ÂUnless there is some other reason that
> the section should be first, but what?
>
> This patch is OK using ORDER_DATA, or with ORDER_NON_RELRO_FIRST if
> there is a reason for it.
>
> Thanks.
>
> Ian

Attachment: patch.txt
Description: Text document