NetBSD Problem Report #48131

From gregoire.sutre@gmail.com  Fri Aug 16 20:02:42 2013
Return-Path: <gregoire.sutre@gmail.com>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id C203171309
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 16 Aug 2013 20:02:42 +0000 (UTC)
Message-Id: <20130816200230.3BCAA705C1@yosemite.localdomain>
Date: Fri, 16 Aug 2013 22:02:30 +0200 (CEST)
From: gregoire.sutre@gmail.com
Reply-To: gregoire.sutre@gmail.com
To: gnats-bugs@NetBSD.org
Subject: __type_fit: bug and floating-point requirement
X-Send-Pr-Version: 3.95

>Number:         48131
>Category:       lib
>Synopsis:       __type_fit: bug and floating-point requirement
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Aug 16 20:05:00 +0000 2013
>Closed-Date:    Thu Sep 05 21:57:45 +0000 2013
>Last-Modified:  Tue Sep 10 12:55: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'

>Release-Note:

>Audit-Trail:
From: =?UTF-8?B?R3LDqWdvaXJlIFN1dHJl?= <gsutre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/48131 CVS commit: src/sys/sys
Date: Thu, 5 Sep 2013 09:03:13 +0000

 Module Name:	src
 Committed By:	gsutre
 Date:		Thu Sep  5 09:03:13 UTC 2013

 Modified Files:
 	src/sys/sys: cdefs.h

 Log Message:
 Check for overflow in __type_fit_s when casting to intmax_t.
 Fixes the first half of PR lib/48131.

 ok christos@


 To generate a diff of this commit:
 cvs rdiff -u -r1.107 -r1.108 src/sys/sys/cdefs.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: =?UTF-8?B?R3LDqWdvaXJlIFN1dHJl?= <gsutre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/48131 CVS commit: src/sys/sys
Date: Thu, 5 Sep 2013 21:00:15 +0000

 Module Name:	src
 Committed By:	gsutre
 Date:		Thu Sep  5 21:00:15 UTC 2013

 Modified Files:
 	src/sys/sys: cdefs.h

 Log Message:
 Implement __negative_p without floating-point arithmetic, using
 a solution proposed by jxh on Stack Overflow.  Fixes the second
 half of PR lib/48131.

 While there, simplify __type_fit_u by using the same logic
 as in __type_fit_s.

 ok christos@


 To generate a diff of this commit:
 cvs rdiff -u -r1.108 -r1.109 src/sys/sys/cdefs.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: gsutre@NetBSD.org
State-Changed-When: Thu, 05 Sep 2013 21:57:45 +0000
State-Changed-Why:
Fixed.


From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, lib-bug-people@netbsd.org, 
	netbsd-bugs@netbsd.org, gnats-admin@netbsd.org, gsutre@NetBSD.org, 
	gregoire.sutre@gmail.com
Cc: 
Subject: Re: lib/48131 (__type_fit: bug and floating-point requirement)
Date: Mon, 9 Sep 2013 17:06:49 -0400

 On Sep 5,  9:57pm, gsutre@NetBSD.org (gsutre@NetBSD.org) wrote:
 -- Subject: Re: lib/48131 (__type_fit: bug and floating-point requirement)

 | Synopsis: __type_fit: bug and floating-point requirement
 | 
 | State-Changed-From-To: open->closed
 | State-Changed-By: gsutre@NetBSD.org
 | State-Changed-When: Thu, 05 Sep 2013 21:57:45 +0000
 | State-Changed-Why:
 | Fixed.

 Hmm....

 /home/builds/ab/HEAD/src/lib/libc/gen/scandir.c: In function '_scandir':
 /home/builds/ab/HEAD/src/lib/libc/gen/scandir.c:142: warning: comparison is always true due to limited range of data type
 /home/builds/ab/HEAD/src/lib/libc/gen/scandir.c:142: warning: comparison is always true due to limited range of data type
 /home/builds/ab/HEAD/src/lib/libc/gen/scandir.c:142: warning: comparison is always true due to limited range of data type

 Seems gcc4.1 does not like it.

 christos

From: =?UTF-8?B?R3LDqWdvaXJlIFN1dHJl?= <gsutre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/48131 CVS commit: src/sys/sys
Date: Tue, 10 Sep 2013 12:54:14 +0000

 Module Name:	src
 Committed By:	gsutre
 Date:		Tue Sep 10 12:54:14 UTC 2013

 Modified Files:
 	src/sys/sys: cdefs.h

 Log Message:
 Unbreak vax build (which still uses gcc 4.1).  See PR lib/48131.


 To generate a diff of this commit:
 cvs rdiff -u -r1.109 -r1.110 src/sys/sys/cdefs.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search