This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: bug in optimised strstr


Sam:
     A potentially quick way of checking for Eric's 64/32-bit question
(in
str-two-way.h):
-      b = CANON_ELEMENT (needle[max_suffix + k]);
+      b = CANON_ELEMENT (needle[(size_t)(max_suffix + k)]);
 
This might be a good thing to do as a patch just in case.  (Since
pointer[index] is effecively changed by compilers (per the std) to
*((pointer) + (index)), the result of the expression is the type of the
pointer operand.  Since pointers are not guaranteed to fit into any
of the integer types, the cast is safest--even if not normal practice.)
     Hey, Jeff, what exact file did you change?  I've checked my local
CVS copy against the repository and am not seeing any changes.
				Craig Howland

-----Original Message-----
From: newlib-owner@sourceware.org [mailto:newlib-owner@sourceware.org]
On Behalf Of Jeff Johnston
Sent: Thursday, October 02, 2008 1:35 PM
To: Eric Blake
Cc: newlib@sources.redhat.com; Sam Clegg
Subject: Re: bug in optimised strstr

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)?

-- Jeff J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]