Re: [PATCH 2/4] Detect EOL on-the-fly in strstr, strcasestr and memmem.

[Maxim Kuvyrkov <maxim at codesourcery dot com>]

On 7/10/2012, at 12:33 PM, Maxim Kuvyrkov wrote:

> On 6/10/2012, at 9:08 AM, Maxim Kuvyrkov wrote:
> 
>> On 6/10/2012, at 8:23 AM, H.J. Lu wrote:
>> 
>>> On Wed, May 30, 2012 at 2:10 AM, Maxim Kuvyrkov <maxim@codesourcery.com> wrote:
>>>> [PATCH 2/4] Detect EOL on-the-fly in strstr, strcasestr and memmem.
>>>> 
>>>> --
>>>> Maxim Kuvyrkov
>>>> CodeSourcery / Mentor Graphics
>>>> 
>>>> 
>>>>      [BZ #11607]
>>>>      * string/str-two-way.h (AVAILABLE1, AVAILABLE2, RET0_IF_0): New macros,
>>>>      define their defaults.
>>>>      (two_way_short_needle): Detect end-of-string on-the-fly.
>>>>      * string/strcasestr.c, string/strstr.c (AVAILABLE): Update.
>>>>      (AVAILABLE1, AVAILABLE2, RET0_IF_0, AVAILABLE_USES_J): Define.
>>>>      * string/bug-strcasestr1.c: New test.
>>>>      * string/Makefile: Run it.
>>> 
>>> This caused:
>>> 
>>> http://www.sourceware.org/bugzilla/show_bug.cgi?id=14602
>>> 
>>> I created hjl/pr14602 branch to add a testcase. On x86-64,
>>> I got
>>> 
>>> /export/build/gnu/glibc/build-x86_64-linux/string/test-strstr: Wrong
>>> result in function simple_strstr , enable_shared,  (null)
>>> /export/build/gnu/glibc/build-x86_64-linux/string/test-strstr-ifunc:
>>> Wrong result in function simple_strstr , enable_shared,  (null)
>>> /export/build/gnu/glibc/build-x86_64-linux/string/test-strstr-ifunc:
>>> Wrong result in function __strstr_sse2 , enable_shared,  (null)v
>> 
>> Thanks for the testcase, I'll investigate this today.

The problem is in initialization phase of matching the needle when length of haystack is about the same as needle.  We match the needle from inside out, starting with some point inside the needle (aka needle[suffix]), match the right side first, then the left side.  If the haystack is short, then haystack[suffix] may turn out to be beyond the end of haystack, thus causing the bug.  Current implementation correctly detects any EOL after haystack[suffix] and the attached patch adds a check for EOL from haystack[0] to haystack[suffix].

The actual fix is this hunk:

+#if CHECK_AVAILABLE_AFTER
+      /* We start matching from the SUFFIX'th element, so make sure we
+        don't hit '\0' before that.  */
+      if (haystack_len < suffix + 1
+         && !AVAILABLE (haystack, haystack_len, 0, suffix + 1))
+       return NULL;
+#endif

... the rest of the patch cleans up macro definitions (the alternative would be to add yet another AVAILABLE3() macro to use it just once in the above check).

Tested on i686-linux-gnu for both -m32 and -m64 multilibs with no regressions.

OK to apply?

--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics

Attachment: 0001-Fix-BZ14602-strstr-and-strcasestr-return-wrong-resul.patch
Description: Binary data