Richard Earnshaw <richard.earns...@foss.arm.com> writes:
> On 17/12/2021 15:52, Andrea Corallo wrote:
>> Hi Richard,
>> thanks for reviewing! Some comments inline.
>> Richard Earnshaw <richard.earns...@foss.arm.com> writes:
>>> On 05/11/2021 08:52, Andrea Corallo via Gcc-patches wrote:
>>>> Hi all,
>>>> this patch enables address return signature and verification based
>>>> on
>>>> Armv8.1-M Pointer Authentication [1].
>>>> To sign the return address, we use the PAC R12, LR, SP instruction
>>>> upon function entry. This is signing LR using SP and storing the
>>>> result in R12. R12 will be pushed into the stack.
>>>> During function epilogue R12 will be popped and AUT R12, LR, SP will
>>>> be used to verify that the content of LR is still valid before return.
>>>> Here an example of PAC instrumented function prologue and epilogue:
>>>> pac r12, lr, sp
>>>> push {r3, r7, lr}
>>>> push {r12}
>>>> sub sp, sp, #4
>>>
>>> Which, as shown here, generates a stack which does not preserve 8-byte
>>> alignment.
>> I'm probably catastrofically wrong but shouldn't the stack be "all
>> times
>> be aligned to a word boundary" [1]?
>
> At a function boundary it must be 8-byte aligned (same reference). I
> don't think GCC really optimizes leaf functions to permit sub 8-byte
> alignment, but since you omitted the body of your function I might be
> wrong in this case.
I see thanks.
>>
>>> Also, what's wrong with
>>>
>>> pac r12, lr, sp
>>> push {r3, r7, ip, lr}
>>> ?
>> AFAIK the AAPCS32 defines the Frame Record to be 2 consecutive
>> 32-bit
>> values of LR and FP on the stack so that's the reason.
>
> GCC does not currently support AAPCS frame chains as that's a
> relatively new feature in the AAPCS; so it's not something you need to
> be concerned about right now. The AAPCS frame chain uses R11 as the
> frame-chain register anyway. However, you are right that this does
> affect -mtpcs-frame and will become relevant when we do add support
> for AAPCS frame chains.
Do you think would be better to go for the "push {r3, r7, ip, lr}"
solution even if we decide -mtpcs-frame is not compatible with pac
instrumentation or we want to stay with the proposed approach for future
compatibility?
Thanks
Andrea
