| Message ID | 20210208125308.23447-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Always save and restore shadow stack pointer | expand |
On Mon, Feb 8, 2021 at 3:07 PM H.J. Lu via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > When the SHSTK feature is not available or not enabled, RDSSP is a NOP, > always save and restore shadow stack pointer to support compiling source > codes, containing __builtin_setjmp and __builtin_longjmp, with different > -fcf-protection options. Is that an ABI change for -fno-cf-protection? > PR target/98997 > * config/i386/i386.md (save_stack_nonlocal): Always save shadow > stack pointer. > (restore_stack_nonlocal): Always restore shadow stack pointer. > (@rdssp<mode>): Make it unconditional. > (@incssp<mode>): Likewise. > --- > gcc/config/i386/i386.md | 150 ++++++++++++++++++---------------------- > 1 file changed, 68 insertions(+), 82 deletions(-) > > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index b60784a2908..2510bd8a73d 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -19398,21 +19398,16 @@ (define_expand "save_stack_nonlocal" > (match_operand 1 "register_operand"))] > "" > { > - rtx stack_slot; > - > - if (flag_cf_protection & CF_RETURN) > - { > - /* Copy shadow stack pointer to the first slot > - and stack pointer to the second slot. */ > - rtx ssp_slot = adjust_address (operands[0], word_mode, 0); > - stack_slot = adjust_address (operands[0], Pmode, UNITS_PER_WORD); > - > - rtx reg_ssp = force_reg (word_mode, const0_rtx); > - emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp)); > - emit_move_insn (ssp_slot, reg_ssp); > - } > - else > - stack_slot = adjust_address (operands[0], Pmode, 0); > + /* Copy shadow stack pointer to the first slot and stack pointer to > + the second slot. */ > + rtx ssp_slot = adjust_address (operands[0], word_mode, 0); > + rtx stack_slot = adjust_address (operands[0], Pmode, UNITS_PER_WORD); > + > + /* If the SHSTK feature is not available or not enabled, the RDSSP > + instruction is a NOP and REG_SSP is 0. */ > + rtx reg_ssp = force_reg (word_mode, const0_rtx); > + emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp)); > + emit_move_insn (ssp_slot, reg_ssp); > emit_move_insn (stack_slot, operands[1]); > DONE; > }) > @@ -19422,72 +19417,63 @@ (define_expand "restore_stack_nonlocal" > (match_operand 1 "memory_operand" ""))] > "" > { > - rtx stack_slot; > + /* Restore shadow stack pointer from the first slot and stack pointer > + from the second slot. */ > + rtx ssp_slot = adjust_address (operands[1], word_mode, 0); > + rtx stack_slot = adjust_address (operands[1], Pmode, UNITS_PER_WORD); > > - if (flag_cf_protection & CF_RETURN) > - { > - /* Restore shadow stack pointer from the first slot > - and stack pointer from the second slot. */ > - rtx ssp_slot = adjust_address (operands[1], word_mode, 0); > - stack_slot = adjust_address (operands[1], Pmode, UNITS_PER_WORD); > - > - /* Get the current shadow stack pointer. The code below will check if > - SHSTK feature is enabled. If it is not enabled the RDSSP instruction > - is a NOP. */ > - rtx reg_ssp = force_reg (word_mode, const0_rtx); > - emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp)); > - > - /* Compare through subtraction the saved and the current ssp > - to decide if ssp has to be adjusted. */ > - reg_ssp = expand_simple_binop (word_mode, MINUS, > - reg_ssp, ssp_slot, > - reg_ssp, 1, OPTAB_DIRECT); > - > - /* Compare and jump over adjustment code. */ > - rtx noadj_label = gen_label_rtx (); > - emit_cmp_and_jump_insns (reg_ssp, const0_rtx, EQ, NULL_RTX, > - word_mode, 1, noadj_label); > - > - /* Compute the number of frames to adjust. */ > - rtx reg_adj = gen_lowpart (ptr_mode, reg_ssp); > - rtx reg_adj_neg = expand_simple_unop (ptr_mode, NEG, reg_adj, > - NULL_RTX, 1); > - > - reg_adj = expand_simple_binop (ptr_mode, LSHIFTRT, reg_adj_neg, > - GEN_INT (exact_log2 (UNITS_PER_WORD)), > - reg_adj, 1, OPTAB_DIRECT); > - > - /* Check if number of frames <= 255 so no loop is needed. */ > - rtx inc_label = gen_label_rtx (); > - emit_cmp_and_jump_insns (reg_adj, GEN_INT (255), LEU, NULL_RTX, > - ptr_mode, 1, inc_label); > - > - /* Adjust the ssp in a loop. */ > - rtx loop_label = gen_label_rtx (); > - emit_label (loop_label); > - LABEL_NUSES (loop_label) = 1; > - > - rtx reg_255 = force_reg (word_mode, GEN_INT (255)); > - emit_insn (gen_incssp (word_mode, reg_255)); > - > - reg_adj = expand_simple_binop (ptr_mode, MINUS, > - reg_adj, GEN_INT (255), > - reg_adj, 1, OPTAB_DIRECT); > - > - /* Compare and jump to the loop label. */ > - emit_cmp_and_jump_insns (reg_adj, GEN_INT (255), GTU, NULL_RTX, > - ptr_mode, 1, loop_label); > - > - emit_label (inc_label); > - LABEL_NUSES (inc_label) = 1; > - > - emit_insn (gen_incssp (word_mode, reg_ssp)); > - > - emit_label (noadj_label); > - LABEL_NUSES (noadj_label) = 1; > - } > - else > - stack_slot = adjust_address (operands[1], Pmode, 0); > + /* If the SHSTK feature is not available or not enabled, the RDSSP > + instruction is a NOP and REG_SSP is 0. */ > + rtx reg_ssp = force_reg (word_mode, const0_rtx); > + emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp)); > + > + /* Compare through subtraction the saved and the current ssp to > + decide if ssp has to be adjusted. */ > + reg_ssp = expand_simple_binop (word_mode, MINUS, reg_ssp, ssp_slot, > + reg_ssp, 1, OPTAB_DIRECT); > + > + /* Compare and jump over adjustment code. */ > + rtx noadj_label = gen_label_rtx (); > + emit_cmp_and_jump_insns (reg_ssp, const0_rtx, EQ, NULL_RTX, > + word_mode, 1, noadj_label); > + > + /* Compute the number of frames to adjust. */ > + rtx reg_adj = gen_lowpart (ptr_mode, reg_ssp); > + rtx reg_adj_neg = expand_simple_unop (ptr_mode, NEG, reg_adj, > + NULL_RTX, 1); > + > + reg_adj = expand_simple_binop (ptr_mode, LSHIFTRT, reg_adj_neg, > + GEN_INT (exact_log2 (UNITS_PER_WORD)), > + reg_adj, 1, OPTAB_DIRECT); > + > + /* Check if number of frames <= 255 so no loop is needed. */ > + rtx inc_label = gen_label_rtx (); > + emit_cmp_and_jump_insns (reg_adj, GEN_INT (255), LEU, NULL_RTX, > + ptr_mode, 1, inc_label); > + > + /* Adjust the ssp in a loop. */ > + rtx loop_label = gen_label_rtx (); > + emit_label (loop_label); > + LABEL_NUSES (loop_label) = 1; > + > + rtx reg_255 = force_reg (word_mode, GEN_INT (255)); > + emit_insn (gen_incssp (word_mode, reg_255)); > + > + reg_adj = expand_simple_binop (ptr_mode, MINUS, > + reg_adj, GEN_INT (255), > + reg_adj, 1, OPTAB_DIRECT); > + > + /* Compare and jump to the loop label. */ > + emit_cmp_and_jump_insns (reg_adj, GEN_INT (255), GTU, NULL_RTX, > + ptr_mode, 1, loop_label); > + > + emit_label (inc_label); > + LABEL_NUSES (inc_label) = 1; > + > + emit_insn (gen_incssp (word_mode, reg_ssp)); > + > + emit_label (noadj_label); > + LABEL_NUSES (noadj_label) = 1; > emit_move_insn (operands[0], stack_slot); > DONE; > }) > @@ -21455,7 +21441,7 @@ (define_insn "@rdssp<mode>" > [(set (match_operand:SWI48 0 "register_operand" "=r") > (unspec_volatile:SWI48 [(match_operand:SWI48 1 "register_operand" "0")] > UNSPECV_NOP_RDSSP))] > - "TARGET_SHSTK || (flag_cf_protection & CF_RETURN)" > + "" > "rdssp<mskmodesuffix>\t%0" > [(set_attr "length" "6") > (set_attr "type" "other")]) > @@ -21463,7 +21449,7 @@ (define_insn "@rdssp<mode>" > (define_insn "@incssp<mode>" > [(unspec_volatile [(match_operand:SWI48 0 "register_operand" "r")] > UNSPECV_INCSSP)] > - "TARGET_SHSTK || (flag_cf_protection & CF_RETURN)" > + "" > "incssp<mskmodesuffix>\t%0" > [(set_attr "length" "4") > (set_attr "type" "other")]) > -- > 2.29.2 >
On Tue, Feb 9, 2021 at 12:59 AM Richard Biener <richard.guenther@gmail.com> wrote: > > On Mon, Feb 8, 2021 at 3:07 PM H.J. Lu via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > When the SHSTK feature is not available or not enabled, RDSSP is a NOP, > > always save and restore shadow stack pointer to support compiling source > > codes, containing __builtin_setjmp and __builtin_longjmp, with different > > -fcf-protection options. > > Is that an ABI change for -fno-cf-protection? > Currently you can't mix object files with __builtin_setjmp and __builtin_longjmp compiled with -fcf-protection and -fcf-protection=none. This patch fixes it for GCC 11 and newer. It does nothing for object files compiled by older versions of GCC.
On Tue, Feb 9, 2021 at 2:11 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Feb 9, 2021 at 12:59 AM Richard Biener > <richard.guenther@gmail.com> wrote: > > > > On Mon, Feb 8, 2021 at 3:07 PM H.J. Lu via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > When the SHSTK feature is not available or not enabled, RDSSP is a NOP, > > > always save and restore shadow stack pointer to support compiling source > > > codes, containing __builtin_setjmp and __builtin_longjmp, with different > > > -fcf-protection options. > > > > Is that an ABI change for -fno-cf-protection? > > > > Currently you can't mix object files with __builtin_setjmp and __builtin_longjmp > compiled with -fcf-protection and -fcf-protection=none. This patch fixes it for > GCC 11 and newer. It does nothing for object files compiled by older versions > of GCC. So object files compiled with GCC 10 still inter-operate with object files from GCC 11 and -fcf-protection=none (aka the long-year default)? Richard. > -- > H.J.
On Tue, Feb 9, 2021 at 6:19 AM Richard Biener <richard.guenther@gmail.com> wrote: > > On Tue, Feb 9, 2021 at 2:11 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Tue, Feb 9, 2021 at 12:59 AM Richard Biener > > <richard.guenther@gmail.com> wrote: > > > > > > On Mon, Feb 8, 2021 at 3:07 PM H.J. Lu via Gcc-patches > > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > > > When the SHSTK feature is not available or not enabled, RDSSP is a NOP, > > > > always save and restore shadow stack pointer to support compiling source > > > > codes, containing __builtin_setjmp and __builtin_longjmp, with different > > > > -fcf-protection options. > > > > > > Is that an ABI change for -fno-cf-protection? > > > > > > > Currently you can't mix object files with __builtin_setjmp and __builtin_longjmp > > compiled with -fcf-protection and -fcf-protection=none. This patch fixes it for > > GCC 11 and newer. It does nothing for object files compiled by older versions > > of GCC. > > So object files compiled with GCC 10 still inter-operate with object My patch does nothing for GCC 10. > files from GCC 11 > and -fcf-protection=none (aka the long-year default)? > Yes, if __builtin_setjmp and __builtin_longjmp aren't used. No, if they are.
On Tue, Feb 09, 2021 at 06:25:10AM -0800, H.J. Lu via Gcc-patches wrote: > My patch does nothing for GCC 10. > > > files from GCC 11 > > and -fcf-protection=none (aka the long-year default)? > > > > Yes, if __builtin_setjmp and __builtin_longjmp aren't used. > No, if they are. Therefore ABI change. Also, is RDSPP really a nop even on say -march=i386/-march=i486? Or was it only ENDBR32 that is problematic for the very old CPUs? Jakub
On Tue, Feb 9, 2021 at 6:33 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Feb 09, 2021 at 06:25:10AM -0800, H.J. Lu via Gcc-patches wrote: > > My patch does nothing for GCC 10. > > > > > files from GCC 11 > > > and -fcf-protection=none (aka the long-year default)? > > > > > > > Yes, if __builtin_setjmp and __builtin_longjmp aren't used. > > No, if they are. > > Therefore ABI change. > > Also, is RDSPP really a nop even on say -march=i386/-march=i486? > Or was it only ENDBR32 that is problematic for the very old CPUs? > Yes, my patch isn't a good idea. It is better to detect such conditions at link-time.
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index b60784a2908..2510bd8a73d 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -19398,21 +19398,16 @@ (define_expand "save_stack_nonlocal" (match_operand 1 "register_operand"))] "" { - rtx stack_slot; - - if (flag_cf_protection & CF_RETURN) - { - /* Copy shadow stack pointer to the first slot - and stack pointer to the second slot. */ - rtx ssp_slot = adjust_address (operands[0], word_mode, 0); - stack_slot = adjust_address (operands[0], Pmode, UNITS_PER_WORD); - - rtx reg_ssp = force_reg (word_mode, const0_rtx); - emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp)); - emit_move_insn (ssp_slot, reg_ssp); - } - else - stack_slot = adjust_address (operands[0], Pmode, 0); + /* Copy shadow stack pointer to the first slot and stack pointer to + the second slot. */ + rtx ssp_slot = adjust_address (operands[0], word_mode, 0); + rtx stack_slot = adjust_address (operands[0], Pmode, UNITS_PER_WORD); + + /* If the SHSTK feature is not available or not enabled, the RDSSP + instruction is a NOP and REG_SSP is 0. */ + rtx reg_ssp = force_reg (word_mode, const0_rtx); + emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp)); + emit_move_insn (ssp_slot, reg_ssp); emit_move_insn (stack_slot, operands[1]); DONE; }) @@ -19422,72 +19417,63 @@ (define_expand "restore_stack_nonlocal" (match_operand 1 "memory_operand" ""))] "" { - rtx stack_slot; + /* Restore shadow stack pointer from the first slot and stack pointer + from the second slot. */ + rtx ssp_slot = adjust_address (operands[1], word_mode, 0); + rtx stack_slot = adjust_address (operands[1], Pmode, UNITS_PER_WORD); - if (flag_cf_protection & CF_RETURN) - { - /* Restore shadow stack pointer from the first slot - and stack pointer from the second slot. */ - rtx ssp_slot = adjust_address (operands[1], word_mode, 0); - stack_slot = adjust_address (operands[1], Pmode, UNITS_PER_WORD); - - /* Get the current shadow stack pointer. The code below will check if - SHSTK feature is enabled. If it is not enabled the RDSSP instruction - is a NOP. */ - rtx reg_ssp = force_reg (word_mode, const0_rtx); - emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp)); - - /* Compare through subtraction the saved and the current ssp - to decide if ssp has to be adjusted. */ - reg_ssp = expand_simple_binop (word_mode, MINUS, - reg_ssp, ssp_slot, - reg_ssp, 1, OPTAB_DIRECT); - - /* Compare and jump over adjustment code. */ - rtx noadj_label = gen_label_rtx (); - emit_cmp_and_jump_insns (reg_ssp, const0_rtx, EQ, NULL_RTX, - word_mode, 1, noadj_label); - - /* Compute the number of frames to adjust. */ - rtx reg_adj = gen_lowpart (ptr_mode, reg_ssp); - rtx reg_adj_neg = expand_simple_unop (ptr_mode, NEG, reg_adj, - NULL_RTX, 1); - - reg_adj = expand_simple_binop (ptr_mode, LSHIFTRT, reg_adj_neg, - GEN_INT (exact_log2 (UNITS_PER_WORD)), - reg_adj, 1, OPTAB_DIRECT); - - /* Check if number of frames <= 255 so no loop is needed. */ - rtx inc_label = gen_label_rtx (); - emit_cmp_and_jump_insns (reg_adj, GEN_INT (255), LEU, NULL_RTX, - ptr_mode, 1, inc_label); - - /* Adjust the ssp in a loop. */ - rtx loop_label = gen_label_rtx (); - emit_label (loop_label); - LABEL_NUSES (loop_label) = 1; - - rtx reg_255 = force_reg (word_mode, GEN_INT (255)); - emit_insn (gen_incssp (word_mode, reg_255)); - - reg_adj = expand_simple_binop (ptr_mode, MINUS, - reg_adj, GEN_INT (255), - reg_adj, 1, OPTAB_DIRECT); - - /* Compare and jump to the loop label. */ - emit_cmp_and_jump_insns (reg_adj, GEN_INT (255), GTU, NULL_RTX, - ptr_mode, 1, loop_label); - - emit_label (inc_label); - LABEL_NUSES (inc_label) = 1; - - emit_insn (gen_incssp (word_mode, reg_ssp)); - - emit_label (noadj_label); - LABEL_NUSES (noadj_label) = 1; - } - else - stack_slot = adjust_address (operands[1], Pmode, 0); + /* If the SHSTK feature is not available or not enabled, the RDSSP + instruction is a NOP and REG_SSP is 0. */ + rtx reg_ssp = force_reg (word_mode, const0_rtx); + emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp)); + + /* Compare through subtraction the saved and the current ssp to + decide if ssp has to be adjusted. */ + reg_ssp = expand_simple_binop (word_mode, MINUS, reg_ssp, ssp_slot, + reg_ssp, 1, OPTAB_DIRECT); + + /* Compare and jump over adjustment code. */ + rtx noadj_label = gen_label_rtx (); + emit_cmp_and_jump_insns (reg_ssp, const0_rtx, EQ, NULL_RTX, + word_mode, 1, noadj_label); + + /* Compute the number of frames to adjust. */ + rtx reg_adj = gen_lowpart (ptr_mode, reg_ssp); + rtx reg_adj_neg = expand_simple_unop (ptr_mode, NEG, reg_adj, + NULL_RTX, 1); + + reg_adj = expand_simple_binop (ptr_mode, LSHIFTRT, reg_adj_neg, + GEN_INT (exact_log2 (UNITS_PER_WORD)), + reg_adj, 1, OPTAB_DIRECT); + + /* Check if number of frames <= 255 so no loop is needed. */ + rtx inc_label = gen_label_rtx (); + emit_cmp_and_jump_insns (reg_adj, GEN_INT (255), LEU, NULL_RTX, + ptr_mode, 1, inc_label); + + /* Adjust the ssp in a loop. */ + rtx loop_label = gen_label_rtx (); + emit_label (loop_label); + LABEL_NUSES (loop_label) = 1; + + rtx reg_255 = force_reg (word_mode, GEN_INT (255)); + emit_insn (gen_incssp (word_mode, reg_255)); + + reg_adj = expand_simple_binop (ptr_mode, MINUS, + reg_adj, GEN_INT (255), + reg_adj, 1, OPTAB_DIRECT); + + /* Compare and jump to the loop label. */ + emit_cmp_and_jump_insns (reg_adj, GEN_INT (255), GTU, NULL_RTX, + ptr_mode, 1, loop_label); + + emit_label (inc_label); + LABEL_NUSES (inc_label) = 1; + + emit_insn (gen_incssp (word_mode, reg_ssp)); + + emit_label (noadj_label); + LABEL_NUSES (noadj_label) = 1; emit_move_insn (operands[0], stack_slot); DONE; }) @@ -21455,7 +21441,7 @@ (define_insn "@rdssp<mode>" [(set (match_operand:SWI48 0 "register_operand" "=r") (unspec_volatile:SWI48 [(match_operand:SWI48 1 "register_operand" "0")] UNSPECV_NOP_RDSSP))] - "TARGET_SHSTK || (flag_cf_protection & CF_RETURN)" + "" "rdssp<mskmodesuffix>\t%0" [(set_attr "length" "6") (set_attr "type" "other")]) @@ -21463,7 +21449,7 @@ (define_insn "@rdssp<mode>" (define_insn "@incssp<mode>" [(unspec_volatile [(match_operand:SWI48 0 "register_operand" "r")] UNSPECV_INCSSP)] - "TARGET_SHSTK || (flag_cf_protection & CF_RETURN)" + "" "incssp<mskmodesuffix>\t%0" [(set_attr "length" "4") (set_attr "type" "other")])