bug-gnulib
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]