Created attachment 58940 [details] add support for strspn() Version: 3.7 SVN OS: Linux false positive when strspn() called Reproducible: Always Steps to Reproduce: $ cat strspn.c #include <string.h> int main() { char s[] = "abcdef"; char accept[] = "234"; return strspn(s, accept); } $ gcc -O0 -g -Wall -Wextra strspn.c $ valgrind --tool=memcheck --track-origins=yes --read-var-info=yes --num-callers=50 ./a.out Actual Results: ==3935== Memcheck, a memory error detector ==3935== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al. ==3935== Using Valgrind-3.7.0.SVN and LibVEX; rerun with -h for copyright info ==3935== Command: ./a.out ==3935== ==3935== Conditional jump or move depends on uninitialised value(s) ==3935== at 0x3A45329F29: __strspn_sse42 (strspn-c.c:106) ==3935== by 0x4004F8: main (strspn.c:9) ==3935== Uninitialised value was created by a stack allocation ==3935== at 0x4004C4: main (strspn.c:4) ==3935== ==3935== Conditional jump or move depends on uninitialised value(s) ==3935== at 0x3A45329F4E: __strspn_sse42 (strspn-c.c:142) ==3935== by 0x4004F8: main (strspn.c:9) ==3935== Uninitialised value was created by a stack allocation ==3935== at 0x4004C4: main (strspn.c:4) ==3935== ==3935== Syscall param exit_group(status) contains uninitialised byte(s) ==3935== at 0x3A452AABA8: _Exit (_exit.c:33) ==3935== by 0x3A452369A1: exit (exit.c:93) ==3935== by 0x3A4521EE63: (below main) (libc-start.c:258) ==3935== Uninitialised value was created by a stack allocation ==3935== at 0x4004C4: main (strspn.c:4) ==3935== ==3935== ==3935== HEAP SUMMARY: ==3935== in use at exit: 0 bytes in 0 blocks ==3935== total heap usage: 0 allocs, 0 frees, 0 bytes allocated ==3935== ==3935== All heap blocks were freed -- no leaks are possible ==3935== ==3935== For counts of detected and suppressed errors, rerun with: -v ==3935== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 6 from 6) Expected Results: $ valgrind --tool=memcheck --track-origins=yes --read-var-info=yes --num-callers=50 ./a.out ==3022== Memcheck, a memory error detector ==3022== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al. ==3022== Using Valgrind-3.7.0.SVN and LibVEX; rerun with -h for copyright info ==3022== Command: ./a.out ==3022== ==3022== ==3022== HEAP SUMMARY: ==3022== in use at exit: 0 bytes in 0 blocks ==3022== total heap usage: 0 allocs, 0 frees, 0 bytes allocated ==3022== ==3022== All heap blocks were freed -- no leaks are possible ==3022== ==3022== For counts of detected and suppressed errors, rerun with: -v ==3022== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 6) Fedora 14/x64. last updated today.
I am seeing the same thing on my Debian unstable system (amd64, libc6 2.13-7). It also happens with strcasestr, which valgrind does not replace. Your patch to replace strspn has a bug; you obviously copied bits from the strcspn implementation, but forgot to change the break condition to accept rather than reject on a match. I'm attaching two patches: a fixed strspn implementation, and a replacement strcasestr implementation.
Created attachment 61095 [details] replacement strspn
Created attachment 61096 [details] replacement strcasestr
(In reply to comment #1) > Your patch to replace strspn has a bug; you obviously copied bits from the > strcspn implementation, but forgot to change the break condition to accept > rather than reject on a match. Really? I looked at it pretty carefully again just now, and I can't see anything wrong with it. Also, it doesn't cause any failures on the 5 tests in memcheck/tests/str_tester.c. Can you clarify?
The problematic lines from Dmitry's patch are: + /* assert(i >= 0 && i <= nrej); */ \ + if (i < nacc) \ + break; \ In the loop just prior to this, we will look through the "accept" string, and "i" will be the index of the character we found, or "nacc" if none was found. In strspn, we want to break only if we found a character not in the "accept" list. In other words: if (i == nacc) break; Using (i < nacc) means we will break if we did find the character, which implements strcspn (and I think this code was simply copied from the strcspn implementation with s/reject/accept/, and he forgot to update the condition. Note that the comment also still mentions nrej, which should be nacc).
Oh, my misunderstanding. Originally the code did have a commented out version of strspn which I had previously verified, and appears to be correct. I thought Dmitry's version merely uncommented it, but on closer examination that wasn't so -- he deleted it and derived strspn from the existing strcspn, without changing the break condition. I expect to commit this fairly soon, as part of the process of fixing the much larger and more complex redirection issue in bug 275284.
oh, sorry :(
Julian committed strspn support in r11985.