[PATCH][gold] Fix offset calculation when applying AArch64 TLSDESC relocations.

[PATCH][gold] Fix offset calculation when applying AArch64 TLSDESC relocations.

[Igor Kudrin]

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

Hi,

If a custom linker script with an unexpected relative layout of .got
and .got.plt sections was used, gold might produce a wrong offset
when applying R_AARCH64_TLSDESC_* relocations.

This patch fixes the issue by calculating "got_tlsdesc_offset"
in a more direct way.

Best regards,
Igor Kudrin
C++ Developer, Access Softek, Inc.

---
gold/ChangeLog

	* aarch64.cc (Target_aarch64::Relocate::relocate_tls):
	Make got_tlsdesc_offset signed and fix its calculation.
	* testsuite/Makefile.am (aarch64_tlsdesc): New test.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/aarch64_tlsdesc.s: New test source file.
	* testsuite/aarch64_tlsdesc.sh: New test script.
	* testsuite/aarch64_tlsdesc.t: New test linker script.

[-- Attachment #2: gold-aarch64-tlsdesc-offset.patch.txt --]
[-- Type: text/plain, Size: 9372 bytes --]

diff --git a/gold/aarch64.cc b/gold/aarch64.cc
index 58c744967a5..2194a21de2a 100644
--- a/gold/aarch64.cc
+++ b/gold/aarch64.cc
@@ -7563,15 +7563,15 @@ Target_aarch64<size, big_endian>::Relocate::relocate_tls(
 	    tls_got_offset_type = (tlsopt == tls::TLSOPT_TO_IE
 				   ? GOT_TYPE_TLS_OFFSET
 				   : GOT_TYPE_TLS_DESC);
-	    unsigned int got_tlsdesc_offset = 0;
+	    int got_tlsdesc_offset = 0;
 	    if (r_type != elfcpp::R_AARCH64_TLSDESC_CALL
 		&& tlsopt == tls::TLSOPT_NONE)
 	      {
 		// We created GOT entries in the .got.tlsdesc portion of the
 		// .got.plt section, but the offset stored in the symbol is the
 		// offset within .got.tlsdesc.
-		got_tlsdesc_offset = (target->got_->data_size()
-				      + target->got_plt_section()->data_size());
+		got_tlsdesc_offset = (target->got_tlsdesc_->address()
+				      - target->got_->address());
 	      }
 	    typename elfcpp::Elf_types<size>::Elf_Addr got_entry_address;
 	    if (gsym != NULL)
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 26ee77abccf..b9d9c8ce92e 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -3829,6 +3829,17 @@ pr21430.stdout: pr21430
 
 MOSTLYCLEANFILES += pr21430
 
+check_SCRIPTS += aarch64_tlsdesc.sh
+check_DATA += aarch64_tlsdesc.stdout
+aarch64_tlsdesc.o: aarch64_tlsdesc.s
+	$(TEST_AS) -o $@ $<
+aarch64_tlsdesc: aarch64_tlsdesc.o $(srcdir)/aarch64_tlsdesc.t ../ld-new
+	../ld-new $< -shared -T $(srcdir)/aarch64_tlsdesc.t -o $@
+aarch64_tlsdesc.stdout: aarch64_tlsdesc
+	$(TEST_OBJDUMP) -dR -j.text -j.got.plt $< > $@
+
+MOSTLYCLEANFILES += aarch64_tlsdesc
+
 endif DEFAULT_TARGET_AARCH64
 
 if DEFAULT_TARGET_S390
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index eae68b56cb4..8e71ba71fe1 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -978,13 +978,16 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @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@	pr21430.sh
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	pr21430.sh \
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	aarch64_tlsdesc.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@	pr21430.stdout
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	pr21430.stdout \
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	aarch64_tlsdesc.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@	pr21430
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	pr21430 \
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	aarch64_tlsdesc
 @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 \
@@ -5310,6 +5313,8 @@ 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)
+aarch64_tlsdesc.sh.log: aarch64_tlsdesc.sh
+	@p='aarch64_tlsdesc.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
@@ -7883,6 +7888,12 @@ uninstall-am:
 @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_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@aarch64_tlsdesc.o: aarch64_tlsdesc.s
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_AS) -o $@ $<
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@aarch64_tlsdesc: aarch64_tlsdesc.o $(srcdir)/aarch64_tlsdesc.t ../ld-new
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	../ld-new $< -shared -T $(srcdir)/aarch64_tlsdesc.t -o $@
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@aarch64_tlsdesc.stdout: aarch64_tlsdesc
+@DEFAULT_TARGET_AARCH64_TRUE@@NATIVE_OR_CROSS_LINKER_TRUE@	$(TEST_OBJDUMP) -dR -j.text -j.got.plt $< > $@
 @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/aarch64_tlsdesc.s b/gold/testsuite/aarch64_tlsdesc.s
new file mode 100644
index 00000000000..b1becd62c7f
--- /dev/null
+++ b/gold/testsuite/aarch64_tlsdesc.s
@@ -0,0 +1,12 @@
+        .global test, var
+        .text
+test:
+        adrp    x0, :tlsdesc:var
+        ldr     x1, [x0, :tlsdesc_lo12:var]
+        add     x0, x0, :tlsdesc_lo12:var
+        .tlsdesccall var
+        blr     x1
+
+        .section        .tbss,"awT",%nobits
+var:
+        .zero   4
diff --git a/gold/testsuite/aarch64_tlsdesc.sh b/gold/testsuite/aarch64_tlsdesc.sh
new file mode 100755
index 00000000000..026f196dd3d
--- /dev/null
+++ b/gold/testsuite/aarch64_tlsdesc.sh
@@ -0,0 +1,110 @@
+#!/bin/sh
+
+# aarch64_tlsdesc.sh -- test R_AARCH64_TLSDESC_* relocations.
+
+# 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=aarch64_tlsdesc.stdout
+
+get_address_by_reloc()
+{
+    var=$1
+    pattern="\<R_AARCH64_TLSDESC\>"
+    found=$(grep "$pattern" "$file")
+    if test -z "$found"; then
+        echo "GOT entry for a TLS symbol is 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 -F'[: ]' '{ print $1 }')"
+}
+
+check_adrp()
+{
+    pattern="\<adrp[[:space:]]\+x0, 0\>"
+    found=$(grep "$pattern" "$file")
+    if test -z "$found"; then
+        echo "An ADRP immediate is supposed to be 0"
+        echo "Search pattern: $pattern"
+        echo ""
+        echo "Actual output below:"
+        cat "$file"
+        exit 1
+    fi
+}
+
+get_address_from_ldr()
+{
+    var=$1
+    pattern="\<ldr[[:space:]]\+x1\>"
+    found=$(grep "$pattern" "$file")
+    if test -z "$found"; then
+        echo "An LDR instruction is not found in file $file."
+        echo "Search pattern: $pattern"
+        echo ""
+        echo "Actual output below:"
+        cat "$file"
+        exit 1
+    fi
+    eval $var="$(echo $found | awk -F'[#\\]]' '{ print $2 }')"
+}
+
+get_address_from_add()
+{
+    var=$1
+    pattern="\<add[[:space:]]\+x0\>"
+    found=$(grep "$pattern" "$file")
+    if test -z "$found"; then
+        echo "An ADD instruction is not found in file $file."
+        echo "Search pattern: $pattern"
+        echo ""
+        echo "Actual output below:"
+        cat "$file"
+        exit 1
+    fi
+    eval $var="$(echo $found | awk -F'#' '{ print $2 }')"
+}
+
+check_adrp
+get_address_by_reloc address_by_reloc
+get_address_from_ldr address_from_ldr
+get_address_from_add address_from_add
+
+if test $(($address_by_reloc)) -ne $(($address_from_ldr)); then
+    echo "The address in LDR instruction is wrong."
+    echo ""
+    echo "Actual output below:"
+    cat "$file"
+    exit 1
+fi
+
+if test $(($address_by_reloc)) -ne $(($address_from_add)); then
+    echo "The address in ADD instruction is wrong."
+    echo ""
+    echo "Actual output below:"
+    cat "$file"
+    exit 1
+fi
+
+exit 0
diff --git a/gold/testsuite/aarch64_tlsdesc.t b/gold/testsuite/aarch64_tlsdesc.t
new file mode 100644
index 00000000000..f0502ee6f4d
--- /dev/null
+++ b/gold/testsuite/aarch64_tlsdesc.t
@@ -0,0 +1,6 @@
+SECTIONS
+{
+  .text : { *(.text) }
+  .got.plt : { *(.got.plt) }
+  .got : { *(.got) }
+}

Re: [PATCH][gold] Fix offset calculation when applying AArch64 TLSDESC relocations.

[Jing Yu via binutils]

Thanks for the patch. It looks good to me.
BTW, the attachment has .txt suffix. Can you resend the patch?

Thanks,
Jing

On Fri, Aug 25, 2017 at 7:10 AM, Igor Kudrin <ikudrin@accesssoftek.com> wrote:
> Hi,
>
> If a custom linker script with an unexpected relative layout of .got
> and .got.plt sections was used, gold might produce a wrong offset
> when applying R_AARCH64_TLSDESC_* relocations.
>
> This patch fixes the issue by calculating "got_tlsdesc_offset"
> in a more direct way.
>
> Best regards,
> Igor Kudrin
> C++ Developer, Access Softek, Inc.
>
> ---
> gold/ChangeLog
>
>         * aarch64.cc (Target_aarch64::Relocate::relocate_tls):
>         Make got_tlsdesc_offset signed and fix its calculation.
>         * testsuite/Makefile.am (aarch64_tlsdesc): New test.
>         * testsuite/Makefile.in: Regenerate.
>         * testsuite/aarch64_tlsdesc.s: New test source file.
>         * testsuite/aarch64_tlsdesc.sh: New test script.
>         * testsuite/aarch64_tlsdesc.t: New test linker script.

Re: [PATCH][gold] Fix offset calculation when applying AArch64 TLSDESC relocations.

[Cary Coutant]

> If a custom linker script with an unexpected relative layout of .got
> and .got.plt sections was used, gold might produce a wrong offset
> when applying R_AARCH64_TLSDESC_* relocations.
>
> This patch fixes the issue by calculating "got_tlsdesc_offset"
> in a more direct way.
>
> Best regards,
> Igor Kudrin
> C++ Developer, Access Softek, Inc.
>
> ---
> gold/ChangeLog
>
>         * aarch64.cc (Target_aarch64::Relocate::relocate_tls):
>         Make got_tlsdesc_offset signed and fix its calculation.
>         * testsuite/Makefile.am (aarch64_tlsdesc): New test.
>         * testsuite/Makefile.in: Regenerate.
>         * testsuite/aarch64_tlsdesc.s: New test source file.
>         * testsuite/aarch64_tlsdesc.sh: New test script.
>         * testsuite/aarch64_tlsdesc.t: New test linker script.

This is OK. Thanks!

-cary

Re: [PATCH][gold] Fix offset calculation when applying AArch64 TLSDESC relocations.

[Igor Kudrin]

Thank you! Could you apply it then?

Best Regards,
Igor Kudrin
C++ Developer, Access Softek, Inc.

________________________________________
From: Cary Coutant <ccoutant@gmail.com>
Sent: Monday, August 28, 2017 12:07 PM
To: Igor Kudrin
Cc: binutils@sourceware.org; Jing Yu; Han Shen
Subject: Re: [PATCH][gold] Fix offset calculation when applying AArch64 TLSDESC relocations.

> If a custom linker script with an unexpected relative layout of .got
> and .got.plt sections was used, gold might produce a wrong offset
> when applying R_AARCH64_TLSDESC_* relocations.
>
> This patch fixes the issue by calculating "got_tlsdesc_offset"
> in a more direct way.
>
> Best regards,
> Igor Kudrin
> C++ Developer, Access Softek, Inc.
>
> ---
> gold/ChangeLog
>
>         * aarch64.cc (Target_aarch64::Relocate::relocate_tls):
>         Make got_tlsdesc_offset signed and fix its calculation.
>         * testsuite/Makefile.am (aarch64_tlsdesc): New test.
>         * testsuite/Makefile.in: Regenerate.
>         * testsuite/aarch64_tlsdesc.s: New test source file.
>         * testsuite/aarch64_tlsdesc.sh: New test script.
>         * testsuite/aarch64_tlsdesc.t: New test linker script.

This is OK. Thanks!

-cary