From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 62002 invoked by alias); 1 Feb 2018 07:42:22 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 60969 invoked by uid 89); 1 Feb 2018 07:42:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-io0-f196.google.com Received: from mail-io0-f196.google.com (HELO mail-io0-f196.google.com) (209.85.223.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Feb 2018 07:42:19 +0000 Received: by mail-io0-f196.google.com with SMTP id b198so18142838iof.6 for ; Wed, 31 Jan 2018 23:42:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=pzYe0oEM8mwqdnFtYWtxkpVlx+FN49+HRQmruRgvDko=; b=CZ8lymzLY467hDboSOoeW7p/mcnGYuAtBQwTdyd8mQVQP1kgPnstZYSjn5M1l7vOXT YTO83vaLPLsSNXj7YRkj7uMmBlT4RFJu7LHIMcgcF28sq7yvnCHDdRl6KSE3yrOjvin4 IY4V+pyGfCkcZbEqXbcB2/3F1YCZ8U+pK45oX/7DRkLkPGrrQ1osIhMcDpjW7b1j24zy QFm4pZDkBJa0OB4QzxDA2QSH4uChRmuuDVnXGw/aJ2UqnLlkFtGbG2O5Is8DNgyk5JEI x/BgwqT6dsmKDGx8Q/WnqxImyAb5RvhPEs3yIsRmXUSNh7SZwe6S2Ai7J6tIKcOV5Nb4 8o3Q== X-Gm-Message-State: AKwxytehmOKiECGlsojJ4Qudnf0TzWqkvCAFf504SxIu9cSvlijgZk3h SoLu3AoKlN/0WlZL29gbPWiuRmFccsEmlBEwnYU= X-Google-Smtp-Source: AH8x227V5sEDQxIC4XjXytNYV4gBeO4XtrpnfTXvo8PH7VErGjS75uxTII6tZO+/MuVYx28VBq+3WEXJdIaKeGIhgj4= X-Received: by 10.107.180.146 with SMTP id d140mr39643336iof.166.1517470937396; Wed, 31 Jan 2018 23:42:17 -0800 (PST) MIME-Version: 1.0 Received: by 10.2.181.171 with HTTP; Wed, 31 Jan 2018 23:42:16 -0800 (PST) In-Reply-To: <20180201001627.GA21117@gmail.com> References: <20180201001627.GA21117@gmail.com> From: Uros Bizjak Date: Thu, 01 Feb 2018 07:42:00 -0000 Message-ID: Subject: Re: [PATCH] Use ptr_mode to save/restore pointers in builtin jmpbuf To: "H.J. Lu" Cc: "gcc-patches@gcc.gnu.org" , Eric Botcazou Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2018-02/txt/msg00007.txt.bz2 On Thu, Feb 1, 2018 at 1:16 AM, H.J. Lu 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. > > 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 | 36 ++++++++++++++++++++++++++ > gcc/testsuite/gcc.dg/pr84150.c | 45 +++++++++++++++++++++++++++++++++ > gcc/testsuite/gcc.target/i386/pr84150.c | 44 ++++++++++++++++++++++++++++++++ > 5 files changed, 153 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..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. > + 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 >