On Fri, Apr 11, 2025 at 11:10 AM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Richard Biener <richard.guent...@gmail.com> writes:
> > On Thu, Apr 10, 2025 at 10:10 PM Richard Sandiford
> > <richard.sandif...@arm.com> wrote:
> >>
> >> PR119610 is about incorrect CFI output for a stack probe when that
> >> probe is not the initial allocation. The main aarch64 stack probe
> >> function, aarch64_allocate_and_probe_stack_space, implicitly assumed
> >> that the incoming stack pointer pointed to the top of the frame,
> >> and thus held the CFA.
> >>
> >> aarch64_save_callee_saves and aarch64_restore_callee_saves use a
> >> parameter called bytes_below_sp to track how far the stack pointer
> >> is above the base of the static frame. This patch does the same
> >> thing for aarch64_allocate_and_probe_stack_space.
> >>
> >> Also, I noticed that the SVE path was attaching the first CFA note
> >> to the wrong instruction: it was attaching the note to the calculation
> >> of the stack size, rather than to the r11<-sp copy.
> >>
> >> Bootstrapped & regression-tested on aarch64-linux-gnu. I'll push on
> >> Monday if there are no comments before then. I'd appreciate a second
> >> pair of eyes though, since this is a sensitive area.
> >
> > Do you happen to know if the backports to older branches you provided for
> > the change that triggered this issue (in particular to GCC 7) are also
> > affected?
>
> GCC 7 and GCC 8 should be ok. The bug relies on stack protection being
> enabled (-fstack-protector-strong for the testcase in the PR, but just
> -fstack-protector for others) and that was added in GCC 9.
Hmm, I see -fstack-protector[-strong] working with GCC 7 on aarch64.
Specifically
the testcase produces for foo():
.text
.align 2
.global _Z3foov
.type _Z3foov, %function
_Z3foov:
.LFB0:
.cfi_startproc
stp x29, x30, [sp, -16]!
.cfi_def_cfa_offset 16
.cfi_offset 29, -16
.cfi_offset 30, -8
add x29, sp, 0
.cfi_def_cfa_register 29
sub sp, sp, #16
sub sp, sp, #524288
adrp x0, __stack_chk_guard
add x0, x0, :lo12:__stack_chk_guard
ldr x1, [x0]
str x1, [x29, -8]
mov x1,0
adrp x0, ptr
add x0, x0, :lo12:ptr
sub x1, x29, #524288
sub x1, x1, #8
str x1, [x0]
mov x0, 4
bl __cxa_allocate_exception
mov x3, x0
mov w0, 1
str w0, [x3]
adrp x0, _ZTIi
add x0, x0, :lo12:_ZTIi
mov x2, 0
mov x1, x0
mov x0, x3
bl __cxa_throw
.cfi_endproc
.LFE0:
.size _Z3foov, .-_Z3foov
> > The testcase from the PR does not fail in the reported way for me, but I
> > guess
> > that very much depends on "details".
>
> Oh, even on trunk? Does adding:
>
> volatile int unused[N] = {};
>
> to main for various N change things?
I didn't try on trunk.
Richard.
>
> Thanks,
> Richard
