On Sat, Mar 28, 2015 at 8:48 PM, Alan Modra <amodra@gmail.com> wrote:
> On Sat, Mar 28, 2015 at 03:46:49PM -0700, H.J. Lu wrote:
>> On Sat, Mar 28, 2015 at 11:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> > On Fri, Mar 27, 2015 at 9:15 PM, Alan Modra <amodra@gmail.com> wrote:
>> >> On Fri, Mar 27, 2015 at 01:00:20PM -0700, H.J. Lu wrote:
>> >>> On Wed, Mar 25, 2015 at 7:22 PM, Alan Modra <amodra@gmail.com> wrote:
>> >>> > Isn't this just re-inventing the commonpage adjustment done for
>> >>> > DATA_SEGMENT_ALIGN?  Why didn't the existing code in ldexp.c work for
>> >>> > you?
>> >>>
>> >>> segment.  In order to properly align PT_GNU_RELRO segmnet, we pad the first
>> >>> section of PT_GNU_RELRO segment by
>> >>
>> >> Adjusting the start of the first section of the PT_GNU_RELRO segment
>> >> is exactly what DATA_SEGMENT_ALIGN is supposed to do.  You are
>> >> patching the wrong place.  Any adjustment to the start of the relro
>> >> segment belongs in ldexp.c code evaluating DATA_SEGMENT_ALIGN.
>> >
>> > Since output_section_statement isn't passed to ldexp.c, I don't see how
>> > DATA_SEGMENT_ALIGN  in ldexp.c can check the section address and size.
>>
>> lang_size_sections aligns expld.dataseg.base:
>
> You're correct.  I thought I knew this code well enough to review your
> patch without looking at the existing code.  Obviously not.
>
> Howver, I'm glad you also had to look at the entirety of the relro
> code as that should make you realize that the patch you posted isn't
> very elegant.  For instance, there is no need for
> output_prev_load_sec_find since the end of the previous section is
> available in expld.dataseg.min_base.  Also, the bulk of the patch

Thanks for the tip.

> still belongs with the other relro code handling DATA_SEGMENT_ALIGN
> (not in ldexp.c as I wrongly claimed before, but in
> lang_size_sections).  I think it important that we keep the relro base
> alignment code all in one place.

It is not about the base alignment.  It is about setting the address of
the first section in PT_GNU_RELRO segment, given the addresses
of PT_GNU_RELRO segment.  It is impossible o set address of the first
section in PT_GNU_RELRO segment from lang_size_sections.
lang_size_sections_1 sets the section address already.  This patch
adds first_sec to expld.dataseg.first_sec and sets it in lang_size_sections.
lang_size_sections_1 checks expld.dataseg.first_sec to increase its
address if it can reduce the file size. OK for master?

-- 
H.J.
---
ld/
PR ld/18176
* ldexp.h (ldexp_control): Add first_sec.
* ldlang.c (lang_size_sections_1): Reduce file size for
PT_GNU_RELRO segment by increasing address of the first section
in PT_GNU_RELRO segment.
(lang_size_sections): Set and reset expld.dataseg.first_sec when
realigning sections in PT_GNU_RELRO segment.

ld/testsuite/

PR ld/18176
* ld-x86-64/pr18176.d: New file.
* ld-x86-64/pr18176.s: Likewise.
* ld-x86-64/pr18176.t: Likewise.
* ld-x86-64/x86-64.exp: Run pr18176.