[PATCH 0/5] aarch64 BTI stub fixes

[PATCH 0/5] aarch64 BTI stub fixes

[Szabolcs Nagy]

Large binaries with BTI can be linked incorrectly with
binutils 2.41 because PR30930.

there are other less critical related issues fixed in
this patchset as well.

Szabolcs Nagy (5):
  bfd: aarch64: Fix BTI stub optimization PR30957
  bfd: aarch64: Fix broken BTI stub PR30930
  bfd: aarch64: Fix leaks in case of BTI stub reuse
  bfd: aarch64: Avoid BTI stub for a PLT that has BTI
  ld: aarch64: Add BTI stub insertion test PR30930

 bfd/elfnn-aarch64.c                     | 89 +++++++++++++++++--------
 ld/testsuite/ld-aarch64/aarch64-elf.exp |  1 +
 ld/testsuite/ld-aarch64/bti-far-3.d     | 87 ++++++++++++++++++++++++
 ld/testsuite/ld-aarch64/bti-far-3.ld    | 12 ++++
 ld/testsuite/ld-aarch64/bti-far-3a.s    | 23 +++++++
 ld/testsuite/ld-aarch64/bti-far-3b.s    | 25 +++++++
 ld/testsuite/ld-aarch64/bti-far-3c.s    | 24 +++++++
 7 files changed, 232 insertions(+), 29 deletions(-)
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3.d
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3.ld
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3a.s
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3b.s
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3c.s

-- 
2.25.1

[PATCH 1/5] bfd: aarch64: Fix BTI stub optimization PR30957

[Szabolcs Nagy]

The instruction was looked up in the wrong input file (file of branch
source instead of branch target) when optimizing away BTI stubs in

  commit 5834f36d93cabf1a8bcc7dd7654141aed3d296bc
  bfd: aarch64: Optimize BTI stubs PR30076

This can cause adding BTI stubs when they are not necessary or removing
them when they are (the latter is a correctness issue but it is very
unlikely in practice).

Fixes bug 30957.
---
 bfd/elfnn-aarch64.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index 4592bd6da27..a0dd17faa7e 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -3675,8 +3675,7 @@ group_sections (struct elf_aarch64_link_hash_table *htab,
 /* True if the inserted stub does not break BTI compatibility.  */
 
 static bool
-aarch64_bti_stub_p (bfd *input_bfd,
-		    struct elf_aarch64_stub_hash_entry *stub_entry)
+aarch64_bti_stub_p (struct elf_aarch64_stub_hash_entry *stub_entry)
 {
   /* Stubs without indirect branch are BTI compatible.  */
   if (stub_entry->stub_type != aarch64_stub_adrp_branch
@@ -3690,7 +3689,7 @@ aarch64_bti_stub_p (bfd *input_bfd,
   file_ptr off = stub_entry->target_value;
   bfd_size_type count = sizeof (loc);
 
-  if (!bfd_get_section_contents (input_bfd, section, loc, off, count))
+  if (!bfd_get_section_contents (section->owner, section, loc, off, count))
     return false;
 
   uint32_t insn = bfd_getl32 (loc);
@@ -4637,7 +4636,7 @@ _bfd_aarch64_add_call_stub_entries (bool *stub_changed, bfd *output_bfd,
 
 	      /* A stub with indirect jump may break BTI compatibility, so
 		 insert another stub with direct jump near the target then.  */
-	      if (need_bti && !aarch64_bti_stub_p (input_bfd, stub_entry))
+	      if (need_bti && !aarch64_bti_stub_p (stub_entry))
 		{
 		  stub_entry->double_stub = true;
 		  htab->has_double_stub = true;
-- 
2.25.1

[PATCH 2/5] bfd: aarch64: Fix broken BTI stub PR30930

[Szabolcs Nagy]

Input sections are grouped together that can use the same stub area
(within reach) and these groups have a stable id.

Stubs have a name generated from the stub group id and target symbol.
When a relocation requires a stub with a name that already exists, the
stub is reused instead of adding a new one.

For an indirect branch stub another BTI stub may be inserted near the
target to provide a BTI landing pad.

The BTI stub can end up with the same stub group id and thus the same
name as the indirect stub. This happens if the target symbol is within
reach of the indirect branch stub. Then, due to the name collision,
only a single stub was emmitted which branched to itself causing an
infinite loop at runtime.

A possible solution is to just name the BTI stubs differently, but
since in the problematic case the indirect and BTI stub are in the
same stub area, a better solution is to emit a single stub with a
direct branch. The stub is still needed since the caller cannot reach
the target directly and we also want a BTI landing pad in the stub in
case other indirect stubs target the same symbol and thus need a BTI
stub.

In short we convert an indirect branch stub into a BTI stub when the
target is within reach and has no BTI. It is a hassle to change the
symbol of the stub so a BTI stub may end up with *_veneer instead of
*_bti_veneer after the conversion, but this should not matter much.
(Refactoring some of _bfd_aarch64_add_call_stub_entries would be
useful but too much for this bug fix patch.)

The same conversion to direct branch could be done even if the target
did not need a BTI. The stub groups are fixed in the current logic so
linking can fail if too many stubs are inserted and the section layout
is changed too much, but this only happens in extreme cases that can
be reasonably ignored. Because of this the target cannot go out of
reach during stub insertion so the optimization is valid, but not
implemented by this patch for the non-BTI case.

Fixes bug 30930.
---
 bfd/elfnn-aarch64.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index a0dd17faa7e..798643ade7c 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -4638,9 +4638,22 @@ _bfd_aarch64_add_call_stub_entries (bool *stub_changed, bfd *output_bfd,
 		 insert another stub with direct jump near the target then.  */
 	      if (need_bti && !aarch64_bti_stub_p (stub_entry))
 		{
+		  id_sec_bti = htab->stub_group[sym_sec->id].link_sec;
+
+		  /* If the stub with indirect jump and the BTI stub are in
+		     the same stub group: change the indirect jump stub into
+		     a BTI stub since a direct branch can reach the target.
+		     The BTI landing pad is still needed in case another
+		     stub indirectly jumps to it.  */
+		  if (id_sec_bti == id_sec)
+		    {
+		      stub_entry->stub_type = aarch64_stub_bti_direct_branch;
+		      goto skip_double_stub;
+		    }
+
 		  stub_entry->double_stub = true;
 		  htab->has_double_stub = true;
-		  id_sec_bti = htab->stub_group[sym_sec->id].link_sec;
+
 		  stub_name_bti =
 		    elfNN_aarch64_stub_name (id_sec_bti, sym_sec, hash, irela);
 		  if (!stub_name_bti)
@@ -4687,7 +4700,7 @@ _bfd_aarch64_add_call_stub_entries (bool *stub_changed, bfd *output_bfd,
 		  stub_entry->h = NULL;
 		  stub_entry->st_type = STT_FUNC;
 		}
-
+skip_double_stub:
 	      *stub_changed = true;
 	    }
 
-- 
2.25.1

[PATCH 3/5] bfd: aarch64: Fix leaks in case of BTI stub reuse

[Szabolcs Nagy]

BTI stub parameters were recomputed even if those were already set up.
This is unnecessary work and leaks the symbol name that is allocated
for the stub.
---
 bfd/elfnn-aarch64.c | 56 ++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index 798643ade7c..3adece30250 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -4665,33 +4665,41 @@ _bfd_aarch64_add_call_stub_entries (bool *stub_changed, bfd *output_bfd,
 		  stub_entry_bti =
 		    aarch64_stub_hash_lookup (&htab->stub_hash_table,
 					      stub_name_bti, false, false);
-		  if (stub_entry_bti == NULL)
-		    stub_entry_bti =
-		      _bfd_aarch64_add_stub_entry_in_group (stub_name_bti,
-							    sym_sec, htab);
-		  if (stub_entry_bti == NULL)
+		  if (stub_entry_bti != NULL)
+		    BFD_ASSERT (stub_entry_bti->stub_type
+				== aarch64_stub_bti_direct_branch);
+		  else
 		    {
-		      free (stub_name);
-		      free (stub_name_bti);
-		      goto error_ret_free_internal;
-		    }
-
-		  stub_entry_bti->target_value = sym_value + irela->r_addend;
-		  stub_entry_bti->target_section = sym_sec;
-		  stub_entry_bti->stub_type = aarch64_stub_bti_direct_branch;
-		  stub_entry_bti->h = hash;
-		  stub_entry_bti->st_type = st_type;
+		      stub_entry_bti =
+			_bfd_aarch64_add_stub_entry_in_group (stub_name_bti,
+							      sym_sec, htab);
+		      if (stub_entry_bti == NULL)
+			{
+			  free (stub_name);
+			  free (stub_name_bti);
+			  goto error_ret_free_internal;
+			}
 
-		  len = sizeof (BTI_STUB_ENTRY_NAME) + strlen (sym_name);
-		  stub_entry_bti->output_name = bfd_alloc (htab->stub_bfd, len);
-		  if (stub_entry_bti->output_name == NULL)
-		    {
-		      free (stub_name);
-		      free (stub_name_bti);
-		      goto error_ret_free_internal;
+		      stub_entry_bti->target_value =
+			sym_value + irela->r_addend;
+		      stub_entry_bti->target_section = sym_sec;
+		      stub_entry_bti->stub_type =
+			aarch64_stub_bti_direct_branch;
+		      stub_entry_bti->h = hash;
+		      stub_entry_bti->st_type = st_type;
+
+		      len = sizeof (BTI_STUB_ENTRY_NAME) + strlen (sym_name);
+		      stub_entry_bti->output_name = bfd_alloc (htab->stub_bfd,
+							       len);
+		      if (stub_entry_bti->output_name == NULL)
+			{
+			  free (stub_name);
+			  free (stub_name_bti);
+			  goto error_ret_free_internal;
+			}
+		      snprintf (stub_entry_bti->output_name, len,
+				BTI_STUB_ENTRY_NAME, sym_name);
 		    }
-		  snprintf (stub_entry_bti->output_name, len,
-			    BTI_STUB_ENTRY_NAME, sym_name);
 
 		  /* Update the indirect call stub to target the BTI stub.  */
 		  stub_entry->target_value = 0;
-- 
2.25.1

[PATCH 4/5] bfd: aarch64: Avoid BTI stub for a PLT that has BTI

[Szabolcs Nagy]

We decide to emit BTI stubs based on the instruction at the target
location. But PLT code is generated later than the stubs so we always
read 0 which is not a valid BTI.

Fix the logic to special case the PLT section: this is code the linker
generates so we know when it will have BTI.

This avoids BTI stubs in large executables where the PLTs have them
already. An alternative is to never emit BTI stubs for PLTs, instead
use BTI in the PLT if a library gets too big, however that may be
more tricky given the ordering of PLT sizing and stub insertion.

Related to bug 30957.
---
 bfd/elfnn-aarch64.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index 3adece30250..4faf642b422 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -3675,7 +3675,8 @@ group_sections (struct elf_aarch64_link_hash_table *htab,
 /* True if the inserted stub does not break BTI compatibility.  */
 
 static bool
-aarch64_bti_stub_p (struct elf_aarch64_stub_hash_entry *stub_entry)
+aarch64_bti_stub_p (struct bfd_link_info *info,
+		    struct elf_aarch64_stub_hash_entry *stub_entry)
 {
   /* Stubs without indirect branch are BTI compatible.  */
   if (stub_entry->stub_type != aarch64_stub_adrp_branch
@@ -3684,12 +3685,22 @@ aarch64_bti_stub_p (struct elf_aarch64_stub_hash_entry *stub_entry)
 
   /* Return true if the target instruction is compatible with BR x16.  */
 
+  struct elf_aarch64_link_hash_table *globals = elf_aarch64_hash_table (info);
   asection *section = stub_entry->target_section;
   bfd_byte loc[4];
   file_ptr off = stub_entry->target_value;
   bfd_size_type count = sizeof (loc);
 
-  if (!bfd_get_section_contents (section->owner, section, loc, off, count))
+  /* PLT code is not generated yet, so treat it specially.
+     Note: Checking elf_aarch64_obj_tdata.plt_type & PLT_BTI is not
+     enough because it only implies BTI in the PLT0 and tlsdesc PLT
+     entries. Normal PLT entries don't have BTI in a shared library
+     (because such PLT is normally not called indirectly and adding
+     the BTI when a stub targets a PLT would change the PLT layout
+     and it's too late for that here).  */
+  if (section == globals->root.splt)
+    memcpy (loc, globals->plt_entry, count);
+  else if (!bfd_get_section_contents (section->owner, section, loc, off, count))
     return false;
 
   uint32_t insn = bfd_getl32 (loc);
@@ -4636,7 +4647,7 @@ _bfd_aarch64_add_call_stub_entries (bool *stub_changed, bfd *output_bfd,
 
 	      /* A stub with indirect jump may break BTI compatibility, so
 		 insert another stub with direct jump near the target then.  */
-	      if (need_bti && !aarch64_bti_stub_p (stub_entry))
+	      if (need_bti && !aarch64_bti_stub_p (info, stub_entry))
 		{
 		  id_sec_bti = htab->stub_group[sym_sec->id].link_sec;
 
-- 
2.25.1

[PATCH 5/5] ld: aarch64: Add BTI stub insertion test PR30930

[Szabolcs Nagy]

The test creates a large shared library and covers a number of
BTI stub insertion cases.
---
 ld/testsuite/ld-aarch64/aarch64-elf.exp |  1 +
 ld/testsuite/ld-aarch64/bti-far-3.d     | 87 +++++++++++++++++++++++++
 ld/testsuite/ld-aarch64/bti-far-3.ld    | 12 ++++
 ld/testsuite/ld-aarch64/bti-far-3a.s    | 23 +++++++
 ld/testsuite/ld-aarch64/bti-far-3b.s    | 25 +++++++
 ld/testsuite/ld-aarch64/bti-far-3c.s    | 24 +++++++
 6 files changed, 172 insertions(+)
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3.d
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3.ld
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3a.s
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3b.s
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3c.s

diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
index b025fcbd567..fa5ac3a2cdd 100644
--- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
+++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
@@ -466,6 +466,7 @@ run_dump_test "undef-tls"
 run_dump_test "bti-far-1"
 run_dump_test "bti-far-2"
 run_dump_test "bti-far-opt"
+run_dump_test "bti-far-3"
 
 if { ![skip_sframe_tests] } {
   run_dump_test "sframe-simple-1"
diff --git a/ld/testsuite/ld-aarch64/bti-far-3.d b/ld/testsuite/ld-aarch64/bti-far-3.d
new file mode 100644
index 00000000000..4c076985a0e
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/bti-far-3.d
@@ -0,0 +1,87 @@
+#name: Check linker stubs with indirect calls handle BTI (exe).
+#source: bti-far-3a.s
+#source: bti-far-3b.s
+#source: bti-far-3c.s
+#as: -mabi=lp64
+#ld: -shared -T bti-far-3.ld
+#objdump: -dr
+
+[^:]*: *file format elf64-.*aarch64
+
+
+Disassembly of section \.plt:
+
+0000000000020000 <\.plt>:
+   20000:	d503245f 	bti	c
+   20004:	a9bf7bf0 	stp	x16, x30, \[sp, #-16\]!
+   20008:	900fff10 	adrp	x16, 20000000 <_GLOBAL_OFFSET_TABLE_>
+   2000c:	f9400e11 	ldr	x17, \[x16, #24\]
+   20010:	91006210 	add	x16, x16, #0x18
+   20014:	d61f0220 	br	x17
+   20018:	d503201f 	nop
+   2001c:	d503201f 	nop
+
+0000000000020020 <extern_func@plt>:
+   20020:	900fff10 	adrp	x16, 20000000 <_GLOBAL_OFFSET_TABLE_>
+   20024:	f9401211 	ldr	x17, \[x16, #32\]
+   20028:	91008210 	add	x16, x16, #0x20
+   2002c:	d61f0220 	br	x17
+   20030:	14000004 	b	20040 <__extern_func_bti_veneer\+0x8>
+   20034:	d503201f 	nop
+
+0000000000020038 <__extern_func_bti_veneer>:
+   20038:	d503245f 	bti	c
+   2003c:	17fffff9 	b	20020 <extern_func@plt>
+
+Disassembly of section \.text:
+
+0000000000030000 <a_func>:
+       30000:	15c00004 	b	7030010 <__b_func_veneer>
+       30004:	17ffc007 	b	20020 <extern_func@plt>
+	\.\.\.
+     7030008:	1400000a 	b	7030030 <__a_func_bti_veneer\+0x8>
+     703000c:	d503201f 	nop
+
+0000000007030010 <__b_func_veneer>:
+     7030010:	90040010 	adrp	x16, f030000 <b_func\+0x6ffffd0>
+     7030014:	9101e210 	add	x16, x16, #0x78
+     7030018:	d61f0200 	br	x16
+	\.\.\.
+
+0000000007030028 <__a_func_bti_veneer>:
+     7030028:	d503245f 	bti	c
+     703002c:	163ffff5 	b	30000 <a_func>
+	\.\.\.
+
+0000000008030030 <b_func>:
+     8030030:	15c00004 	b	f030040 <__c_func_veneer>
+     8030034:	15c00005 	b	f030048 <__a_func_veneer>
+	\.\.\.
+     f030038:	14000012 	b	f030080 <__b_func_bti_veneer\+0x8>
+     f03003c:	d503201f 	nop
+
+000000000f030040 <__c_func_veneer>:
+     f030040:	d503245f 	bti	c
+     f030044:	1440000f 	b	10030080 <c_func>
+
+000000000f030048 <__a_func_veneer>:
+     f030048:	90fc0010 	adrp	x16, 7030000 <a_func\+0x7000000>
+     f03004c:	9100a210 	add	x16, x16, #0x28
+     f030050:	d61f0200 	br	x16
+	\.\.\.
+
+000000000f030060 <__extern_func_veneer>:
+     f030060:	90f87f90 	adrp	x16, 20000 <\.plt>
+     f030064:	9100e210 	add	x16, x16, #0x38
+     f030068:	d61f0200 	br	x16
+	\.\.\.
+
+000000000f030078 <__b_func_bti_veneer>:
+     f030078:	d503245f 	bti	c
+     f03007c:	163fffed 	b	8030030 <b_func>
+	\.\.\.
+
+0000000010030080 <c_func>:
+    10030080:	17bffff2 	b	f030048 <__a_func_veneer>
+    10030084:	17bffffd 	b	f030078 <__b_func_bti_veneer>
+    10030088:	17bffff6 	b	f030060 <__extern_func_veneer>
diff --git a/ld/testsuite/ld-aarch64/bti-far-3.ld b/ld/testsuite/ld-aarch64/bti-far-3.ld
new file mode 100644
index 00000000000..3265c55b8cc
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/bti-far-3.ld
@@ -0,0 +1,12 @@
+OUTPUT_ARCH(aarch64)
+SECTIONS
+{
+  . = 0x00010000;
+  .rela.plt       : { *(.rela.plt) *(.rela.iplt) }
+  . = 0x00020000;
+  .plt            : { *(.plt) *(.iplt) }
+  . = 0x00030000;
+  .text           : { *(.text) }
+  . = 0x20000000;
+  .got            : { *(.got) *(.got.plt) }
+}
diff --git a/ld/testsuite/ld-aarch64/bti-far-3a.s b/ld/testsuite/ld-aarch64/bti-far-3a.s
new file mode 100644
index 00000000000..ed53545fed7
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/bti-far-3a.s
@@ -0,0 +1,23 @@
+	.text
+	.hidden	a_func
+	.hidden	b_func
+	.hidden	c_func
+
+	.global	a_func
+	.type	a_func, %function
+a_func:
+	b	b_func
+	b	extern_func
+
+.zero	0x07000000
+
+	.section	.note.gnu.property,"a"
+	.align	3
+	.word	4
+	.word	16
+	.word	5
+	.string	"GNU"
+	.word	3221225472
+	.word	4
+	.word	1
+	.align	3
diff --git a/ld/testsuite/ld-aarch64/bti-far-3b.s b/ld/testsuite/ld-aarch64/bti-far-3b.s
new file mode 100644
index 00000000000..1f3be3d15cf
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/bti-far-3b.s
@@ -0,0 +1,25 @@
+	.text
+	.hidden	a_func
+	.hidden	b_func
+	.hidden	c_func
+
+.zero	0x01000000
+
+	.global	b_func
+	.type	b_func, %function
+b_func:
+	b	c_func
+	b	a_func
+
+.zero	0x07000000
+
+	.section	.note.gnu.property,"a"
+	.align	3
+	.word	4
+	.word	16
+	.word	5
+	.string	"GNU"
+	.word	3221225472
+	.word	4
+	.word	1
+	.align	3
diff --git a/ld/testsuite/ld-aarch64/bti-far-3c.s b/ld/testsuite/ld-aarch64/bti-far-3c.s
new file mode 100644
index 00000000000..8e2361fd771
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/bti-far-3c.s
@@ -0,0 +1,24 @@
+	.text
+	.hidden	a_func
+	.hidden	b_func
+	.hidden	c_func
+
+.zero	0x01000000
+
+	.global	c_func
+	.type	c_func, %function
+c_func:
+	b	a_func
+	b	b_func
+	b	extern_func
+
+	.section	.note.gnu.property,"a"
+	.align	3
+	.word	4
+	.word	16
+	.word	5
+	.string	"GNU"
+	.word	3221225472
+	.word	4
+	.word	1
+	.align	3
-- 
2.25.1

[PATCH 5/5] ld: aarch64: Add BTI stub insertion test PR30930

[Szabolcs Nagy]

The test creates a large shared library and covers a number of
BTI stub insertion cases.
---
 ld/testsuite/ld-aarch64/aarch64-elf.exp |  1 +
 ld/testsuite/ld-aarch64/bti-far-3.d     | 87 +++++++++++++++++++++++++
 ld/testsuite/ld-aarch64/bti-far-3.ld    | 12 ++++
 ld/testsuite/ld-aarch64/bti-far-3a.s    | 23 +++++++
 ld/testsuite/ld-aarch64/bti-far-3b.s    | 25 +++++++
 ld/testsuite/ld-aarch64/bti-far-3c.s    | 24 +++++++
 6 files changed, 172 insertions(+)
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3.d
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3.ld
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3a.s
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3b.s
 create mode 100644 ld/testsuite/ld-aarch64/bti-far-3c.s

diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
index b025fcbd567..fa5ac3a2cdd 100644
--- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
+++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
@@ -466,6 +466,7 @@ run_dump_test "undef-tls"
 run_dump_test "bti-far-1"
 run_dump_test "bti-far-2"
 run_dump_test "bti-far-opt"
+run_dump_test "bti-far-3"
 
 if { ![skip_sframe_tests] } {
   run_dump_test "sframe-simple-1"
diff --git a/ld/testsuite/ld-aarch64/bti-far-3.d b/ld/testsuite/ld-aarch64/bti-far-3.d
new file mode 100644
index 00000000000..4c076985a0e
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/bti-far-3.d
@@ -0,0 +1,87 @@
+#name: Check linker stubs with indirect calls handle BTI (exe).
+#source: bti-far-3a.s
+#source: bti-far-3b.s
+#source: bti-far-3c.s
+#as: -mabi=lp64
+#ld: -shared -T bti-far-3.ld
+#objdump: -dr
+
+[^:]*: *file format elf64-.*aarch64
+
+
+Disassembly of section \.plt:
+
+0000000000020000 <\.plt>:
+   20000:	d503245f 	bti	c
+   20004:	a9bf7bf0 	stp	x16, x30, \[sp, #-16\]!
+   20008:	900fff10 	adrp	x16, 20000000 <_GLOBAL_OFFSET_TABLE_>
+   2000c:	f9400e11 	ldr	x17, \[x16, #24\]
+   20010:	91006210 	add	x16, x16, #0x18
+   20014:	d61f0220 	br	x17
+   20018:	d503201f 	nop
+   2001c:	d503201f 	nop
+
+0000000000020020 <extern_func@plt>:
+   20020:	900fff10 	adrp	x16, 20000000 <_GLOBAL_OFFSET_TABLE_>
+   20024:	f9401211 	ldr	x17, \[x16, #32\]
+   20028:	91008210 	add	x16, x16, #0x20
+   2002c:	d61f0220 	br	x17
+   20030:	14000004 	b	20040 <__extern_func_bti_veneer\+0x8>
+   20034:	d503201f 	nop
+
+0000000000020038 <__extern_func_bti_veneer>:
+   20038:	d503245f 	bti	c
+   2003c:	17fffff9 	b	20020 <extern_func@plt>
+
+Disassembly of section \.text:
+
+0000000000030000 <a_func>:
+       30000:	15c00004 	b	7030010 <__b_func_veneer>
+       30004:	17ffc007 	b	20020 <extern_func@plt>
+	\.\.\.
+     7030008:	1400000a 	b	7030030 <__a_func_bti_veneer\+0x8>
+     703000c:	d503201f 	nop
+
+0000000007030010 <__b_func_veneer>:
+     7030010:	90040010 	adrp	x16, f030000 <b_func\+0x6ffffd0>
+     7030014:	9101e210 	add	x16, x16, #0x78
+     7030018:	d61f0200 	br	x16
+	\.\.\.
+
+0000000007030028 <__a_func_bti_veneer>:
+     7030028:	d503245f 	bti	c
+     703002c:	163ffff5 	b	30000 <a_func>
+	\.\.\.
+
+0000000008030030 <b_func>:
+     8030030:	15c00004 	b	f030040 <__c_func_veneer>
+     8030034:	15c00005 	b	f030048 <__a_func_veneer>
+	\.\.\.
+     f030038:	14000012 	b	f030080 <__b_func_bti_veneer\+0x8>
+     f03003c:	d503201f 	nop
+
+000000000f030040 <__c_func_veneer>:
+     f030040:	d503245f 	bti	c
+     f030044:	1440000f 	b	10030080 <c_func>
+
+000000000f030048 <__a_func_veneer>:
+     f030048:	90fc0010 	adrp	x16, 7030000 <a_func\+0x7000000>
+     f03004c:	9100a210 	add	x16, x16, #0x28
+     f030050:	d61f0200 	br	x16
+	\.\.\.
+
+000000000f030060 <__extern_func_veneer>:
+     f030060:	90f87f90 	adrp	x16, 20000 <\.plt>
+     f030064:	9100e210 	add	x16, x16, #0x38
+     f030068:	d61f0200 	br	x16
+	\.\.\.
+
+000000000f030078 <__b_func_bti_veneer>:
+     f030078:	d503245f 	bti	c
+     f03007c:	163fffed 	b	8030030 <b_func>
+	\.\.\.
+
+0000000010030080 <c_func>:
+    10030080:	17bffff2 	b	f030048 <__a_func_veneer>
+    10030084:	17bffffd 	b	f030078 <__b_func_bti_veneer>
+    10030088:	17bffff6 	b	f030060 <__extern_func_veneer>
diff --git a/ld/testsuite/ld-aarch64/bti-far-3.ld b/ld/testsuite/ld-aarch64/bti-far-3.ld
new file mode 100644
index 00000000000..3265c55b8cc
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/bti-far-3.ld
@@ -0,0 +1,12 @@
+OUTPUT_ARCH(aarch64)
+SECTIONS
+{
+  . = 0x00010000;
+  .rela.plt       : { *(.rela.plt) *(.rela.iplt) }
+  . = 0x00020000;
+  .plt            : { *(.plt) *(.iplt) }
+  . = 0x00030000;
+  .text           : { *(.text) }
+  . = 0x20000000;
+  .got            : { *(.got) *(.got.plt) }
+}
diff --git a/ld/testsuite/ld-aarch64/bti-far-3a.s b/ld/testsuite/ld-aarch64/bti-far-3a.s
new file mode 100644
index 00000000000..ed53545fed7
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/bti-far-3a.s
@@ -0,0 +1,23 @@
+	.text
+	.hidden	a_func
+	.hidden	b_func
+	.hidden	c_func
+
+	.global	a_func
+	.type	a_func, %function
+a_func:
+	b	b_func
+	b	extern_func
+
+.zero	0x07000000
+
+	.section	.note.gnu.property,"a"
+	.align	3
+	.word	4
+	.word	16
+	.word	5
+	.string	"GNU"
+	.word	3221225472
+	.word	4
+	.word	1
+	.align	3
diff --git a/ld/testsuite/ld-aarch64/bti-far-3b.s b/ld/testsuite/ld-aarch64/bti-far-3b.s
new file mode 100644
index 00000000000..1f3be3d15cf
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/bti-far-3b.s
@@ -0,0 +1,25 @@
+	.text
+	.hidden	a_func
+	.hidden	b_func
+	.hidden	c_func
+
+.zero	0x01000000
+
+	.global	b_func
+	.type	b_func, %function
+b_func:
+	b	c_func
+	b	a_func
+
+.zero	0x07000000
+
+	.section	.note.gnu.property,"a"
+	.align	3
+	.word	4
+	.word	16
+	.word	5
+	.string	"GNU"
+	.word	3221225472
+	.word	4
+	.word	1
+	.align	3
diff --git a/ld/testsuite/ld-aarch64/bti-far-3c.s b/ld/testsuite/ld-aarch64/bti-far-3c.s
new file mode 100644
index 00000000000..8e2361fd771
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/bti-far-3c.s
@@ -0,0 +1,24 @@
+	.text
+	.hidden	a_func
+	.hidden	b_func
+	.hidden	c_func
+
+.zero	0x01000000
+
+	.global	c_func
+	.type	c_func, %function
+c_func:
+	b	a_func
+	b	b_func
+	b	extern_func
+
+	.section	.note.gnu.property,"a"
+	.align	3
+	.word	4
+	.word	16
+	.word	5
+	.string	"GNU"
+	.word	3221225472
+	.word	4
+	.word	1
+	.align	3
-- 
2.25.1

Re: [PATCH 0/5] aarch64 BTI stub fixes

[Nick Clifton]

Hi Szabolcs,

> Large binaries with BTI can be linked incorrectly with
> binutils 2.41 because PR30930.
> 
> there are other less critical related issues fixed in
> this patchset as well.
> 
> Szabolcs Nagy (5):
>    bfd: aarch64: Fix BTI stub optimization PR30957
>    bfd: aarch64: Fix broken BTI stub PR30930
>    bfd: aarch64: Fix leaks in case of BTI stub reuse
>    bfd: aarch64: Avoid BTI stub for a PLT that has BTI
>    ld: aarch64: Add BTI stub insertion test PR30930

This patch series looks good to me, so please apply if you have not already done so.


FYI - I did find one regression in the linker testsuite with the patch applied:

   FAIL: Check linker stubs with indirect calls handle BTI (exe).

However there are mitigating circumstances:

   * The failure was for an unusual toolchain configuration:

       --target=aarch64_be-linux-gnu_ilp32

   * The three other tests in this part of the testsuite also fail, both
     with and without your patch applied:

       FAIL: Check linker stubs with indirect calls handle BTI (shared lib).
       FAIL: Check linker stubs with indirect calls handle BTI (exe).
       FAIL: Check linker stubs with indirect calls handle BTI when target has BTI.

    Note: there appear to be *two* tests called "check linker stubs with indirect calls handle BTI (exe)."

I leave it up to you to decide if it is worth fixing these tests.

Cheers
   Nick

PS.  My gut feeling at the moment is that we do not need a 2.41.1 release
for this patch, since AArch64 systems with BTI enabled are not yet the norm,
and those that do exist are being used to test for problems like this rather
than general use.  Since your patch will be in the 2.42 release, due January
next year, I think that that will be a reasonable timescale for getting the
fix out there.  Do you agree ?

Re: [PATCH 0/5] aarch64 BTI stub fixes

[Szabolcs Nagy]

The 11/07/2023 11:38, Nick Clifton wrote:
> Hi Szabolcs,
> 
> > Large binaries with BTI can be linked incorrectly with
> > binutils 2.41 because PR30930.
> > 
> > there are other less critical related issues fixed in
> > this patchset as well.
> > 
> > Szabolcs Nagy (5):
> >    bfd: aarch64: Fix BTI stub optimization PR30957
> >    bfd: aarch64: Fix broken BTI stub PR30930
> >    bfd: aarch64: Fix leaks in case of BTI stub reuse
> >    bfd: aarch64: Avoid BTI stub for a PLT that has BTI
> >    ld: aarch64: Add BTI stub insertion test PR30930
> 
> This patch series looks good to me, so please apply if you have not already done so.
> 
> 
> FYI - I did find one regression in the linker testsuite with the patch applied:
> 
>   FAIL: Check linker stubs with indirect calls handle BTI (exe).
> 
> However there are mitigating circumstances:
> 
>   * The failure was for an unusual toolchain configuration:
> 
>       --target=aarch64_be-linux-gnu_ilp32
> 
>   * The three other tests in this part of the testsuite also fail, both
>     with and without your patch applied:
> 
>       FAIL: Check linker stubs with indirect calls handle BTI (shared lib).
>       FAIL: Check linker stubs with indirect calls handle BTI (exe).
>       FAIL: Check linker stubs with indirect calls handle BTI when target has BTI.
> 
>    Note: there appear to be *two* tests called "check linker stubs with indirect calls handle BTI (exe)."
> 
> I leave it up to you to decide if it is worth fixing these tests.

thanks for looking at this.
i will have to check what's going on.

> PS.  My gut feeling at the moment is that we do not need a 2.41.1 release
> for this patch, since AArch64 systems with BTI enabled are not yet the norm,
> and those that do exist are being used to test for problems like this rather
> than general use.  Since your patch will be in the 2.42 release, due January
> next year, I think that that will be a reasonable timescale for getting the
> fix out there.  Do you agree ?

makes sense,
unless it affects hardened kernel builds.
i will have to check if that's an issue.

thanks.

Re: [PATCH 0/5] aarch64 BTI stub fixes

[Szabolcs Nagy]

The 11/07/2023 13:08, Szabolcs Nagy wrote:
> The 11/07/2023 11:38, Nick Clifton wrote:
> > FYI - I did find one regression in the linker testsuite with the patch applied:
> > 
> >   FAIL: Check linker stubs with indirect calls handle BTI (exe).
> > 
> > However there are mitigating circumstances:
> > 
> >   * The failure was for an unusual toolchain configuration:
> > 
> >       --target=aarch64_be-linux-gnu_ilp32
> > 
> >   * The three other tests in this part of the testsuite also fail, both
> >     with and without your patch applied:
> > 
> >       FAIL: Check linker stubs with indirect calls handle BTI (shared lib).
> >       FAIL: Check linker stubs with indirect calls handle BTI (exe).
> >       FAIL: Check linker stubs with indirect calls handle BTI when target has BTI.
> > 
> >    Note: there appear to be *two* tests called "check linker stubs with indirect calls handle BTI (exe)."
> > 
> > I leave it up to you to decide if it is worth fixing these tests.
> 
> thanks for looking at this.
> i will have to check what's going on.

i will change the name of the new test to be unique.

cleaning up ilp32 test issues will be a fair bit of work.

i can commit a followup patch to force the tests i added to use
lp64 abi, like we do on other tests, but this is not ideal.

> > PS.  My gut feeling at the moment is that we do not need a 2.41.1 release
> > for this patch, since AArch64 systems with BTI enabled are not yet the norm,
> > and those that do exist are being used to test for problems like this rather
> > than general use.  Since your patch will be in the 2.42 release, due January
> > next year, I think that that will be a reasonable timescale for getting the
> > fix out there.  Do you agree ?
> 
> makes sense,
> unless it affects hardened kernel builds.
> i will have to check if that's an issue.

in practice i think the kernel should be fine.
(it's hard to say conclusively)

but i plan to backport the patches even if we don't arrange
a new release.

Re: [PATCH 0/5] aarch64 BTI stub fixes

[Fangrui Song]

On Thu, Nov 9, 2023 at 6:58 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 11/07/2023 13:08, Szabolcs Nagy wrote:
> > The 11/07/2023 11:38, Nick Clifton wrote:
> > > FYI - I did find one regression in the linker testsuite with the patch applied:
> > >
> > >   FAIL: Check linker stubs with indirect calls handle BTI (exe).
> > >
> > > However there are mitigating circumstances:
> > >
> > >   * The failure was for an unusual toolchain configuration:
> > >
> > >       --target=aarch64_be-linux-gnu_ilp32
> > >
> > >   * The three other tests in this part of the testsuite also fail, both
> > >     with and without your patch applied:
> > >
> > >       FAIL: Check linker stubs with indirect calls handle BTI (shared lib).
> > >       FAIL: Check linker stubs with indirect calls handle BTI (exe).
> > >       FAIL: Check linker stubs with indirect calls handle BTI when target has BTI.
> > >
> > >    Note: there appear to be *two* tests called "check linker stubs with indirect calls handle BTI (exe)."
> > >
> > > I leave it up to you to decide if it is worth fixing these tests.
> >
> > thanks for looking at this.
> > i will have to check what's going on.
>
> i will change the name of the new test to be unique.
>
> cleaning up ilp32 test issues will be a fair bit of work.
>
> i can commit a followup patch to force the tests i added to use
> lp64 abi, like we do on other tests, but this is not ideal.
>
> > > PS.  My gut feeling at the moment is that we do not need a 2.41.1 release
> > > for this patch, since AArch64 systems with BTI enabled are not yet the norm,
> > > and those that do exist are being used to test for problems like this rather
> > > than general use.  Since your patch will be in the 2.42 release, due January
> > > next year, I think that that will be a reasonable timescale for getting the
> > > fix out there.  Do you agree ?
> >
> > makes sense,
> > unless it affects hardened kernel builds.
> > i will have to check if that's an issue.
>
> in practice i think the kernel should be fine.
> (it's hard to say conclusively)
>
> but i plan to backport the patches even if we don't arrange
> a new release.

% grep '\.zero' ld/testsuite/ld-aarch64/bti-far*
ld/testsuite/ld-aarch64/bti-far-3a.s:.zero      0x07000000
ld/testsuite/ld-aarch64/bti-far-3b.s:.zero      0x01000000
ld/testsuite/ld-aarch64/bti-far-3b.s:.zero      0x07000000
ld/testsuite/ld-aarch64/bti-far-3c.s:.zero      0x01000000
% size ld/tmpdir/bti-far-*
   text    data     bss     dec     hex filename
117440552             0       0 117440552       7000028 ld/tmpdir/bti-far-3a.o
134217768             0       0 134217768       8000028 ld/tmpdir/bti-far-3b.o
16777260              0       0 16777260        100002c ld/tmpdir/bti-far-3c.o
    132       0       0     132      84 ld/tmpdir/bti-far-opt.o


`.zero      0x07000000` is 112MiB.  Perhaps use output section
addresses (e.g. [1]) to make the files smaller?

[1]: https://github.com/llvm/llvm-project/blob/de176d8c5496d6cf20e82aface98e102c593dbe2/lld/test/ELF/aarch64-thunk-pi.s#L112

Re: [PATCH 0/5] aarch64 BTI stub fixes

[Szabolcs Nagy]

The 11/17/2023 00:42, Fangrui Song wrote:
> On Thu, Nov 9, 2023 at 6:58 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> `.zero      0x07000000` is 112MiB.  Perhaps use output section
> addresses (e.g. [1]) to make the files smaller?

i cannot reproduce the original bug with different
output sections.

the bug requires the next input section after the
inserted stubs to be grouped with those stubs.
which i think does not happen in bfd ld if the next
input section goes to a different output section.

> 
> [1]: https://github.com/llvm/llvm-project/blob/de176d8c5496d6cf20e82aface98e102c593dbe2/lld/test/ELF/aarch64-thunk-pi.s#L112