On Wed, Jan 31, 2018 at 11:42 PM, Uros Bizjak <ubiz...@gmail.com> wrote: > On Thu, Feb 1, 2018 at 1:16 AM, H.J. Lu <hongjiu...@intel.com> wrote: >> >> We currently read and write beyond the builtin jmpbuf on ILP32 targets >> where Pmode == DImode and ptr_mode == SImode. Since the builtin jmpbuf >> is an array of 5 pointers, ptr_mode should be used to save and restore >> frame and program pointers. Since x86 only saves stack pointer in >> stack save area, STACK_SAVEAREA_MODE should be ptr_mode, not Pmode. >> >> Note: If ptr_mode != Pmode, builtin setjmp/longjmp tests may fail. When >> it happens, please check if there are correct save_stack_nonlocal and >> restore_stack_nonlocal expand patterns. >> >> Tested on i686 and x86-64. OK for trunk? >> >> H.J. >> ---- >> gcc/ >> >> PR middle-end/84150 >> * builtins.c (expand_builtin_setjmp_setup): Use ptr_mode to >> save frame and program pointers. >> (expand_builtin_longjmp): Use ptr_mode to restore frame and >> program pointers. >> * config/i386/i386.md (save_stack_nonlocal): New expand pattern. >> (restore_stack_nonlocal): Likewise. >> * config/i386/i386.h (STACK_SAVEAREA_MODE): New.
>> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md >> index c08e4f55cff..c563a26cd60 100644 >> --- a/gcc/config/i386/i386.md >> +++ b/gcc/config/i386/i386.md >> @@ -18375,6 +18375,42 @@ >> "* return output_probe_stack_range (operands[0], operands[2]);" >> [(set_attr "type" "multi")]) >> >> +;; Non-local goto support. >> + >> +(define_expand "save_stack_nonlocal" >> + [(use (match_operand 0 "memory_operand" "")) >> + (use (match_operand 1 "register_operand" ""))] >> + "" >> +{ >> + /* Stack is saved in ptr_mode and stack register is in Pmode. */ >> + if (GET_MODE (operands[0]) != GET_MODE (operands[1])) >> + { >> + if (GET_MODE (operands[0]) != ptr_mode >> + || GET_MODE (operands[1]) != Pmode) >> + gcc_unreachable (); > > gcc_assert instead of if (...) gcc_unreachable. > >> + operands[1] = gen_rtx_SUBREG (ptr_mode, operands[1], 0); > > gen_lowpart > >> + } >> + emit_move_insn (operands[0], operands[1]); >> + DONE; >> +}) >> + >> +(define_expand "restore_stack_nonlocal" >> + [(use (match_operand 0 "register_operand" "")) >> + (use (match_operand 1 "memory_operand" ""))] >> + "" >> +{ >> + /* Stack is saved in ptr_mode and stack register is in Pmode. */ >> + if (GET_MODE (operands[0]) != GET_MODE (operands[1])) >> + { >> + if (GET_MODE (operands[0]) != Pmode >> + || GET_MODE (operands[1]) != ptr_mode) >> + gcc_unreachable (); > > Also here. > Here is the updated patch. OK for trunk? BTW, the -maddress-mode=long test in Igor's patch for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84066 is expected to fail without this fix. -- H.J.
From f8c86b9fc727dbd7e4de0691bd81d2d2bea8904b Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.to...@gmail.com> Date: Wed, 31 Jan 2018 09:47:57 -0800 Subject: [PATCH] Use ptr_mode to save/restore pointers in builtin jmpbuf We currently read and write beyond the builtin jmpbuf on ILP32 targets where Pmode == DImode and ptr_mode == SImode. Since the builtin jmpbuf is an array of 5 pointers, ptr_mode should be used to save and restore frame and program pointers. Since x86 only saves stack pointer in stack save area, STACK_SAVEAREA_MODE should be ptr_mode, not Pmode. Note: If ptr_mode != Pmode, builtin setjmp/longjmp tests may fail. When it happens, please check if there are correct save_stack_nonlocal and restore_stack_nonlocal expand patterns. gcc/ PR middle-end/84150 * builtins.c (expand_builtin_setjmp_setup): Use ptr_mode to save frame and program pointers. (expand_builtin_longjmp): Use ptr_mode to restore frame and program pointers. * config/i386/i386.md (save_stack_nonlocal): New expand pattern. (restore_stack_nonlocal): Likewise. * config/i386/i386.h (STACK_SAVEAREA_MODE): New. gcc/testsuite/ PR middle-end/84150 * gcc.dg/pr84150.c: New test. * gcc.target/i386/pr84150.c: Likewisde. --- gcc/builtins.c | 36 +++++++++++++++++--------- gcc/config/i386/i386.h | 4 +++ gcc/config/i386/i386.md | 34 +++++++++++++++++++++++++ gcc/testsuite/gcc.dg/pr84150.c | 45 +++++++++++++++++++++++++++++++++ gcc/testsuite/gcc.target/i386/pr84150.c | 44 ++++++++++++++++++++++++++++++++ 5 files changed, 151 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr84150.c create mode 100644 gcc/testsuite/gcc.target/i386/pr84150.c diff --git a/gcc/builtins.c b/gcc/builtins.c index 683c6ec6e5b..c3d45d5e3fa 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -840,6 +840,7 @@ expand_builtin_setjmp_setup (rtx buf_addr, rtx receiver_label) machine_mode sa_mode = STACK_SAVEAREA_MODE (SAVE_NONLOCAL); rtx stack_save; rtx mem; + rtx tmp; if (setjmp_alias_set == -1) setjmp_alias_set = new_alias_set (); @@ -852,20 +853,25 @@ expand_builtin_setjmp_setup (rtx buf_addr, rtx receiver_label) the buffer and use the rest of it for the stack save area, which is machine-dependent. */ - mem = gen_rtx_MEM (Pmode, buf_addr); + mem = gen_rtx_MEM (ptr_mode, buf_addr); set_mem_alias_set (mem, setjmp_alias_set); - emit_move_insn (mem, targetm.builtin_setjmp_frame_value ()); + tmp = targetm.builtin_setjmp_frame_value (); + if (GET_MODE (tmp) != ptr_mode) + tmp = gen_lowpart (ptr_mode, tmp); + emit_move_insn (mem, tmp); - mem = gen_rtx_MEM (Pmode, plus_constant (Pmode, buf_addr, - GET_MODE_SIZE (Pmode))), + mem = gen_rtx_MEM (ptr_mode, plus_constant (Pmode, buf_addr, + GET_MODE_SIZE (ptr_mode))), set_mem_alias_set (mem, setjmp_alias_set); - emit_move_insn (validize_mem (mem), - force_reg (Pmode, gen_rtx_LABEL_REF (Pmode, receiver_label))); + tmp = force_reg (Pmode, gen_rtx_LABEL_REF (Pmode, receiver_label)); + if (Pmode != ptr_mode) + tmp = gen_lowpart (ptr_mode, tmp); + emit_move_insn (validize_mem (mem), tmp); stack_save = gen_rtx_MEM (sa_mode, plus_constant (Pmode, buf_addr, - 2 * GET_MODE_SIZE (Pmode))); + 2 * GET_MODE_SIZE (ptr_mode))); set_mem_alias_set (stack_save, setjmp_alias_set); emit_stack_save (SAVE_NONLOCAL, &stack_save); @@ -991,12 +997,14 @@ expand_builtin_longjmp (rtx buf_addr, rtx value) emit_insn (targetm.gen_builtin_longjmp (buf_addr)); else { - fp = gen_rtx_MEM (Pmode, buf_addr); - lab = gen_rtx_MEM (Pmode, plus_constant (Pmode, buf_addr, - GET_MODE_SIZE (Pmode))); + fp = gen_rtx_MEM (ptr_mode, buf_addr); + lab = gen_rtx_MEM (ptr_mode, + plus_constant (Pmode, buf_addr, + GET_MODE_SIZE (ptr_mode))); - stack = gen_rtx_MEM (sa_mode, plus_constant (Pmode, buf_addr, - 2 * GET_MODE_SIZE (Pmode))); + stack = gen_rtx_MEM (sa_mode, + plus_constant (Pmode, buf_addr, + 2 * GET_MODE_SIZE (ptr_mode))); set_mem_alias_set (fp, setjmp_alias_set); set_mem_alias_set (lab, setjmp_alias_set); set_mem_alias_set (stack, setjmp_alias_set); @@ -1015,6 +1023,10 @@ expand_builtin_longjmp (rtx buf_addr, rtx value) emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode))); emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx)); +#ifdef POINTERS_EXTEND_UNSIGNED + if (GET_MODE (fp) != Pmode) + fp = convert_to_mode (Pmode, fp, POINTERS_EXTEND_UNSIGNED); +#endif emit_move_insn (hard_frame_pointer_rtx, fp); emit_stack_restore (SAVE_NONLOCAL, stack); diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h index 59522ccba02..16aace86e48 100644 --- a/gcc/config/i386/i386.h +++ b/gcc/config/i386/i386.h @@ -1937,6 +1937,10 @@ do { \ between pointers and any other objects of this machine mode. */ #define Pmode (ix86_pmode == PMODE_DI ? DImode : SImode) +/* Supply a definition of STACK_SAVEAREA_MODE for emit_stack_save. We + only need save and restore stack pointer in ptr_mode. */ +#define STACK_SAVEAREA_MODE(LEVEL) ptr_mode + /* Specify the machine mode that bounds have. */ #define BNDmode (ix86_pmode == PMODE_DI ? BND64mode : BND32mode) diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index c08e4f55cff..bce45e0e4b4 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -18375,6 +18375,40 @@ "* return output_probe_stack_range (operands[0], operands[2]);" [(set_attr "type" "multi")]) +;; Non-local goto support. + +(define_expand "save_stack_nonlocal" + [(use (match_operand 0 "memory_operand" "")) + (use (match_operand 1 "register_operand" ""))] + "" +{ + /* Stack is saved in ptr_mode and stack register is in Pmode. */ + if (GET_MODE (operands[0]) != GET_MODE (operands[1])) + { + gcc_assert (GET_MODE (operands[0]) == ptr_mode + && GET_MODE (operands[1]) == Pmode); + operands[1] = gen_lowpart (ptr_mode, operands[1]); + } + emit_move_insn (operands[0], operands[1]); + DONE; +}) + +(define_expand "restore_stack_nonlocal" + [(use (match_operand 0 "register_operand" "")) + (use (match_operand 1 "memory_operand" ""))] + "" +{ + /* Stack is saved in ptr_mode and stack register is in Pmode. */ + if (GET_MODE (operands[0]) != GET_MODE (operands[1])) + { + gcc_assert (GET_MODE (operands[0]) == Pmode + && GET_MODE (operands[1]) == ptr_mode); + operands[1] = gen_rtx_ZERO_EXTEND (Pmode, operands[1]); + } + emit_move_insn (operands[0], operands[1]); + DONE; +}) + /* Additional processing for builtin_setjmp. Store the shadow stack pointer as a forth element in jmpbuf. */ (define_expand "builtin_setjmp_setup" diff --git a/gcc/testsuite/gcc.dg/pr84150.c b/gcc/testsuite/gcc.dg/pr84150.c new file mode 100644 index 00000000000..1e7a04c1e9c --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr84150.c @@ -0,0 +1,45 @@ +/* { dg-do run } */ +/* { dg-options "-O" } */ +/* { dg-require-effective-target indirect_jumps } */ + +void *buf[6] = { + (void *) -1, + (void *) -1, + (void *) -1, + (void *) -1, + (void *) -1, + (void *) -1 +}; + +void raise0(void) +{ + __builtin_longjmp (buf, 1); +} + +int execute(int cmd) +{ + int last = 0; + + if (__builtin_setjmp (buf) == 0) + while (1) + { + last = 1; + raise0 (); + } + + if (last == 0) + return 0; + else + return cmd; +} + +int main(void) +{ + if (execute (1) == 0) + __builtin_abort (); + + if (buf[5] != (void *) -1) + __builtin_abort (); + + return 0; +} diff --git a/gcc/testsuite/gcc.target/i386/pr84150.c b/gcc/testsuite/gcc.target/i386/pr84150.c new file mode 100644 index 00000000000..11d3d361487 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr84150.c @@ -0,0 +1,44 @@ +/* { dg-do run { target x32 } } */ +/* { dg-options "-O -maddress-mode=long" } */ + +void *buf[6] = { + (void *) -1, + (void *) -1, + (void *) -1, + (void *) -1, + (void *) -1, + (void *) -1 +}; + +void raise0(void) +{ + __builtin_longjmp (buf, 1); +} + +int execute(int cmd) +{ + int last = 0; + + if (__builtin_setjmp (buf) == 0) + while (1) + { + last = 1; + raise0 (); + } + + if (last == 0) + return 0; + else + return cmd; +} + +int main(void) +{ + if (execute (1) == 0) + __builtin_abort (); + + if (buf[5] != (void *) -1) + __builtin_abort (); + + return 0; +} -- 2.14.3