[PR,target/79449,7,regression] fix ppc strncmp builtin expansion runtime boundary crossing check
diff mbox

Message ID 1486698338.30084.17.camel@linux.vnet.ibm.com
State New
Headers show

Commit Message

Aaron Sawdey Feb. 10, 2017, 3:45 a.m. UTC
Basic problem is that the runtime check for a 4k boundary crossing was
checking that it didn't happen in the N bytes strncmp was being asked
to look at. But, the code generation after the check assumed it could
generate an 8-byte read and so it could step over the boundary. 

The fix is to make sure that the minimum distance to the boundary being
checked and the maximum read size being generated for these short
comparisons are in sync.

The added test case tests for this condition against both memcmp and
strncmp builtin expansion.

Assuming bootstrap/regtest passes on ppc variants and the new test case
passes on x86_64 as well, ok for trunk?


2017-02-09  Aaron Sawdey  <acsawdey@linux.vnet.ibm.com>

	PR target/79449
	* gcc.dg/strncmp-2.c: New.  Test strncmp and memcmp builtin
	expansion for reading beyond a 4k boundary.

2017-02-09  Aaron Sawdey  <acsawdey@linux.vnet.ibm.com>

	PR target/79449
	* config/rs6000/rs6000.c (expand_block_compare): Make sure runtime
	boundary crossing check and subsequent code generation agree.

Comments

Segher Boessenkool Feb. 10, 2017, 4:21 p.m. UTC | #1
Hi Aaron,

On Thu, Feb 09, 2017 at 09:45:38PM -0600, Aaron Sawdey wrote:
> --- gcc/config/rs6000/rs6000.c	(revision 245294)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -19931,15 +19931,26 @@
>  	 cmpldi	cr7,r8,4096-16
>  	 bgt	cr7,L(pagecross) */
>  
> +      /* Make sure that the length we use for the alignment test and
> +         the subsequent code generation are in agreement so we do not
> +         go past the length we tested for a 4k boundary crossing.  */
> +      unsigned HOST_WIDE_INT align_test = compare_length;
> +      if (align_test < 8)
> +        {
> +          align_test = 1 << ceil_log2 (align_test);

HOST_WIDE_INT_1U here.  Okay for trunk with that fixed, thanks!


Segher

Patch
diff mbox

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 245294)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -19931,15 +19931,26 @@ 
 	 cmpldi	cr7,r8,4096-16
 	 bgt	cr7,L(pagecross) */
 
+      /* Make sure that the length we use for the alignment test and
+         the subsequent code generation are in agreement so we do not
+         go past the length we tested for a 4k boundary crossing.  */
+      unsigned HOST_WIDE_INT align_test = compare_length;
+      if (align_test < 8)
+        {
+          align_test = 1 << ceil_log2 (align_test);
+          base_align = align_test;
+        }
+      else
+        {
+          align_test = ROUND_UP (align_test, 8);
+          base_align = 8;
+        }
+
       if (align1 < 8)
-	expand_strncmp_align_check (strncmp_label, src1, compare_length);
+        expand_strncmp_align_check (strncmp_label, src1, align_test);
       if (align2 < 8)
-	expand_strncmp_align_check (strncmp_label, src2, compare_length);
+        expand_strncmp_align_check (strncmp_label, src2, align_test);
 
-      /* After the runtime alignment checks, we can use any alignment we
-	 like as we know there is no 4k boundary crossing.  */
-      base_align = 8;
-
       /* Now generate the following sequence:
 	 - branch to begin_compare
 	 - strncmp_label
Index: gcc/testsuite/gcc.dg/strncmp-2.c
===================================================================
--- gcc/testsuite/gcc.dg/strncmp-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/strncmp-2.c	(working copy)
@@ -0,0 +1,99 @@ 
+/* Test strncmp builtin expansion for compilation and proper execution.  */
+/* { dg-do run { target *-*-linux* *-*-gnu* } } */
+/* { dg-options "-O2" } */
+/* { dg-require-effective-target ptr32plus } */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+int lib_memcmp(const void *a, const void *b, size_t n) asm("memcmp");
+int lib_strncmp(const char *a, const char *b, size_t n) asm("strncmp");
+
+static void test_driver_strncmp (void (test_strncmp)(const char *, const char *, int),
+				 void (test_memcmp)(const void *, const void *, int),
+				 size_t sz)
+{
+  long pgsz = sysconf(_SC_PAGESIZE);
+  char buf1[sz+1];
+  char *buf2 = aligned_alloc(pgsz,2*pgsz);
+  char *p2;
+  int r,i,e;
+
+  r = mprotect (buf2+pgsz,pgsz,PROT_NONE);
+  if (r < 0) abort();
+  
+  memset(buf1,'A',sz);
+  for(i=10; i>=0; i--) {
+    p2 = buf2+pgsz-sz-i;
+    memset(p2,'A',sz);
+    e = lib_strncmp(buf1,p2,sz);
+    (*test_strncmp)(buf1,p2,e);
+    e = lib_memcmp(buf1,p2,sz);
+    (*test_memcmp)(buf1,p2,e);
+  }
+}
+
+#define RUN_TEST(SZ) test_driver_strncmp (test_strncmp_ ## SZ, test_memcmp_ ## SZ, SZ);
+
+#define DEF_TEST(SZ) \
+__attribute__((noinline))						  \
+void test_strncmp_ ## SZ (const char *str1, const char *str2, int expect) \
+{									  \
+  int r;								  \
+  r = strncmp(str1,str2,SZ);						  \
+  if ( r < 0 && !(expect < 0) ) abort();				  \
+  if ( r > 0 && !(expect > 0) )	abort();				  \
+  if ( r == 0 && !(expect == 0) ) abort();				  \
+}                                                                         \
+__attribute__((noinline))						  \
+void test_memcmp_ ## SZ (const void *p1, const void *p2, int expect)      \
+{									  \
+  int r;								  \
+  r = memcmp(p1,p2,SZ);						          \
+  if ( r < 0 && !(expect < 0) ) abort();				  \
+  if ( r > 0 && !(expect > 0) )	abort();				  \
+  if ( r == 0 && !(expect == 0) ) abort();				  \
+}
+
+DEF_TEST(1)
+DEF_TEST(2)
+DEF_TEST(3)
+DEF_TEST(4)
+DEF_TEST(5)
+DEF_TEST(6)
+DEF_TEST(7)
+DEF_TEST(8)
+DEF_TEST(9)
+DEF_TEST(10)
+DEF_TEST(11)
+DEF_TEST(12)
+DEF_TEST(13)
+DEF_TEST(14)
+DEF_TEST(15)
+DEF_TEST(16)
+
+int
+main(int argc, char **argv)
+{
+  RUN_TEST(1) ;
+  RUN_TEST(2) ;
+  RUN_TEST(3) ;
+  RUN_TEST(4) ;
+  RUN_TEST(5) ;
+  RUN_TEST(6) ;
+  RUN_TEST(7) ;
+  RUN_TEST(8) ;
+  RUN_TEST(9) ;
+  RUN_TEST(10);
+  RUN_TEST(11);
+  RUN_TEST(12);
+  RUN_TEST(13);
+  RUN_TEST(14);
+  RUN_TEST(15);
+  RUN_TEST(16);
+  return 0;
+}