On 19 Nov 18:19, Richard Biener wrote:
> On November 19, 2015 6:12:30 PM GMT+01:00, Bernd Schmidt
> <bschm...@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.
Right. We also may inline in case we know no pointers are copied. Below is a
version with extended condition and a couple more tests. Bootstrapped and
regtested on x86_64-unknown-linux-gnu. Does it OK for trunk and gcc-5-branch?
>
> Richard.
>
> >
> >Bernd
>
>
Thanks,
Ilya
--
gcc/
2015-11-20 Ilya Enkovich <enkovich....@gmail.com>
* gimple-fold.c (gimple_fold_builtin_memory_op): Don't
fold call if we are going to instrument it and it may
copy pointers.
gcc/testsuite/
2015-11-20 Ilya Enkovich <enkovich....@gmail.com>
* gcc.target/i386/mpx/pr68337-1.c: New test.
* gcc.target/i386/mpx/pr68337-2.c: New test.
* gcc.target/i386/mpx/pr68337-3.c: New test.
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 1ab20d1..dd9f80b 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -53,6 +53,8 @@ along with GCC; see the file COPYING3. If not see
#include "gomp-constants.h"
#include "optabs-query.h"
#include "omp-low.h"
+#include "tree-chkp.h"
+#include "ipa-chkp.h"
/* Return true when DECL can be referenced from current unit.
@@ -664,6 +666,23 @@ 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 in case pointers are copied. */
+ if (flag_check_pointer_bounds
+ && chkp_instrumentable_p (cfun->decl)
+ /* Even if data may contain pointers we can inline if copy
+ less than a pointer size. */
+ && (!tree_fits_uhwi_p (len)
+ || compare_tree_int (len, POINTER_SIZE_UNITS) >= 0)
+ /* Check data type for pointers. */
+ && (!TREE_TYPE (src)
+ || !TREE_TYPE (TREE_TYPE (src))
+ || VOID_TYPE_P (TREE_TYPE (TREE_TYPE (src)))
+ || chkp_type_has_pointer (TREE_TYPE (TREE_TYPE (src)))))
+ 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-1.c
b/gcc/testsuite/gcc.target/i386/mpx/pr68337-1.c
new file mode 100644
index 0000000..3f8d79d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/pr68337-1.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;
+}
diff --git a/gcc/testsuite/gcc.target/i386/mpx/pr68337-2.c
b/gcc/testsuite/gcc.target/i386/mpx/pr68337-2.c
new file mode 100644
index 0000000..16736b4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/pr68337-2.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
+/* { dg-final { scan-assembler-not "memcpy" } } */
+
+void
+test1 (char *dst, char *src)
+{
+ __builtin_memcpy (dst, src, sizeof (char *) * 2);
+}
+
+void
+test2 (void *dst, void *src)
+{
+ __builtin_memcpy (dst, src, sizeof (char *) / 2);
+}
+
+struct s
+{
+ int a;
+ int b;
+};
+
+void
+test3 (struct s *dst, struct s *src)
+{
+ __builtin_memcpy (dst, src, sizeof (struct s));
+}
diff --git a/gcc/testsuite/gcc.target/i386/mpx/pr68337-3.c
b/gcc/testsuite/gcc.target/i386/mpx/pr68337-3.c
new file mode 100644
index 0000000..095425a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/pr68337-3.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
+/* { dg-final { scan-assembler-times "bnd\[^\n\]*__mpx_wrapper_memcpy" 3 } } */
+
+void
+test1 (char **dst, char **src)
+{
+ __builtin_memcpy (dst, src, sizeof (char *) * 2);
+}
+
+void
+test2 (void *dst, void *src)
+{
+ __builtin_memcpy (dst, src, sizeof (char *));
+}
+
+struct s
+{
+ int a;
+ int *b;
+};
+
+void
+test3 (struct s *dst, struct s *src)
+{
+ __builtin_memcpy (dst, src, sizeof (struct s));
+}
