[GOLD][PATCH] Fix -z relro bugs

[Doug Kwan (關振德)]

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
-------------- next part --------------
Index: gold/arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.147
diff -u -u -p -r1.147 arm.cc
--- gold/arm.cc	14 Mar 2012 23:07:07 -0000	1.147
+++ gold/arm.cc	15 Mar 2012 18:14:41 -0000
@@ -4174,11 +4174,22 @@ Target_arm<big_endian>::got_section(Symb
     {
       gold_assert(symtab != NULL && layout != NULL);
 
+      // 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_DATA);
+
+      // Unlike some targets (.e.g x86), ARM does not use separate .got and
+      // .got.plt sections in output.  The output .got section contains both
+      // PLT and non-PLT GOT entries.
       this->got_ = new Arm_output_data_got<big_endian>(symtab, layout);
 
       layout->add_output_section_data(".got", elfcpp::SHT_PROGBITS,
 				      (elfcpp::SHF_ALLOC | elfcpp::SHF_WRITE),
-				      this->got_, ORDER_DATA, false);
+				      this->got_, got_order, is_got_relro);
 
       // The old GNU linker creates a .got.plt section.  We just
       // create another set of data in the .got section.  Note that we
@@ -4187,7 +4198,7 @@ Target_arm<big_endian>::got_section(Symb
       this->got_plt_ = new Output_data_space(4, "** GOT PLT");
       layout->add_output_section_data(".got", elfcpp::SHT_PROGBITS,
 				      (elfcpp::SHF_ALLOC | elfcpp::SHF_WRITE),
-				      this->got_plt_, ORDER_DATA, false);
+				      this->got_plt_, got_order, is_got_relro);
 
       // The first three entries are reserved.
       this->got_plt_->set_current_data_size(3 * 4);