This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Reduce file size for PT_GNU_RELRO segment


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]