Re: bug in optimised strstr

[Sam Clegg <sam at ideaworks3d dot com>]

On Thu, 2008-10-02 at 13:35 -0400, Jeff Johnston wrote:
> Jeff Johnston wrote:
> > Eric Blake wrote:
> >> Jeff Johnston <jjohnstn <at> redhat.com> writes:
> >>
> >>  
> >>>   max_suffix = SIZE_MAX;
> >>>   j = 0;
> >>>   k = p = 1;
> >>>   while (j + k < needle_len)
> >>>     {
> >>>       a = CANON_ELEMENT (needle[j + k]);
> >>>       b = CANON_ELEMENT (needle[max_suffix + k]);
> >>>
> >>> it is the line b=....
> >>>
> >>> It cannot be correct as you are trying to reference SIZE_MAX + 1 the 
> >>> first time through the loop.
> >>>     
> >>
> >> But the comments state:
> >>
> >>   /* Invariants:
> >>      0 <= j < NEEDLE_LEN - 1
> >>      -1 <= max_suffix{,_rev} < j (treating SIZE_MAX as if it were 
> >> signed)
> >> ...
> >>
> >> On cygwin, this works (in other words, I'm not reproducing the 
> >> crash).  The intent is that this line is referencing needle[0].  What 
> >> type is size_t on your platform, and the value of SIZE_MAX?  Could it 
> >> be that there is some type promotion going on, where the result of 
> >> SIZE_MAX+1 results in a 64-bit type containing 2**32 instead of 0, as 
> >> is required by modulo math since size_t is unsigned?
> >>
> >>   
> > Ok, I got mixed up on what SIZE_MAX was supposed to be.  For x86, 
> > stdint.h is being overridden with one in libc/sys/linux/include that 
> > has a wrong value for SIZE_MAX (LONG_MAX).  I am rebuilding now.  I 
> > can't say what is happening for arm since it should be using the same 
> > stdint.h from libc/include.
> >
> > -- Jeff J.
> >
> >
> Patch works for x86-linux and is checked in.  I don't have an arm system 
> to play around with.  Sam, are you running arm-linux (which isn't in 
> newlib)?  That would use the code in question.  Otherwise, can you 
> provide more details (e.g. run under gdb and print out the various 
> information above)?

Looks like there were three places where SIZE_MAX was wrongly defined
and all of them are now corrected.

I must have been compiling codesourcery last drop when I had the problem
on an ARM build since it predated the fix in libc/include/stdint.h a
couple of months ago.

http://sourceware.org/cgi-bin/cvsweb.cgi/src/newlib/libc/include/stdint.h.diff?r1=1.10&r2=1.11&cvsroot=src

If your latest submission also fixes x86-linux then I'm sure all is
good. I'll let you know if i see this issue again.

thanks for you help,
sam

-- 
sam clegg
sam@ideaworks3d.com

Attachment: signature.asc
Description: This is a digitally signed message part