| Message ID | 20151119163110.GG42296@msticlxl57.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
On 11/19/2015 05:31 PM, Ilya Enkovich wrote: > Currently we fold all memcpy/memmove calls with a known data size. > It causes two problems when used with Pointer Bounds Checker. > The first problem is that we may copy pointers as integer data > and thus loose bounds. The second problem is that if we inline > memcpy, we also have to inline bounds copy and this may result > in a huge amount of code and significant compilation time growth. > This patch disables folding for functions we want to instrument. > > Does it look reasonable for trunk and GCC5 branch? Bootstrapped > and regtested on x86_64-unknown-linux-gnu. Can't see anything wrong with it. Ok. Bernd
On November 19, 2015 6:12:30 PM GMT+01:00, Bernd Schmidt <bschmidt@redhat.com> wrote: >On 11/19/2015 05:31 PM, Ilya Enkovich wrote: >> Currently we fold all memcpy/memmove calls with a known data size. >> It causes two problems when used with Pointer Bounds Checker. >> The first problem is that we may copy pointers as integer data >> and thus loose bounds. The second problem is that if we inline >> memcpy, we also have to inline bounds copy and this may result >> in a huge amount of code and significant compilation time growth. >> This patch disables folding for functions we want to instrument. >> >> Does it look reasonable for trunk and GCC5 branch? Bootstrapped >> and regtested on x86_64-unknown-linux-gnu. > >Can't see anything wrong with it. Ok. But for small sizes this can have a huge impact on optimization. Which is why we have the code in the first place. I'd make the check less broad, for example inlining copies of size less than a pointer shouldn't be affected. Richard. > >Bernd
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index 1ab20d1..b3a1229 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -53,6 +53,7 @@ along with GCC; see the file COPYING3. If not see #include "gomp-constants.h" #include "optabs-query.h" #include "omp-low.h" +#include "ipa-chkp.h" /* Return true when DECL can be referenced from current unit. @@ -664,6 +665,13 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi, unsigned int src_align, dest_align; tree off0; + /* Inlining of memcpy/memmove may cause bounds lost (if we copy + pointers as wide integer) and also may result in huge function + size because of inlined bounds copy. Thus don't inline for + functions we want to instrument. */ + if (flag_check_pointer_bounds && chkp_instrumentable_p (cfun->decl)) + return false; + /* Build accesses at offset zero with a ref-all character type. */ off0 = build_int_cst (build_pointer_type_for_mode (char_type_node, ptr_mode, true), 0); diff --git a/gcc/testsuite/gcc.target/i386/mpx/pr68337.c b/gcc/testsuite/gcc.target/i386/mpx/pr68337.c new file mode 100644 index 0000000..3f8d79d --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/mpx/pr68337.c @@ -0,0 +1,32 @@ +/* { dg-do run } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */ + +#include "mpx-check.h" + +#define N 2 + +extern void abort (); + +static int +mpx_test (int argc, const char **argv) +{ + char ** src = (char **)malloc (sizeof (char *) * N); + char ** dst = (char **)malloc (sizeof (char *) * N); + int i; + + for (i = 0; i < N; i++) + src[i] = __bnd_set_ptr_bounds (argv[0] + i, i + 1); + + __builtin_memcpy(dst, src, sizeof (char *) * N); + + for (i = 0; i < N; i++) + { + char *p = dst[i]; + if (p != argv[0] + i + || __bnd_get_ptr_lbound (p) != p + || __bnd_get_ptr_ubound (p) != p + i) + abort (); + } + + return 0; +}