From owner-freebsd-hackers@freebsd.org Mon Jan 18 11:47:15 2021 Return-Path: Delivered-To: freebsd-hackers@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 7218E4E86F6 for ; Mon, 18 Jan 2021 11:47:15 +0000 (UTC) (envelope-from "") Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.141]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4DK94f3nglz3pJ3 for ; Mon, 18 Jan 2021 11:47:14 +0000 (UTC) (envelope-from "") Received: from submission (posteo.de [89.146.220.130]) by mout01.posteo.de (Postfix) with ESMTPS id 0698716005C for ; Mon, 18 Jan 2021 12:47:11 +0100 (CET) Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4DK94Z4Cydz6tm6; Mon, 18 Jan 2021 12:47:10 +0100 (CET) From: Walter von Entferndt To: Mark Millard Cc: freebsd-hackers@freebsd.org Subject: Re: Implicit assumptions (was: Re: Some fun with -O2) Date: Mon, 18 Jan 2021 12:46:58 +0100 Message-ID: <4907874.QyqR4RcxXA@t450s.local.lan> In-Reply-To: References: <22093525.sr7ieKrsik@t450s.local.lan> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart3148181.qzLpRxe9IO" Content-Transfer-Encoding: 7Bit X-Rspamd-Queue-Id: 4DK94f3nglz3pJ3 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=fail reason="No valid SPF, No valid DKIM" header.from=posteo.net (policy=none); spf=none (mx1.freebsd.org: domain of mout01.posteo.de has no SPF policy when checking 185.67.36.141) smtp.helo=mout01.posteo.de X-Spamd-Result: default: False [-2.70 / 15.00]; RCVD_TLS_ALL(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_LONG(-1.00)[-1.000]; HAS_ATTACHMENT(0.00)[]; MIME_GOOD(-0.10)[multipart/mixed,text/plain,text/x-patch]; PREVIOUSLY_DELIVERED(0.00)[freebsd-hackers@freebsd.org]; RCVD_COUNT_THREE(0.00)[3]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCVD_IN_DNSWL_MED(-0.20)[185.67.36.141:from]; RCPT_COUNT_TWO(0.00)[2]; NEURAL_HAM_SHORT(-1.00)[-1.000]; R_SPF_NA(0.00)[no SPF record]; FREEMAIL_TO(0.00)[yahoo.com]; R_DKIM_NA(0.00)[]; CTE_CASE(0.50)[]; ASN(0.00)[asn:8495, ipnet:185.67.36.0/23, country:DE]; MIME_TRACE(0.00)[0:+,1:+,2:+]; MAILMAN_DEST(0.00)[freebsd-hackers]; DMARC_POLICY_SOFTFAIL(0.10)[posteo.net : No valid SPF, No valid DKIM,none] X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jan 2021 11:47:15 -0000 This is a multi-part message in MIME format. --nextPart3148181.qzLpRxe9IO Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Now to test the above solution (and out of curiosity), I tried it with *time_t* changed to *signed char* in /guess_time_t_max()/ and it gave: DEBUG: prec(signed_char) = 8 schar_t_max = 0xFFFFFFFFFFFFFFFF sizeof(schar_t) = 1 byte precision = 63 bit padding = -56 bit bits per byte = 8 bit Exitcode 1 which is from the check I added at the start of /main()/ (0xFF = -1, anyway it would fall into an endless loop because the delta gets =0 then), while with *signed int* instead of *time_t* it gives: DEBUG: prec(signed int) = 32 xxx_t_max = 0x7FFFFFFF sizeof(xxx_t) = 4 byte precision = 63 bit padding = -32 bit bits per byte = 8 bit and the program runs ok (I have set /printexitvalue/ in my tcsh). It does not concern me that the program fails if *sizeof(time_t)* = 1, but I didn't expect these other curiosities. Now I went back to a simple switch-on-storage-width, that I believe will cover most (all?) cases except when *time_t* is a floating point. Rationale: the standard says *time_t* is either a floating point or integer type, and from that I conclude it's equivalent to one of the standard types. I tested with time_t faked beeing 1 (char), 2 (short), 4 (int) and 8 (standard on my 64-bit Intel). Of course with a 1-byte *time_t* it falls into the endless loop as expected. At Montag, 18. Januar 2021, 00:31:46 CET, Mark Millard wrote: > On 2021-Jan-17, at 13:25, Walter von Entferndt wrote: > > . . . > > - for (j = 1; 0 < j; j *= 2) > > + /* NOTE This does not reach very large values > time_t_max/2 > > + neither the (susceptible for overflow) version > > + for (j = 1; 0 < j; j *= 2) does */ > > + for (i = j = 1; i < PRECISION(INT_MAX); i++, j *= 2) > > I'm unclear on why INT_MAX here. > Because i and j are *int* s. My intent was to avoid sign overflow, which can happen silently (j *= 2), but the standard says it's undefined, and that was the starting point of this thread. The genuine version relies on silent overflow, which is sub-optimal. The above version is (intentionally) not equivalent to the genuine one. Instead this would do what the author intended (from my understanding), but without any chance for overflow: for (i = 0; i < PRECISION (INT_MAX); i++) { j = (int) 1 << i; bigtime_test (j); } bigtime_test (INT_MAX); } The point is that on sign overflow j switches to INT_MIN, and the author expects the difference INT_MIN - 1 = INT_MAX, which is reasonable, but mathematically bogus, as well as j *= 2 will mathematically always be >0 as long you start with a positive value. And today's clever optimizing compilers do what you write, not what you intend; e.g. produce an endless loop when you write an expression that it can prove to be always true as the loop condition. > It leaves me to wonder about using something that > invovled PRECISION(time_t_max). > You can't do that as long you don't know if it's really the maximum value. In the code above (i < PRECISION (INT_MAX)): to account for any padding bits. To be portable. > > +/*! vi: set ai tabstop=8 shiftwidth=2: */ When fixing code, I think it's polite to adopt the genuine formating. Since in this case it's rather unusual, I'm telling my editor to use sw=2 (I use 4). -- =|o) "Stell' Dir vor es geht und keiner kriegt's hin." (Wolfgang Neuss) --nextPart3148181.qzLpRxe9IO Content-Disposition: attachment; filename="check_mktime.c.patch" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="check_mktime.c.patch" --- check_mktime.c.orig 2021-01-15 03:19:33.962253000 +0100 +++ check_mktime.c 2021-01-18 12:33:10.068968000 +0100 @@ -3,6 +3,11 @@ # include # include # include +# include /* putenv(), exit(), EXIT_xxx */ +# include /* intmax_t */ +# include /* printf() */ +# include /* CHAR_BIT, xxx_MAX */ +# include /* format spec PRIX64: ll/l + X on 32/64-bit arch */ /* Work around redefinition to rpl_putenv by other config tests. */ #undef putenv @@ -16,6 +21,97 @@ }; #define N_STRINGS (sizeof (tz_strings) / sizeof (tz_strings[0])) +/* The following two routines (should) work for any integer representation + in either 2's or 1's complement and for any #bits per byte. */ + +/* Count the bits set in any unsigned integer type. + Returns the precision (width - padding bits - sign bit) iff given + the xxx_MAX value of any integer type, signed or unsigned. + From SEI CERT C Coding Standard: + Rules for Developing Safe, Reliable, and Secure Systems (2016) */ +size_t +popcount (num) +uintmax_t num; +{ + size_t cnt = 0; + + while (num != 0) + { + if (num % 2 == 1) + cnt++; + num >>= 1; + } + return cnt; +} +#define PRECISION(max_value) popcount(max_value) +#define SIGN_BIT (1) + +/* Get the maximum value of a signed integer type from it's storage width. + Parameters: + size_t size: storage width in bytes as given by the sizeof operator. + On error: + Returns (intmax_t)(-1) iff xxxint_t is longer than an intmax_t. + ASSERTIONS the type is equivalent to one of the C standard integer types. + I.e. if it's 6 bytes wide, but no standard type is 6 bytes, we're out. + NOTE the maximum of any unsigned integer type can be derived by + umax = (-max) - 1 on 2's-complement machines; on 1's-complement it's simply + simply umax = -max. Test for 1's complement: (-1)|1 == (-0) */ +intmax_t +get_xxxint_t_max (size) +size_t size; +{ + size_t prec = 0; + intmax_t max = (intmax_t)(-1); + + if (size > sizeof(intmax_t)) + prec = 0; + else if (size == sizeof(intmax_t)) + { + prec = PRECISION (INTMAX_MAX); + max = (intmax_t) INTMAX_MAX; + } +#ifdef __LONG_LONG_SUPPORTED + else if (size == sizeof(long long)) + { + prec = PRECISION (LLONG_MAX); + max = (intmax_t) LLONG_MAX; + } +#endif /* __LONG_LONG_SUPPORTED */ + else if (size == sizeof(long)) + { + prec = PRECISION (LONG_MAX); + max= (intmax_t) LONG_MAX; + } + else if (size == sizeof(int)) + { + prec = PRECISION (INT_MAX); + max = (intmax_t) INT_MAX; + } + else if (size == sizeof(short)) + { + prec = PRECISION (SHRT_MAX); + max = (intmax_t) SHRT_MAX; + } + else if (size == sizeof(signed char)) + { + prec = PRECISION (SCHAR_MAX); + max = (intmax_t) SCHAR_MAX; + } + else + /* out of options... */ + prec = 0; + +#ifdef DEBUG + fprintf (stderr, "xxx_t_max\t= 0x%"PRIX64"\n", (uint64_t) max); + fprintf (stderr, "sizeof(xxx_t)\t= %3zd byte\n", size); + fprintf (stderr, "precision\t= %3zd bit\n", prec); + fprintf (stderr, "padding\t\t= %3zd bit\n", CHAR_BIT*size - prec - SIGN_BIT); + fprintf (stderr, "bits per byte\t= %3d bit\n", CHAR_BIT); +#endif /* DEBUG */ + return max; +} +#undef SIGN_BIT + /* Fail if mktime fails to convert a date in the spring-forward gap. Based on a problem report from Andreas Jaeger. */ static void @@ -37,8 +133,8 @@ tm.tm_min = 0; tm.tm_sec = 0; tm.tm_isdst = -1; - if (mktime (&tm) == (time_t)-1) - exit (1); + if (mktime (&tm) == (time_t)(-1)) + exit (EXIT_FAILURE); } static void @@ -47,10 +143,10 @@ { struct tm *lt; if ((lt = localtime (&now)) && mktime (lt) != now) - exit (1); + exit (EXIT_FAILURE); now = time_t_max - now; if ((lt = localtime (&now)) && mktime (lt) != now) - exit (1); + exit (EXIT_FAILURE); } static void @@ -67,7 +163,7 @@ tm.tm_isdst = -1; mktime (&tm); if (tm.tm_mon != 2 || tm.tm_mday != 31) - exit (1); + exit (EXIT_FAILURE); } static void @@ -82,7 +178,7 @@ alarm (10); now = mktime (&tm); alarm (0); - if (now != (time_t) -1) + if (now != (time_t)(-1)) { struct tm *lt = localtime (&now); if (! (lt @@ -96,7 +192,7 @@ && lt->tm_wday == tm.tm_wday && ((lt->tm_isdst < 0 ? -1 : 0 < lt->tm_isdst) == (tm.tm_isdst < 0 ? -1 : 0 < tm.tm_isdst)))) - exit (1); + exit (EXIT_FAILURE); } } @@ -106,9 +202,10 @@ time_t t, delta; int i, j; - for (time_t_max = 1; 0 < time_t_max; time_t_max *= 2) - continue; - time_t_max--; + time_t_max = get_xxxint_t_max (sizeof (time_t_max)); + if (time_t_max == (time_t)(-1)) + exit (EXIT_FAILURE); + delta = time_t_max / 997; /* a suitable prime number */ for (i = 0; i < N_STRINGS; i++) { @@ -120,11 +217,16 @@ mktime_test ((time_t) 60 * 60); mktime_test ((time_t) 60 * 60 * 24); - for (j = 1; 0 < j; j *= 2) - bigtime_test (j); - bigtime_test (j - 1); + for (i = 0; i < PRECISION (INT_MAX); i++) + { + j = (int) 1 << i; + bigtime_test (j); + } + j = INT_MAX; + bigtime_test (j); } irix_6_4_bug (); spring_forward_gap (); - exit (0); + exit (EXIT_SUCCESS); } +/*! vi: set ai tabstop=8 shiftwidth=2: */ --nextPart3148181.qzLpRxe9IO--