[PATCH] PR gold/21430 Fix misplacement of a relaxed section on AArch64

[PATCH] PR gold/21430 Fix misplacement of a relaxed section on AArch64

[Igor Kudrin]

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

Hi,

When linking a large executable file for AArch64, gold might put a relaxed
section to a wrong position.

Here is a simplified reproducer:

$ cat > test.s << EOF
.globl _start, foo, bar

.section ".text.start", "ax"
_start:
    bl foo
    .space 0x7000000

.section ".text.bar", "ax"
bar:
    .space 0x1000000
    .size bar, .-bar

.section ".text.foo", "ax"
foo:
    b _start
EOF
$ aarch64-linux-gnu-as test.s -o test.o
$ ./gold-master test.o -o test.out
$ nm -S test.out | grep "foo\|bar"
00000000074000c4 0000000001000000 T bar
00000000084000b4 T foo

As you can see, the address for 'foo' lays inside 'bar', and so does
the content of its section.

See also https://sourceware.org/bugzilla/show_bug.cgi?id=21430.

Best regards,
Igor Kudrin

---
gold/ChangeLog

	* aarch64.cc
	(AArch64_relobj::convert_input_section_to_relaxed_section):
	Set the section offset to -1ULL.
	(Target_aarch64::relocate_section): Adjust the view in case
	of a relaxed input section.
	* testsuite/Makefile.am (pr21430): New test.
	* testsuite/Makefile.in: Regenerate
	* testsuite/pr21430.s: New test source file.
	* testsuite/pr21430.sh: New test script.

[-- Attachment #2: gold-pr21430-misplacing-relaxed-section-aarch64.patch.txt --]
[-- Type: text/plain, Size: 8726 bytes --]

diff --git a/gold/aarch64.cc b/gold/aarch64.cc
index b282ccf..c9bb6b7 100644
--- a/gold/aarch64.cc
+++ b/gold/aarch64.cc
@@ -1769,10 +1769,11 @@ class AArch64_relobj : public Sized_relobj_file<size, big_endian>
 
   // Convert regular input section with index SHNDX to a relaxed section.
   void
-  convert_input_section_to_relaxed_section(unsigned /* shndx */)
+  convert_input_section_to_relaxed_section(unsigned shndx)
   {
     // The stubs have relocations and we need to process them after writing
     // out the stubs.  So relocation now must follow section write.
+    this->set_section_offset(shndx, -1ULL);
     this->set_relocs_must_follow_section_writes();
   }
 
@@ -7960,6 +7961,7 @@ Target_aarch64<size, big_endian>::relocate_section(
     section_size_type view_size,
     const Reloc_symbol_changes* reloc_symbol_changes)
 {
+  typedef typename elfcpp::Elf_types<size>::Elf_Addr Address;
   typedef Target_aarch64<size, big_endian> Aarch64;
   typedef typename Target_aarch64<size, big_endian>::Relocate AArch64_relocate;
   typedef gold::Default_classify_reloc<elfcpp::SHT_RELA, size, big_endian>
@@ -7967,6 +7969,29 @@ Target_aarch64<size, big_endian>::relocate_section(
 
   gold_assert(sh_type == elfcpp::SHT_RELA);
 
+  // See if we are relocating a relaxed input section.  If so, the view
+  // covers the whole output section and we need to adjust accordingly.
+  if (needs_special_offset_handling)
+    {
+      const Output_relaxed_input_section* poris =
+	output_section->find_relaxed_input_section(relinfo->object,
+						   relinfo->data_shndx);
+      if (poris != NULL)
+	{
+	  Address section_address = poris->address();
+	  section_size_type section_size = poris->data_size();
+
+	  gold_assert((section_address >= address)
+		      && ((section_address + section_size)
+			  <= (address + view_size)));
+
+	  off_t offset = section_address - address;
+	  view += offset;
+	  address += offset;
+	  view_size = section_size;
+	}
+    }
+
   gold::relocate_section<size, big_endian, Aarch64, AArch64_relocate,
 			 gold::Default_comdat_behavior, Classify_reloc>(
     relinfo,
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index d0803d2..26ee77a 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -3818,6 +3818,17 @@ aarch64_relocs.stdout: aarch64_relocs
 
 MOSTLYCLEANFILES += aarch64_relocs
 
+check_SCRIPTS += pr21430.sh
+check_DATA += pr21430.stdout
+pr21430.o: pr21430.s
+	$(TEST_AS) -o $@ $<
+pr21430: pr21430.o ../ld-new
+	../ld-new -o $@ $<
+pr21430.stdout: pr21430
+	$(TEST_NM) -S $< > $@
+
+MOSTLYCLEANFILES += pr21430
+
 endif DEFAULT_TARGET_AARCH64
 
 if DEFAULT_TARGET_S390
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index 133e733..eae68b5 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -977,11 +977,14 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_target2_got_rel \
 @DEFAULT_TARGET_ARM_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	arm_target_lazy_init
 @DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_99 = aarch64_reloc_none.sh \
-@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	aarch64_relocs.sh
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	aarch64_relocs.sh \
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	pr21430.sh
 @DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_100 = aarch64_reloc_none.stdout \
-@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	aarch64_relocs.stdout
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	aarch64_relocs.stdout \
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	pr21430.stdout
 @DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_101 = aarch64_reloc_none \
-@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	aarch64_relocs
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	aarch64_relocs \
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	pr21430
 @DEFAULT_TARGET_S390_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_102 = split_s390.sh
 @DEFAULT_TARGET_S390_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@am__append_103 = split_s390_z1.stdout split_s390_z2.stdout split_s390_z3.stdout \
 @DEFAULT_TARGET_S390_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	split_s390_z4.stdout split_s390_n1.stdout split_s390_n2.stdout \
@@ -5305,6 +5308,8 @@ aarch64_reloc_none.sh.log: aarch64_reloc_none.sh
 	@p='aarch64_reloc_none.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 aarch64_relocs.sh.log: aarch64_relocs.sh
 	@p='aarch64_relocs.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+pr21430.sh.log: pr21430.sh
+	@p='pr21430.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 split_s390.sh.log: split_s390.sh
 	@p='split_s390.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 dwp_test_1.sh.log: dwp_test_1.sh
@@ -7872,6 +7877,12 @@ uninstall-am:
 @DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	../ld-new -o $@ aarch64_relocs.o aarch64_globals.o -e0 --emit-relocs
 @DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@aarch64_relocs.stdout: aarch64_relocs
 @DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_OBJDUMP) -dr $< > $@
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@pr21430.o: pr21430.s
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_AS) -o $@ $<
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@pr21430: pr21430.o ../ld-new
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	../ld-new -o $@ $<
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@pr21430.stdout: pr21430
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_NM) -S $< > $@
 @DEFAULT_TARGET_S390_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@split_s390_1_z1.o: split_s390_1_z1.s
 @DEFAULT_TARGET_S390_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_AS) -m31 -o $@ $<
 @DEFAULT_TARGET_S390_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@split_s390_1_z2.o: split_s390_1_z2.s
diff --git a/gold/testsuite/pr21430.s b/gold/testsuite/pr21430.s
new file mode 100644
index 0000000..3c6c1fa
--- /dev/null
+++ b/gold/testsuite/pr21430.s
@@ -0,0 +1,15 @@
+.globl _start, foo, bar
+
+.section ".text.start", "ax"
+_start:
+    bl foo
+    .space 0x7000000
+
+.section ".text.bar", "ax"
+bar:
+    .space 0x1000000
+    .size bar, .-bar
+
+.section ".text.foo", "ax"
+foo:
+    b _start
diff --git a/gold/testsuite/pr21430.sh b/gold/testsuite/pr21430.sh
new file mode 100755
index 0000000..826a3c3
--- /dev/null
+++ b/gold/testsuite/pr21430.sh
@@ -0,0 +1,73 @@
+#!/bin/sh
+
+# pr21430.sh -- test the position of a relaxed section.
+
+# Copyright (C) 2017 Free Software Foundation, Inc.
+# Written by Igor Kudrin  <ikudrin@accesssoftek.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.
+
+file=pr21430.stdout
+
+get_symbol_address()
+{
+    symbol=$1
+    var=$2
+    pattern="\<$symbol\>"
+    found=`grep "$pattern" "$file"`
+    if test -z "$found"; then
+        echo "Symbol '$symbol' not found in file $file."
+        echo "Search pattern: $pattern"
+        echo ""
+        echo "Actual output below:"
+        cat "$file"
+        exit 1
+    fi
+    eval $var="0x0`echo $found | awk '{ print $1 }'`"
+}
+
+get_symbol_size()
+{
+    symbol=$1
+    var=$2
+    pattern="\<$symbol\>"
+    found=`grep "$pattern" "$file"`
+    if test -z "$found"; then
+        echo "Symbol '$symbol' not found in file $file."
+        echo "Search pattern: $pattern"
+        echo ""
+        echo "Actual output below:"
+        cat "$file"
+        exit 1
+    fi
+    eval $var="0x0`echo $found | awk '{ print $2 }'`"
+}
+
+get_symbol_address "bar" bar_address
+get_symbol_size "bar" bar_size
+get_symbol_address "foo" foo_address
+
+if test $(($foo_address)) -lt $(($bar_address+$bar_size)); then
+    echo "'foo' should not overlap the content of 'bar'."
+    echo ""
+    echo "Actual output below:"
+    cat "$file"
+    exit 1
+fi
+
+exit 0

Re: [PATCH] PR gold/21430 Fix misplacement of a relaxed section on AArch64

[Han Shen via binutils]

Hi Igor, thanks for quick fixing this. This looks good to me. Waiting
for Cary's review & approval.

Thanks,
Han

On Wed, Apr 26, 2017 at 10:01 AM, Igor Kudrin <ikudrin@accesssoftek.com> wrote:
> Hi,
>
> When linking a large executable file for AArch64, gold might put a relaxed
> section to a wrong position.
>
> Here is a simplified reproducer:
>
> $ cat > test.s << EOF
> .globl _start, foo, bar
>
> .section ".text.start", "ax"
> _start:
>     bl foo
>     .space 0x7000000
>
> .section ".text.bar", "ax"
> bar:
>     .space 0x1000000
>     .size bar, .-bar
>
> .section ".text.foo", "ax"
> foo:
>     b _start
> EOF
> $ aarch64-linux-gnu-as test.s -o test.o
> $ ./gold-master test.o -o test.out
> $ nm -S test.out | grep "foo\|bar"
> 00000000074000c4 0000000001000000 T bar
> 00000000084000b4 T foo
>
> As you can see, the address for 'foo' lays inside 'bar', and so does
> the content of its section.
>
> See also https://sourceware.org/bugzilla/show_bug.cgi?id=21430.
>
> Best regards,
> Igor Kudrin
>
> ---
> gold/ChangeLog
>
>         * aarch64.cc
>         (AArch64_relobj::convert_input_section_to_relaxed_section):
>         Set the section offset to -1ULL.
>         (Target_aarch64::relocate_section): Adjust the view in case
>         of a relaxed input section.
>         * testsuite/Makefile.am (pr21430): New test.
>         * testsuite/Makefile.in: Regenerate
>         * testsuite/pr21430.s: New test source file.
>         * testsuite/pr21430.sh: New test script.



-- 
Han Shen |  Software Engineer |  shenhan@google.com |  +1-650-440-3330

Re: [PATCH] PR gold/21430 Fix misplacement of a relaxed section on AArch64

[Cary Coutant]

> gold/ChangeLog
>
>         PR gold/21430
>         * aarch64.cc
>         (AArch64_relobj::convert_input_section_to_relaxed_section):
>         Set the section offset to -1ULL.
>         (Target_aarch64::relocate_section): Adjust the view in case
>         of a relaxed input section.
>         * testsuite/Makefile.am (pr21430): New test.
>         * testsuite/Makefile.in: Regenerate
>         * testsuite/pr21430.s: New test source file.
>         * testsuite/pr21430.sh: New test script.

Approved and applied. Thanks!

-cary