lib/48131: __type_fit: bug and floating-point requirement

[gregoire.sutre%gmail.com@localhost]

>Number:         48131
>Category:       lib
>Synopsis:       __type_fit: bug and floating-point requirement
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Aug 16 20:05:00 +0000 2013
>Originator:     Grégoire Sutre
>Release:        NetBSD 6.99.23 (201308140140Z)
>Organization:
>Environment:
System: NetBSD yosemite 6.99.23 NetBSD 6.99.23 (GENERIC) #0: Thu Aug 15 
00:59:23 CEST 2013 
instsoft@yosemite:/tmp/netbsd-drmgem.usr/src/sys/arch/amd64/compile/GENERIC 
amd64
Architecture: x86_64
Machine: amd64
>Description:
This report addresses two problems with the macro __type_fix that
is defined /usr/include/sys/cdefs.h.

Firstly, on my NetBSD/amd64 system, for a signed integer type t, the
macro __type_fit(t, x) evaluates to 1 when x > LONG_MAX.  See below
for an example.

Secondly, the macro __type_fit relies on the inline function

static __inline int __negative_p(double x) { return x < 0; }

This function definition introduces a dependency on a floating-point
data type in many (most?) system headers.  This breaks programs that
explicitly require integer-only data types, such as GRUB (current
development version). See:

http://lists.gnu.org/archive/html/grub-devel/2013-08/msg00048.html
>How-To-Repeat:
As an example, consider the C program:

#include <sys/types.h>
#include <stdint.h>
#include <stdio.h>
#include <limits.h>

int main()
{
        uintmax_t x = (uintmax_t)LONG_MAX+1;
        printf("fit(int, %jx) : %d\n", x, __type_fit(int, x));
        return 0;
}

Running this program gives fit(int, 8000000000000000) : 1
>Fix:
The attached patch attempts to fix both problems.  However, there is
a build failure caused by lint with the patch.  I believe that this
is because lint does not understand __typeof__.

The first part of the patch fixes the issue with the double data type.
This fix was suggested by Vladimir Serbinenko.

The second part fixes __type_fit(t, x) when x is positive by checking
that its sign bit is not set.  There is a probably a better way to do
that.

Index: cdefs.h
===================================================================
RCS file: /cvsroot/src/sys/sys/cdefs.h,v
retrieving revision 1.107
diff -u -r1.107 cdefs.h
--- cdefs.h     29 May 2013 19:02:30 -0000      1.107
+++ cdefs.h     16 Aug 2013 16:57:10 -0000
@@ -555,12 +555,13 @@
 
 #ifndef __ASSEMBLER__
 static __inline long long __zeroll(void) { return 0; }
-static __inline int __negative_p(double x) { return x < 0; }
 #else
 #define __zeroll() (0LL)
-#define __negative_p(x) ((x) < 0)
 #endif
 
+#define __negative_p(x) (__type_is_signed(__typeof__(x)) && \
+    ((x) & (1ULL << (sizeof(x) * NBBY - 1))))
+
 #define __type_min_s(t) ((t)((1ULL << (sizeof(t) * NBBY - 1))))
 #define __type_max_s(t) ((t)~((1ULL << (sizeof(t) * NBBY - 1))))
 #define __type_min_u(t) ((t)0ULL)
@@ -575,7 +576,8 @@
 
 #define __type_fit_s(t, a) (/*LINTED*/__negative_p(a) ? \
     ((intmax_t)((a) + __zeroll()) >= (intmax_t)__type_min_s(t)) : \
-    ((intmax_t)((a) + __zeroll()) <= (intmax_t)__type_max_s(t)))
+    (((a) & (1ULL << (sizeof(intmax_t) * NBBY - 1))) == 0 && \
+     (intmax_t)((a) + __zeroll()) <= (intmax_t)__type_max_s(t)))
 
 /*
  * return true if value 'a' fits in type 't'