fix <ctype.h> bugs on 64-bit machine

Jon TURNEY jon.turney@dronecode.org.uk
Fri Nov 6 14:14:00 GMT 2009


This change seems to introduce a regression if there is an assignment inside 
the ctype macro argument, e.g.

#include <ctype.h>

int main()
{
         int c;
         int t = isspace(c = ' ');
}

after this ctype.h change, with the current cygwin gcc4, compilation fails 
'error: lvalue required as left operand of assignment'.

A trivial patch to fix this attached.

On 19:59, Jeff Johnston wrote:
> Go ahead.  You might as well put in the parentheses as suggested by Craig.
>
> -- Jeff J.
>
> On 19/10/09 04:21 PM, Eric Blake wrote:
>> Bruno Haible pointed out to me that on 64-bit machines, the use of
>> isalpha
>> (0x100000001LL) will cause an out-of-bounds array dereference, rather
>> than
>> returning the correct result of isalpha(1). 32-bit machines are
>> immune. All
>> of this stems from our QoI effort to warn about raw char arguments. OK to
>> apply this fix? I've verified that on 32-bit architectures, there is no
>> difference in the generated code, with or without optimization. I've also
>> verified that 'gcc -Wall' still gives the desired warning.
>>
>> diff --git i/newlib/libc/include/ctype.h w/newlib/libc/include/ctype.h
>> index e1d2d01..abc080e 100644
>> --- i/newlib/libc/include/ctype.h
>> +++ w/newlib/libc/include/ctype.h
>> @@ -47,24 +47,32 @@ extern __IMPORT char *__ctype_ptr__;
>> #ifndef __cplusplus
>> /* These macros are intentionally written in a manner that will trigger
>> a gcc -Wall warning if the user mistakenly passes a 'char' instead
>> - of an int containing an 'unsigned char'. */
>> -#define isalpha(__c) ((__ctype_ptr__+1)[__c]&(_U|_L))
>> -#define isupper(__c) (((__ctype_ptr__+1)[__c]&(_U|_L))==_U)
>> -#define islower(__c) (((__ctype_ptr__+1)[__c]&(_U|_L))==_L)
>> -#define isdigit(__c) ((__ctype_ptr__+1)[__c]&_N)
>> -#define isxdigit(__c) ((__ctype_ptr__+1)[__c]&(_X|_N))
>> -#define isspace(__c) ((__ctype_ptr__+1)[__c]&_S)
>> -#define ispunct(__c) ((__ctype_ptr__+1)[__c]&_P)
>> -#define isalnum(__c) ((__ctype_ptr__+1)[__c]&(_U|_L|_N))
>> -#define isprint(__c) ((__ctype_ptr__+1)[__c]&(_P|_U|_L|_N|_B))
>> -#define isgraph(__c) ((__ctype_ptr__+1)[__c]&(_P|_U|_L|_N))
>> -#define iscntrl(__c) ((__ctype_ptr__+1)[__c]&_C)
>> + of an int containing an 'unsigned char'. Note that the sizeof will
>> + always be 1, which is what we want for mapping EOF to __ctype_ptr__[0];
>> + the use of a raw index inside the sizeof triggers the gcc warning if
>> + __c was of type char, and sizeof masks side effects of the extra __c.
>> + Meanwhile, the real index to __ctype_ptr__+1 must be cast to int,
>> + since isalpha(0x100000001LL) must equal isalpha(1), rather than being
>> + an out-of-bounds reference on a 64-bit machine. */
>> +#define __ctype_lookup(__c) ((__ctype_ptr__+sizeof""[__c])[(int)__c])
>> +
>> +#define isalpha(__c) (__ctype_lookup(__c)&(_U|_L))
>> +#define isupper(__c) ((__ctype_lookup(__c)&(_U|_L))==_U)
>> +#define islower(__c) ((__ctype_lookup(__c)&(_U|_L))==_L)
>> +#define isdigit(__c) (__ctype_lookup(__c)&_N)
>> +#define isxdigit(__c) (__ctype_lookup(__c)&(_X|_N))
>> +#define isspace(__c) (__ctype_lookup(__c)&_S)
>> +#define ispunct(__c) (__ctype_lookup(__c)&_P)
>> +#define isalnum(__c) (__ctype_lookup(__c)&(_U|_L|_N))
>> +#define isprint(__c) (__ctype_lookup(__c)&(_P|_U|_L|_N|_B))
>> +#define isgraph(__c) (__ctype_lookup(__c)&(_P|_U|_L|_N))
>> +#define iscntrl(__c) (__ctype_lookup(__c)&_C)
>>
>> #if defined(__GNUC__)&& \
>> (!defined(__STRICT_ANSI__) || __STDC_VERSION__>= 199901L)
>> #define isblank(__c) \
>> __extension__ ({ __typeof__ (__c) __x = (__c); \
>> - ((__ctype_ptr__+1)[__x]&_B) || (__x) == '\t';})
>> + (__ctype_lookup(__x)&_B) || (int) (__x) == '\t';})
>> #endif

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ctype.h.patch
Type: application/x-itunes-itlp
Size: 578 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/newlib/attachments/20091106/a2d9c83b/attachment.bin>


More information about the Newlib mailing list