On Fri, Jun 17, 2011 at 08:28:57AM +0200, Pierre Habouzit wrote:

> >   $ gcc -g -o foo foo.c
> >   $ valgrind ./foo
> >   ...
> >   ==5854== Conditional jump or move depends on uninitialised value(s)
> >   ==5854==    at 0x4B3623E: __strspn_sse42 (strspn-c.c:142)
> >   ==5854==    by 0x400509: main (foo.c:8)
> >   ...
> > 
> > Looks like it was reported elsewhere with a patch a few months ago:
> > 
> >   https://bugs.kde.org/show_bug.cgi?id=270925
> > 
> > I tried the version of valgrind in experimental, but it shows the same
> > behavior.
> 
> This isn't very surprising as the libc over-reads memory in its strspn
> implementation. valgrind has to reimplement it and divert it (like it
> does for dozens of other str* functions) but it doesn't sadly.

Yeah, they even ship a commented-out implementation of strspn; the patch
I mentioned above basically just uncomments it.

Beware of that patch, though. I was getting some weird behavior with it,
and I finally tracked down the bug. Instead of simply uncommenting the
strspn implementation, he copied the boilerplate from the strcspn
implementation just above it, and botched it! The replacement strspn
actually behaves like strcspn.

You need this patch on top of it:

diff --git a/memcheck/mc_replace_strmem.c b/memcheck/mc_replace_strmem.c
index 21f572f..7439fb4 100644
--- a/memcheck/mc_replace_strmem.c
+++ b/memcheck/mc_replace_strmem.c
@@ -1144,7 +1144,7 @@ STRCSPN(VG_Z_LIBC_SONAME,          strcspn)
                break; \
          } \
          /* assert(i >= 0 && i <= nrej); */ \
-         if (i < nacc) \
+         if (i == nacc) \
             break; \
          s++; \
          len++; \

I'll attach the full pristine patch from valgrind 3.6.1-5 that fixes it
for me.

-Peff
diff --git a/memcheck/mc_replace_strmem.c b/memcheck/mc_replace_strmem.c
index ebef4d8..0622a5e 100644
--- a/memcheck/mc_replace_strmem.c
+++ b/memcheck/mc_replace_strmem.c
@@ -1120,35 +1120,42 @@ STRCSPN(VG_Z_LIBC_SONAME,          strcspn)
 #endif
 
 
-// And here's a validated strspn replacement, should it
-// become necessary.
-//UWord mystrspn( UChar* s, UChar* accept )
-//{
-//   /* find the length of 'accept', not including terminating zero */
-//   UWord nacc = 0;
-//   while (accept[nacc]) nacc++;
-//   if (nacc == 0) return 0;
-//
-//   UWord len = 0;
-//   while (1) {
-//      UWord i;
-//      UChar sc = *s;
-//      if (sc == 0)
-//         break;
-//      for (i = 0; i < nacc; i++) {
-//         if (sc == accept[i])
-//            break;
-//      }
-//      assert(i >= 0 && i <= nacc);
-//      if (i == nacc)
-//         break;
-//      s++;
-//      len++;
-//   }
-//
-//   return len;
-//}
+#define STRSPN(soname, fnname) \
+   SizeT VG_REPLACE_FUNCTION_ZU(soname,fnname) \
+         (void* sV, void* acceptV); \
+   SizeT VG_REPLACE_FUNCTION_ZU(soname,fnname) \
+         (void* sV, void* acceptV) \
+   { \
+      UChar* s = (UChar*)sV; \
+      UChar* accept = (UChar*)acceptV; \
+      \
+      /* find the length of 'accept', not including terminating zero */ \
+      UWord nacc = 0; \
+      while (accept[nacc]) nacc++; \
+      \
+      UWord len = 0; \
+      while (1) { \
+         UWord i; \
+         UChar sc = *s; \
+         if (sc == 0) \
+            break; \
+         for (i = 0; i < nacc; i++) { \
+            if (sc == accept[i]) \
+               break; \
+         } \
+         /* assert(i >= 0 && i <= nacc); */ \
+         if (i == nacc) \
+            break; \
+         s++; \
+         len++; \
+      } \
+      \
+      return len; \
+   }
 
+#if defined(VGO_linux)
+STRSPN(VG_Z_LIBC_SONAME,          strspn)
+#endif
 
 /*------------------------------------------------------------*/
 /*--- Improve definedness checking of process environment  ---*/

Reply via email to