Re: [GCC][PATCH 13/15, v5] arm: Add support for dwarf debug directives and pseudo hard-register for PAC feature.

[Richard Earnshaw <Richard.Earnshaw@foss.arm.com>]

On 13/01/2023 17:44, Srinath Parvathaneni via Gcc-patches wrote:
> Hello,
> 
> This patch teaches the DWARF support in gcc about RA_AUTH_CODE pseudo hard-register and also
> updates the ".save", ".cfi_register", ".cfi_offset", ".cfi_restore" directives accordingly.
> This patch also adds support to emit ".pacspval" directive when "pac ip, lr, sp" instruction
> in generated in the assembly.
> 
> RA_AUTH_CODE register number is 107 and it's dwarf register number is 143.
> 
> Applying this patch on top of PACBTI series posted here
> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599658.html and when compiling the following
> test.c with "-march=armv8.1-m.main+mve+pacbti -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard
> fasynchronous-unwind-tables -g -O0 -S" command line options, the assembly output after this patch
> looks like below:
> 
> $cat test.c
> 
> void fun1(int a);
> void fun(int a,...)
> {
>    fun1(a);
> }
> 
> int main()
> {
>    fun (10);
>    return 0;
> }
> 
> $ arm-none-eabi-gcc -march=armv8.1-m.main+mve+pacbti -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard
> -fasynchronous-unwind-tables -g -O0 -S test.s
> 
> Assembly output:
> ...
> fun:
> ...
>          .pacspval
>          pac     ip, lr, sp
>          .cfi_register 143, 12
>          push    {r3, r7, ip, lr}
>          .save {r3, r7, ra_auth_code, lr}
> ...
>          .cfi_offset 143, -24
> ...
>          .cfi_restore 143
> ...
>          aut     ip, lr, sp
>          bx      lr
> ...
> main:
> ...
>          .pacspval
>          pac     ip, lr, sp
>          .cfi_register 143, 12
>          push    {r3, r7, ip, lr}
>          .save {r3, r7, ra_auth_code, lr}
> ...
>          .cfi_offset 143, -8
> ...
>          .cfi_restore 143
> ...
>          aut     ip, lr, sp
>          bx      lr
> ...
> 
> Regression tested on arm-none-eabi target and found no regressions.
> 
> Ok for master?
> 
> Regards,
> Srinath.
> 
> 2023-01-11  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>
> 
>          * config/arm/aout.h (ra_auth_code): Add entry in enum.
>          (emit_multi_reg_push): Add RA_AUTH_CODE register to
>          dwarf frame expression.
>          (arm_emit_multi_reg_pop): Restore RA_AUTH_CODE register.
>          (arm_expand_prologue): Update frame related information and reg notes
>          for pac/pacbit insn.
>          (arm_regno_class): Check for pac pseudo reigster.
>          (arm_dbx_register_number): Assign ra_auth_code register number in dwarf.
>          (arm_init_machine_status): Set pacspval_needed to zero.
>          (arm_debugger_regno): Check for PAC register.
>          (arm_unwind_emit_sequence): Print .save directive with ra_auth_code
>          register.
>          (arm_unwind_emit_set): Add entry for IP_REGNUM in switch case.
>          (arm_unwind_emit): Update REG_CFA_REGISTER case._
>          * config/arm/arm.h (FIRST_PSEUDO_REGISTER): Modify.
>          (DWARF_PAC_REGNUM): Define.
>          (IS_PAC_REGNUM): Likewise.
>          (enum reg_class): Add PAC_REG entry.
>          (machine_function): Add pacbti_needed state to structure.
>          * config/arm/arm.md (RA_AUTH_CODE): Define.
> 
> gcc/testsuite/ChangeLog:
> 
> 2023-01-11  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>
> 
>          * g++.target/arm/pac-1.C: New test.
>          * gcc.target/arm/pac-15.c: Likewise.

Your attachments are still not being correctly detected.  Perhaps this 
is because of the filename you've chosen, which has no recognizable 
extension.  If you name your files <xxx>.patch (or <xxx>.diff, or even 
<xxx>.txt) then the system should automatically pick the right mime type 
for encoding.

+	  /* NOTE: Dwarf code emitter handle reg-reg copies correctly and in the
+	     following example reg-reg copy of SP to IP register is handled
+	     through .cfi_def_cfa_register directive and the .cfi_offset
+	     directive for IP register is skipped by dwarf code emitter.
+	     Example:
+		mov     ip, sp
+		.cfi_def_cfa_register 12
+		push    {fp, ip, lr, pc}
+		.cfi_offset 11, -16
+		.cfi_offset 13, -12
+		.cfi_offset 14, -8
+
+	     Where as Arm-specific .save directive reg-reg copy handling is
+	     buggy.  After the reg-reg copy, the copied registers need to be

It's not buggy (if it were you'd need to fix it :).  It just works in a 
different way to the dwarf tracker and doesn't need to handle reg->reg 
copies.  So please rephrase this.

+	     populated in .save directive register list but with the current
+	     implementation of .save directive original registers are getting
+	     populated in the register list.  So to avoid this issue for IP
+	     register when PACBTI is enabled we manually updated the .save
+	     directive register list to use "ra_auth_code" (pseduo register 143)
+	     instead of IP register as shown in following example.
+	     Example:
+		pacbti  ip, lr, sp
+		.cfi_register 143, 12
+		push    {r3, r7, ip, lr}
+		.save {r3, r7, ra_auth_code, lr}
+	  */

R.