Re: [PATCH v2, rs6000] Add -msafe-indirect-jumps option and implement safe bctr / bctrl

[Richard Biener <richard dot guenther at gmail dot com>]

On Sun, Jan 14, 2018 at 5:53 AM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> [This patch supercedes and extends https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01135.html.
> There was a small error in the assembly code produced by that patch (bad
> memory on my account of how to spell "crset eq").  I've also increased the
> function provided; see below.]
>
> This patch adds a new option for the compiler to produce only "safe" indirect
> jumps, in the sense that these jumps are deliberately mispredicted to inhibit
> speculative execution.  For now, this option is undocumented; this may change
> at some future date.  It is intended eventually for the linker to also honor
> this flag when creating PLT stubs, for example.
>
> In addition to the new option, I've included changes to indirect calls for
> the ELFv2 ABI when the option is specified.  In place of bctrl, we generate
> a "crset eq" followed by a beqctrl-.  Using the CR0.eq bit is safe since CR0
> is volatile over the call.
>
> I've also added code to replace uses of bctr when the new option is specified,
> with the sequence
>
>         crset 4x[CRb]+2
>         beqctr- CRb
>         b .
>
> where CRb is an available condition register field.  This applies to all
> subtargets, and in particular is not restricted to ELFv2.  The use cases
> covered here are computed gotos and switch statements.
>
> NOT yet covered by this patch: indirect calls for ELFv1.  That will come later.
>
> Please let me know if there is a better way to represent the crset without
> an unspec.  For the indirect jump, I don't see a way around it due to the
> expected form of indirect jumps in cfganal.c.
>
> Bootstrapped and tested on powerpc64-linux-gnu and powerpc64le-linux-gnu with
> no regressions.  Is this okay for trunk?

As this sounds Spectre related feel free to backport this to the GCC 7 branch
as well (even if you don't hit the Wednesday deadline for RC1 of GCC 7.3).

Thanks,
Richard.

> Thanks,
> Bill
>
>
> [gcc]
>
> 2018-01-13  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * config/rs6000/rs6000.c (rs6000_opt_vars): Add entry for
>         -msafe-indirect-jumps.
>         * config/rs6000/rs6000.md (UNSPEC_CRSET_EQ): New UNSPEC enum.
>         (UNSPEC_COMP_GOTO_CR): Likewise.
>         (*call_indirect_elfv2<mode>): Disable for -msafe-indirect-jumps.
>         (*call_indirect_elfv2<mode>_safe): New define_insn.
>         (*call_value_indirect_elfv2<mode>): Disable for
>         -msafe-indirect-jumps.
>         (*call_value_indirect_elfv2<mode>_safe): New define_insn.
>         (indirect_jump): Emit different RTL for -msafe-indirect-jumps.
>         (*indirect_jump<mode>): Disable for -msafe-indirect-jumps.
>         (*indirect_jump<mode>_safe): New define_insn.
>         (*set_cr_eq): New define_insn.
>         (tablejump): Emit different RTL for -msafe-indirect-jumps.
>         (tablejumpsi): Disable for -msafe-indirect-jumps.
>         (tablejumpsi_safe): New define_expand.
>         (tablejumpdi): Disable for -msafe-indirect-jumps.
>         (tablejumpdi_safe): New define_expand.
>         (*tablejump<mode>_internal1): Disable for -msafe-indirect-jumps.
>         (*tablejump<mode>_internal1_safe): New define_insn.
>         * config/rs6000/rs6000.opt (msafe-indirect-jumps): New option.
>
> [gcc/testsuite]
>
> 2018-01-13  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * gcc.target/powerpc/safe-indirect-jump-1.c: New file.
>         * gcc.target/powerpc/safe-indirect-jump-2.c: New file.
>         * gcc.target/powerpc/safe-indirect-jump-3.c: New file.
>
>
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c  (revision 256364)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -36726,6 +36726,9 @@ static struct rs6000_opt_var const rs6000_opt_vars
>    { "sched-epilog",
>      offsetof (struct gcc_options, x_TARGET_SCHED_PROLOG),
>      offsetof (struct cl_target_option, x_TARGET_SCHED_PROLOG), },
> +  { "safe-indirect-jumps",
> +    offsetof (struct gcc_options, x_rs6000_safe_indirect_jumps),
> +    offsetof (struct cl_target_option, x_rs6000_safe_indirect_jumps), },
>  };
>
>  /* Inner function to handle attribute((target("..."))) and #pragma GCC target
> Index: gcc/config/rs6000/rs6000.md
> ===================================================================
> --- gcc/config/rs6000/rs6000.md (revision 256364)
> +++ gcc/config/rs6000/rs6000.md (working copy)
> @@ -150,6 +150,8 @@
>     UNSPEC_SIGNBIT
>     UNSPEC_SF_FROM_SI
>     UNSPEC_SI_FROM_SF
> +   UNSPEC_CRSET_EQ
> +   UNSPEC_COMP_GOTO_CR
>    ])
>
>  ;;
> @@ -11222,11 +11224,22 @@
>          (match_operand 1 "" "g,g"))
>     (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 2 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
>     (clobber (reg:P LR_REGNO))]
> -  "DEFAULT_ABI == ABI_ELFv2"
> +  "DEFAULT_ABI == ABI_ELFv2 && !rs6000_safe_indirect_jumps"
>    "b%T0l\;<ptrload> 2,%2(1)"
>    [(set_attr "type" "jmpreg")
>     (set_attr "length" "8")])
>
> +;; Variant with deliberate misprediction.
> +(define_insn "*call_indirect_elfv2<mode>_safe"
> +  [(call (mem:SI (match_operand:P 0 "register_operand" "c,*l"))
> +        (match_operand 1 "" "g,g"))
> +   (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 2 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
> +   (clobber (reg:P LR_REGNO))]
> +  "DEFAULT_ABI == ABI_ELFv2 && rs6000_safe_indirect_jumps"
> +  "crset eq\;beq%T0l-\;<ptrload> 2,%2(1)"
> +  [(set_attr "type" "jmpreg")
> +   (set_attr "length" "12")])
> +
>  (define_insn "*call_value_indirect_elfv2<mode>"
>    [(set (match_operand 0 "" "")
>         (call (mem:SI (match_operand:P 1 "register_operand" "c,*l"))
> @@ -11233,11 +11246,22 @@
>               (match_operand 2 "" "g,g")))
>     (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 3 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
>     (clobber (reg:P LR_REGNO))]
> -  "DEFAULT_ABI == ABI_ELFv2"
> +  "DEFAULT_ABI == ABI_ELFv2 && !rs6000_safe_indirect_jumps"
>    "b%T1l\;<ptrload> 2,%3(1)"
>    [(set_attr "type" "jmpreg")
>     (set_attr "length" "8")])
>
> +; Variant with deliberate misprediction.
> +(define_insn "*call_value_indirect_elfv2<mode>_safe"
> +  [(set (match_operand 0 "" "")
> +       (call (mem:SI (match_operand:P 1 "register_operand" "c,*l"))
> +             (match_operand 2 "" "g,g")))
> +   (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 3 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
> +   (clobber (reg:P LR_REGNO))]
> +  "DEFAULT_ABI == ABI_ELFv2 && rs6000_safe_indirect_jumps"
> +  "crset eq\;beq%T1l-\;<ptrload> 2,%3(1)"
> +  [(set_attr "type" "jmpreg")
> +   (set_attr "length" "12")])
>
>  ;; Call subroutine returning any type.
>  (define_expand "untyped_call"
> @@ -12917,15 +12941,50 @@
>    [(set_attr "type" "jmpreg")])
>
>  (define_expand "indirect_jump"
> -  [(set (pc) (match_operand 0 "register_operand"))])
> +  [(set (pc) (match_operand 0 "register_operand"))]
> + ""
> +{
> +  /* We need to reserve a CR when forcing a mispredicted jump.  */
> +  if (rs6000_safe_indirect_jumps) {
> +    rtx ccreg = gen_reg_rtx (CCmode);
> +    emit_insn (gen_rtx_SET (ccreg,
> +                           gen_rtx_UNSPEC (CCmode,
> +                                           gen_rtvec (1, const0_rtx),
> +                                           UNSPEC_CRSET_EQ)));
> +    rtvec v = rtvec_alloc (2);
> +    RTVEC_ELT (v, 0) = operands[0];
> +    RTVEC_ELT (v, 1) = ccreg;
> +    emit_jump_insn (gen_rtx_SET (pc_rtx,
> +                                gen_rtx_UNSPEC (Pmode, v,
> +                                                UNSPEC_COMP_GOTO_CR)));
> +    DONE;
> +  }
> +})
>
>  (define_insn "*indirect_jump<mode>"
>    [(set (pc)
>         (match_operand:P 0 "register_operand" "c,*l"))]
> -  ""
> +  "!rs6000_safe_indirect_jumps"
>    "b%T0"
>    [(set_attr "type" "jmpreg")])
>
> +(define_insn "*indirect_jump<mode>_safe"
> +  [(set (pc)
> +       (unspec:P [(match_operand:P 0 "register_operand" "c,*l")
> +                  (match_operand:CC 1 "cc_reg_operand" "y,y")]
> +                  UNSPEC_COMP_GOTO_CR))]
> +  "rs6000_safe_indirect_jumps"
> +  "beq%T0- %1\;b ."
> +  [(set_attr "type" "jmpreg")
> +   (set_attr "length" "8")])
> +
> +(define_insn "*set_cr_eq"
> +  [(set (match_operand:CC 0 "cc_reg_operand" "=y")
> +       (unspec:CC [(const_int 0)] UNSPEC_CRSET_EQ))]
> +  "rs6000_safe_indirect_jumps"
> +  "crset %E0"
> +  [(set_attr "type" "cr_logical")])
> +
>  ;; Table jump for switch statements:
>  (define_expand "tablejump"
>    [(use (match_operand 0))
> @@ -12933,9 +12992,19 @@
>    ""
>  {
>    if (TARGET_32BIT)
> -    emit_jump_insn (gen_tablejumpsi (operands[0], operands[1]));
> +    {
> +      if (rs6000_safe_indirect_jumps)
> +       emit_jump_insn (gen_tablejumpsi_safe (operands[0], operands[1]));
> +      else
> +       emit_jump_insn (gen_tablejumpsi (operands[0], operands[1]));
> +    }
>    else
> -    emit_jump_insn (gen_tablejumpdi (operands[0], operands[1]));
> +    {
> +      if (rs6000_safe_indirect_jumps)
> +       emit_jump_insn (gen_tablejumpdi_safe (operands[0], operands[1]));
> +      else
> +       emit_jump_insn (gen_tablejumpdi (operands[0], operands[1]));
> +    }
>    DONE;
>  })
>
> @@ -12946,7 +13015,7 @@
>     (parallel [(set (pc)
>                    (match_dup 3))
>               (use (label_ref (match_operand 1)))])]
> -  "TARGET_32BIT"
> +  "TARGET_32BIT && !rs6000_safe_indirect_jumps"
>  {
>    operands[0] = force_reg (SImode, operands[0]);
>    operands[2] = force_reg (SImode, gen_rtx_LABEL_REF (SImode, operands[1]));
> @@ -12953,6 +13022,23 @@
>    operands[3] = gen_reg_rtx (SImode);
>  })
>
> +(define_expand "tablejumpsi_safe"
> +  [(set (match_dup 3)
> +       (plus:SI (match_operand:SI 0)
> +                (match_dup 2)))
> +   (set (match_dup 4) (unspec:CC [(const_int 0)] UNSPEC_CRSET_EQ))
> +   (parallel [(set (pc)
> +                  (match_dup 3))
> +             (use (label_ref (match_operand 1)))
> +             (use (match_dup 4))])]
> +  "TARGET_32BIT && rs6000_safe_indirect_jumps"
> +{
> +  operands[0] = force_reg (SImode, operands[0]);
> +  operands[2] = force_reg (SImode, gen_rtx_LABEL_REF (SImode, operands[1]));
> +  operands[3] = gen_reg_rtx (SImode);
> +  operands[4] = gen_reg_rtx (CCmode);
> +})
> +
>  (define_expand "tablejumpdi"
>    [(set (match_dup 4)
>          (sign_extend:DI (match_operand:SI 0 "lwa_operand")))
> @@ -12962,7 +13048,7 @@
>     (parallel [(set (pc)
>                    (match_dup 3))
>               (use (label_ref (match_operand 1)))])]
> -  "TARGET_64BIT"
> +  "TARGET_64BIT && !rs6000_safe_indirect_jumps"
>  {
>    operands[2] = force_reg (DImode, gen_rtx_LABEL_REF (DImode, operands[1]));
>    operands[3] = gen_reg_rtx (DImode);
> @@ -12969,14 +13055,43 @@
>    operands[4] = gen_reg_rtx (DImode);
>  })
>
> +(define_expand "tablejumpdi_safe"
> +  [(set (match_dup 4)
> +        (sign_extend:DI (match_operand:SI 0 "lwa_operand")))
> +   (set (match_dup 5) (unspec:CC [(const_int 0)] UNSPEC_CRSET_EQ))
> +   (set (match_dup 3)
> +       (plus:DI (match_dup 4)
> +                (match_dup 2)))
> +   (parallel [(set (pc)
> +                  (match_dup 3))
> +             (use (label_ref (match_operand 1)))
> +             (use (match_dup 5))])]
> +  "TARGET_64BIT && rs6000_safe_indirect_jumps"
> +{
> +  operands[2] = force_reg (DImode, gen_rtx_LABEL_REF (DImode, operands[1]));
> +  operands[3] = gen_reg_rtx (DImode);
> +  operands[4] = gen_reg_rtx (DImode);
> +  operands[5] = gen_reg_rtx (CCmode);
> +})
> +
>  (define_insn "*tablejump<mode>_internal1"
>    [(set (pc)
>         (match_operand:P 0 "register_operand" "c,*l"))
>     (use (label_ref (match_operand 1)))]
> -  ""
> +  "!rs6000_safe_indirect_jumps"
>    "b%T0"
>    [(set_attr "type" "jmpreg")])
>
> +(define_insn "*tablejump<mode>_internal1_safe"
> +  [(set (pc)
> +       (match_operand:P 0 "register_operand" "c,*l"))
> +   (use (label_ref (match_operand 1)))
> +   (use (match_operand:CC 2 "cc_reg_operand" "y,y"))]
> +  "rs6000_safe_indirect_jumps"
> +  "beq%T0- %2\;b ."
> +  [(set_attr "type" "jmpreg")
> +   (set_attr "length" "8")])
> +
>  (define_insn "nop"
>    [(unspec [(const_int 0)] UNSPEC_NOP)]
>    ""
> Index: gcc/config/rs6000/rs6000.opt
> ===================================================================
> --- gcc/config/rs6000/rs6000.opt        (revision 256364)
> +++ gcc/config/rs6000/rs6000.opt        (working copy)
> @@ -617,3 +617,8 @@ Use the given offset for addressing the stack-prot
>
>  TargetVariable
>  long rs6000_stack_protector_guard_offset = 0
> +
> +;; -msafe-indirect-jumps adds deliberate misprediction to indirect
> +;; branches via the CTR.
> +msafe-indirect-jumps
> +Target Undocumented Var(rs6000_safe_indirect_jumps) Init(0) Save
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-1.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-1.c     (nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-1.c     (working copy)
> @@ -0,0 +1,14 @@
> +/* { dg-do compile { target { powerpc64le-*-* } } } */
> +/* { dg-options "-msafe-indirect-jumps" } */
> +
> +/* Test for deliberate misprediction of indirect calls for ELFv2.  */
> +
> +extern int (*f)();
> +
> +int bar ()
> +{
> +  return (*f) ();
> +}
> +
> +/* { dg-final { scan-assembler "crset eq" } } */
> +/* { dg-final { scan-assembler "beqctrl-" } } */
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c     (nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c     (working copy)
> @@ -0,0 +1,33 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */
> +/* { dg-options "-msafe-indirect-jumps" } */
> +
> +/* Test for deliberate misprediction of computed goto.  */
> +
> +int bar (int);
> +int baz (int);
> +int spaz (int);
> +
> +int foo (int x)
> +{
> +  static void *labptr[] = { &&lab0, &&lab1, &&lab2 };
> +
> +  if (x < 0 || x > 2)
> +    return -1;
> +
> +  goto *labptr[x];
> +
> + lab0:
> +  return bar (x);
> +
> + lab1:
> +  return baz (x) + 1;
> +
> + lab2:
> +  return spaz (x) / 2;
> +}
> +
> +/* The following assumes CR7 as the first chosen volatile.  */
> +
> +/* { dg-final { scan-assembler "crset 30" } } */
> +/* { dg-final { scan-assembler "beqctr- 7" } } */
> +/* { dg-final { scan-assembler "b ." } } */
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c     (nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c     (working copy)
> @@ -0,0 +1,52 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */
> +/* { dg-options "-msafe-indirect-jumps" } */
> +
> +/* Test for deliberate misprediction of jump tables.  */
> +
> +void bar (void);
> +
> +int foo (int x)
> +{
> +  int a;
> +
> +  switch (x)
> +    {
> +    default:
> +      a = -1;
> +      break;
> +    case 0:
> +      a = x * x;
> +      break;
> +    case 1:
> +      a = x + 1;
> +      break;
> +    case 2:
> +      a = x + x;
> +      break;
> +    case 3:
> +      a = x << 3;
> +      break;
> +    case 4:
> +      a = x >> 1;
> +      break;
> +    case 5:
> +      a = x;
> +      break;
> +    case 6:
> +      a = 0;
> +      break;
> +    case 7:
> +      a = x * x + x;
> +      break;
> +    }
> +
> +  bar();
> +
> +  return a;
> +}
> +
> +/* The following assumes CR7 as the first chosen volatile.  */
> +
> +/* { dg-final { scan-assembler "crset 30" } } */
> +/* { dg-final { scan-assembler "beqctr- 7" } } */
> +/* { dg-final { scan-assembler "b ." } } */
>