diff mbox

[PR68337] Don't fold memcpy/memmove we want to instrument

Message ID 20151123134505.GC11184@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Nov. 23, 2015, 1:45 p.m. UTC
On 23 Nov 14:29, Richard Biener wrote:
> On Mon, Nov 23, 2015 at 12:33 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> >
> > I see.  But it should still be OK to check type in case of strict aliasing, right?
> 
> No, memcpy is always "no-strict-aliasing"
> 

Thanks a lot for help!  Here is a variant with a size check only as
you originally suggested.  Is it OK for trunk and gcc-5-branch if
no regressions?

Thanks,
Ilya
--
gcc/

2015-11-23  Ilya Enkovich  <enkovich.gnu@gmail.com>

	* gimple-fold.c: Include ipa-chkp.h.
	(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-23  Ilya Enkovich  <enkovich.gnu@gmail.com>

	* gcc.target/i386/mpx/pr68337-1.c: New test.
	* gcc.target/i386/mpx/pr68337-2.c: New test.

Comments

Richard Biener Nov. 23, 2015, 3:33 p.m. UTC | #1
On Mon, Nov 23, 2015 at 2:45 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> On 23 Nov 14:29, Richard Biener wrote:
>> On Mon, Nov 23, 2015 at 12:33 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> >
>> > I see.  But it should still be OK to check type in case of strict aliasing, right?
>>
>> No, memcpy is always "no-strict-aliasing"
>>
>
> Thanks a lot for help!  Here is a variant with a size check only as
> you originally suggested.  Is it OK for trunk and gcc-5-branch if
> no regressions?

Ok.

Thanks,
Richard.

> Thanks,
> Ilya
> --
> gcc/
>
> 2015-11-23  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * gimple-fold.c: Include ipa-chkp.h.
>         (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-23  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * gcc.target/i386/mpx/pr68337-1.c: New test.
>         * gcc.target/i386/mpx/pr68337-2.c: New test.
>
>
> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
> index 1ab20d1..6ff5e26 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,18 @@ 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)
> +         /* 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))
> +       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..8845cca
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mpx/pr68337-2.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
> +/* { dg-final { scan-assembler-not "memcpy" } } */
> +
> +void
> +test (void *dst, void *src)
> +{
> +  __builtin_memcpy (dst, src, sizeof (char *) / 2);
> +}
diff mbox

Patch

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 1ab20d1..6ff5e26 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,18 @@  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)
+	  /* 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))
+	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..8845cca
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/pr68337-2.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fcheck-pointer-bounds -mmpx" } */
+/* { dg-final { scan-assembler-not "memcpy" } } */
+
+void
+test (void *dst, void *src)
+{
+  __builtin_memcpy (dst, src, sizeof (char *) / 2);
+}