[Bug middle-end/88950] New: stack_protect_prologue can be reordered by sched1 around memory accesses

[matmal01 at gcc dot gnu.org]

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

            Bug ID: 88950
           Summary: stack_protect_prologue can be reordered by sched1
                    around memory accesses
           Product: gcc
           Version: 9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: matmal01 at gcc dot gnu.org
  Target Milestone: ---

I've found a testcase where the stack protector code generated through
`-fstack-protector-all` doesn't actually protect anything.

With the following testcase

int foo (int a, int b, int c) {
        char buf[64];
        buf[a] = 1;
        buf[b] = c;

        // Just add something so that the assignments above have some
        // observable behaviour.
        int retval = 0;
        for (size_t i = 0; i < 32; i++)
        {
                retval += buf[i];
        }
        return retval;
}


When compiling for aarch64 with
gcc -fstack-protector-all -g -S stack-reorder.c  -o test.s -O3 -fdump-rtl-final
(with ~gcc (GCC) 9.0.0 20181214 (experimental)~)

We get an RTL dump on the final pass that has the snippet
(insn 8 21 130 (parallel [
            (set (mem/v/f/c:DI (plus:DI (reg/f:DI 31 sp)
                        (const_int 88 [0x58])) [1 D.4227+0 S8 A64])
                (unspec:DI [
                        (mem/v/f/c:DI (reg/f:DI 0 x0 [116]) [1
__stack_chk_guard+0 S8 A64])
                    ] UNSPEC_SP_SET))
            (set (reg:DI 1 x1 [141])
                (const_int 0 [0]))
        ]) "stack-reorder.c":3:31 1046 {stack_protect_set_di}
     (expr_list:REG_UNUSED (reg:DI 1 x1 [141])
        (nil)))
(note 130 8 117 (var_location b (entry_value:SI (reg:SI 1 x1 [ b ])))
NOTE_INSN_VAR_LOCATION)
(note 117 130 118 stack-reorder.c:4 NOTE_INSN_BEGIN_STMT)
(note 118 117 119 stack-reorder.c:5 NOTE_INSN_BEGIN_STMT)
(note 119 118 120 stack-reorder.c:6 NOTE_INSN_BEGIN_STMT)
(note 120 119 131 stack-reorder.c:10 NOTE_INSN_BEGIN_STMT)
(note 131 120 121 (var_location retval (const_int 0 [0]))
NOTE_INSN_VAR_LOCATION)
(note 121 131 144 stack-reorder.c:11 NOTE_INSN_BEGIN_STMT)
(note 144 121 122 0xffffb76fd960 NOTE_INSN_BLOCK_BEG)
(note 122 144 132 stack-reorder.c:11 NOTE_INSN_BEGIN_STMT)
(note 132 122 123 (var_location retval (nil)) NOTE_INSN_VAR_LOCATION)
(note 123 132 145 stack-reorder.c:13 NOTE_INSN_BEGIN_STMT)
(note 145 123 75 0xffffb76fd960 NOTE_INSN_BLOCK_END)
(insn:TI 75 145 133 (parallel [
            (set (reg:DI 1 x1 [137])
                (unspec:DI [
                        (mem/v/f/c:DI (plus:DI (reg/f:DI 31 sp)
                                (const_int 88 [0x58])) [1 D.4227+0 S8 A64])
                        (mem/v/f/c:DI (reg/f:DI 0 x0 [116]) [1
__stack_chk_guard+0 S8 A64])
                    ] UNSPEC_SP_TEST))
            (clobber (reg:DI 2 x2 [142]))
        ]) "stack-reorder.c":16:1 1048 {stack_protect_test_di}
     (expr_list:REG_DEAD (reg/f:DI 0 x0 [116])
        (expr_list:REG_UNUSED (reg:DI 2 x2 [142])
            (nil))))


In this snippet the stack protect set and test patterns are right next to each
other, causing the stack protector to essentially do nothing.
The RTL insns to set the two elements in `buf[]` are before this snippet.

The stack_protect_set and stack_protect_test patterns are put together in the
sched1 pass (as seen by the change in the RTL between the previous dump and
that
one).

I would like to know what is supposed to stop RTL from the stack_protect_set
pattern from being reordered around the code it protects like this?
I don't believe aarch64 is doing anything special here -- the stack protect set
and test patterns are very similar to those of other backends.

I recognise this is an unlikely pattern of code and that it doesn't present as
much of a security risk as things like calling memcpy or setting memory through
some sort of loop.