From patchwork Fri Dec 18 20:25:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Eggert X-Patchwork-Id: 41481 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 28C003939C1D; Fri, 18 Dec 2020 20:25:31 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from zimbra.cs.ucla.edu (zimbra.cs.ucla.edu [131.179.128.68]) by sourceware.org (Postfix) with ESMTPS id BD26A3854813 for ; Fri, 18 Dec 2020 20:25:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BD26A3854813 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=cs.ucla.edu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=eggert@cs.ucla.edu Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id E66EA160095; Fri, 18 Dec 2020 12:25:26 -0800 (PST) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id WyCVR5_u_xMD; Fri, 18 Dec 2020 12:25:24 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id CEBA5160106; Fri, 18 Dec 2020 12:25:24 -0800 (PST) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 4LWh41kB0nYc; Fri, 18 Dec 2020 12:25:24 -0800 (PST) Received: from [192.168.1.9] (cpe-23-243-218-95.socal.res.rr.com [23.243.218.95]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 8F2F3160095; Fri, 18 Dec 2020 12:25:24 -0800 (PST) To: GNU C Library From: Paul Eggert Organization: UCLA Computer Science Department Subject: [PATCH] glibc __builtin_add_overflow problem with GCC < 7 Message-ID: <8ff7de3a-690b-3213-16df-be479cdccae3@cs.ucla.edu> Date: Fri, 18 Dec 2020 12:25:24 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 MIME-Version: 1.0 Content-Language: en-US X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_NUMSUBJECT, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Stefan Liebler , Bruno Haible Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Stefan Liebler's recent glibc commit "s390x: Require GCC 7.1 or later to build glibc" is motivated by a bug with __builtin_add_overflow that he encountered on s390x. However, it appears that the underlying GCC bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98269 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78559 is platform-independent (I am not sure of this, and am being conservative). In Gnulib, Bruno Haible worked around the bug today by updating intprops.h so that INT_ADD_WRAPV does not use __builtin_add_overflow in older GCCs. When Gnulib intprops.h is copied to glibc this should fix glibc modules like posix/regexec.c and time/mktime.c that are shared with Gnulib and use intprops.h and INT_ADD_WRAPV etc. However, several glibc modules (e.g., malloc/malloc.c) are not shared with Gnulib and use __builtin_add_overflow directly, so they won't benefit from the intprops.h fix. A simple workaround would be to change INSTALL to recommend GCC 7.1 or higher, as in the first attached patch (configure and sysdeps/s390/configure files would also need to be regenerated). This patch also rejects icc, as icc 2021.1 pretends to be GCC 10 but doesn't do proper integer overflow checking. Alternatively, if requiring GCC 7.1 is too drastic, glibc could change all direct uses of __builtin_add_overflow etc. to use intprops.h and INT_ADD_WRAPV etc., as Adhemerval suggested recently . The first step in this would be to update intprops.h from Gnulib (which should be routine anyway), as in the second attached patch. We'd also need to change malloc/malloc.c etc. From c31fe81a71ba7e0dfb13ceeb602b453c73cd7384 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 18 Dec 2020 12:10:16 -0800 Subject: [PATCH] Sync intprops.h from Gnulib * include/intprops.h: Sync from Gnulib. This fixes a bug with INT_ADD_WRAPV and GCC < 7. --- include/intprops.h | 70 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/include/intprops.h b/include/intprops.h index 6de65b067d..52e60e5e2d 100644 --- a/include/intprops.h +++ b/include/intprops.h @@ -48,7 +48,7 @@ /* Minimum and maximum values for integer types and expressions. */ /* The width in bits of the integer type or expression T. - Do not evaluate T. + Do not evaluate T. T must not be a bit-field expression. Padding bits are not supported; this is checked at compile-time below. */ #define TYPE_WIDTH(t) (sizeof (t) * CHAR_BIT) @@ -70,7 +70,7 @@ ? _GL_SIGNED_INT_MAXIMUM (e) \ : _GL_INT_NEGATE_CONVERT (e, 1)) #define _GL_SIGNED_INT_MAXIMUM(e) \ - (((_GL_INT_CONVERT (e, 1) << (TYPE_WIDTH ((e) + 0) - 2)) - 1) * 2 + 1) + (((_GL_INT_CONVERT (e, 1) << (TYPE_WIDTH (+ (e)) - 2)) - 1) * 2 + 1) /* Work around OpenVMS incompatibility with C99. */ #if !defined LLONG_MAX && defined __INT64_MAX @@ -86,6 +86,7 @@ /* Does the __typeof__ keyword work? This could be done by 'configure', but for now it's easier to do it by hand. */ #if (2 <= __GNUC__ \ + || (4 <= __clang_major__) \ || (1210 <= __IBMC__ && defined __IBM__TYPEOF__) \ || (0x5110 <= __SUNPRO_C && !__STDC__)) # define _GL_HAVE___TYPEOF__ 1 @@ -94,8 +95,9 @@ #endif /* Return 1 if the integer type or expression T might be signed. Return 0 - if it is definitely unsigned. This macro does not evaluate its argument, - and expands to an integer constant expression. */ + if it is definitely unsigned. T must not be a bit-field expression. + This macro does not evaluate its argument, and expands to an + integer constant expression. */ #if _GL_HAVE___TYPEOF__ # define _GL_SIGNED_TYPE_OR_EXPR(t) TYPE_SIGNED (__typeof__ (t)) #else @@ -108,6 +110,8 @@ #define INT_BITS_STRLEN_BOUND(b) (((b) * 146 + 484) / 485) /* Bound on length of the string representing an integer type or expression T. + T must not be a bit-field expression. + Subtract 1 for the sign bit if T is signed, and then add 1 more for a minus sign if needed. @@ -119,7 +123,7 @@ + _GL_SIGNED_TYPE_OR_EXPR (t)) /* Bound on buffer size needed to represent an integer type or expression T, - including the terminating null. */ + including the terminating null. T must not be a bit-field expression. */ #define INT_BUFSIZE_BOUND(t) (INT_STRLEN_BOUND (t) + 1) @@ -222,7 +226,9 @@ /* True if __builtin_add_overflow (A, B, P) and __builtin_sub_overflow (A, B, P) work when P is non-null. */ -#if 5 <= __GNUC__ && !defined __ICC +/* __builtin_{add,sub}_overflow exists but is not reliable in GCC 5.x and 6.x, + see . */ +#if 7 <= __GNUC__ && !defined __ICC # define _GL_HAS_BUILTIN_ADD_OVERFLOW 1 #elif defined __has_builtin # define _GL_HAS_BUILTIN_ADD_OVERFLOW __has_builtin (__builtin_add_overflow) @@ -239,8 +245,18 @@ #endif /* True if __builtin_add_overflow_p (A, B, C) works, and similarly for - __builtin_mul_overflow_p and __builtin_mul_overflow_p. */ -#define _GL_HAS_BUILTIN_OVERFLOW_P (7 <= __GNUC__) + __builtin_sub_overflow_p and __builtin_mul_overflow_p. */ +#if defined __clang__ || defined __ICC +/* Clang 11 lacks __builtin_mul_overflow_p, and even if it did it + would presumably run afoul of Clang bug 16404. ICC 2021.1's + __builtin_add_overflow_p etc. are not treated as integral constant + expressions even when all arguments are. */ +# define _GL_HAS_BUILTIN_OVERFLOW_P 0 +#elif defined __has_builtin +# define _GL_HAS_BUILTIN_OVERFLOW_P __has_builtin (__builtin_mul_overflow_p) +#else +# define _GL_HAS_BUILTIN_OVERFLOW_P (7 <= __GNUC__) +#endif /* The _GL*_OVERFLOW macros have the same restrictions as the *_RANGE_OVERFLOW macros, except that they do not assume that operands @@ -373,8 +389,9 @@ _GL_INT_OP_WRAPV (a, b, r, -, _GL_INT_SUBTRACT_RANGE_OVERFLOW) #endif #if _GL_HAS_BUILTIN_MUL_OVERFLOW -# if (9 < __GNUC__ + (3 <= __GNUC_MINOR__) \ - || (__GNUC__ == 8 && 4 <= __GNUC_MINOR__)) +# if ((9 < __GNUC__ + (3 <= __GNUC_MINOR__) \ + || (__GNUC__ == 8 && 4 <= __GNUC_MINOR__)) \ + && !defined __ICC) # define INT_MULTIPLY_WRAPV(a, b, r) __builtin_mul_overflow (a, b, r) # else /* Work around GCC bug 91450. */ @@ -395,7 +412,7 @@ For now, assume all versions of GCC-like compilers generate bogus warnings for _Generic. This matters only for compilers that lack relevant builtins. */ -#if __GNUC__ +#if __GNUC__ || defined __clang__ # define _GL__GENERIC_BOGUS 1 #else # define _GL__GENERIC_BOGUS 0 @@ -565,7 +582,7 @@ ? (EXPR_SIGNED (_GL_INT_CONVERT (tmax, b)) \ ? (a) < (tmax) / (b) \ : ((INT_NEGATE_OVERFLOW (b) \ - ? _GL_INT_CONVERT (b, tmax) >> (TYPE_WIDTH (b) - 1) \ + ? _GL_INT_CONVERT (b, tmax) >> (TYPE_WIDTH (+ (b)) - 1) \ : (tmax) / -(b)) \ <= -1 - (a))) \ : INT_NEGATE_OVERFLOW (_GL_INT_CONVERT (b, tmin)) && (b) == -1 \ @@ -581,4 +598,33 @@ : (tmin) / (a) < (b)) \ : (tmax) / (b) < (a))) +/* The following macros compute A + B, A - B, and A * B, respectively. + If no overflow occurs, they set *R to the result and return 1; + otherwise, they return 0 and may modify *R. + + Example usage: + + long int result; + if (INT_ADD_OK (a, b, &result)) + printf ("result is %ld\n", result); + else + printf ("overflow\n"); + + A, B, and *R should be integers; they need not be the same type, + and they need not be all signed or all unsigned. + + These macros work correctly on all known practical hosts, and do not rely + on undefined behavior due to signed arithmetic overflow. + + These macros are not constant expressions. + + These macros may evaluate their arguments zero or multiple times, so the + arguments should not have side effects. + + These macros are tuned for B being a constant. */ + +#define INT_ADD_OK(a, b, r) ! INT_ADD_WRAPV (a, b, r) +#define INT_SUBTRACT_OK(a, b, r) ! INT_SUBTRACT_WRAPV (a, b, r) +#define INT_MULTIPLY_OK(a, b, r) ! INT_MULTIPLY_WRAPV (a, b, r) + #endif /* _GL_INTPROPS_H */ -- 2.29.2