public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Reduce file size for PT_GNU_RELRO segment
@ 2015-03-19 13:02 H.J. Lu
  2015-03-26  2:22 ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2015-03-19 13:02 UTC (permalink / raw)
  To: binutils

When a padding in file is used to align PT_GNU_RELRO segment, the maximum
padding size is maximum page size minus 1.  This patch trades memory size
for file size by increasing memory size to avoid padding if file size
will be reduced by more than maximum page minus a page and memory size
will be increased by less than a page.

OK for master?

Thanks.

H.J.
---
	* ldlang.c (output_prev_load_sec_find): New.
	(lang_size_sections_1): Reduce file size for PT_GNU_RELRO
	segment by increasing memory size.
---
 ld/ldlang.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/ld/ldlang.c b/ld/ldlang.c
index 8880821..3450c9d 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -1718,6 +1718,31 @@ output_prev_sec_find (lang_output_section_statement_type *os)
   return NULL;
 }
 
+/* Find the last output load section with contents before given output
+   statement.  */
+
+static asection *
+output_prev_load_sec_find (lang_output_section_statement_type *os)
+{
+  lang_output_section_statement_type *lookup;
+
+  for (lookup = os->prev; lookup != NULL; lookup = lookup->prev)
+    {
+      if (lookup->constraint < 0)
+	continue;
+
+      if (lookup->bfd_section != NULL
+	  && lookup->bfd_section->owner != NULL
+	  && ((lookup->bfd_section->flags & (SEC_ALLOC
+					     | SEC_LOAD
+					     | SEC_HAS_CONTENTS))
+	      == (SEC_ALLOC | SEC_LOAD | SEC_HAS_CONTENTS)))
+	return lookup->bfd_section;
+    }
+
+  return NULL;
+}
+
 /* Look for a suitable place for a new output section statement.  The
    idea is to skip over anything that might be inside a SECTIONS {}
    statement in a script, before we find another output section
@@ -4776,6 +4801,12 @@ lang_size_sections_1
    bfd_boolean check_regions)
 {
   lang_statement_union_type *s;
+  bfd_boolean reduce_padding
+    = (link_info.relro
+       && expld.dataseg.phase == exp_dataseg_relro_adjust
+       && expld.dataseg.maxpagesize > expld.dataseg.pagesize);
+  bfd_vma pagsizediff
+    = expld.dataseg.maxpagesize - expld.dataseg.pagesize;
 
   /* Size up the sections from their constituent parts.  */
   for (s = *prev; s != NULL; s = s->header.next)
@@ -4918,6 +4949,38 @@ lang_size_sections_1
 		  {
 		    bfd_vma savedot = newdot;
 		    newdot = align_power (newdot, section_alignment);
+		    if (reduce_padding && expld.dataseg.base == newdot)
+		      {
+			/* If a padding in file will be used for
+			   PT_GNU_RELRO segment, check if we can reduce
+			   the padding by increase the address of the
+			   first relro section.  It is a tradeoff between
+			   memory size and file size.  We only do it if
+			   file size will be reduced by more than maximum
+			   page minus a page and memory size will be
+			   increased by less than a page.  */
+			asection *prev_sec
+			  = output_prev_load_sec_find (os);
+			if (prev_sec)
+			  {
+			    bfd_vma pad
+			      = ((newdot - (bfd_section_vma (prev_sec->owner,
+							     prev_sec)
+					    + bfd_get_section_size (prev_sec)))
+				 % expld.dataseg.maxpagesize);
+			    if (pad)
+			      {
+				bfd_vma nextdot
+				  = expld.dataseg.maxpagesize - pad;
+				nextdot = align_power (newdot + nextdot,
+						       section_alignment);
+				if (((nextdot - newdot)
+				     < expld.dataseg.pagesize)
+				    && pad > pagsizediff)
+				  newdot = nextdot;
+			      }
+			  }
+		      }
 
 		    dotdelta = newdot - savedot;
 		    if (dotdelta != 0
-- 
2.1.0

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-19 13:02 [PATCH] Reduce file size for PT_GNU_RELRO segment H.J. Lu
@ 2015-03-26  2:22 ` Alan Modra
  2015-03-27 20:00   ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2015-03-26  2:22 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Thu, Mar 19, 2015 at 06:02:44AM -0700, H.J. Lu wrote:
> When a padding in file is used to align PT_GNU_RELRO segment, the maximum
> padding size is maximum page size minus 1.  This patch trades memory size
> for file size by increasing memory size to avoid padding if file size
> will be reduced by more than maximum page minus a page and memory size
> will be increased by less than a page.

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?

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-26  2:22 ` Alan Modra
@ 2015-03-27 20:00   ` H.J. Lu
  2015-03-28  4:15     ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2015-03-27 20:00 UTC (permalink / raw)
  To: Binutils

On Wed, Mar 25, 2015 at 7:22 PM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Mar 19, 2015 at 06:02:44AM -0700, H.J. Lu wrote:
>> When a padding in file is used to align PT_GNU_RELRO segment, the maximum
>> padding size is maximum page size minus 1.  This patch trades memory size
>> for file size by increasing memory size to avoid padding if file size
>> will be reduced by more than maximum page minus a page and memory size
>> will be increased by less than a page.
>
> 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?

The existing code in  ldexp.c only deals with sections within PT_GNU_RELRO
segment.  It doesn't consider impact of the section just before PT_GNU_RELRO
segment on the file size due to the alignment requirement for PT_GNU_RELRO
segment.  In order to properly align PT_GNU_RELRO segment, we pad the first
section of PT_GNU_RELRO segment by

((sec->vma - (prev_sec->vma + prev_sec->size)) % common_pagesize

If we can reduce the padding significantly by increasing the address
of the first
section of PT_GNU_RELRO segment just a little bit, we limit the padding
to the minimum and generate a much smaller binary.  That is what my patch
does.


-- 
H.J.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-27 20:00   ` H.J. Lu
@ 2015-03-28  4:15     ` Alan Modra
  2015-03-28 18:30       ` H.J. Lu
  2015-03-29  3:19       ` [PATCH] Reduce file size for PT_GNU_RELRO segment H.J. Lu
  0 siblings, 2 replies; 15+ messages in thread
From: Alan Modra @ 2015-03-28  4:15 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

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 segment, 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.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-28  4:15     ` Alan Modra
@ 2015-03-28 18:30       ` H.J. Lu
  2015-03-28 22:46         ` H.J. Lu
  2015-03-29  3:19       ` [PATCH] Reduce file size for PT_GNU_RELRO segment H.J. Lu
  1 sibling, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2015-03-28 18:30 UTC (permalink / raw)
  To: Binutils

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 segment, 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.


-- 
H.J.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-28 18:30       ` H.J. Lu
@ 2015-03-28 22:46         ` H.J. Lu
  2015-03-29  3:49           ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2015-03-28 22:46 UTC (permalink / raw)
  To: Binutils

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:

5422      /* Aligning the adjusted base guarantees the padding
5423 between sections won't change.  This is better than
5424 simply subtracting 1 << max_alignment_power which is
5425 what we used to do here.  */
5426      expld.dataseg.base &= ~((1 << max_alignment_power) - 1);
5427      lang_reset_memory_regions ();
5428      one_lang_size_sections_pass (relax, check_regions);
5429    }
5430 }
5431      link_info.relro_start = expld.dataseg.base;
(gdb) p/x expld.dataseg
$290 = {phase = 0x4, base = 0x24bd00, min_base = 0x4bcc7,
  relro_end = 0x24f000, end = 0x64eae0, pagesize = 0x1000,
  maxpagesize = 0x200000, relro = 0x0, relro_start_stat = 0x8203e0,
  relro_end_stat = 0x822b20}
(gdb)

and calls  lang_size_sections_1 to align the first section of
PT_GNU_RELRO segmnet:

(gdb) bt
#0  lang_size_sections_1 (prev=0x81f258, output_section_statement=0x8135b0,
    fill=0x0, dot=2407680, relax=0x0, check_regions=1)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldlang.c:4919
#1  0x0000000000415219 in one_lang_size_sections_pass (relax=0x0,
    check_regions=1)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldlang.c:5368
#2  0x000000000041541d in lang_size_sections (relax=0x0, check_regions=1)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldlang.c:5428
#3  0x0000000000417758 in lang_process ()
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldlang.c:6766
#4  0x000000000041b588 in main (argc=74, argv=0x7fffffffd328)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldmain.c:419
(gdb) list
4914 if (section_alignment > 0)
4915  {
4916    bfd_vma savedot = newdot;
4917    newdot = align_power (newdot, section_alignment);
4918
4919    dotdelta = newdot - savedot;
4920    if (dotdelta != 0
4921 && (config.warn_section_align
4922    || os->addr_tree != NULL)
4923 && expld.phase != lang_mark_phase_enum)
(gdb)

After that, DATA_SEGMENT_ALIGN is processed:

(gdb) bt
#0  fold_binary (tree=0x820380)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldexp.c:570
#1  0x000000000042065d in exp_fold_tree_1 (tree=0x820380)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldexp.c:989
#2  0x00000000004206f2 in exp_fold_tree_1 (tree=0x8203b0)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldexp.c:1008
#3  0x0000000000420bef in exp_fold_tree (tree=0x8203b0,
    current_section=0x8078b0 <_bfd_std_section+560>, dotp=0x7fffffffcf68)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldexp.c:1166
#4  0x0000000000414e36 in lang_size_sections_1 (prev=0x820300,
    output_section_statement=0x8135b0, fill=0x0, dot=310471, relax=0x0,
    check_regions=1)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldlang.c:5211
#5  0x0000000000415219 in one_lang_size_sections_pass (relax=0x0,
    check_regions=1)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldlang.c:5368
#6  0x000000000041524f in lang_size_sections (relax=0x0, check_regions=1)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldlang.c:5378
#7  0x000000000041724a in lang_relax_sections (need_layout=1)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldlang.c:6529
#8  0x0000000000427d83 in gldelf_x86_64_map_segments (need_layout=1)
    at eelf_x86_64.c:68
#9  0x000000000042ae3f in gldelf_x86_64_after_allocation ()
---Type <return> to continue, or q <return> to quit---
    at eelf_x86_64.c:1787
#10 0x0000000000421899 in ldemul_after_allocation ()
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldemul.c:70
#11 0x000000000041775d in lang_process ()
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldlang.c:6770
#12 0x000000000041b588 in main (argc=74, argv=0x7fffffffd328)
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldmain.c:419
(gdb) list
565  else if (expld.dataseg.phase == exp_dataseg_none)
566    {
567      expld.dataseg.phase = exp_dataseg_align_seen;
568      expld.dataseg.min_base = expld.dot;
569      expld.dataseg.base = expld.result.value;
570      expld.dataseg.pagesize = commonpage;
571      expld.dataseg.maxpagesize = maxpage;
572      expld.dataseg.relro_end = 0;
573    }
574  else
(gdb) f 11
#11 0x000000000041775d in lang_process ()
    at /export/gnu/import/git/sources/binutils-gdb/ld/ldlang.c:6770
6770  ldemul_after_allocation ();
(gdb) list
6765  /* Size up the sections.  */
6766  lang_size_sections (NULL, ! RELAXATION_ENABLED);
6767
6768  /* See if anything special should be done now we know how big
6769     everything is.  This is where relaxation is done.  */
6770  ldemul_after_allocation ();
6771
6772  /* Fix any .startof. or .sizeof. symbols.  */
6773  lang_set_startof ();
6774
(gdb)

I don't see DATA_SEGMENT_ALIGN changes section address.

-- 
H.J.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-28  4:15     ` Alan Modra
  2015-03-28 18:30       ` H.J. Lu
@ 2015-03-29  3:19       ` H.J. Lu
  1 sibling, 0 replies; 15+ messages in thread
From: H.J. Lu @ 2015-03-29  3:19 UTC (permalink / raw)
  To: Binutils

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 segment, 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.
>

I opened a bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=18176

-- 
H.J.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-28 22:46         ` H.J. Lu
@ 2015-03-29  3:49           ` Alan Modra
  2015-03-29 13:58             ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2015-03-29  3:49 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

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
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.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-29  3:49           ` Alan Modra
@ 2015-03-29 13:58             ` H.J. Lu
  2015-03-30 12:39               ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2015-03-29 13:58 UTC (permalink / raw)
  To: Binutils

[-- Attachment #1: Type: text/plain, Size: 2962 bytes --]

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.

[-- Attachment #2: 0001-Reduce-file-size-for-PT_GNU_RELRO-segment.patch --]
[-- Type: text/x-patch, Size: 9901 bytes --]

From 72bcdf3ce44148548ca3d73029bf9eb4b4fd1db5 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 29 Mar 2015 05:54:11 -0700
Subject: [PATCH] Reduce file size for PT_GNU_RELRO segment

When a padding in file is used to align PT_GNU_RELRO segment, the maximum
padding size is maximum page size minus 1.  This patch trades memory size
for file size by increasing address of the first section in PT_GNU_RELRO
segment to avoid padding if file size will be reduced by more than maximum
page minus a page and memory size will be increased by less than a page.

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.
---
 ld/ldexp.h                        |  2 ++
 ld/ldlang.c                       | 67 +++++++++++++++++++++++++++++----------
 ld/testsuite/ld-x86-64/pr18176.d  |  9 ++++++
 ld/testsuite/ld-x86-64/pr18176.s  | 52 ++++++++++++++++++++++++++++++
 ld/testsuite/ld-x86-64/pr18176.t  | 39 +++++++++++++++++++++++
 ld/testsuite/ld-x86-64/x86-64.exp |  1 +
 6 files changed, 153 insertions(+), 17 deletions(-)
 create mode 100644 ld/testsuite/ld-x86-64/pr18176.d
 create mode 100644 ld/testsuite/ld-x86-64/pr18176.s
 create mode 100644 ld/testsuite/ld-x86-64/pr18176.t

diff --git a/ld/ldexp.h b/ld/ldexp.h
index 10fcf3d..44d30fb 100644
--- a/ld/ldexp.h
+++ b/ld/ldexp.h
@@ -158,6 +158,8 @@ struct ldexp_control {
 
     bfd_vma base, min_base, relro_end, end, pagesize, maxpagesize;
 
+    asection *first_sec;
+
     enum relro_enum relro;
 
     union lang_statement_union *relro_start_stat;
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 8880821..8cb440d 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -4776,6 +4776,8 @@ lang_size_sections_1
    bfd_boolean check_regions)
 {
   lang_statement_union_type *s;
+  bfd_vma pagsizediff
+    = expld.dataseg.maxpagesize - expld.dataseg.pagesize;
 
   /* Size up the sections from their constituent parts.  */
   for (s = *prev; s != NULL; s = s->header.next)
@@ -4858,6 +4860,7 @@ lang_size_sections_1
 	      }
 	    else
 	      {
+		bfd_vma savedot;
 		if (os->addr_tree == NULL)
 		  {
 		    /* No address specified for this section, get one
@@ -4914,21 +4917,44 @@ lang_size_sections_1
 		  section_alignment = os->section_alignment;
 
 		/* Align to what the section needs.  */
+		savedot = newdot;
 		if (section_alignment > 0)
-		  {
-		    bfd_vma savedot = newdot;
-		    newdot = align_power (newdot, section_alignment);
+		  newdot = align_power (newdot, section_alignment);
 
-		    dotdelta = newdot - savedot;
-		    if (dotdelta != 0
-			&& (config.warn_section_align
-			    || os->addr_tree != NULL)
-			&& expld.phase != lang_mark_phase_enum)
-		      einfo (_("%P: warning: changing start of section"
-			       " %s by %lu bytes\n"),
-			     os->name, (unsigned long) dotdelta);
+		if (expld.dataseg.first_sec == os->bfd_section)
+		  {
+		    /* If a padding in file will be used for PT_GNU_RELRO
+		       segment, check if we can reduce the padding by
+		       increase the address of the first relro section.
+		       It is a tradeoff between memory size and file size.
+		       We only do it if file size will be reduced by more
+		       than maximum page minus a page and memory size will
+		       be increased by less than a page.  */
+		    bfd_vma pad
+		      = ((newdot - expld.dataseg.min_base)
+			 % expld.dataseg.maxpagesize);
+		    if (pad)
+		      {
+			bfd_vma nextdot
+			  = expld.dataseg.maxpagesize - pad;
+			nextdot = align_power (newdot + nextdot,
+					       section_alignment);
+			if (((nextdot - newdot)
+			     < expld.dataseg.pagesize)
+			    && pad > pagsizediff)
+			  newdot = nextdot;
+		      }
 		  }
 
+		dotdelta = newdot - savedot;
+		if (dotdelta != 0
+		    && (config.warn_section_align
+			|| os->addr_tree != NULL)
+		    && expld.phase != lang_mark_phase_enum)
+		  einfo (_("%P: warning: changing start of section"
+			   " %s by %lu bytes\n"),
+			 os->name, (unsigned long) dotdelta);
+
 		bfd_set_section_vma (0, os->bfd_section, newdot);
 
 		os->bfd_section->output_offset = 0;
@@ -5409,16 +5435,21 @@ lang_size_sections (bfd_boolean *relax, bfd_boolean check_regions)
 	     and DATA_SEGMENT_RELRO_END can cause excessive padding to
 	     be inserted at DATA_SEGMENT_RELRO_END.  Try to start a
 	     bit lower so that the section alignments will fit in.  */
-	  asection *sec;
+	  asection *sec, *first_sec = NULL;
 	  unsigned int max_alignment_power = 0;
 
 	  /* Find maximum alignment power of sections between
 	     DATA_SEGMENT_ALIGN and DATA_SEGMENT_RELRO_END.  */
-	  for (sec = link_info.output_bfd->sections; sec; sec = sec->next)
-	    if (sec->vma >= expld.dataseg.base
-		&& sec->vma < expld.dataseg.relro_end
-		&& sec->alignment_power > max_alignment_power)
-	      max_alignment_power = sec->alignment_power;
+	  for (sec = link_info.output_bfd->sections;
+	       sec && sec->vma < expld.dataseg.relro_end;
+	       sec = sec->next)
+	    if (sec->vma >= expld.dataseg.base)
+	      {
+		if (first_sec == NULL)
+		  first_sec = sec;
+		if (sec->alignment_power > max_alignment_power)
+		  max_alignment_power = sec->alignment_power;
+	      }
 
 	  if (((bfd_vma) 1 << max_alignment_power) < expld.dataseg.pagesize)
 	    {
@@ -5427,8 +5458,10 @@ lang_size_sections (bfd_boolean *relax, bfd_boolean check_regions)
 		 simply subtracting 1 << max_alignment_power which is
 		 what we used to do here.  */
 	      expld.dataseg.base &= ~((1 << max_alignment_power) - 1);
+	      expld.dataseg.first_sec = first_sec;
 	      lang_reset_memory_regions ();
 	      one_lang_size_sections_pass (relax, check_regions);
+	      expld.dataseg.first_sec = NULL;
 	    }
 	}
       link_info.relro_start = expld.dataseg.base;
diff --git a/ld/testsuite/ld-x86-64/pr18176.d b/ld/testsuite/ld-x86-64/pr18176.d
new file mode 100644
index 0000000..3a08539
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr18176.d
@@ -0,0 +1,9 @@
+#name: PR ld/18176
+#as: --64
+#ld: -melf_x86_64 -shared -z relro -T pr18176.t -z max-page-size=0x200000 -z common-page-size=0x1000
+#readelf: -l --wide
+#target: x86_64-*-linux*
+
+#...
+  GNU_RELRO      0x04bd07 0x000000000024bd07 0x000000000024bd07 0x0022f9 0x0022f9 R   0x1
+#pass
diff --git a/ld/testsuite/ld-x86-64/pr18176.s b/ld/testsuite/ld-x86-64/pr18176.s
new file mode 100644
index 0000000..405355f
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr18176.s
@@ -0,0 +1,52 @@
+	.section .init,"ax",@progbits
+	.p2align 2
+	.space 0x1a
+	.section .text,"ax",@progbits
+	.p2align 4
+	.space 0x3ab96
+	.section .fini,"ax",@progbits
+	.p2align 2
+	.space 0x9
+	.section .foo_hdr,"a",@progbits
+	.p2align 2
+	.space 0xd14
+	.section .foo,"a",@progbits
+	.p2align 3
+	.space 0x4d5c
+	.section .rodata,"a",@progbits
+	.p2align 6
+	.space 0xa54d
+	.section .xxx,"a",@progbits
+	.space 0xf43
+	.section .yyy,"aw",@progbits
+	.space 0x1
+	.section .init_array,"aw",@init_array
+	.p2align 3
+	.space 0x10
+	.section .fini_array,"aw",@fini_array
+	.p2align 3
+	.space 0x8
+	.section	.tdata,"awT",%progbits
+	.p2align 3
+	.space 0x4
+	.section	.tbss,"awT",%nobits
+	.p2align 3
+	.space 0x40
+	.section	.data.rel.ro,"aw",%progbits
+	.p2align 6
+	.space 0x1b60
+	.section .jcr,"aw",@progbits
+	.p2align 3
+	.space 0x8
+	.section .bar,"aw",@progbits
+	.p2align 3
+	.space 0x640
+	.section	.data,"aw",%progbits
+	.p2align 5
+	.space 0x70
+	.section	.bss,"aw",%nobits
+	.p2align 6
+	.space 0x8a0
+	.section	BUS_ERROR_MAP,"aw",%progbits
+	.p2align 3
+	.space 0x230
diff --git a/ld/testsuite/ld-x86-64/pr18176.t b/ld/testsuite/ld-x86-64/pr18176.t
new file mode 100644
index 0000000..480c0cd
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr18176.t
@@ -0,0 +1,39 @@
+SECTIONS
+{
+  /* Read-only sections, merged into text segment: */
+  . = SEGMENT_START("text-segment", 0) + SIZEOF_HEADERS;
+  .hash           : { *(.hash) }
+  .gnu.hash       : { *(.gnu.hash) }
+  .dynsym         : { *(.dynsym) }
+  .dynstr         : { *(.dynstr) }
+  .init           : { *(.init) }
+  .text           : { *(.text) }
+  .fini           : { *(.fini) }
+  .rodata         : { *(.rodata) }
+  .foo_hdr : { *(.foo_hdr) }
+  .foo : { *(.foo) }
+  .xxx : { *(.xxx) }
+  . = ALIGN (CONSTANT (MAXPAGESIZE)) - ((CONSTANT (MAXPAGESIZE) - .) & (CONSTANT (MAXPAGESIZE) - 1)); . = DATA_SEGMENT_ALIGN (CONSTANT (MAXPAGESIZE), CONSTANT (COMMONPAGESIZE));
+  .yyy : { *(.yyy) }
+  .tdata	  : { *(.tdata) }
+  .tbss		  : { *(.tbss) }
+  .init_array     : { *(.init_array) }
+  .fini_array     : { *(.fini_array) }
+  .jcr            : { *(.jcr) }
+  .data.rel.ro : { *(.data.rel.ro.local* .gnu.linkonce.d.rel.ro.local.*) *(.data.rel.ro .data.rel.ro.* .gnu.linkonce.d.rel.ro.*) }
+  .dynamic        : { *(.dynamic) }
+  .bar            : { *(.bar) }
+  . = DATA_SEGMENT_RELRO_END (SIZEOF (.got.plt) >= 24 ? 24 : 0, .);
+  .got.plt        : { *(.got.plt) }
+  .data           : { *(.data) }
+  __bss_start = .;
+  .bss            :
+  {
+   *(.bss)
+   . = ALIGN(. != 0 ? 64 / 8 : 1);
+  }
+  . = ALIGN(64 / 8);
+  _end = .; PROVIDE (end = .);
+  . = DATA_SEGMENT_END (.);
+  /DISCARD/ : { *(.*) }
+}
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 7bceb7f..98514ed 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -318,6 +318,7 @@ run_dump_test "mov1d"
 run_dump_test "pr17935-1"
 run_dump_test "pr17935-2"
 run_dump_test "pr18160"
+run_dump_test "pr18176"
 
 # Must be native with the C compiler
 if { [isnative] && [which $CC] != 0 } {
-- 
2.1.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-29 13:58             ` H.J. Lu
@ 2015-03-30 12:39               ` Alan Modra
  2015-03-30 13:00                 ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2015-03-30 12:39 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On Sun, Mar 29, 2015 at 06:58:45AM -0700, H.J. Lu wrote:
> On Sat, Mar 28, 2015 at 8:48 PM, Alan Modra <amodra@gmail.com> wrote:
> > 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 to set address of the first
> section in PT_GNU_RELRO segment from lang_size_sections.

Look again at the code for exp_dataseg_relro_adjust.  If you change
expld.dataseg.base there you will change addresses of sections in the
relro segment.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-30 12:39               ` Alan Modra
@ 2015-03-30 13:00                 ` H.J. Lu
  2015-03-30 14:14                   ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2015-03-30 13:00 UTC (permalink / raw)
  To: Binutils

On Mon, Mar 30, 2015 at 5:39 AM, Alan Modra <amodra@gmail.com> wrote:
> On Sun, Mar 29, 2015 at 06:58:45AM -0700, H.J. Lu wrote:
>> On Sat, Mar 28, 2015 at 8:48 PM, Alan Modra <amodra@gmail.com> wrote:
>> > 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 to set address of the first
>> section in PT_GNU_RELRO segment from lang_size_sections.
>
> Look again at the code for exp_dataseg_relro_adjust.  If you change
> expld.dataseg.base there you will change addresses of sections in the
> relro segment.

But I don't want to change expld.dataseg.base.  I just want to
bump the address of the first section a bit.  Currently I have

                    bfd_vma pad
                      = ((newdot - expld.dataseg.min_base)
                         % expld.dataseg.maxpagesize);
                    if (pad)
                      {
                        bfd_vma nextdot
                          = expld.dataseg.maxpagesize - pad;
                        nextdot = align_power (newdot + nextdot,
                                               section_alignment);
                        if (((nextdot - newdot)
                             < expld.dataseg.pagesize)
                            && pad > (expld.dataseg.maxpagesize
                                      - expld.dataseg.pagesize))
                          newdot = nextdot;
                      }

I can keep the address increase, nextdot - newdot, even lower
than the page size.

-- 
H.J.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-30 13:00                 ` H.J. Lu
@ 2015-03-30 14:14                   ` Alan Modra
  2015-03-30 16:12                     ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2015-03-30 14:14 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On Mon, Mar 30, 2015 at 06:00:20AM -0700, H.J. Lu wrote:
> On Mon, Mar 30, 2015 at 5:39 AM, Alan Modra <amodra@gmail.com> wrote:
> > On Sun, Mar 29, 2015 at 06:58:45AM -0700, H.J. Lu wrote:
> >> On Sat, Mar 28, 2015 at 8:48 PM, Alan Modra <amodra@gmail.com> wrote:
> >> > 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 to set address of the first
> >> section in PT_GNU_RELRO segment from lang_size_sections.
> >
> > Look again at the code for exp_dataseg_relro_adjust.  If you change
> > expld.dataseg.base there you will change addresses of sections in the
> > relro segment.
> 
> But I don't want to change expld.dataseg.base.  I just want to
> bump the address of the first section a bit.

This says to me that you haven't analyzed the current relro code.

That gives me no confidence that your patch is good for more that just
the testcase you submitted.  It's clear to me that it will interact
with the existing exp_dataseg_relro_adjust code, perhaps badly in some
cases.  I'm not going to OK a patch that makes the relro code even
more difficult to maintain.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Reduce file size for PT_GNU_RELRO segment
  2015-03-30 14:14                   ` Alan Modra
@ 2015-03-30 16:12                     ` H.J. Lu
  2015-04-01  9:12                       ` Start of relro segment adjustment Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2015-03-30 16:12 UTC (permalink / raw)
  To: Binutils

On Mon, Mar 30, 2015 at 7:14 AM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Mar 30, 2015 at 06:00:20AM -0700, H.J. Lu wrote:
>> On Mon, Mar 30, 2015 at 5:39 AM, Alan Modra <amodra@gmail.com> wrote:
>> > On Sun, Mar 29, 2015 at 06:58:45AM -0700, H.J. Lu wrote:
>> >> On Sat, Mar 28, 2015 at 8:48 PM, Alan Modra <amodra@gmail.com> wrote:
>> >> > 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 to set address of the first
>> >> section in PT_GNU_RELRO segment from lang_size_sections.
>> >
>> > Look again at the code for exp_dataseg_relro_adjust.  If you change
>> > expld.dataseg.base there you will change addresses of sections in the
>> > relro segment.
>>
>> But I don't want to change expld.dataseg.base.  I just want to
>> bump the address of the first section a bit.
>
> This says to me that you haven't analyzed the current relro code.
>
> That gives me no confidence that your patch is good for more that just
> the testcase you submitted.  It's clear to me that it will interact
> with the existing exp_dataseg_relro_adjust code, perhaps badly in some
> cases.  I'm not going to OK a patch that makes the relro code even
> more difficult to maintain.
>

I am open to all suggestion as long as expld.dataseg.base is not
changed.


-- 
H.J.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Start of relro segment adjustment
  2015-03-30 16:12                     ` H.J. Lu
@ 2015-04-01  9:12                       ` Alan Modra
  2015-04-01 11:26                         ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2015-04-01  9:12 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

Adjusting the start of the relro segment in order to make it end
exactly on a page boundary runs into difficulties when sections in the
relro segment are aligned;  Adjusting the start by (next_page - end)
sometimes results in more than that adjustment occurring at the end,
overrunning the page boundary.  So when that occurs we try a new lower
start position by masking the adjusted start with the maximum section
alignment.  However, we didn't consider that this masked start address
may in fact be before the initial relro base, which is silly since
that can only increase padding at the relro end.

I've also moved some calculations closer to where they are used, and
comments closer to the relevant statements.

	* ldlang.c (lang_size_sections): When alignment of sections
	results in relro base adjustment being too large, don't go lower
	than the initial value.
	* ldexp.c (fold_binary <DATA_SEGMENT_RELRO_END>): Comment.
	* scripttempl/elf.sc (DATA_SEGMENT_ALIGN): Omit SEGMENT_SIZE
	alignment when SEGMENT_SIZE is the same as MAXPAGESIZE.

diff --git a/ld/ldexp.c b/ld/ldexp.c
index ac66cc0..9cd9e29 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -588,6 +588,8 @@ fold_binary (etree_type *tree)
 	  break;
 
 	case DATA_SEGMENT_RELRO_END:
+	  /* Operands swapped!  DATA_SEGMENT_RELRO_END(offset,exp)
+	     has offset in expld.result and exp in lhs.  */
 	  expld.dataseg.relro = exp_dataseg_relro_end;
 	  if (expld.phase == lang_first_phase_enum
 	      || expld.section != bfd_abs_section_ptr)
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 8880821..13e7b1a 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -5382,20 +5382,20 @@ lang_size_sections (bfd_boolean *relax, bfd_boolean check_regions)
   if (expld.dataseg.phase == exp_dataseg_end_seen
       && link_info.relro && expld.dataseg.relro_end)
     {
-      /* If DATA_SEGMENT_ALIGN DATA_SEGMENT_RELRO_END pair was seen, try
-	 to put expld.dataseg.relro_end on a (common) page boundary.  */
-      bfd_vma min_base, relro_end, maxpage;
+      bfd_vma initial_base, min_base, relro_end, maxpage;
 
       expld.dataseg.phase = exp_dataseg_relro_adjust;
       maxpage = expld.dataseg.maxpagesize;
-      /* MIN_BASE is the absolute minimum address we are allowed to start the
-	 read-write segment (byte before will be mapped read-only).  */
-      min_base = (expld.dataseg.min_base + maxpage - 1) & ~(maxpage - 1);
+      initial_base = expld.dataseg.base;
+      /* Try to put expld.dataseg.relro_end on a (common) page boundary.  */
       expld.dataseg.base += (-expld.dataseg.relro_end
 			     & (expld.dataseg.pagesize - 1));
       /* Compute the expected PT_GNU_RELRO segment end.  */
       relro_end = ((expld.dataseg.relro_end + expld.dataseg.pagesize - 1)
 		   & ~(expld.dataseg.pagesize - 1));
+      /* MIN_BASE is the absolute minimum address we are allowed to start the
+	 read-write segment (byte before will be mapped read-only).  */
+      min_base = (expld.dataseg.min_base + maxpage - 1) & ~(maxpage - 1);
       if (min_base + maxpage < expld.dataseg.base)
 	{
 	  expld.dataseg.base -= maxpage;
@@ -5420,16 +5420,17 @@ lang_size_sections (bfd_boolean *relax, bfd_boolean check_regions)
 		&& sec->alignment_power > max_alignment_power)
 	      max_alignment_power = sec->alignment_power;
 
-	  if (((bfd_vma) 1 << max_alignment_power) < expld.dataseg.pagesize)
-	    {
-	      /* Aligning the adjusted base guarantees the padding
-		 between sections won't change.  This is better than
-		 simply subtracting 1 << max_alignment_power which is
-		 what we used to do here.  */
-	      expld.dataseg.base &= ~((1 << max_alignment_power) - 1);
-	      lang_reset_memory_regions ();
-	      one_lang_size_sections_pass (relax, check_regions);
-	    }
+	  /* Aligning the adjusted base guarantees the padding
+	     between sections won't change.  This is better than
+	     simply subtracting 1 << max_alignment_power which is
+	     what we used to do here.  */
+	  expld.dataseg.base &= ~((1 << max_alignment_power) - 1);
+	  /* It doesn't make much sense to go lower than the initial
+	     base.  That can only increase padding.  */
+	  if (expld.dataseg.base < initial_base)
+	    expld.dataseg.base = initial_base;
+	  lang_reset_memory_regions ();
+	  one_lang_size_sections_pass (relax, check_regions);
 	}
       link_info.relro_start = expld.dataseg.base;
       link_info.relro_end = expld.dataseg.relro_end;
diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
index c9c80b0..b4f52eb 100644
--- a/ld/scripttempl/elf.sc
+++ b/ld/scripttempl/elf.sc
@@ -128,7 +128,11 @@ DATA_SEGMENT_ALIGN="ALIGN(${SEGMENT_SIZE}) + (. & (${MAXPAGESIZE} - 1))"
 DATA_SEGMENT_RELRO_END=""
 DATA_SEGMENT_END=""
 if test -n "${COMMONPAGESIZE}"; then
-  DATA_SEGMENT_ALIGN="ALIGN (${SEGMENT_SIZE}) - ((${MAXPAGESIZE} - .) & (${MAXPAGESIZE} - 1)); . = DATA_SEGMENT_ALIGN (${MAXPAGESIZE}, ${COMMONPAGESIZE})"
+  if test "${SEGMENT_SIZE}" != "${MAXPAGESIZE}"; then
+    DATA_SEGMENT_ALIGN="ALIGN (${SEGMENT_SIZE}) - ((${MAXPAGESIZE} - .) & (${MAXPAGESIZE} - 1)); . = DATA_SEGMENT_ALIGN (${MAXPAGESIZE}, ${COMMONPAGESIZE})"
+  else
+    DATA_SEGMENT_ALIGN="DATA_SEGMENT_ALIGN (${MAXPAGESIZE}, ${COMMONPAGESIZE})"
+  fi
   DATA_SEGMENT_END=". = DATA_SEGMENT_END (.);"
   DATA_SEGMENT_RELRO_END=". = DATA_SEGMENT_RELRO_END (${SEPARATE_GOTPLT-0}, .);"
 fi

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Start of relro segment adjustment
  2015-04-01  9:12                       ` Start of relro segment adjustment Alan Modra
@ 2015-04-01 11:26                         ` H.J. Lu
  0 siblings, 0 replies; 15+ messages in thread
From: H.J. Lu @ 2015-04-01 11:26 UTC (permalink / raw)
  To: Binutils

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

On Wed, Apr 1, 2015 at 2:11 AM, Alan Modra <amodra@gmail.com> wrote:
> Adjusting the start of the relro segment in order to make it end
> exactly on a page boundary runs into difficulties when sections in the
> relro segment are aligned;  Adjusting the start by (next_page - end)
> sometimes results in more than that adjustment occurring at the end,
> overrunning the page boundary.  So when that occurs we try a new lower
> start position by masking the adjusted start with the maximum section
> alignment.  However, we didn't consider that this masked start address
> may in fact be before the initial relro base, which is silly since
> that can only increase padding at the relro end.
>
> I've also moved some calculations closer to where they are used, and
> comments closer to the relevant statements.
>
>         * ldlang.c (lang_size_sections): When alignment of sections
>         results in relro base adjustment being too large, don't go lower
>         than the initial value.
>         * ldexp.c (fold_binary <DATA_SEGMENT_RELRO_END>): Comment.
>         * scripttempl/elf.sc (DATA_SEGMENT_ALIGN): Omit SEGMENT_SIZE
>         alignment when SEGMENT_SIZE is the same as MAXPAGESIZE.
>

Thanks.

I checked in this testcase.


-- 
H.J.

[-- Attachment #2: 0001-Add-a-testcase-for-PR-ld-18176.patch --]
[-- Type: text/x-patch, Size: 5466 bytes --]

From 875b5b9d147d37c99a189aa95354f9bebdd64ef5 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 1 Apr 2015 04:24:05 -0700
Subject: [PATCH] Add a testcase for PR ld/18176

	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.
---
 ld/ChangeLog                      |  1 +
 ld/testsuite/ChangeLog            |  8 ++++++
 ld/testsuite/ld-x86-64/pr18176.d  |  9 +++++++
 ld/testsuite/ld-x86-64/pr18176.s  | 52 +++++++++++++++++++++++++++++++++++++++
 ld/testsuite/ld-x86-64/pr18176.t  | 39 +++++++++++++++++++++++++++++
 ld/testsuite/ld-x86-64/x86-64.exp |  1 +
 6 files changed, 110 insertions(+)
 create mode 100644 ld/testsuite/ld-x86-64/pr18176.d
 create mode 100644 ld/testsuite/ld-x86-64/pr18176.s
 create mode 100644 ld/testsuite/ld-x86-64/pr18176.t

diff --git a/ld/ChangeLog b/ld/ChangeLog
index 718cdb6..6082e28 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -5,6 +5,7 @@
 
 2015-04-01  Alan Modra  <amodra@gmail.com>
 
+	PR ld/18176
 	* ldlang.c (lang_size_sections): When alignment of sections
 	results in relro base adjustment being too large, don't go lower
 	than the initial value.
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 47bb489..b35ab3f 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2015-04-01  H.J. Lu  <hongjiu.lu@intel.com>
+
+	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.
+
 2015-03-31  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* ld-bootstrap/bootstrap.exp (extralibs): Add -lz.
diff --git a/ld/testsuite/ld-x86-64/pr18176.d b/ld/testsuite/ld-x86-64/pr18176.d
new file mode 100644
index 0000000..3a08539
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr18176.d
@@ -0,0 +1,9 @@
+#name: PR ld/18176
+#as: --64
+#ld: -melf_x86_64 -shared -z relro -T pr18176.t -z max-page-size=0x200000 -z common-page-size=0x1000
+#readelf: -l --wide
+#target: x86_64-*-linux*
+
+#...
+  GNU_RELRO      0x04bd07 0x000000000024bd07 0x000000000024bd07 0x0022f9 0x0022f9 R   0x1
+#pass
diff --git a/ld/testsuite/ld-x86-64/pr18176.s b/ld/testsuite/ld-x86-64/pr18176.s
new file mode 100644
index 0000000..405355f
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr18176.s
@@ -0,0 +1,52 @@
+	.section .init,"ax",@progbits
+	.p2align 2
+	.space 0x1a
+	.section .text,"ax",@progbits
+	.p2align 4
+	.space 0x3ab96
+	.section .fini,"ax",@progbits
+	.p2align 2
+	.space 0x9
+	.section .foo_hdr,"a",@progbits
+	.p2align 2
+	.space 0xd14
+	.section .foo,"a",@progbits
+	.p2align 3
+	.space 0x4d5c
+	.section .rodata,"a",@progbits
+	.p2align 6
+	.space 0xa54d
+	.section .xxx,"a",@progbits
+	.space 0xf43
+	.section .yyy,"aw",@progbits
+	.space 0x1
+	.section .init_array,"aw",@init_array
+	.p2align 3
+	.space 0x10
+	.section .fini_array,"aw",@fini_array
+	.p2align 3
+	.space 0x8
+	.section	.tdata,"awT",%progbits
+	.p2align 3
+	.space 0x4
+	.section	.tbss,"awT",%nobits
+	.p2align 3
+	.space 0x40
+	.section	.data.rel.ro,"aw",%progbits
+	.p2align 6
+	.space 0x1b60
+	.section .jcr,"aw",@progbits
+	.p2align 3
+	.space 0x8
+	.section .bar,"aw",@progbits
+	.p2align 3
+	.space 0x640
+	.section	.data,"aw",%progbits
+	.p2align 5
+	.space 0x70
+	.section	.bss,"aw",%nobits
+	.p2align 6
+	.space 0x8a0
+	.section	BUS_ERROR_MAP,"aw",%progbits
+	.p2align 3
+	.space 0x230
diff --git a/ld/testsuite/ld-x86-64/pr18176.t b/ld/testsuite/ld-x86-64/pr18176.t
new file mode 100644
index 0000000..480c0cd
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr18176.t
@@ -0,0 +1,39 @@
+SECTIONS
+{
+  /* Read-only sections, merged into text segment: */
+  . = SEGMENT_START("text-segment", 0) + SIZEOF_HEADERS;
+  .hash           : { *(.hash) }
+  .gnu.hash       : { *(.gnu.hash) }
+  .dynsym         : { *(.dynsym) }
+  .dynstr         : { *(.dynstr) }
+  .init           : { *(.init) }
+  .text           : { *(.text) }
+  .fini           : { *(.fini) }
+  .rodata         : { *(.rodata) }
+  .foo_hdr : { *(.foo_hdr) }
+  .foo : { *(.foo) }
+  .xxx : { *(.xxx) }
+  . = ALIGN (CONSTANT (MAXPAGESIZE)) - ((CONSTANT (MAXPAGESIZE) - .) & (CONSTANT (MAXPAGESIZE) - 1)); . = DATA_SEGMENT_ALIGN (CONSTANT (MAXPAGESIZE), CONSTANT (COMMONPAGESIZE));
+  .yyy : { *(.yyy) }
+  .tdata	  : { *(.tdata) }
+  .tbss		  : { *(.tbss) }
+  .init_array     : { *(.init_array) }
+  .fini_array     : { *(.fini_array) }
+  .jcr            : { *(.jcr) }
+  .data.rel.ro : { *(.data.rel.ro.local* .gnu.linkonce.d.rel.ro.local.*) *(.data.rel.ro .data.rel.ro.* .gnu.linkonce.d.rel.ro.*) }
+  .dynamic        : { *(.dynamic) }
+  .bar            : { *(.bar) }
+  . = DATA_SEGMENT_RELRO_END (SIZEOF (.got.plt) >= 24 ? 24 : 0, .);
+  .got.plt        : { *(.got.plt) }
+  .data           : { *(.data) }
+  __bss_start = .;
+  .bss            :
+  {
+   *(.bss)
+   . = ALIGN(. != 0 ? 64 / 8 : 1);
+  }
+  . = ALIGN(64 / 8);
+  _end = .; PROVIDE (end = .);
+  . = DATA_SEGMENT_END (.);
+  /DISCARD/ : { *(.*) }
+}
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 7bceb7f..98514ed 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -318,6 +318,7 @@ run_dump_test "mov1d"
 run_dump_test "pr17935-1"
 run_dump_test "pr17935-2"
 run_dump_test "pr18160"
+run_dump_test "pr18176"
 
 # Must be native with the C compiler
 if { [isnative] && [which $CC] != 0 } {
-- 
2.1.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-04-01 11:26 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-19 13:02 [PATCH] Reduce file size for PT_GNU_RELRO segment H.J. Lu
2015-03-26  2:22 ` Alan Modra
2015-03-27 20:00   ` H.J. Lu
2015-03-28  4:15     ` Alan Modra
2015-03-28 18:30       ` H.J. Lu
2015-03-28 22:46         ` H.J. Lu
2015-03-29  3:49           ` Alan Modra
2015-03-29 13:58             ` H.J. Lu
2015-03-30 12:39               ` Alan Modra
2015-03-30 13:00                 ` H.J. Lu
2015-03-30 14:14                   ` Alan Modra
2015-03-30 16:12                     ` H.J. Lu
2015-04-01  9:12                       ` Start of relro segment adjustment Alan Modra
2015-04-01 11:26                         ` H.J. Lu
2015-03-29  3:19       ` [PATCH] Reduce file size for PT_GNU_RELRO segment H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).