public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrea Corallo <andrea.corallo@arm.com>
To: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>
Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>, nd <nd@arm.com>,
	"Andrea Corallo via Gcc-patches" <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 9/12 V2] arm: Make libgcc bti compatible
Date: Mon, 12 Dec 2022 15:54:33 +0100	[thread overview]
Message-ID: <gkr4ju0u0ye.fsf@arm.com> (raw)
In-Reply-To: <86a735ce-f41c-967c-45cd-8e3965d283bf@foss.arm.com> (Richard Earnshaw's message of "Mon, 25 Jul 2022 11:41:34 +0100")

Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes:

> On 22/07/2022 16:09, Andrea Corallo via Gcc-patches wrote:
>> Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes:
>> 
>>> On 21/07/2022 10:17, Andrea Corallo via Gcc-patches wrote:
>>>> Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes:
>>>>
>>>>> On 28/04/2022 10:48, Andrea Corallo via Gcc-patches wrote:
>>>>>> This change add bti instructions at the beginning of arm specific
>>>>>> libgcc hand written assembly routines.
>>>>>> 2022-03-31  Andrea Corallo  <andrea.corallo@arm.com>
>>>>>> 	* libgcc/config/arm/crti.S (FUNC_START): Add bti instruction
>>>>>> if
>>>>>> 	necessary.
>>>>>> 	* libgcc/config/arm/lib1funcs.S (THUMB_FUNC_START, FUNC_START):
>>>>>> 	Likewise.
>>>>>>
>>>>>
>>>>> +#if defined(__ARM_FEATURE_BTI)
>>>>>
>>>>> Wouldn't it be better to use __ARM_FEATURE_BTI_DEFAULT?  That way we
>>>>> only get BTI instructions in multilib variants that have asked for
>>>>> BTI.
>>>>>
>>>>> R.
>>>> Hi Richard,
>>>> good point, yes I think so.
>>>> Please find attached the updated patch.
>>>> BR
>>>>     Andrea
>>>>
>>>
>>> I've been pondering this patch.  The way it is implemented would put a
>>> BTI instruction at the start of every assembler routine in libgcc.
>>> But the vast majority of functions in libgcc cannot have their address
>>> taken, so a BTI isn't needed (BTI is only needed when an indirect jump
>>> could be used).  So I wonder if we really need to do this so
>>> aggressively?
>>>
>>> Perhaps a better approach would be to define a macro (eg MAYBEBTI)
>>> which expands a BTI if the compilation requires it and nothing
>>> otherwise), and then manually insert that in any functions that really
>>> need this (if any).
>> I guess the main downside of this approach would be the maintanace
>> burden, we'll have to remember forever that every time an asm function
>> is called by function pointer we have to add the bti landing pad
>> manually, otherwise this will be broken when pacbti enabled. WDYT?
>> If we want to go this way I'll start reworking the patch in this
>> direction (tho this might not be trivial).
>> 
>
> Yes, it's a trade-off.  The lazy way, however, costs all users even if
> a function is never addressed (which I think is the case for
> practically all functions in libgcc).
>
> So I think in this case it's worth taking that extra development pain.
>
> R.

As a late follow-up to this.

I believe there are no hand written asm functions in libgcc that are
addressed, so this patch was dropped from the series in the following
iteration.  It is true that we could pac instrument them but ATM we
don't.

  Andrea

  reply	other threads:[~2022-12-12 14:55 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-28  8:39 [PATCH 0/12] arm: Enables return address verification and branch target identification on Cortex-M Andrea Corallo
2022-04-28  9:08 ` [PATCH 1/12] arm: Make mbranch-protection opts parsing common to AArch32/64 Andrea Corallo
2022-07-01 10:49   ` Richard Earnshaw
2022-04-28  9:37 ` [PATCH 2/12] arm: Add Armv8.1-M Mainline target feature +pacbti Andrea Corallo
2022-07-01 10:51   ` Richard Earnshaw
2022-04-28  9:38 ` [PATCH 3/12] arm: Add option -mbranch-protection Andrea Corallo
2022-07-01 10:59   ` Richard Earnshaw
2022-07-04  9:27     ` [PATCH 3/12 V2] " Andrea Corallo
2022-07-04 10:55       ` Richard Earnshaw
2022-04-28  9:40 ` [PATCH 4/12] arm: Add testsuite library support for PACBTI target Andrea Corallo
2022-07-01 13:03   ` Richard Earnshaw
2022-07-01 14:17     ` Richard Earnshaw
2022-07-04 14:47       ` Andrea Corallo
2022-07-05 10:05         ` Richard Earnshaw
2022-04-28  9:42 ` [PATCH 5/12] arm: Implement target feature macros for PACBTI Andrea Corallo
2022-07-01 14:26   ` Richard Earnshaw
2022-07-12 15:45     ` [PATCH 5/12 V2] " Andrea Corallo
2022-07-21 11:01       ` Richard Earnshaw
2022-07-22 10:35         ` [PATCH 5/12 V3] " Andrea Corallo
2022-07-22 14:34           ` [PATCH 5/12 V4] " Andrea Corallo
2022-04-28  9:44 ` [PATCH 6/12] arm: Add pointer authentication for stack-unwinding runtime Andrea Corallo
2022-07-01 14:41   ` Richard Earnshaw
2022-11-09 11:17     ` [PATCH 6/12 V2] " Andrea Corallo
2022-04-28  9:45 ` [PATCH 7/12] arm: Emit build attributes for PACBTI target feature Andrea Corallo
2022-07-01 14:49   ` Richard Earnshaw
2022-07-13  8:58     ` [PATCH 7/12 V2] " Andrea Corallo
2022-07-21 11:03       ` Richard Earnshaw
2022-07-22 14:57     ` Andrea Corallo
2022-04-28  9:46 ` [PATCH 8/12] arm: Introduce multilibs " Andrea Corallo
2022-06-01 12:32   ` [PATCH 8/12 V2] " Andrea Corallo
2022-07-01 14:54     ` Richard Earnshaw
2022-07-01 14:57       ` Richard Earnshaw
2022-07-21  9:04         ` [PATCH 8/12 V3] " Andrea Corallo
2022-07-21 11:09           ` Richard Earnshaw
2022-04-28  9:48 ` [PATCH 9/12] arm: Make libgcc bti compatible Andrea Corallo
2022-07-01 15:03   ` Richard Earnshaw
2022-07-21  9:17     ` [PATCH 9/12 V2] " Andrea Corallo
2022-07-21 11:41       ` Richard Earnshaw
2022-07-22 15:09         ` Andrea Corallo
2022-07-25 10:41           ` Richard Earnshaw
2022-12-12 14:54             ` Andrea Corallo [this message]
2022-04-28  9:50 ` [PATCH 10/12] arm: Implement cortex-M return signing address codegen Andrea Corallo
2022-06-28  9:17   ` [PATCH 10/12 V2] " Andrea Corallo
2022-07-01 15:43     ` Richard Earnshaw
2022-08-08  9:33       ` Andrea Corallo
2022-10-20 14:53         ` Kyrylo Tkachov
2022-04-28  9:51 ` [PATCH 11/12] aarch64: Make bti pass generic so it can be used by the arm backend Andrea Corallo
2022-05-06  8:23   ` Richard Sandiford
2022-07-01 15:53   ` Richard Earnshaw
2022-04-28  9:53 ` [PATCH 12/12] arm: implement bti injection Andrea Corallo
2022-06-28  9:21   ` [PATCH 12/12 V2] " Andrea Corallo
2022-07-01 16:04     ` Richard Earnshaw
2022-06-01 12:34 ` [PATCH 0/12] arm: Enables return address verification and branch target identification on Cortex-M Andrea Corallo

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=gkr4ju0u0ye.fsf@arm.com \
    --to=andrea.corallo@arm.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=Richard.Earnshaw@foss.arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=nd@arm.com \
    /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).