From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81315 invoked by alias); 23 Mar 2017 19:57:56 -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 81297 invoked by uid 89); 23 Mar 2017 19:57:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.4 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=ub, UD:u.b, business X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-vk0-f54.google.com Received: from mail-vk0-f54.google.com (HELO mail-vk0-f54.google.com) (209.85.213.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Mar 2017 19:57:53 +0000 Received: by mail-vk0-f54.google.com with SMTP id d188so153064713vka.0; Thu, 23 Mar 2017 12:57:54 -0700 (PDT) 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:content-transfer-encoding; bh=LQkyUeyCSd4VykRpNxmbknHSArj+sMx+kvWBsMzIgW8=; b=fRi/imIgFLGkFCpKFljuQBjeaWCYts05SIG1Pihn1C7+mbdhKLGpeSmqGGlb7Knar3 1HEZUlV2ag+kvuCEvxSnjiUm1L6xN5Kp/IFlT4aJEIeLDGTNa8MKjyj/VA/VFD3EOy2z zfjzFys6goIuScvAWyEaKAqBC9NWBv6JEqdA9xtR3oNlez9QpjGcrG1SF5xul2X7fr69 AjG2yMzFxsXdBx1wX6zqdBbhCa520ldTXFzz8D9auZDKOytBZ4HQ2L7Vpy2WgIUI1saD Lcqj7W6zmju8HmjZbPGn9FDPYfl7Cn+aK6ZTS+HzPa/RUPfg9wbqQbUt6fUpxLWcYxmr SLUw== X-Gm-Message-State: AFeK/H3p2H1koU1fMNjOxtUluRezhLTR3k4oNoUcUyhqRVhzUdKWEk8DQFxGiHyB9YPt+n3kyA8XgjgsMLf8pw== X-Received: by 10.31.99.1 with SMTP id x1mr2252571vkb.65.1490299072924; Thu, 23 Mar 2017 12:57:52 -0700 (PDT) MIME-Version: 1.0 Received: by 10.159.36.151 with HTTP; Thu, 23 Mar 2017 12:57:52 -0700 (PDT) In-Reply-To: References: From: Ilya Enkovich Date: Thu, 23 Mar 2017 20:34:00 -0000 Message-ID: Subject: Re: [CHKP] Fix for PR79990 To: Alexander Ivchenko Cc: marxin@gcc.gnu.org, GCC Patches Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-03/txt/msg01254.txt.bz2 2017-03-23 17:18 GMT+03:00 Alexander Ivchenko : > Hi, > > The patch below attempts to fix the PR. I checked that it did not > break any of mpx.exp tests, but I did not run the full testing yet. > Would like to know whether this approach is generally correct or not. > > The issue is that we have the hard reg vector variable: > > typedef int U __attribute__ ((vector_size (16))); > register U u asm("xmm0"); > > and chkp tries to instrument access to it: > > return u[i]; > > by doing that: > > __bound_tmp.0_4 =3D __builtin_ia32_bndmk (&u, 16); > > However, you cannot take an address of a register variable (in fact if > you do that, the compiler will give you "address of register variable > =E2=80=98u=E2=80=99 requested" error), so expand, sensibly, gives an ICE = on on &u > here. I believe that if there is no pointers, pointer bounds checker > shouldn't get involved into that business. What do you think? Hi! I think with this patch I can call foo with any index and thus access some random stack slot. The first thing we should answer is 'do we want to catch array index overflows in such cases'? If we want to (and this looks reasonable thing to do because it prevents invalid memory accesses) then this patch doesn't resolve the problem. I'm not sure it can affect the patch, but please consider more complex cases. E.g.: typedef int v8 __attribute__ ((vector_size(8))); struct U { v8 a; v8 b; }; int foo (int i) { register struct U u asm ("xmm0"); return u.b[i]; } One way to catch overflow in such cases might be to use some fake pointer value (e.g. 0) for such not addressible variable. This fake value would be used as base for memory access and as lower bound. I don't see other cases except array_ref overflow check where such value might be used. So this fake value will not be passed somewhere and will not be stored to Bounds Table. Thanks, Ilya > > > > > diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c > index 75caf83..e39ec9a 100644 > --- a/gcc/tree-chkp.c > +++ b/gcc/tree-chkp.c > @@ -3383,6 +3383,7 @@ chkp_parse_array_and_component_ref (tree node, tree= *ptr, > tree comp_to_narrow =3D NULL_TREE; > tree last_comp =3D NULL_TREE; > bool array_ref_found =3D false; > + bool is_register_var =3D false; > tree *nodes; > tree var; > int len; > @@ -3440,6 +3441,9 @@ chkp_parse_array_and_component_ref (tree node, tree= *ptr, > || TREE_CODE (var) =3D=3D STRING_CST > || TREE_CODE (var) =3D=3D SSA_NAME); > > + if (VAR_P (var) && DECL_HARD_REGISTER (var)) > + is_register_var =3D true; > + > *ptr =3D chkp_build_addr_expr (var); > } > > @@ -3455,7 +3459,11 @@ chkp_parse_array_and_component_ref (tree node, tre= e *ptr, > > if (TREE_CODE (var) =3D=3D ARRAY_REF) > { > - *safe =3D false; > + // Mark it as unsafe, unless the array being accessed > + // has been explicitly placed on a register: in this > + // case we cannot take a pointer of this variable, > + // so we don't instrument the access. > + *safe =3D is_register_var; > array_ref_found =3D true; > if (flag_chkp_narrow_bounds > && !flag_chkp_narrow_to_innermost_arrray > @@ -4001,6 +4009,19 @@ chkp_process_stmt (gimple_stmt_iterator *iter, tre= e node, > bool bitfield; > tree elt; > > + { > + // We don't instrument accesses to arrays that > + // are explicitely assigned to hard registers. > + HOST_WIDE_INT bitsize, bitpos; > + tree base, offset; > + machine_mode mode; > + int unsignedp, reversep, volatilep =3D 0; > + base =3D get_inner_reference (node, &bitsize, &bitpos, &offset,= &mode, > + &unsignedp, &reversep, &volatilep); > + if (VAR_P (base) && DECL_HARD_REGISTER (base)) > + safe =3D true; > + } > + > if (safe) > { > /* We are not going to generate any checks, so do not > > diff --git a/gcc/testsuite/gcc.target/i386/mpx/pr79990.c > b/gcc/testsuite/gcc.target/i386/mpx/pr79990.c > new file mode 100644 > index 0000000..a27734d > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/mpx/pr79990.c > @@ -0,0 +1,18 @@ > +/* { dg-do compile } */ > +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */ > + > +typedef int U __attribute__ ((vector_size (16))); > + > +int > +foo (int i) > +{ > +#if __SSE2__ > + register > +#endif > + U u > +#if __SSE2__ > + asm ("xmm0") > +#endif > + ; > + return u[i]; > +} > > regards, > Alexander