[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freebsd-hackers
Subject:    Re: Implicit assumptions (was: Re: Some fun with -O2)
From:       Walter von Entferndt <walter.von.entferndt () posteo ! net>
Date:       2021-01-18 11:46:58
Message-ID: 4907874.QyqR4RcxXA () t450s ! local ! lan
[Download RAW message or body]

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 
<walter.von.entferndt@posteo.net> 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)
["check_mktime.c.patch" (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 <sys/time.h>
 # include <time.h>
 # include <unistd.h>
+# include <stdlib.h>	/* putenv(), exit(), EXIT_xxx */
+# include <stdint.h>	/* intmax_t */
+# include <stdio.h>	/* printf() */
+# include <limits.h>	/* CHAR_BIT, xxx_MAX */
+# include <inttypes.h>	/* 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: */


_______________________________________________
freebsd-hackers@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic