[Bug libc/10296] New: QoI: ctype macros should trigger gcc -Wall warnings on 'char' argument

[Bug libc/10296] New: QoI: ctype macros should trigger gcc -Wall warnings on 'char' argument

[ebb9 at byu dot net]

Consider this program:

#include <ctype.h>
int
main()
{
  char c = -2;
  unsigned char uc = -2;
  int table[2] = {0};
  c = isalpha (c);
  uc = isalpha (uc);
  c = table[c];
  uc = table[uc];
  return c + uc;
}

Depending on whether 'char' is a signed type, line 8 potentially violates the
standard, by passing an argument to isalpha that is outside the range of EOF or
unsigned char.  As a QoI point, glibc already handles this situation, by
allowing negative offsets (for all 'char' except -1, since that collides with
EOF) into the table lookup provided under the hood of the ctype macros.

But the QoI should be taken one step further.  It would be nice if <ctype.h>
were rewritten in such a manner as to warn the user that their program is not
portable.  For example, contrast compilation of the above program when using
newlib headers:
$ gcc -o foo -Wall foo.c
foo.c: In function `main':
foo.c:8: warning: subscript has type `char'
foo.c:10: warning: array subscript has type `char'

vs. when using glibc headers:
% gcc -o foo -Wall foo.c
foo.c: In function `main':
foo.c:10: warning: array subscript has type `char'

The difference? Look at the preprocessed results for newlib:
$ gcc -E foo.c | grep -C1 table
  unsigned char uc = -2;
  int table[2] = {0};
  c = ((__ctype_ptr__+1)[c]&(01|02));
  uc = ((__ctype_ptr__+1)[uc]&(01|02));
  c = table[c];
  uc = table[uc];
  return c + uc;

vs. for glibc (this from a machine with glibc 2.3.4, but even glibc 2.10 still
has a similar expansion)
% gcc -E foo.c | grep -C1 table
  unsigned char uc = -2;
  int table[2] = {0};
  c = ((*__ctype_b_loc ())[(int) ((c))] & (unsigned short int) _ISalpha);
  uc = ((*__ctype_b_loc ())[(int) ((uc))] & (unsigned short int) _ISalpha);
  c = table[c];
  uc = table[uc];
  return c + uc;

The difference is that newlib INTENTIONALLY used "c" as the array index, rather
than "(int) ((c))", since it is only by using a raw char as an index that you
can trigger the gcc -Wall warning.  All glibc has to do to help users find their
buggy uses of the ctype macros is to remove the spurious cast to int in the
__isctype macro of ctype/ctype.h.

-- 
           Summary: QoI: ctype macros should trigger gcc -Wall warnings on
                    'char' argument
           Product: glibc
           Version: 2.10
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: drepper at redhat dot com
        ReportedBy: ebb9 at byu dot net
                CC: glibc-bugs at sources dot redhat dot com


http://sourceware.org/bugzilla/show_bug.cgi?id=10296

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/10296] QoI: ctype macros should trigger gcc -Wall warnings on 'char' argument

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2009-10-30 07:32 -------
Then come up with a portable patch.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |SUSPENDED


http://sourceware.org/bugzilla/show_bug.cgi?id=10296

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/10296] QoI: ctype macros should trigger gcc -Wall warnings on 'char' argument

[ebb9 at byu dot net]

------- Additional Comments From ebb9 at byu dot net  2009-10-31 19:53 -------
Created an attachment (id=4347)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4347&action=view)
The requested patch.

This should do it.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=10296

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/10296] QoI: ctype macros should trigger gcc -Wall warnings on 'char' argument

[ebb9 at byu dot net]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|SUSPENDED                   |WAITING


http://sourceware.org/bugzilla/show_bug.cgi?id=10296

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/10296] QoI: ctype macros should trigger gcc -Wall warnings on 'char' argument

[pasky at suse dot cz]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW
   Last reconfirmed|0000-00-00 00:00:00         |2010-06-01 02:57:53
               date|                            |


http://sourceware.org/bugzilla/show_bug.cgi?id=10296

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/10296] QoI: ctype macros should trigger gcc -Wall warnings on 'char' argument

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2010-07-09 22:33 -------
Looking more at this, I won't change anything.

First of all, nothing what you describe is what current gcc versions for Linux
do.  You do get warnings:

foo.c:8: warning: overflow in implicit constant conversion
foo.c:9: warning: overflow in implicit constant conversion

Second, with this "improvement" perfectly correct code will get warnings. 
Almost all the code out there uses char* for strings and with the change all
situations where a character of a string is passed to an is* or to* function
will provoke a warning.

This is perfectly valid code.  The is* and to* interfaces are not required to be
macros and they take int parameters.  Just like in all similar situations the
parameters of type char are propagated to int before being used.  And this is
what the code does.  A bit more efficient than other solutions but the effect is
the same.

I won't make this "QoI change" (which it really isn't) because the positives are
by far outweighed by the negative, the countless of programs with correct code
which would provoke warnings.  And all this not because the code would be
executed incorrectly.  No, only because there are other lousy implementations
this might be a problem.  No way.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |WONTFIX


http://sourceware.org/bugzilla/show_bug.cgi?id=10296

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.