[gold patch] Fix problem with alignment of relro sections

Cary Coutant ccoutant@google.com
Fri Oct 15 19:16:00 GMT 2010


If any relro sections have unusual alignment that cause padding to be
introduced, the alignment of the PT_GNU_RELRO segment is not
calculated correctly. This patch fixes gold to account for any
alignment padding correctly. It also fixes a problem where gold was
trying to optimize the page alignment of the data segment to make it
fit on one less page, while simultaneously trying to align the relro
segment. The result was a waste of a page in the output file.

There is still an open question about the ARM port -- the .got
section, which includes the GOT PLT, is currently placed entirely
within relro. I don't think that's intended (all but the very
beginning of the GOT PLT should be on the page following the relro
segment), but I'll leave it for a separate patch.

OK?

-cary

        * layout.cc (Layout::set_segment_offsets): Don't try to realign data
        segment that has been aligned for relro.
        * output.cc (Output_segment::set_section_addresses): Change signature;
        adjust all callers.  Account for alignment when totalling
        size of relro sections.
        * output.h (Output_segment::set_section_addresses): Change signature.
        * testsuite/Makefile.am (relro_test.sh, relro_test.stdout): New
        targets.
        * testsuite/Makefile.in: Regenerate.
        * testsuite/relro_test.cc: Add alignment attributes to test proper
        alignment of relro sections when padding is necessary.
        * testsuite/relro_test.sh: New script.
-------------- next part --------------
Index: layout.cc
===================================================================
RCS file: /cvs/src/src/gold/layout.cc,v
retrieving revision 1.182
diff -u -p -r1.182 layout.cc
--- layout.cc	14 Oct 2010 22:10:22 -0000	1.182
+++ layout.cc	15 Oct 2010 18:58:26 -0000
@@ -2656,8 +2656,10 @@ Layout::set_segment_offsets(const Target
 	    }
 
 	  unsigned int shndx_hold = *pshndx;
+	  bool has_relro = false;
 	  uint64_t new_addr = (*p)->set_section_addresses(this, false, addr,
-							  increase_relro,
+							  &increase_relro,
+							  &has_relro,
                                                           &off, pshndx);
 
 	  // Now that we know the size of this segment, we may be able
@@ -2666,7 +2668,7 @@ Layout::set_segment_offsets(const Target
 	  // page.  Here we use the real machine page size rather than
 	  // the ABI mandated page size.
 
-	  if (!are_addresses_set && aligned_addr != addr)
+	  if (!are_addresses_set && !has_relro && aligned_addr != addr)
 	    {
 	      uint64_t first_off = (common_pagesize
 				    - (aligned_addr
@@ -2684,7 +2686,8 @@ Layout::set_segment_offsets(const Target
 		  off = orig_off + ((addr - orig_addr) & (abi_pagesize - 1));
 		  off = align_file_offset(off, addr, abi_pagesize);
 		  new_addr = (*p)->set_section_addresses(this, true, addr,
-							 increase_relro,
+							 &increase_relro,
+							 &has_relro,
                                                          &off, pshndx);
 		}
 	    }
Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.134
diff -u -p -r1.134 output.cc
--- output.cc	25 Aug 2010 08:36:54 -0000	1.134
+++ output.cc	15 Oct 2010 18:58:26 -0000
@@ -3662,12 +3662,14 @@ Output_segment::has_dynamic_reloc_list(c
 uint64_t
 Output_segment::set_section_addresses(const Layout* layout, bool reset,
                                       uint64_t addr,
-				      unsigned int increase_relro,
+				      unsigned int* pincrease_relro,
+				      bool* has_relro,
 				      off_t* poff,
 				      unsigned int* pshndx)
 {
   gold_assert(this->type_ == elfcpp::PT_LOAD);
 
+  uint64_t last_relro_pad = 0;
   off_t orig_off = *poff;
 
   // If we have relro sections, we need to pad forward now so that the
@@ -3678,6 +3680,7 @@ Output_segment::set_section_addresses(co
     {
       uint64_t relro_size = 0;
       off_t off = *poff;
+      uint64_t max_align = 0;
       for (int i = 0; i < static_cast<int>(ORDER_MAX); ++i)
 	{
 	  Output_data_list* pdl = &this->output_lists_[i];
@@ -3689,6 +3692,10 @@ Output_segment::set_section_addresses(co
 	      Output_section* pos = (*p)->output_section();
 	      if (!pos->is_relro())
 		break;
+	      uint64_t align = (*p)->addralign();
+	      if (align > max_align)
+		max_align = align;
+	      relro_size = align_address(relro_size, align);
 	      if ((*p)->is_address_valid())
 		relro_size += (*p)->data_size();
 	      else
@@ -3703,12 +3710,20 @@ Output_segment::set_section_addresses(co
 	  if (p != pdl->end())
 	    break;
 	}
-      relro_size += increase_relro;
+      relro_size += *pincrease_relro;
+      // Pad the total relro size to a multiple of the maximum
+      // section alignment seen.
+      uint64_t aligned_size = align_address(relro_size, max_align);
+      // Note the amount of padding added after the last relro section.
+      last_relro_pad = aligned_size - relro_size;
+      // Adjust the INCREASE_RELRO parameter to include the padding.
+      *pincrease_relro += last_relro_pad;
+      *has_relro = true;
 
       uint64_t page_align = parameters->target().common_pagesize();
 
       // Align to offset N such that (N + RELRO_SIZE) % PAGE_ALIGN == 0.
-      uint64_t desired_align = page_align - (relro_size % page_align);
+      uint64_t desired_align = page_align - (aligned_size % page_align);
       if (desired_align < *poff % page_align)
 	*poff += page_align - *poff % page_align;
       *poff += desired_align - *poff % page_align;
@@ -3739,6 +3754,11 @@ Output_segment::set_section_addresses(co
       addr = this->set_section_list_addresses(layout, reset,
 					      &this->output_lists_[i],
 					      addr, poff, pshndx, &in_tls);
+      if (i == static_cast<int>(ORDER_RELRO_LAST))
+	{
+	  *poff += last_relro_pad;
+	  addr += last_relro_pad;
+	}
       if (i < static_cast<int>(ORDER_SMALL_BSS))
 	{
 	  this->filesz_ = *poff - orig_off;
Index: output.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.113
diff -u -p -r1.113 output.h
--- output.h	25 Aug 2010 08:36:54 -0000	1.113
+++ output.h	15 Oct 2010 18:58:26 -0000
@@ -3908,8 +3908,8 @@ class Output_segment
   // *PSHNDX.  This should only be called for a PT_LOAD segment.
   uint64_t
   set_section_addresses(const Layout*, bool reset, uint64_t addr,
-			unsigned int increase_relro, off_t* poff,
-			unsigned int* pshndx);
+			unsigned int* pincrease_relro, bool* has_relro,
+			off_t* poff, unsigned int* pshndx);
 
   // Set the minimum alignment of this segment.  This may be adjusted
   // upward based on the section alignments.
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.150
diff -u -p -r1.150 Makefile.am
--- testsuite/Makefile.am	14 Oct 2010 22:10:22 -0000	1.150
+++ testsuite/Makefile.am	15 Oct 2010 18:58:26 -0000
@@ -1076,6 +1076,8 @@ protected_3.err: protected_4_pic.o gccte
 	fi
 
 check_PROGRAMS += relro_test
+check_SCRIPTS += relro_test.sh
+check_DATA += relro_test.stdout
 relro_test_SOURCES = relro_test_main.cc
 relro_test_DEPENDENCIES = gcctestdir/ld relro_test.so
 relro_test_LDFLAGS = -Bgcctestdir/ -Wl,-R,.
@@ -1084,6 +1086,8 @@ relro_test.so: gcctestdir/ld relro_test_
 	$(CXXLINK) -Bgcctestdir/ -shared -Wl,-z,relro relro_test_pic.o
 relro_test_pic.o: relro_test.cc
 	$(CXXCOMPILE) -c -fpic -o $@ $<
+relro_test.stdout: relro_test.so
+	$(TEST_READELF) -SlW relro_test.so > relro_test.stdout
 
 check_PROGRAMS += relro_strip_test
 relro_strip_test_SOURCES = relro_test_main.cc
Index: testsuite/Makefile.in
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.in,v
retrieving revision 1.159
diff -u -p -r1.159 Makefile.in
--- testsuite/Makefile.in	14 Oct 2010 22:10:22 -0000	1.159
+++ testsuite/Makefile.in	15 Oct 2010 18:58:26 -0000
@@ -75,6 +75,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__E
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_1.sh ver_test_2.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_4.sh ver_test_5.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_7.sh ver_test_10.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	relro_test.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_matching_test.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	script_test_3.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	script_test_4.sh \
@@ -112,6 +113,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__E
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_2.syms ver_test_4.syms \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_5.syms ver_test_7.syms \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_10.syms protected_3.err \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	relro_test.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_matching_test.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	script_test_3.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	script_test_4.stdout \
@@ -3301,6 +3303,8 @@ ver_test_7.sh.log: ver_test_7.sh
 	@p='ver_test_7.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 ver_test_10.sh.log: ver_test_10.sh
 	@p='ver_test_10.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+relro_test.sh.log: relro_test.sh
+	@p='relro_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 ver_matching_test.sh.log: ver_matching_test.sh
 	@p='ver_matching_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 script_test_3.sh.log: script_test_3.sh
@@ -4196,6 +4200,8 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -shared -Wl,-z,relro relro_test_pic.o
 @GCC_TRUE@@NATIVE_LINKER_TRUE@relro_test_pic.o: relro_test.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXCOMPILE) -c -fpic -o $@ $<
+@GCC_TRUE@@NATIVE_LINKER_TRUE@relro_test.stdout: relro_test.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_READELF) -SlW relro_test.so > relro_test.stdout
 @GCC_TRUE@@NATIVE_LINKER_TRUE@relro_strip_test.so: relro_test.so
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_STRIP) -o $@ $<
 @GCC_TRUE@@NATIVE_LINKER_TRUE@relro_script_test.so: gcctestdir/ld relro_script_test.t relro_test_pic.o
@@ -4560,10 +4566,10 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_READELF) -lWS  $< > $@
 @NATIVE_OR_CROSS_LINKER_TRUE@script_test_10.o: script_test_10.s
 @NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_AS) -o $@ $<
-@NATIVE_OR_CROSS_LINKER_TRUE@script_test_10: $(srcdir)/script_test_10.t script_test_10.o
-@NATIVE_OR_CROSS_LINKER_TRUE@	../ld-new -o $@ script_test_10.o -T $(srcdir)/script_test_10.t
+@NATIVE_OR_CROSS_LINKER_TRUE@script_test_10: $(srcdir)/script_test_10.t script_test_10.o gcctestdir/ld
+@NATIVE_OR_CROSS_LINKER_TRUE@	gcctestdir/ld -o $@ script_test_10.o -T $(srcdir)/script_test_10.t
 @NATIVE_OR_CROSS_LINKER_TRUE@script_test_10.stdout: script_test_10
-@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_READELF) -SW script_test_10 > script_test_10.stdout
+@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_READELF) -SW script_test_10 > $@
 @DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@split_i386_1.o: split_i386_1.s
 @DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_AS) -o $@ $<
 @DEFAULT_TARGET_I386_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@split_i386_2.o: split_i386_2.s
Index: testsuite/relro_test.cc
===================================================================
RCS file: /cvs/src/src/gold/testsuite/relro_test.cc,v
retrieving revision 1.3
diff -u -p -r1.3 relro_test.cc
--- testsuite/relro_test.cc	13 Aug 2008 07:37:46 -0000	1.3
+++ testsuite/relro_test.cc	15 Oct 2010 18:58:26 -0000
@@ -40,10 +40,10 @@ int i1 = 1;
 static int i2 = 2;
 
 // P1 is a global relro variable.
-int* const p1 = &i1;
+int* const p1 __attribute__ ((aligned(64))) = &i1;
 
 // P2 is a local relro variable.
-int* const p2 = &i2;
+int* const p2 __attribute__ ((aligned(64))) = &i2;
 
 // Test symbol addresses.
 
Index: testsuite/relro_test.sh
===================================================================
RCS file: testsuite/relro_test.sh
diff -N testsuite/relro_test.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/relro_test.sh	15 Oct 2010 18:58:26 -0000
@@ -0,0 +1,73 @@
+#!/bin/sh
+
+# relro_test.sh -- test -z relro
+
+# Copyright 2010 Free Software Foundation, Inc.
+# Written by Cary Coutant <ccoutant@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# This test checks that the PT_GNU_RELRO segment is properly
+# aligned and is coincident with the beginning of the data segment.
+
+
+# Cleans a hexadecimal number for input to dc.
+clean_hex()
+{
+  echo "$1" | sed -e 's/0x//' -e 'y/abcdef/ABCDEF/'
+}
+
+check()
+{
+  # Get the address and length of the PT_GNU_RELRO segment.
+  RELRO_START=`grep GNU_RELRO "$1" | awk '{ print $3; }'`
+  RELRO_LEN=`grep GNU_RELRO "$1" | awk '{ print $6; }'`
+
+  if test -z "$RELRO_START"
+  then
+    echo "Did not find a PT_GNU_RELRO segment."
+    exit 1
+  fi
+
+  # Get the address and alignment of the PT_LOAD segment whose address
+  # matches the PT_GNU_RELRO segment.
+  LOAD_ALIGN=`grep LOAD "$1" | awk -v A=$RELRO_START '$3 == A { print $NF; }'`
+  LOAD_LEN=`grep LOAD "$1" | awk -v A=$RELRO_START '$3 == A { print $6; }'`
+
+  if test -z "$LOAD_LEN"
+  then
+    echo "Did not find a PT_LOAD segment matching the PT_GNU_RELRO segment."
+    exit 1
+  fi
+
+  # Compute the address of the end of the PT_GNU_RELRO segment,
+  # modulo the alignment of the PT_LOAD segment.
+  RELRO_START=`clean_hex "$RELRO_START"`
+  RELRO_LEN=`clean_hex "$RELRO_LEN"`
+  LOAD_ALIGN=`clean_hex "$LOAD_ALIGN"`
+  RELRO_END=`echo "16o 16i $RELRO_START $RELRO_LEN + p" | dc`
+  REM=`echo "16i $RELRO_END $LOAD_ALIGN % p" | dc`
+
+  if test "$REM" -ne 0
+  then
+    echo "PT_GNU_RELRO segment does not end at page boundary."
+    exit 1
+  fi
+}
+
+check relro_test.stdout


More information about the Binutils mailing list