arm: Fix canary address calculation for non-PIC
authorRichard Sandiford <richard.sandiford@arm.com>
Thu, 24 Sep 2020 09:06:11 +0000 (10:06 +0100)
committerRichard Sandiford <richard.sandiford@arm.com>
Thu, 24 Sep 2020 09:06:11 +0000 (10:06 +0100)
For non-PIC, the stack protector patterns did:

  rtx mem = XEXP (force_const_mem (SImode, operands[1]), 0);
  emit_move_insn (operands[2], mem);

Here, operands[1] is the address of the canary (&__stack_chk_guard)
and operands[2] is the register that we want to move that address into.
However, the code above instead sets operands[2] to the address of a
constant pool entry that contains &__stack_chk_guard, rather than to
&__stack_chk_guard itself.  The sequence therefore does one less
pointer indirection than it should.

The net effect was to use &__stack_chk_guard for stack-smash detection,
instead of using __stack_chk_guard itself.

gcc/
* config/arm/arm.md (*stack_protect_combined_set_insn): For non-PIC,
load the address of the canary rather than the address of the
constant pool entry that points to it.
(*stack_protect_combined_test_insn): Likewise.

gcc/testsuite/
* gcc.target/arm/stack-protector-3.c: New test.
* gcc.target/arm/stack-protector-4.c: Likewise.

gcc/config/arm/arm.md
gcc/testsuite/gcc.target/arm/stack-protector-3.c [new file with mode: 0644]
gcc/testsuite/gcc.target/arm/stack-protector-4.c [new file with mode: 0644]

index bffdb0b3987f640491fb798ff97503451a74f299..c4fa116ab77ce0f3d54c4ac10de9c83d3836264e 100644 (file)
        operands[2] = operands[1];
       else
        {
-         rtx mem = XEXP (force_const_mem (SImode, operands[1]), 0);
+         rtx mem = force_const_mem (SImode, operands[1]);
          emit_move_insn (operands[2], mem);
        }
     }
        operands[3] = operands[1];
       else
        {
-         rtx mem = XEXP (force_const_mem (SImode, operands[1]), 0);
+         rtx mem = force_const_mem (SImode, operands[1]);
          emit_move_insn (operands[3], mem);
        }
     }
diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-3.c b/gcc/testsuite/gcc.target/arm/stack-protector-3.c
new file mode 100644 (file)
index 0000000..b8f77fa
--- /dev/null
@@ -0,0 +1,38 @@
+/* { dg-do run } */
+/* { dg-require-effective-target fstack_protector } */
+/* { dg-options "-fstack-protector-all -O2" } */
+
+extern volatile long *stack_chk_guard_ptr;
+
+void __attribute__ ((noipa))
+f (void)
+{
+  volatile int x;
+  /* Munging the contents of __stack_chk_guard should trigger a
+     stack-smashing failure for this function.  */
+  *stack_chk_guard_ptr += 1;
+}
+
+asm (
+"      .data\n"
+"      .align  3\n"
+"      .globl  stack_chk_guard_ptr\n"
+"stack_chk_guard_ptr:\n"
+"      .word   __stack_chk_guard\n"
+"      .weak   __stack_chk_guard\n"
+"__stack_chk_guard:\n"
+"      .word   0xdead4321\n"
+"      .text\n"
+"      .type   __stack_chk_fail, %function\n"
+"__stack_chk_fail:\n"
+"      movs    r0, #0\n"
+"      b       exit\n"
+"      .size   __stack_chk_fail, .-__stack_chk_fail"
+);
+
+int
+main (void)
+{
+  f ();
+  __builtin_abort ();
+}
diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-4.c b/gcc/testsuite/gcc.target/arm/stack-protector-4.c
new file mode 100644 (file)
index 0000000..6334dd0
--- /dev/null
@@ -0,0 +1,6 @@
+/* { dg-do run } */
+/* { dg-require-effective-target fstack_protector } */
+/* { dg-require-effective-target fpic } */
+/* { dg-options "-fstack-protector-all -O2 -fpic" } */
+
+#include "stack-protector-3.c"