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

[Jon TURNEY <jon dot turney at dronecode dot org dot uk>]

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

Attachment: ctype.h.patch
Description: application/itunes-itlp