bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: checked integer arithmetic


From: Bruno Haible
Subject: Re: checked integer arithmetic
Date: Fri, 16 Dec 2016 00:55:09 +0100
User-agent: KMail/4.8.5 (Linux/3.8.0-44-generic; KDE/4.8.5; x86_64; ; )

Hi Paul,

> I installed the patch I proposed yesterday, along with the 
> additional patches attached, which merely change the x* functions to 
> check for both kinds of overflow.

These changes give me some stomach-ache. I perfectly understand that
you're making a departure from 20 years of C tradition, to get overflows
detected better. I see three issues:

1) You're basically saying "let's use signed integer types for indices",
and you do that in the quotearg.c change.

1.1) The declaration 'unsigned int i;' served also as an information for
the programmer: "Attention, never assign negative values to this variable!"
Changing that to "int i;" hampers maintainability.
This could be solved through a typedef
  typedef int index_t; /* keep always >= 0 */
but such a typedef doesn't solve 1.2).

1.2) Essentially, the only remaining use of 'unsigned int' is for packed
bit arrays and bit masks, and for multi-precision arithmetic. But as C
programmer I do want to have integer variables which are always >= 0,
<= PTRDIFF_MAX, <= SIZE_MAX, _and_ I would like to have them checked
against overflow.

For this purpose, it would be good if GCC had a type, say, __gcc_index_t,
that -fsanitize=undefined will make produce a diagnostic is a value < 0
or > PTRDIFF_MAX is assigned.

2) The type __xalloc_count_type is sometimes signed, sometimes unsigned,
depending on platform (like 'char' and 'wchar_t').
Such types produce extra trouble:
  * Some bugs can occur only one kind of platform and cannot be reproduced
    even with the best intrumentation (-fsanitize=undefined, valgrind, etc.)
    on the other kind of platforms.
  * Code which is necessary for one platform produces warnings on the
    other platforms. Such as:
      wchar_t wc = ...;
      if (wc >= 0x0000 && wc < 0x110000)
    This produces a warning on platforms where wchar_t is unsigned.

3) The type defined in xalloc-oversized.h has much wider applicability:

   #if PTRDIFF_MAX < SIZE_MAX
   typedef ptrdiff_t __xalloc_count_type;
   #else
   typedef size_t __xalloc_count_type;
   #endif

   It becomes one of the basic C types and should therefore deserve a
   name with wider scope. Possibly 'gl_index_t' or such.

Bruno




reply via email to

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