[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: ctype functions with char argument
From: |
Bruno Haible |
Subject: |
Re: ctype functions with char argument |
Date: |
Mon, 19 Oct 2009 01:36:57 +0200 |
User-agent: |
KMail/1.9.9 |
Hi Eric,
> Can we go ahead and make the replacement ctype.h catch additional bugs, by
> rewriting the various ctype macros to cause gcc -Wall warnings when passed
> a char (rather than int or unsigned char)?
Gnulib is not the right place for implementing this kind of warnings:
1. because the number of users of gnulib is small, compared to the number
of users of gcc + glibc,
2. because producing warnings for standard C library function calls
is logically in the business of the compiler and C library.
> Newlib has already made the
> change, and it has caught several real bugs in existing code, but my plea
> to get glibc to improve QoI has, so far, fallen on deaf ears:
> http://sources.redhat.com/bugzilla/show_bug.cgi?id=10296
Newlib's macroexpansion is wrong for 64-bit platforms: When
sizeof (int) < sizeof (ptrdiff_t), then when a user invokes
isalpha (0x100000041L)
the result *must* be the same as
isalpha ((int) 0x100000041L) = isalpha (0x41L) = nonzero
(because the standard says that 'isalpha' is a function that takes an
'int' parameter),
whereas newlib macroexpands it to
((__ctype_ptr__+1)[0x100000041L]&(01|02))
which yields an out-of-range memory access.
What you need, IMO, in order to fix this bug, is a primitive for warning
if an expression is of type 'char' and signed. Fortunately GCC already
provides this primitive: the array access.
Try this:
============================== foo.c ===================================
extern int *__ctype_ptr__;
/* Incorrect when c = 0x100000041L on 64-bit platforms */
#if 0
#define isalpha(c) ((__ctype_ptr__+1)[c]&(01|02))
#endif
/* Correct but evaluates c twice */
#if 0
#define isalpha(c) ((void)(0 ? (__ctype_ptr__+1)[c] : 0),
(__ctype_ptr__+1)[(int)(c)]&(01|02))
#endif
/* Correct and evaluates c only once */
#define isalpha(c) ((void)sizeof((__ctype_ptr__+1)[c]),
(__ctype_ptr__+1)[(int)(c)]&(01|02))
int
main()
{
char c = -2;
unsigned char uc = -2;
c = isalpha (c);
uc = isalpha (uc);
return c + uc;
}
===========================================================================
So, this kind of warning can be enabled in newlib or glibc headers.
Bruno