public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
To: <binutils@sourceware.org>
Subject: [PATCH 2/5] bfd: aarch64: Fix broken BTI stub PR30930
Date: Fri, 3 Nov 2023 13:15:38 +0000	[thread overview]
Message-ID: <8a36dd5ca319bd4d0fe2425bddcf4868c3426a96.1699016830.git.szabolcs.nagy@arm.com> (raw)
In-Reply-To: <cover.1699016830.git.szabolcs.nagy@arm.com>

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


  parent reply	other threads:[~2023-11-03 13:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-03 13:15 [PATCH 0/5] aarch64 BTI stub fixes Szabolcs Nagy
2023-11-03 13:15 ` [PATCH 1/5] bfd: aarch64: Fix BTI stub optimization PR30957 Szabolcs Nagy
2023-11-03 13:15 ` Szabolcs Nagy [this message]
2023-11-03 13:15 ` [PATCH 3/5] bfd: aarch64: Fix leaks in case of BTI stub reuse Szabolcs Nagy
2023-11-03 13:15 ` [PATCH 4/5] bfd: aarch64: Avoid BTI stub for a PLT that has BTI Szabolcs Nagy
2023-11-03 13:22 ` [PATCH 5/5] ld: aarch64: Add BTI stub insertion test PR30930 Szabolcs Nagy
2023-11-03 13:15   ` Szabolcs Nagy
2023-11-07 11:38 ` [PATCH 0/5] aarch64 BTI stub fixes Nick Clifton
2023-11-07 13:08   ` Szabolcs Nagy
2023-11-09 14:58     ` Szabolcs Nagy
2023-11-17  8:42       ` Fangrui Song
     [not found]       ` <DS7PR12MB5765CE9DC452CD430E68B89BCBB7A@DS7PR12MB5765.namprd12.prod.outlook.com>
2023-11-20 14:56         ` Szabolcs Nagy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8a36dd5ca319bd4d0fe2425bddcf4868c3426a96.1699016830.git.szabolcs.nagy@arm.com \
    --to=szabolcs.nagy@arm.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).