From patchwork Mon Sep 22 15:33:43 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Enkovich X-Patchwork-Id: 392019 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3446914003E for ; Tue, 23 Sep 2014 01:34:17 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=KYBAsBHVYU7oi31SI iwmAAA09T6r+C83aozFE8AAXgoBZedO8YX95qyWHmoWd86tOtACCsQg7rPrBoHm/ hKj3doGIyNKiP07reVmFhiWy4kUeohimPdblvDca8MlQcGz4GQaehktZEVL7mL8n xM/JLm2lMYNydguFKV70Z9Nuho= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=iNDnyPOfieYlUcqm1SXzw4q Chzk=; b=m/vqBjoB47DfFxnxH5/NtrUE3C0nYJL+he7XOHyMbsnwsQLB/kTBiLV nmEvUwKeiCyVcH7J5fLl1e2RxvhmGUn6hxjBWjx4kpQY0v6gAxSO18AH6cMGFBhd ZNZBtYpcJtxkjiajN6mEOgmisQSLsp9LDlg4+Y4SiiIpJ782ai6g= Received: (qmail 25929 invoked by alias); 22 Sep 2014 15:34:11 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 25918 invoked by uid 89); 22 Sep 2014 15:34:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f42.google.com Received: from mail-pa0-f42.google.com (HELO mail-pa0-f42.google.com) (209.85.220.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 22 Sep 2014 15:34:08 +0000 Received: by mail-pa0-f42.google.com with SMTP id et14so4800131pad.29 for ; Mon, 22 Sep 2014 08:34:07 -0700 (PDT) X-Received: by 10.66.97.41 with SMTP id dx9mr26931812pab.65.1411400047068; Mon, 22 Sep 2014 08:34:07 -0700 (PDT) Received: from msticlxl57.ims.intel.com (fmdmzpr01-ext.fm.intel.com. [192.55.54.36]) by mx.google.com with ESMTPSA id p1sm9711877pdr.15.2014.09.22.08.34.05 for (version=TLSv1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 22 Sep 2014 08:34:06 -0700 (PDT) Date: Mon, 22 Sep 2014 19:33:43 +0400 From: Ilya Enkovich To: Uros Bizjak Cc: "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH, i386, Pointer Bounds Checker 34/x] Vararg functions support Message-ID: <20140922153343.GB35262@msticlxl57.ims.intel.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes On 21 Sep 18:08, Uros Bizjak wrote: > Hello! > > > This patch introduces initialization of incoming bounds for vararg function on i386 target. > > > > Bootstrapped and tested on linux-x86_64. > > > > Thanks, > > Ilya > > -- > > gcc/ > > > > 2014-06-11 Ilya Enkovich > > > > * config/i386/i386.c (ix86_setup_incoming_varargs): New. > > (ix86_va_start): Initialize bounds for pointers in va_list. > > (TARGET_SETUP_INCOMING_VARARG_BOUNDS): New. > > OK with a couple of smallchanges below. > > Thanks, > Uros. > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > index a67e6e7..c520f26 100644 > > --- a/gcc/config/i386/i386.c > > +++ b/gcc/config/i386/i386.c > > @@ -8456,6 +8456,72 @@ ix86_setup_incoming_varargs (cumulative_args_t cum_v, enum machine_mode mode, > > setup_incoming_varargs_64 (&next_cum); > > } > > > > +static void > > +ix86_setup_incoming_vararg_bounds (cumulative_args_t cum_v, > > + enum machine_mode mode, > > + tree type, > > + int *pretend_size ATTRIBUTE_UNUSED, > > + int no_rtl) > > +{ > > + CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v); > > + CUMULATIVE_ARGS next_cum; > > + tree fntype; > > + rtx save_area; > > + int bnd_reg, i, max; > > + > > + gcc_assert (!no_rtl); > > + > > Please add a comment for following condition. > > > + if (!TARGET_64BIT) > > + return; > > + > > + fntype = TREE_TYPE (current_function_decl); > > + > > + /* For varargs, we do not want to skip the dummy va_dcl argument. > > + For stdargs, we do want to skip the last named argument. */ > > + next_cum = *cum; > > + if (stdarg_p (fntype)) > > + ix86_function_arg_advance (pack_cumulative_args (&next_cum), mode, type, > > + true); > > + if (cum->call_abi == MS_ABI) > > + return; > > Put this early exit at the top of the function. > > > + save_area = frame_pointer_rtx; > > + > > + max = cum->regno + cfun->va_list_gpr_size / UNITS_PER_WORD; > > + if (max > X86_64_REGPARM_MAX) > > + max = X86_64_REGPARM_MAX; > > + > > + bnd_reg = cum->bnd_regno + cum->force_bnd_pass; > > + if (chkp_function_instrumented_p (current_function_decl)) > > + for (i = cum->regno; i < max; i++) > > + { > > + rtx addr = plus_constant (Pmode, save_area, i * UNITS_PER_WORD); > > + rtx reg = gen_rtx_REG (DImode, > > + x86_64_int_parameter_registers[i]); > > + rtx ptr = reg; > > + rtx bounds; > > + > > + if (bnd_reg <= LAST_BND_REG) > > + bounds = gen_rtx_REG (BNDmode, bnd_reg); > > + else > > + { > > + rtx ldx_addr = plus_constant (Pmode, arg_pointer_rtx, > > + (LAST_BND_REG - bnd_reg) * 8); > > No magic constants! > > > + bounds = gen_reg_rtx (BNDmode); > > + emit_insn (TARGET_64BIT > > + ? gen_bnd64_ldx (bounds, ldx_addr, ptr) > > + : gen_bnd32_ldx (bounds, ldx_addr, ptr)); > > + } > > + > > + emit_insn (TARGET_64BIT > > + ? gen_bnd64_stx (addr, ptr, bounds) > > + : gen_bnd32_stx (addr, ptr, bounds)); > > Please check BNDmode instead of TARGET_64BIT. > > > + bnd_reg++; > > + } > > +} > > + > > + > > /* Checks if TYPE is of kind va_list char *. */ > > > > static bool > > @@ -8478,7 +8544,7 @@ ix86_va_start (tree valist, rtx nextarg) > > { > > HOST_WIDE_INT words, n_gpr, n_fpr; > > tree f_gpr, f_fpr, f_ovf, f_sav; > > - tree gpr, fpr, ovf, sav, t; > > + tree gpr, fpr, ovf, sav, t, t1; > > tree type; > > rtx ovf_rtx; > > > > @@ -8529,6 +8595,13 @@ ix86_va_start (tree valist, rtx nextarg) > > crtl->args.arg_offset_rtx, > > NULL_RTX, 0, OPTAB_LIB_WIDEN); > > convert_move (va_r, next, 0); > > + > > + /* Store zero bounds for va_list. */ > > + if (chkp_function_instrumented_p (current_function_decl)) > > + chkp_expand_bounds_reset_for_mem (valist, > > + make_tree (TREE_TYPE (valist), > > + next)); > > + > > } > > return; > > } > > @@ -8582,10 +8655,15 @@ ix86_va_start (tree valist, rtx nextarg) > > t = make_tree (type, ovf_rtx); > > if (words != 0) > > t = fold_build_pointer_plus_hwi (t, words * UNITS_PER_WORD); > > + t1 = t; > > t = build2 (MODIFY_EXPR, type, ovf, t); > > TREE_SIDE_EFFECTS (t) = 1; > > expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL); > > > > + /* Store zero bounds for overflow area pointer. */ > > + if (chkp_function_instrumented_p (current_function_decl)) > > + chkp_expand_bounds_reset_for_mem (ovf, t1); > > Can you please move this above to avoid another temporary (t1)? > > > if (ix86_varargs_gpr_size || ix86_varargs_fpr_size) > > { > > /* Find the register save area. > > @@ -8594,9 +8672,14 @@ ix86_va_start (tree valist, rtx nextarg) > > t = make_tree (type, frame_pointer_rtx); > > if (!ix86_varargs_gpr_size) > > t = fold_build_pointer_plus_hwi (t, -8 * X86_64_REGPARM_MAX); > > + t1 = t; > > t = build2 (MODIFY_EXPR, type, sav, t); > > TREE_SIDE_EFFECTS (t) = 1; > > expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL); > > + > > + /* Store zero bounds for save area pointer. */ > > + if (chkp_function_instrumented_p (current_function_decl)) > > + chkp_expand_bounds_reset_for_mem (sav, t1); > > Also move this part upper to remove another temporary. > > > } > > } > > > > @@ -48057,6 +48140,9 @@ ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts) > > #undef TARGET_CHKP_INITIALIZE_BOUNDS > > #define TARGET_CHKP_INITIALIZE_BOUNDS ix86_initialize_bounds > > > > +#undef TARGET_SETUP_INCOMING_VARARG_BOUNDS > > +#define TARGET_SETUP_INCOMING_VARARG_BOUNDS ix86_setup_incoming_vararg_bounds > > + > > struct gcc_target targetm = TARGET_INITIALIZER; > > > > #include "gt-i386.h" Hi, Thank you for review. Below is a fixed version. Thanks, Ilya diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 66622d2..c183501 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -8477,6 +8477,71 @@ ix86_setup_incoming_varargs (cumulative_args_t cum_v, enum machine_mode mode, setup_incoming_varargs_64 (&next_cum); } +static void +ix86_setup_incoming_vararg_bounds (cumulative_args_t cum_v, + enum machine_mode mode, + tree type, + int *pretend_size ATTRIBUTE_UNUSED, + int no_rtl) +{ + CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v); + CUMULATIVE_ARGS next_cum; + tree fntype; + rtx save_area; + int bnd_reg, i, max; + + gcc_assert (!no_rtl); + + /* Do nothing if we use plain pointer to argument area. */ + if (!TARGET_64BIT || cum->call_abi == MS_ABI) + return; + + fntype = TREE_TYPE (current_function_decl); + + /* For varargs, we do not want to skip the dummy va_dcl argument. + For stdargs, we do want to skip the last named argument. */ + next_cum = *cum; + if (stdarg_p (fntype)) + ix86_function_arg_advance (pack_cumulative_args (&next_cum), mode, type, + true); + save_area = frame_pointer_rtx; + + max = cum->regno + cfun->va_list_gpr_size / UNITS_PER_WORD; + if (max > X86_64_REGPARM_MAX) + max = X86_64_REGPARM_MAX; + + bnd_reg = cum->bnd_regno + cum->force_bnd_pass; + if (chkp_function_instrumented_p (current_function_decl)) + for (i = cum->regno; i < max; i++) + { + rtx addr = plus_constant (Pmode, save_area, i * UNITS_PER_WORD); + rtx reg = gen_rtx_REG (DImode, + x86_64_int_parameter_registers[i]); + rtx ptr = reg; + rtx bounds; + + if (bnd_reg <= LAST_BND_REG) + bounds = gen_rtx_REG (BNDmode, bnd_reg); + else + { + rtx ldx_addr = + plus_constant (Pmode, arg_pointer_rtx, + (LAST_BND_REG - bnd_reg) * GET_MODE_SIZE (Pmode)); + bounds = gen_reg_rtx (BNDmode); + emit_insn (BNDmode == BND64mode + ? gen_bnd64_ldx (bounds, ldx_addr, ptr) + : gen_bnd32_ldx (bounds, ldx_addr, ptr)); + } + + emit_insn (BNDmode == BND64mode + ? gen_bnd64_stx (addr, ptr, bounds) + : gen_bnd32_stx (addr, ptr, bounds)); + + bnd_reg++; + } +} + + /* Checks if TYPE is of kind va_list char *. */ static bool @@ -8550,6 +8615,13 @@ ix86_va_start (tree valist, rtx nextarg) crtl->args.arg_offset_rtx, NULL_RTX, 0, OPTAB_LIB_WIDEN); convert_move (va_r, next, 0); + + /* Store zero bounds for va_list. */ + if (chkp_function_instrumented_p (current_function_decl)) + chkp_expand_bounds_reset_for_mem (valist, + make_tree (TREE_TYPE (valist), + next)); + } return; } @@ -8603,6 +8675,11 @@ ix86_va_start (tree valist, rtx nextarg) t = make_tree (type, ovf_rtx); if (words != 0) t = fold_build_pointer_plus_hwi (t, words * UNITS_PER_WORD); + + /* Store zero bounds for overflow area pointer. */ + if (chkp_function_instrumented_p (current_function_decl)) + chkp_expand_bounds_reset_for_mem (ovf, t); + t = build2 (MODIFY_EXPR, type, ovf, t); TREE_SIDE_EFFECTS (t) = 1; expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL); @@ -8615,6 +8692,11 @@ ix86_va_start (tree valist, rtx nextarg) t = make_tree (type, frame_pointer_rtx); if (!ix86_varargs_gpr_size) t = fold_build_pointer_plus_hwi (t, -8 * X86_64_REGPARM_MAX); + + /* Store zero bounds for save area pointer. */ + if (chkp_function_instrumented_p (current_function_decl)) + chkp_expand_bounds_reset_for_mem (sav, t); + t = build2 (MODIFY_EXPR, type, sav, t); TREE_SIDE_EFFECTS (t) = 1; expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL); @@ -48313,6 +48395,9 @@ ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts) #undef TARGET_CHKP_INITIALIZE_BOUNDS #define TARGET_CHKP_INITIALIZE_BOUNDS ix86_initialize_bounds +#undef TARGET_SETUP_INCOMING_VARARG_BOUNDS +#define TARGET_SETUP_INCOMING_VARARG_BOUNDS ix86_setup_incoming_vararg_bounds + struct gcc_target targetm = TARGET_INITIALIZER; #include "gt-i386.h"