Hi, Richard,
I have sent out my v4[1], please let me know if i got something wrong :).

[1] https://gcc.gnu.org/pipermail/gcc-patches/2022-February/589921.html

Thanks,
Dan.


On 1/31/22 09:00, Richard Sandiford wrote:
Dan Li <ashim...@linux.alibaba.com> writes:
Shadow Call Stack can be used to protect the return address of a
function at runtime, and clang already supports this feature[1].

To enable SCS in user mode, in addition to compiler, other support
is also required (as discussed in [2]). This patch only adds basic
support for SCS from the compiler side, and provides convenience
for users to enable SCS.

For linux kernel, only the support of the compiler is required.

[1] https://clang.llvm.org/docs/ShadowCallStack.html
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102768

Signed-off-by: Dan Li <ashim...@linux.alibaba.com>

gcc/ChangeLog:

        * config/aarch64/aarch64.c (aarch64_layout_frame):
        Change callee_adjust when scs is enabled.
        (aarch64_restore_callee_saves): Avoid pop x30 twice
        when scs is enabled.
        (aarch64_expand_prologue): Push x30 onto SCS before it's
        pushed onto stack.
        (aarch64_expand_epilogue): Pop x30 frome SCS, while
        preventing it from being popped from the regular stack again.
        (aarch64_override_options_internal): Add SCS compile option check.
        (TARGET_HAVE_SHADOW_CALL_STACK): New hook.
        * config/aarch64/aarch64.h (struct GTY): Add is_scs_enabled.
        * config/aarch64/aarch64.md (scs_push):  New template.
        (scs_pop): Likewise.
        * doc/invoke.texi: Document -fsanitize=shadow-call-stack.
        * doc/tm.texi: Regenerate.
        * doc/tm.texi.in: Add hook have_shadow_call_stack.
        * flag-types.h (enum sanitize_code):
        Add SANITIZE_SHADOW_CALL_STACK.
        * opts.c: Add shadow-call-stack.
        * target.def: New hook.
        * toplev.c (process_options): Add SCS compile option check.

gcc/testsuite/ChangeLog:

        * gcc.target/aarch64/shadow_call_stack_1.c: New test.
        * gcc.target/aarch64/shadow_call_stack_2.c: New test.
        * gcc.target/aarch64/shadow_call_stack_3.c: New test.
        * gcc.target/aarch64/shadow_call_stack_4.c: New test.
        * gcc.target/aarch64/shadow_call_stack_5.c: New test.
        * gcc.target/aarch64/shadow_call_stack_6.c: New test.
        * gcc.target/aarch64/shadow_call_stack_7.c: New test.
        * gcc.target/aarch64/shadow_call_stack_8.c: New test.
---
V3:
- Change scs_push/pop to standard move patterns.
- Optimize scs_pop to avoid pop x30 twice when shadow stack is enabled.

  gcc/config/aarch64/aarch64.c                  | 66 +++++++++++++++++--
  gcc/config/aarch64/aarch64.h                  |  4 ++
  gcc/config/aarch64/aarch64.md                 | 10 +++
  gcc/doc/invoke.texi                           | 30 +++++++++
  gcc/doc/tm.texi                               |  5 ++
  gcc/doc/tm.texi.in                            |  2 +
  gcc/flag-types.h                              |  2 +
  gcc/opts.c                                    |  1 +
  gcc/target.def                                |  8 +++
  .../gcc.target/aarch64/shadow_call_stack_1.c  |  6 ++
  .../gcc.target/aarch64/shadow_call_stack_2.c  |  6 ++
  .../gcc.target/aarch64/shadow_call_stack_3.c  | 45 +++++++++++++
  .../gcc.target/aarch64/shadow_call_stack_4.c  | 20 ++++++
  .../gcc.target/aarch64/shadow_call_stack_5.c  | 18 +++++
  .../gcc.target/aarch64/shadow_call_stack_6.c  | 18 +++++
  .../gcc.target/aarch64/shadow_call_stack_7.c  | 18 +++++
  .../gcc.target/aarch64/shadow_call_stack_8.c  | 24 +++++++
  gcc/toplev.c                                  | 10 +++
  18 files changed, 289 insertions(+), 4 deletions(-)
  create mode 100644 gcc/testsuite/gcc.target/aarch64/shadow_call_stack_1.c
  create mode 100644 gcc/testsuite/gcc.target/aarch64/shadow_call_stack_2.c
  create mode 100644 gcc/testsuite/gcc.target/aarch64/shadow_call_stack_3.c
  create mode 100644 gcc/testsuite/gcc.target/aarch64/shadow_call_stack_4.c
  create mode 100644 gcc/testsuite/gcc.target/aarch64/shadow_call_stack_5.c
  create mode 100644 gcc/testsuite/gcc.target/aarch64/shadow_call_stack_6.c
  create mode 100644 gcc/testsuite/gcc.target/aarch64/shadow_call_stack_7.c
  create mode 100644 gcc/testsuite/gcc.target/aarch64/shadow_call_stack_8.c

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 699c105a42a..461c205010e 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -79,6 +79,7 @@
  #include "tree-ssa-loop-niter.h"
  #include "fractional-cost.h"
  #include "rtlanal.h"
+#include "asan.h"
/* This file should be included last. */
  #include "target-def.h"
@@ -7478,10 +7479,31 @@ aarch64_layout_frame (void)
    frame.sve_callee_adjust = 0;
    frame.callee_offset = 0;
+ /* Shadow call stack only deal with functions where the LR is pushed

Typo: s/deal/deals/

+     onto the stack and without specifying the "no_sanitize" attribute
+     with the argument "shadow-call-stack".  */
+  frame.is_scs_enabled
+    = (!crtl->calls_eh_return
+       && (sanitize_flags_p (SANITIZE_SHADOW_CALL_STACK)
+          && known_ge (cfun->machine->frame.reg_offset[LR_REGNUM], 0)));

Nit, but normal GCC style would be to use a single chain of &&s here:

   frame.is_scs_enabled
     = (!crtl->calls_eh_return
        && sanitize_flags_p (SANITIZE_SHADOW_CALL_STACK)
        && known_ge (cfun->machine->frame.reg_offset[LR_REGNUM], 0));

+
+  /* When shadow call stack is enabled, the scs_pop in the epilogue will
+     restore x30, and we don't need to pop x30 again in the traditional
+     way.  At this time, if candidate2 is x30, we need to adjust
+     max_push_offset to 256 to ensure that the offset meets the requirements
+     of emit_move_insn.  Similarly, if candidate1 is x30, we need to set
+     max_push_offset to 0, because x30 is not popped up at this time, so
+     callee_adjust cannot be adjusted.  */
    HOST_WIDE_INT max_push_offset = 0;
    if (frame.wb_candidate2 != INVALID_REGNUM)
-    max_push_offset = 512;
-  else if (frame.wb_candidate1 != INVALID_REGNUM)
+    {
+      if (frame.is_scs_enabled && frame.wb_candidate2 == R30_REGNUM)
+       max_push_offset = 256;
+      else
+       max_push_offset = 512;
+    }
+  else if ((frame.wb_candidate1 != INVALID_REGNUM)
+          && !(frame.is_scs_enabled && frame.wb_candidate1 == R30_REGNUM))
      max_push_offset = 256;
    HOST_WIDE_INT const_size, const_outgoing_args_size, const_fp_offset;

Maybe we should instead add separate fields for wb_push_candidate[12] and
wb_pop_candidate[12].  The pop candidates would start out the same as the
push candidates, but R30_REGNUM would get replaced with INVALID_REGNUM
for SCS.

Admittedly, suppressing the restore of x30 is turning out to be a bit
more difficult than I'd realised :-/

[…]
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 2792bb29adb..1610a4fd74c 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -916,6 +916,10 @@ struct GTY (()) aarch64_frame
    unsigned spare_pred_reg;
bool laid_out;
+
+  /* Nonzero if shadow call stack should be enabled for the current
+     function, otherwise return FALSE.  */

“True” seems better than “Nonzero” given that this is a bool.
(A lot of GCC bools were originally ints, which is why “nonzero”
still appears in non-obvious places.)

I think we can just drop “otherwise return FALSE”: “return” doesn't
seem appropriate here, given that it's a variable.

Looks great otherwise.  Thanks especially for testing the corner cases. :-)

One minor thing:

+/* { dg-final { scan-assembler-times "str\tx30, \\\[x18\\\], \[#|$\]?8" 2 } } 
*/
+/* { dg-final { scan-assembler-times "ldr\tx30, \\\[x18, \[#|$\]?-8\\\]!" 2 } 
} */

This sort of regexp can be easier to write if you quote them using {…}
rather than "…", since it reduces the number of backslashes needed.  E.g.:

/* { dg-final { scan-assembler-times {str\tx30, \[x18\], [#|$]?8} 2 } } */

The current version is fine too though, and is widely used.  Just mentioning
it in case it's useful in future.

Also, [#|$]? can be written #?.

Thanks,
Richard

Reply via email to