diff mbox

[SH] Check for 0 length with inlined strnlen builtin

Message ID 54ABAAD0.4020302@st.com
State New
Headers show

Commit Message

Christian Bruel Jan. 6, 2015, 9:28 a.m. UTC 
Hello,

We should not enter the first iteration when length is 0. Testcase 
attached. Difficult to reduce because register allocation generated 
accidentally the correct return value.

testsuite OK

OK for 4.9 and trunk ?

Christian

Comments

Oleg Endo Jan. 6, 2015, 9:41 a.m. UTC | #1 
On Tue, 2015-01-06 at 10:28 +0100, Christian Bruel wrote:
> Hello,
> 
> We should not enter the first iteration when length is 0. Testcase 
> attached. Difficult to reduce because register allocation generated 
> accidentally the correct return value.
> 
> testsuite OK
> 
> OK for 4.9 and trunk ?

In your patch:

+      emit_insn (gen_tstsi_t (len, len));

Please use 'gen_cmpeqsi_t (len, const0_rtx)' for comparing a value
against zero instead of the bit test insn.  Otherwise, I think it's OK
for trunk and 4.9.

Cheers,
Oleg
diff mbox

Patch

2015-01-08  Christian Bruel  <christian.bruel@st.com>

	PR target/64507
	* config/sh/sh-mem.cc (sh_expand_cmpnstr): Check 0 length.

2015-01-08  Christian Bruel  <christian.bruel@st.com>

	PR target/64507
	* gcc.target/sh/pr64507.c: New test.

Index: gcc/config/sh/sh-mem.cc
===================================================================
--- gcc/config/sh/sh-mem.cc	(revision 219182)
+++ gcc/config/sh/sh-mem.cc	(working copy)
@@ -1,5 +1,5 @@ 
 /* Helper routines for memory move and comparison insns.
-   Copyright (C) 2013-2014 Free Software Foundation, Inc.
+   Copyright (C) 2013-2015 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -421,6 +421,7 @@ 
 	  /* end loop.  Reached max iterations.  */
 	  if (sbytes == 0)
 	    {
+	      emit_insn (gen_subsi3 (operands[0], tmp1, tmp2));
 	      jump = emit_jump_insn (gen_jump_compact (L_return));
 	      emit_barrier_after (jump);
 	    }
@@ -496,6 +497,13 @@ 
       jump = emit_jump_insn (gen_jump_compact( L_end_loop_byte));
       emit_barrier_after (jump);
     }
+  else
+    {
+      emit_insn (gen_tstsi_t (len, len));
+      emit_move_insn (operands[0], const0_rtx);
+      jump = emit_jump_insn (gen_branch_true (L_return));
+      add_int_reg_note (jump, REG_BR_PROB, prob_unlikely);
+    }
 
   addr1 = adjust_automodify_address (addr1, QImode, s1_addr, 0);
   addr2 = adjust_automodify_address (addr2, QImode, s2_addr, 0);
@@ -536,10 +544,10 @@ 
     emit_insn (gen_zero_extendqisi2 (tmp2, gen_lowpart (QImode, tmp2)));
   emit_insn (gen_zero_extendqisi2 (tmp1, gen_lowpart (QImode, tmp1)));
 
-  emit_label (L_return);
-
   emit_insn (gen_subsi3 (operands[0], tmp1, tmp2));
 
+  emit_label (L_return);
+
   return true;
 }
 
Index: gcc/testsuite/gcc.target/sh/pr64507.c
===================================================================
--- gcc/testsuite/gcc.target/sh/pr64507.c	(revision 0)
+++ gcc/testsuite/gcc.target/sh/pr64507.c	(working copy)
@@ -0,0 +1,25 @@ 
+/* Check that the __builtin_strnlen returns 0 with with 
+   non-constant 0 length.  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+extern int snprintf(char *, int, const char *, ...);
+extern void abort (void);
+
+int main()
+ {
+   int i;
+   int cmp = 0;
+   char buffer[1024];
+   const char* s = "the string";
+
+   snprintf(buffer, 4, "%s", s);
+
+   for (i = 1; i < 4; i++)
+     cmp += __builtin_strncmp(buffer, s, i - 1);
+
+  if (cmp)
+    abort();
+
+  return 0;
+}