[Bug c/106153] New: Generated arm64 code writing below stack pointer without updating SP

[Bug c/106153] New: Generated arm64 code writing below stack pointer without updating SP

[andy at plausible dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106153

            Bug ID: 106153
           Summary: Generated arm64 code writing below stack pointer
                    without updating SP
           Product: gcc
           Version: 10.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: andy at plausible dot org
  Target Milestone: ---

The compiler is a arm64 gcc 10.3.0 from the Zephyr project SDK.  It's
a pretty standard crosstools-ng build.  Binaries at zephyrproject.org,
config/source at https://github.com/zephyrproject-rtos/sdk-ng

The short summary is that variable length array usage is generating
incorrect stack pointers; the memory gets used before the SP register
is decremented, leading to a race with the Zephyr interrupt handling
(which uses the interrupted stack during interrupt entry in some
configurations) that then corrupts the interrupted context.

See upstream Zephyr bug here:
https://github.com/zephyrproject-rtos/zephyr/pull/46967

A preprocessed "main.i" is attached that shows the bug fairly easily.
We build it with these arguments:

   aarch64-zephyr-elf-gcc -Os -fno-common -g -gdwarf-4 -mcpu=cortex-a53
-mabi=lp64 -Wall -Wformat -Wformat-security -Wno-format-zero-length -Wno-main
-Wno-pointer-sign -Wpointer-arith -Wexpansion-to-defined
-Wno-unused-but-set-variable -Werror=implicit-int
-fno-asynchronous-unwind-tables -fno-pie -fno-pic -ftls-model=local-exec
-fno-reorder-functions -fno-defer-pop -ffunction-sections -fdata-sections
-std=c99 -S -o - main.i

Doing so, it's easy to see constructions like this (scan down for the
"log_msgs:" function):

        mov     x19, sp           ;<-- x19 == sp
        mov     x22, x19          ;<-- x22 == sp
        str     xzr, [x29, 128]
        str     wzr, [x29, 136]
        str     x0, [x19, 24]
        mov     x0, x20
        str     x23, [x22, 16]!   ;<-- store through x22 into memory BELOW sp!
        strh    wzr, [x29, 140]
        mov     x2, x22
        bl      z_log_msg_static_create ;<-- and hand it to a function as an
argument!

We've worked around it with a compiler barrier for now.  As far as I can tell
it's only happening with arm64, plausibly because of the auto indexed "push"
addressing mode generation?

[Bug target/106153] Generated arm64 code writing below stack pointer without updating SP

[andy at plausible dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106153

--- Comment #1 from Andy Ross <andy at plausible dot org> ---
Just submitted the same code at godbolt and their "ARM64 gcc trunk" build shows
the same behavior.

[Bug target/106153] Generated arm64 code writing below stack pointer without updating SP

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106153

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2022-06-30
     Ever confirmed|0                           |1

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Can you attach the preprocessed source where the assembly was generated from?

[Bug target/106153] Generated arm64 code writing below stack pointer without updating SP

[andy at plausible dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106153

--- Comment #3 from Andy Ross <andy at plausible dot org> ---
Created attachment 53231
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53231&action=edit
Preprocessed source file (gzipped)

Sorry, I thought I attached it with the submission.  Looks like it got kicked
out for being 50 bytes over the limit.  Compressed and resubmitted.  The
macrobatics is admittedly pretty thick in this API, and the expansions are
messy.  The upstream source (discussed in the linked github issue) might help
clarify.

[Bug target/106153] Generated arm64 code writing below stack pointer without updating SP

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106153

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|1                           |0
             Status|WAITING                     |UNCONFIRMED

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
  saved_stack.50_2 = __builtin_stack_save ();
  _ll_buf.24_379 = __builtin_alloca_with_align (/*size=*/32,
/*align(bits)=*/64);
  _pbuf_381 = &MEM[(struct log_msg *)_ll_buf.24_379].data;
  MEM[(const void * *)_ll_buf.24_379 + 24B] = "Error message example.";
  MEM[(union cbprintf_package_hdr *)&hdr] = 4;
  MEM[(union cbprintf_package_hdr *)_ll_buf.24_379 + 16B] = hdr;
  hdr ={v} {CLOBBER};
  MEM[(struct log_msg_desc *)&_desc] = 8256;
  z_log_msg_static_create (&log_const_syst, _desc, _pbuf_381, 0B);
  __builtin_stack_restore (saved_stack.50_2);




The alloca gets expanded as:
(insn 15 14 16 (set (reg/f:DI 31 sp)
        (plus:DI (reg/f:DI 31 sp)
            (const_int -32 [0xffffffffffffffe0])))
"/home/andyross/z/zephyr/samples/subsys/logging/syst/src/main.c":47:49 -1
     (nil))

(insn 16 15 17 (set (reg:DI 214)
        (reg/f:DI 88 virtual-stack-dynamic))
"/home/andyross/z/zephyr/samples/subsys/logging/syst/src/main.c":47:49 -1
     (nil))

(insn 17 16 18 (set (reg:DI 215)
        (plus:DI (reg:DI 214)
            (const_int 7 [0x7])))
"/home/andyross/z/zephyr/samples/subsys/logging/syst/src/main.c":47:49 -1
     (nil))

(insn 18 17 19 (set (reg:DI 216)
        (lshiftrt:DI (reg:DI 215)
            (const_int 3 [0x3])))
"/home/andyross/z/zephyr/samples/subsys/logging/syst/src/main.c":47:49 -1
     (expr_list:REG_EQUAL (udiv:DI (reg:DI 215)
            (const_int 8 [0x8]))
        (nil)))

(insn 19 18 20 (set (reg/f:DI 217)
        (ashift:DI (reg:DI 216)
            (const_int 3 [0x3])))
"/home/andyross/z/zephyr/samples/subsys/logging/syst/src/main.c":47:49 -1
     (nil))

(insn 20 19 0 (set (reg/f:DI 117 [ _ll_buf.24 ])
        (reg/f:DI 217))
"/home/andyross/z/zephyr/samples/subsys/logging/syst/src/main.c":47:49 -1
     (nil))


Which looks correct (64 bit aligned is 8 byte aligned).

[Bug target/106153] Generated arm64 code writing below stack pointer without updating SP

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106153

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I don't see any below accesses either.

Right before the assembler code you posted has:
        sub     sp, sp, #32 

So ....
Maybe I am missing something.

[Bug target/106153] Generated arm64 code writing below stack pointer without updating SP

[andy at plausible dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106153

--- Comment #6 from Andy Ross <andy at plausible dot org> ---
No, I just had a thinko (hur dur stack grows down, sigh) and jumped too quickly
once I thought I had it.  All the circumstantial evidence is pointing at a
compiler bug here, but this smoking gun isn't.  I'll keep looking.  Feel free
to close this if you need to, I can always reopen if I find something.

[Bug target/106153] Generated arm64 code writing below stack pointer without updating SP

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106153

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
.