Re: [PATCH v2] aarch64: Fix .cfi_window_save with pac-ret [PR94515]

[Richard Sandiford]

Szabolcs Nagy <szabolcs.n...@arm.com> writes:
> @@ -2221,6 +2239,14 @@ change_cfi_row (dw_cfi_row *old_row, dw_cfi_row 
> *new_row)
>        cfi->dw_cfi_opc = DW_CFA_GNU_window_save;
>        add_cfi (cfi);
>      }
> +
> +  if (old_row->ra_mangled != new_row->ra_mangled)
> +    {
> +      dw_cfi_ref cfi = new_cfi ();
> +      /* DW_CFA_GNU_window_save is reused for toggling RA mangle state.  */
> +      cfi->dw_cfi_opc = DW_CFA_GNU_window_save;
> +      add_cfi (cfi);
> +    }
>  }
>  
>  /* Examine CFI and return true if a cfi label and set_loc is needed

I wonder if it would be worth asserting:

  gcc_assert (!old_row->ra_mangled && !new_row->ra_mangled);

in the "SPARC" block and:

  gcc_assert (!old_row->window_save && !new_row->window_save);

in the "aarch64" block?

> diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-2.C 
> b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
> new file mode 100644
> index 00000000000..1f13e1200ad
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
> @@ -0,0 +1,41 @@
> +/* PR target/94515. Check .cfi_window_save with multiple return paths.  */
> +/* { dg-do run } */
> +/* { dg-require-effective-target lp64 } */
> +/* { dg-additional-options "-O2 -mbranch-protection=pac-ret" } */
> +
> +volatile int zero = 0;
> +int global = 0;
> +
> +__attribute__((noinline))
> +int bar(void)
> +{
> +  if (zero == 0) return 3;
> +  return 0;
> +}
> +
> +__attribute__((noinline, noreturn))
> +void unwind (void)
> +{
> +  throw 42;
> +}
> +
> +__attribute__((noinline, noipa, target("branch-protection=pac-ret")))

I'm probably just showing my ignorance, but is this target attribute
needed?  It looks like all functions in the test are compiled with pac-ret.

LGTM otherwise, but please give others 24 hrs to object.

Thanks,
Richard