Re: [CHKP] Fix for PR79990

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

2017-04-02 23:52 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
> Hi,
>
> Here is the patch that roughly follows your idea.
> Some comments:
>
> - There are more cases than array_ref overflow. We need to take care
> of component_ref and both underflows/overflows are possible
> - I could not make it work with "0" as a fake address, because then
> catching lower bounds violation is getting hard at O2 and above. E.g.
> consider this:
>
>    0x00000000004005f8 <+8>:     bndmk  0x7(%rax),%bnd0
>    0x00000000004005fd <+13>:    mov    $0x400734,%edi
> => 0x0000000000400602 <+18>:    bndcl  0xfffffffffffffffc,%bnd0
>             (gdb) p $bnd0
>             $1 = {lbound = 0x0, ubound = 0x7} : size 8
>   0x000000000040060b <+27>:    callq  0x400500 <printf@plt>
>
>     - bndcu is removed as not necessary and underflowed access is not
> caught. I used another fake value for lower bound address, which is
> 2^(bitness - 1)

Hi,

Looks like CHKP optimizations don't let us catch cases when pointer
arithmetc overflows. Using any fake value doesn't guarantee you don't
have overflow.

This overoptimization is definately a separate issue. It should be easy
to write a test where usage of a huge index in array causes
uncought bounds violation because of removed bndcl/bndcu. You should
file a bug for that.

If we don't try to work around overflow issues in this patch then using 0
should be more efficient because it allows you to always use bndcu only
(you just can't violate zero lower bound).

BTW please don't forget ChangeLogs for your patches.

>
> - hard-reg-3-[1,2]* tests fail with ICE right now because of PR80270.
> I will mark them as XFAIL if the patch is approved and the mentioned
> bug is not fixed
>
>
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-2-lbv.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-2-lbv.c
> new file mode 100644
> index 0000000..319e1ec
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-2-lbv.c
> @@ -0,0 +1,21 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +typedef int v16 __attribute__((vector_size(16)));
> +
> +int foo(int i) {
> +  register v16 u asm("xmm0");
> +  return u[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo (-1));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-2-nov.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-2-nov.c
> new file mode 100644
> index 0000000..3c6d39a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-2-nov.c
> @@ -0,0 +1,18 @@
> +/* { dg-do run } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +#include "mpx-check.h"
> +
> +typedef int v16 __attribute__((vector_size(16)));
> +
> +int foo (int i) {
> +  register v16 u asm ("xmm0");
> +  return u[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo (3));
> +  printf ("%d\n", foo (0));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-2-ubv.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-2-ubv.c
> new file mode 100644
> index 0000000..7fe76c4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-2-ubv.c
> @@ -0,0 +1,21 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +typedef int v16 __attribute__((vector_size(16)));
> +
> +int foo (int i) {
> +  register v16 u asm ("xmm0");
> +  return u[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo (5));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-1-lbv.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-1-lbv.c
> new file mode 100644
> index 0000000..7e4451f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-1-lbv.c
> @@ -0,0 +1,33 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +typedef int v8 __attribute__ ((vector_size (8)));
> +
> +struct S1
> +{
> +  v8 s1f;
> +};
> +
> +struct S2
> +{
> +  struct S1 s2f1;
> +  v8 s2f2;
> +};
> +
> +int foo_s2f1 (int i)
> +{
> +  register struct S2 b asm ("xmm0");
> +  return b.s2f1.s1f[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo_s2f1 (-1));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-1-nov.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-1-nov.c
> new file mode 100644
> index 0000000..73bd7fb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-1-nov.c
> @@ -0,0 +1,30 @@
> +/* { dg-do run } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#include "mpx-check.h"
> +
> +typedef int v8 __attribute__ ((vector_size (8)));
> +
> +struct S1
> +{
> +  v8 s1f;
> +};
> +
> +struct S2
> +{
> +  struct S1 s2f1;
> +  v8 s2f2;
> +};
> +
> +int foo_s2f1 (int i)
> +{
> +  register struct S2 b asm ("xmm0");
> +  return b.s2f1.s1f[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo_s2f1 (0));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-1-ubv.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-1-ubv.c
> new file mode 100644
> index 0000000..166b6b9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-1-ubv.c
> @@ -0,0 +1,33 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +typedef int v8 __attribute__ ((vector_size (8)));
> +
> +struct S1
> +{
> +  v8 s1f;
> +};
> +
> +struct S2
> +{
> +  struct S1 s2f1;
> +  v8 s2f2;
> +};
> +
> +int foo_s2f1 (int i)
> +{
> +  register struct S2 b asm ("xmm0");
> +  return b.s2f1.s1f[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo_s2f1 (3));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-2-lbv.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-2-lbv.c
> new file mode 100644
> index 0000000..7820c2f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-2-lbv.c
> @@ -0,0 +1,33 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +typedef int v8 __attribute__ ((vector_size (8)));
> +
> +struct S1
> +{
> +  v8 s1f;
> +};
> +
> +struct S2
> +{
> +  struct S1 s2f1;
> +  v8 s2f2;
> +};
> +
> +int foo_s2f2 (int i)
> +{
> +  register struct S2 b asm ("xmm0");
> +  return b.s2f2[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo_s2f2 (-1));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-2-nov.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-2-nov.c
> new file mode 100644
> index 0000000..0816e58
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-2-nov.c
> @@ -0,0 +1,30 @@
> +/* { dg-do run } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#include "mpx-check.h"
> +
> +typedef int v8 __attribute__ ((vector_size (8)));
> +
> +struct S1
> +{
> +  v8 s1f;
> +};
> +
> +struct S2
> +{
> +  struct S1 s2f1;
> +  v8 s2f2;
> +};
> +
> +int foo_s2f2 (int i)
> +{
> +  register struct S2 b asm ("xmm0");
> +  return b.s2f2[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo_s2f2 (0));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-2-ubv.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-2-ubv.c
> new file mode 100644
> index 0000000..94261a7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-2-ubv.c
> @@ -0,0 +1,33 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +typedef int v8 __attribute__ ((vector_size (8)));
> +
> +struct S1
> +{
> +  v8 s1f;
> +};
> +
> +struct S2
> +{
> +  struct S1 s2f1;
> +  v8 s2f2;
> +};
> +
> +int foo_s2f2 (int i)
> +{
> +  register struct S2 b asm ("xmm0");
> +  return b.s2f2[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo_s2f2 (3));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-lbv.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-lbv.c
> new file mode 100644
> index 0000000..f273d58
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-lbv.c
> @@ -0,0 +1,27 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +typedef int v16 __attribute__ ((vector_size (16)));
> +
> +struct S1
> +{
> +  v16 s1f1;
> +};
> +
> +int foo_s1f1 (int i)
> +{
> +  register struct S1 b asm ("xmm0");
> +  return b.s1f1[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo_s1f1 (-1));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-nov.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-nov.c
> new file mode 100644
> index 0000000..aa8f7b9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-nov.c
> @@ -0,0 +1,24 @@
> +/* { dg-do run } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#include "mpx-check.h"
> +
> +typedef int v16 __attribute__ ((vector_size (16)));
> +
> +struct S1
> +{
> +  v16 s1f1;
> +};
> +
> +int foo_s1f1 (int i)
> +{
> +  register struct S1 b asm ("xmm0");
> +  return b.s1f1[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo_s1f1 (0));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-ubv.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-ubv.c
> new file mode 100644
> index 0000000..3d0c9b2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-3-ubv.c
> @@ -0,0 +1,27 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +typedef int v16 __attribute__ ((vector_size (16)));
> +
> +struct S1
> +{
> +  v16 s1f1;
> +};
> +
> +int foo_s1f1 (int i)
> +{
> +  register struct S1 b asm ("xmm0");
> +  return b.s1f1[i];
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo_s1f1 (7));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-4-lbv.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-4-lbv.c
> new file mode 100644
> index 0000000..e81b942
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-4-lbv.c
> @@ -0,0 +1,31 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +typedef int v8 __attribute__ ((vector_size (8)));
> +
> +struct S2
> +{
> +  v8 s2f2;
> +  int* f3;
> +};
> +
> +int foo (int i)
> +{
> +  register struct S2 b asm ("xmm0");
> +  int k = 5;
> +  b.f3 = &k;
> +  b.f3 = b.f3 + i;
> +  return *b.f3;
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo (-1));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-4-nov.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-4-nov.c
> new file mode 100644
> index 0000000..4b1f1ac
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-4-nov.c
> @@ -0,0 +1,28 @@
> +/* { dg-do run } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#include "mpx-check.h"
> +
> +typedef int v8 __attribute__ ((vector_size (8)));
> +
> +struct S2
> +{
> +  v8 s2f2;
> +  int* f3;
> +};
> +
> +int foo (int i)
> +{
> +  register struct S2 b asm ("xmm0");
> +  int k = 5;
> +  b.f3 = &k;
> +  b.f3 = b.f3 + i;
> +  return *b.f3;
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo (0));
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/mpx/hard-reg-4-ubv.c
> b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-4-ubv.c
> new file mode 100644
> index 0000000..e95e68f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/hard-reg-4-ubv.c
> @@ -0,0 +1,31 @@
> +/* { dg-do run } */
> +/* { dg-shouldfail "bounds violation" } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +
> +
> +#define SHOULDFAIL
> +
> +#include "mpx-check.h"
> +
> +typedef int v8 __attribute__ ((vector_size (8)));
> +
> +struct S2
> +{
> +  v8 s2f2;
> +  int* f3;
> +};
> +
> +int foo (int i)
> +{
> +  register struct S2 b asm ("xmm0");
> +  int k = 5;
> +  b.f3 = &k;
> +  b.f3 = b.f3 + i;
> +  return *b.f3;
> +}
> +
> +int mpx_test (int argc, const char **argv)
> +{
> +  printf ("%d\n", foo (1));
> +  return 0;
> +}
> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
> index b1ff218..15c0da6 100644
> --- a/gcc/tree-chkp.c
> +++ b/gcc/tree-chkp.c
> @@ -679,6 +679,17 @@ chkp_erase_completed_bounds (void)
>    chkp_completed_bounds_set = new hash_set<tree>;
>  }
>
> +/* If we check bounds for a hard register variable, we cannot
> +   use its address - it is illegal, so instead of that we use
> +   this fake value.  */
> +static tree
> +chkp_get_hard_register_var_fake_address ()
> +{
> +  tree base = fold_convert (ptr_type_node, integer_zero_node);
> +  unsigned HOST_WIDE_INT offset = 1 << (TYPE_PRECISION (ptr_type_node) - 1);
> +  return fold_build_pointer_plus_hwi (base, offset);
> +}
> +
>  /* Mark BOUNDS associated with PTR as incomplete.  */
>  static void
>  chkp_register_incomplete_bounds (tree bounds, tree ptr)
> @@ -1040,10 +1051,24 @@ chkp_add_modification_to_stmt_list (tree lhs,
>    stmts->avail--;
>  }
>
> -/* Build and return ADDR_EXPR for specified object OBJ.  */
> +/* Build and return ADDR_EXPR for specified object OBJ.
> +   There is a special case for which we cannot return
> +   ADDR_EXPR - if the object is declared to be placed
> +   on a fixed hard register - in this case we cannot
> +   take its address, so we use the object itself. The
> +   caller of this function must be aware of that and
> +   use proper checks if necessary.  */

I don't follow the idea of this change.

If we want to use fake address for register vars then why not to
return this fake value by this function? In case of accessing a
component of register var we should return fake value + required
offset then.

>  static tree
>  chkp_build_addr_expr (tree obj)
>  {
> +  /* We first check whether it is a "hard reg case".  */
> +  tree outer = obj;
> +  while (TREE_CODE (outer) == COMPONENT_REF)
> +    outer = TREE_OPERAND (outer, 0);

What about ARRAY_REF? Probably get_base_address is what you need.

> +  if (VAR_P (outer) && DECL_HARD_REGISTER (outer))
> +      return obj;
> +
> +  /* If not - return regular ADDR_EXPR.  */
>    return TREE_CODE (obj) == TARGET_MEM_REF
>      ? tree_mem_ref_addr (ptr_type_node, obj)
>      : build_fold_addr_expr (obj);
> @@ -3170,6 +3195,11 @@ chkp_get_bounds_for_decl_addr (tree decl)
>        gcc_assert (VAR_P (decl));
>        bounds = chkp_generate_extern_var_bounds (decl);
>      }
> +  else if (VAR_P (decl) && DECL_HARD_REGISTER (decl))
> +    {
> +      tree lb = chkp_get_hard_register_var_fake_address ();
> +      bounds = chkp_make_bounds(lb, DECL_SIZE_UNIT (decl), NULL, false);
> +    }
>    else
>      {
>        tree lb = chkp_build_addr_expr (decl);
> @@ -3351,6 +3381,8 @@ chkp_narrow_bounds_to_field (tree bounds, tree component,
>    tree field_ptr = chkp_build_addr_expr (component);
>    tree field_bounds;
>
> +  if (!BOUNDED_P (field_ptr))
> +    field_ptr = chkp_get_hard_register_var_fake_address ();

This gives you fake pointer instead of fake pointer + offset. No?

>    field_bounds = chkp_make_bounds (field_ptr, size, iter, false);
>
>    return chkp_intersect_bounds (field_bounds, bounds, iter);
> @@ -3639,6 +3671,8 @@ chkp_find_bounds_1 (tree ptr, tree ptr_src,
> gimple_stmt_iterator *iter)
>      addr = chkp_build_addr_expr (ptr_src);
>      bounds = chkp_build_bndldx (addr, ptr, iter);
>    }
> +      else if (VAR_P (ptr) && DECL_HARD_REGISTER (ptr))
> + bounds = chkp_make_addressed_object_bounds (ptr_src, iter);

I don't get what this piece of code is for.

>        else
>   bounds = chkp_get_nonpointer_load_bounds ();
>        break;
> @@ -4031,7 +4065,19 @@ chkp_process_stmt (gimple_stmt_iterator *iter, tree node,
>        addr_first,
>        byte_position (field));
>            }
> -        else
> +        else if (VAR_P (ptr) && DECL_HARD_REGISTER (ptr))
> +  {
> +    gcc_assert (TREE_CODE (node) == ARRAY_REF
> + || TREE_CODE (node) == COMPONENT_REF);

This may never hit because this code is in a switch block filtering
node to ARRAY_REF and COMPONENT_REF already.

> +
> +    tree base = chkp_get_hard_register_var_fake_address ();
> +    tree indx = fold_convert_loc (loc,
> +  size_type_node,
> +  TREE_OPERAND (node, 1));

Code looks like it expects ARRAY_REF only but COMPONENT_REF
is also possible.

> +    tree offset = size_binop_loc (loc, MULT_EXPR, size, indx);

'size' here holds a size of memory access, not size of array element.

Overall it looks like all you need is a proper fix in chkp_build_addr_expr
to use fake value when required. Many (all?) other changes might just
go away then.

Thanks,
Ilya

> +    addr_first = fold_build_pointer_plus_loc (loc, base, offset);
> +  }
> + else
>            addr_first = chkp_build_addr_expr (node);
>        }
>        break;
>
> 2017-03-23 20:57 GMT+01:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>> 2017-03-23 17:18 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>>> 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 = __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
>>> ‘u’ 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 = NULL_TREE;
>>>    tree last_comp = NULL_TREE;
>>>    bool array_ref_found = false;
>>> +  bool is_register_var = false;
>>>    tree *nodes;
>>>    tree var;
>>>    int len;
>>> @@ -3440,6 +3441,9 @@ chkp_parse_array_and_component_ref (tree node, tree *ptr,
>>>                   || TREE_CODE (var) == STRING_CST
>>>                   || TREE_CODE (var) == SSA_NAME);
>>>
>>> +      if (VAR_P (var) && DECL_HARD_REGISTER (var))
>>> +       is_register_var = true;
>>> +
>>>        *ptr = chkp_build_addr_expr (var);
>>>      }
>>>
>>> @@ -3455,7 +3459,11 @@ chkp_parse_array_and_component_ref (tree node, tree *ptr,
>>>
>>>        if (TREE_CODE (var) == ARRAY_REF)
>>>         {
>>> -         *safe = 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 = is_register_var;
>>>           array_ref_found = true;
>>>           if (flag_chkp_narrow_bounds
>>>               && !flag_chkp_narrow_to_innermost_arrray
>>> @@ -4001,6 +4009,19 @@ chkp_process_stmt (gimple_stmt_iterator *iter, tree 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 = 0;
>>> +         base = get_inner_reference (node, &bitsize, &bitpos, &offset, &mode,
>>> +                                     &unsignedp, &reversep, &volatilep);
>>> +         if (VAR_P (base) && DECL_HARD_REGISTER (base))
>>> +           safe = 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