This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Use ptr_mode to save/restore pointers in builtin jmpbuf
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Uros Bizjak <ubizjak at gmail dot com>, "Tsimbalist, Igor V" <igor dot v dot tsimbalist at intel dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Eric Botcazou <ebotcazou at libertysurf dot fr>
- Date: Thu, 1 Feb 2018 05:43:12 -0800
- Subject: Re: [PATCH] Use ptr_mode to save/restore pointers in builtin jmpbuf
- Authentication-results: sourceware.org; auth=none
- References: <20180201001627.GA21117@gmail.com> <CAFULd4ZomG0yGThKBPPNmOjHHnqQ61DLe59es3UGboNCva1j2A@mail.gmail.com>
On Wed, Jan 31, 2018 at 11:42 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Thu, Feb 1, 2018 at 1:16 AM, H.J. Lu <hongjiu.lu@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.tools@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