public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][PR target/79449][7 regression] fix ppc strncmp builtin expansion runtime boundary crossing check
@ 2017-02-10  3:45 Aaron Sawdey
  2017-02-10 16:22 ` Segher Boessenkool
  0 siblings, 1 reply; 2+ messages in thread
From: Aaron Sawdey @ 2017-02-10  3:45 UTC (permalink / raw)
  To: gcc-patches; +Cc: Segher Boessenkool, Bill Schmidt

[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]

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.

-- 
Aaron Sawdey, Ph.D.  acsawdey@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain

[-- Attachment #2: strncmp.20170209a.patch --]
[-- Type: text/x-patch, Size: 4113 bytes --]

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;
+}

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH][PR target/79449][7 regression] fix ppc strncmp builtin expansion runtime boundary crossing check
  2017-02-10  3:45 [PATCH][PR target/79449][7 regression] fix ppc strncmp builtin expansion runtime boundary crossing check Aaron Sawdey
@ 2017-02-10 16:22 ` Segher Boessenkool
  0 siblings, 0 replies; 2+ messages in thread
From: Segher Boessenkool @ 2017-02-10 16:22 UTC (permalink / raw)
  To: Aaron Sawdey; +Cc: gcc-patches, Bill Schmidt

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-02-10 16:22 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-10  3:45 [PATCH][PR target/79449][7 regression] fix ppc strncmp builtin expansion runtime boundary crossing check Aaron Sawdey
2017-02-10 16:22 ` Segher Boessenkool

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).