Emacs current-time-string core dump on 64-bit hosts

Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

GNU Emacs 21.4.1 (sparc-sun-solaris2.8, X toolkit), built with
"CC='gcc -m64' ./configure" to get 64-bit mode, reliably dumps core
when I execute (current-time-string '(2814749767106 0)), due to a
buffer overrun inside the C library.  This is because Emacs is relying
on undefined behavior within the ctime() call.

I discovered this problem after reporting a bug in the POSIX
specification for ctime; see
<http://www.opengroup.org/austin/mailarchives/ag/msg09294.html>.
In practice, ctime (as well as ctime_r, asctime, and asctime_r)
cannot be trusted with arbitrary time stamps, any more than gets
can be trusted with arbitrary inputs: if the time stamps generate
a string that is too long, the program can do anything.

Here is a proposed patch, relative to CVS head.

2006-03-16  Paul Eggert  <eggert@cs.ucla.edu>

	Do not use ctime, since it has undefined behavior with
	out-of-range time stamps.  This fixes a bug where
	(current-time-string '(2814749767106 0)) would make Emacs dump
	core on 64-bit Solaris 8; the fix is to remove all uses of ctime
	from the Emacs source code.  Please see
	<http://www.opengroup.org/austin/mailarchives/ag/msg09294.html>
	for more details about this portability problem.

	* lib-src/b2m.c: Include <limits.h>, for CHAR_BIT.
	(TM_YEAR_BASE): New macro.
	(string_from_time): New function.
	(main): Use it instead of ctime, to avoid buffer overflows with
	outlandish time stamps.
	* lib-src/fakemail.c: Likewise.
	* src/editfns.c: Include <limits.h>, for CHAR_BIT.
	(TM_YEAR_BASE): Move up, so new code can use it.
	(Fdecode_time, Fencode_time):
	Use TM_YEAR_BASE instead of hardwiring 1900.
	(Fdecode_time): Cast tm_year to EMACS_INT, to avoid overflow when
	int is narrow than EMACS_INT.
	(Fcurrent_time_string): Do not use ctime, since it might
	cause a buffer overflow with outlandish time stamps.
	Instead, implement its algorithm ourselves.
	As with Fformat_time_string, report an invalid time specification
	if the argument is invalid, and report an out-of-range time stamp
	if that problem occurs.  This is better than overrunning the buffer,
	and it preserves the historic behavior of always returning a
	fixed-size string.
	* lib-src/ntlib.c (sys_ctime): Remove, since Emacs never calls
	ctime any more.
	* lib-src/ntlib.h (ctime): Likewise.
	* src/w32.c (sys_ctime): Likewise.
	* src/s/ms-w32.h (ctime): Likewise.

Index: lib-src/b2m.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/b2m.c,v
retrieving revision 1.30
diff -p -u -r1.30 b2m.c
--- lib-src/b2m.c	7 May 2004 15:26:21 -0000	1.30
+++ lib-src/b2m.c	17 Mar 2006 05:22:00 -0000
@@ -26,6 +26,7 @@
 #undef static
 #endif
 
+#include <limits.h>
 #include <stdio.h>
 #include <time.h>
 #include <sys/types.h>
@@ -68,6 +69,35 @@ void fatal ();
 #define xnew(n, Type)	((Type *) xmalloc ((n) * sizeof (Type)))
 
 
+#define TM_YEAR_BASE 1900
+
+static char *
+string_from_time (time_t t)
+{
+  static char buf[sizeof "Sun Sep 16 01:03:52 "
+		  + sizeof (long int) * CHAR_BIT / 3];
+  struct tm const *tm = localtime (&t);
+  if (! tm)
+    sprintf (buf, "@%ld", (long int) t);
+  else
+    {
+      static char const wday[][4] =
+	{
+	  "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+	};
+      static char const mon[][4] =
+	{
+	  "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+	  "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+	};
+      sprintf (buf, "%s %s %2d %.2d:%.2d:%.2d %ld",
+	       wday[tm->tm_wday], mon[tm->tm_mon], tm->tm_mday,
+	       tm->tm_hour, tm->tm_min, tm->tm_sec,
+	       TM_YEAR_BASE + (long int) tm->tm_year);
+    }
+  return buf;
+}
+
 
 char *progname;
 
@@ -131,7 +161,7 @@ main (argc, argv)
 
   labels_saved = printing = header = FALSE;
   ltoday = time (0);
-  today = ctime (&ltoday);
+  today = string_from_time (ltoday);
   data.size = 200;
   data.buffer = xnew (200, char);
 
@@ -144,7 +174,7 @@ main (argc, argv)
       if (streq (data.buffer, "*** EOOH ***") && !printing)
 	{
 	  printing = header = TRUE;
-	  printf ("From \"Babyl to mail by %s\" %s", progname, today);
+	  printf ("From \"Babyl to mail by %s\" %s\n", progname, today);
 	  continue;
 	}
 
Index: lib-src/fakemail.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/fakemail.c,v
retrieving revision 1.35
diff -p -u -r1.35 fakemail.c
--- lib-src/fakemail.c	6 Feb 2006 11:28:28 -0000	1.35
+++ lib-src/fakemail.c	17 Mar 2006 05:22:01 -0000
@@ -53,6 +53,7 @@ main ()
 #include "ntlib.h"
 #endif
 
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 #include <ctype.h>
@@ -349,22 +350,47 @@ add_field (the_list, field, where)
   return where;
 }
 \f
+#define TM_YEAR_BASE 1900
+
+static char *
+string_from_time (time_t t)
+{
+  static char buf[sizeof "Sun Sep 16 01:03:52 "
+		  + sizeof (long int) * CHAR_BIT / 3];
+  struct tm const *tm = localtime (&t);
+  if (! tm)
+    sprintf (buf, "@%ld", (long int) t);
+  else
+    {
+      static char const wday[][4] =
+	{
+	  "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+	};
+      static char const mon[][4] =
+	{
+	  "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+	  "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+	};
+      sprintf (buf, "%s %s %2d %.2d:%.2d:%.2d %ld",
+	       wday[tm->tm_wday], mon[tm->tm_mon], tm->tm_mday,
+	       tm->tm_hour, tm->tm_min, tm->tm_sec,
+	       TM_YEAR_BASE + (long int) tm->tm_year);
+    }
+  return buf;
+}
+
 line_list
 make_file_preface ()
 {
   char *the_string, *temp;
-  long idiotic_interface;
   long prefix_length;
   long user_length;
   long date_length;
   line_list result;
 
   prefix_length = strlen (FROM_PREFIX);
-  time (&idiotic_interface);
-  the_date = ctime (&idiotic_interface);
-  /* the_date has an unwanted newline at the end */
-  date_length = strlen (the_date) - 1;
-  the_date[date_length] = '\0';
+  the_date = string_from_time (time (NULL));
+  date_length = strlen (the_date);
   temp = cuserid ((char *) NULL);
   user_length = strlen (temp);
   the_user = alloc_string (user_length + 1);
Index: src/editfns.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/editfns.c,v
retrieving revision 1.409
diff -p -u -r1.409 editfns.c
--- src/editfns.c	7 Feb 2006 09:08:53 -0000	1.409
+++ src/editfns.c	17 Mar 2006 05:22:01 -0000
@@ -49,6 +49,7 @@ Boston, MA 02110-1301, USA.  */
 #endif
 
 #include <ctype.h>
+#include <limits.h>
 
 #include "intervals.h"
 #include "buffer.h"
@@ -72,6 +73,8 @@ Boston, MA 02110-1301, USA.  */
 extern char **environ;
 #endif
 
+#define TM_YEAR_BASE 1900
+
 extern size_t emacs_strftimeu P_ ((char *, size_t, const char *,
 				   const struct tm *, int));
 static int tm_diff P_ ((struct tm *, struct tm *));
@@ -1722,7 +1725,7 @@ DOW and ZONE.)  */)
   XSETFASTINT (list_args[2], decoded_time->tm_hour);
   XSETFASTINT (list_args[3], decoded_time->tm_mday);
   XSETFASTINT (list_args[4], decoded_time->tm_mon + 1);
-  XSETINT (list_args[5], decoded_time->tm_year + 1900);
+  XSETINT (list_args[5], TM_YEAR_BASE + (EMACS_INT) decoded_time->tm_year);
   XSETFASTINT (list_args[6], decoded_time->tm_wday);
   list_args[7] = (decoded_time->tm_isdst)? Qt : Qnil;
 
@@ -1778,7 +1781,7 @@ usage: (encode-time SECOND MINUTE HOUR D
   tm.tm_hour = XINT (args[2]);
   tm.tm_mday = XINT (args[3]);
   tm.tm_mon = XINT (args[4]) - 1;
-  tm.tm_year = XINT (args[5]) - 1900;
+  tm.tm_year = XINT (args[5]) - TM_YEAR_BASE;
   tm.tm_isdst = -1;
 
   if (CONSP (zone))
@@ -1843,21 +1846,34 @@ but this is considered obsolete.  */)
      Lisp_Object specified_time;
 {
   time_t value;
-  char buf[30];
-  register char *tem;
+  char buf[sizeof "Sun Sep 16 01:03:52 1973"];
+  struct tm const *tm;
+  static char const wday[][4] =
+    {
+      "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+    };
+  static char const mon[][4] =
+    {
+      "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+      "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+    };
 
   if (! lisp_time_argument (specified_time, &value, NULL))
-    value = -1;
-  tem = (char *) ctime (&value);
+    error ("Invalid time specification");
 
-  strncpy (buf, tem, 24);
-  buf[24] = 0;
+  tm = localtime (&value);
+  if (! (tm
+	 && -999 - TM_YEAR_BASE <= tm->tm_year
+	 && tm->tm_year <= 9999 - TM_YEAR_BASE))
+    error ("Specified time is not representable");
 
+  sprintf (buf, "%s %s %2d %02d:%02d:%02d %04d",
+	   wday[tm->tm_wday], mon[tm->tm_mon], tm->tm_mday,
+	   tm->tm_hour, tm->tm_min, tm->tm_sec,
+	   TM_YEAR_BASE + tm->tm_year);
   return build_string (buf);
 }
 
-#define TM_YEAR_BASE 1900
-
 /* Yield A - B, measured in seconds.
    This function is copied from the GNU C Library.  */
 static int
Index: lib-src/ntlib.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/ntlib.c,v
retrieving revision 1.13
diff -p -u -r1.13 ntlib.c
--- lib-src/ntlib.c	6 Feb 2006 11:28:28 -0000	1.13
+++ lib-src/ntlib.c	17 Mar 2006 05:22:01 -0000
@@ -188,16 +188,6 @@ fchown (int fd, int uid, int gid)
   return 0;
 }
 
-/* Place a wrapper around the MSVC version of ctime.  It returns NULL
-   on network directories, so we handle that case here.
-   (Ulrich Leodolter, 1/11/95).  */
-char *
-sys_ctime (const time_t *t)
-{
-  char *str = (char *) ctime (t);
-  return (str ? str : "Sun Jan 01 00:00:00 1970");
-}
-
 FILE *
 sys_fopen(const char * path, const char * mode)
 {
Index: lib-src/ntlib.h
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/ntlib.h,v
retrieving revision 1.11
diff -p -u -r1.11 ntlib.h
--- lib-src/ntlib.h	6 Feb 2006 11:28:28 -0000	1.11
+++ lib-src/ntlib.h	17 Mar 2006 05:22:01 -0000
@@ -61,7 +61,6 @@ int fchown (int fd, int uid, int gid);
 #define close   _close
 #undef creat
 #define creat   _creat
-#undef ctime
 #undef dup
 #define dup     _dup
 #undef dup2
Index: src/w32.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/w32.c,v
retrieving revision 1.100
diff -p -u -r1.100 w32.c
--- src/w32.c	27 Feb 2006 02:07:37 -0000	1.100
+++ src/w32.c	17 Mar 2006 05:22:01 -0000
@@ -1325,16 +1325,6 @@ gettimeofday (struct timeval *tv, struct
 /* IO support and wrapper functions for W32 API. */
 /* ------------------------------------------------------------------------- */
 
-/* Place a wrapper around the MSVC version of ctime.  It returns NULL
-   on network directories, so we handle that case here.
-   (Ulrich Leodolter, 1/11/95).  */
-char *
-sys_ctime (const time_t *t)
-{
-  char *str = (char *) ctime (t);
-  return (str ? str : "Sun Jan 01 00:00:00 1970");
-}
-
 /* Emulate sleep...we could have done this with a define, but that
    would necessitate including windows.h in the files that used it.
    This is much easier.  */
Index: src/s/ms-w32.h
===================================================================
RCS file: /cvsroot/emacs/emacs/src/s/ms-w32.h,v
retrieving revision 1.37
diff -p -u -r1.37 ms-w32.h
--- src/s/ms-w32.h	6 Feb 2006 15:23:23 -0000	1.37
+++ src/s/ms-w32.h	17 Mar 2006 05:22:01 -0000
@@ -317,7 +317,6 @@ Boston, MA 02110-1301, USA.  */
 #define close   sys_close
 #undef creat
 #define creat   sys_creat
-#define ctime	sys_ctime
 #undef dup
 #define dup     sys_dup
 #undef dup2

Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

GNU Emacs 21.4.1 (sparc-sun-solaris2.8, X toolkit), built with
"CC='gcc -m64' ./configure" to get 64-bit mode, reliably dumps core
when I execute (current-time-string '(2814749767106 0)), due to a
buffer overrun inside the C library.  This is because Emacs is relying
on undefined behavior within the ctime() call.

I discovered this problem after reporting a bug in the POSIX
specification for ctime; see
<http://www.opengroup.org/austin/mailarchives/ag/msg09294.html>.
In practice, ctime (as well as ctime_r, asctime, and asctime_r)
cannot be trusted with arbitrary time stamps, any more than gets
can be trusted with arbitrary inputs: if the time stamps generate
a string that is too long, the program can do anything.

Here is a proposed patch, relative to CVS head.

2006-03-16  Paul Eggert  <eggert@cs.ucla.edu>

	Do not use ctime, since it has undefined behavior with
	out-of-range time stamps.  This fixes a bug where
	(current-time-string '(2814749767106 0)) would make Emacs dump
	core on 64-bit Solaris 8; the fix is to remove all uses of ctime
	from the Emacs source code.  Please see
	<http://www.opengroup.org/austin/mailarchives/ag/msg09294.html>
	for more details about this portability problem.

	* lib-src/b2m.c: Include <limits.h>, for CHAR_BIT.
	(TM_YEAR_BASE): New macro.
	(string_from_time): New function.
	(main): Use it instead of ctime, to avoid buffer overflows with
	outlandish time stamps.
	* lib-src/fakemail.c: Likewise.
	* src/editfns.c: Include <limits.h>, for CHAR_BIT.
	(TM_YEAR_BASE): Move up, so new code can use it.
	(Fdecode_time, Fencode_time):
	Use TM_YEAR_BASE instead of hardwiring 1900.
	(Fdecode_time): Cast tm_year to EMACS_INT, to avoid overflow when
	int is narrow than EMACS_INT.
	(Fcurrent_time_string): Do not use ctime, since it might
	cause a buffer overflow with outlandish time stamps.
	Instead, implement its algorithm ourselves.
	As with Fformat_time_string, report an invalid time specification
	if the argument is invalid, and report an out-of-range time stamp
	if that problem occurs.  This is better than overrunning the buffer,
	and it preserves the historic behavior of always returning a
	fixed-size string.
	* lib-src/ntlib.c (sys_ctime): Remove, since Emacs never calls
	ctime any more.
	* lib-src/ntlib.h (ctime): Likewise.
	* src/w32.c (sys_ctime): Likewise.
	* src/s/ms-w32.h (ctime): Likewise.

Index: lib-src/b2m.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/b2m.c,v
retrieving revision 1.30
diff -p -u -r1.30 b2m.c
--- lib-src/b2m.c	7 May 2004 15:26:21 -0000	1.30
+++ lib-src/b2m.c	17 Mar 2006 05:22:00 -0000
@@ -26,6 +26,7 @@
 #undef static
 #endif
 
+#include <limits.h>
 #include <stdio.h>
 #include <time.h>
 #include <sys/types.h>
@@ -68,6 +69,35 @@ void fatal ();
 #define xnew(n, Type)	((Type *) xmalloc ((n) * sizeof (Type)))
 
 
+#define TM_YEAR_BASE 1900
+
+static char *
+string_from_time (time_t t)
+{
+  static char buf[sizeof "Sun Sep 16 01:03:52 "
+		  + sizeof (long int) * CHAR_BIT / 3];
+  struct tm const *tm = localtime (&t);
+  if (! tm)
+    sprintf (buf, "@%ld", (long int) t);
+  else
+    {
+      static char const wday[][4] =
+	{
+	  "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+	};
+      static char const mon[][4] =
+	{
+	  "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+	  "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+	};
+      sprintf (buf, "%s %s %2d %.2d:%.2d:%.2d %ld",
+	       wday[tm->tm_wday], mon[tm->tm_mon], tm->tm_mday,
+	       tm->tm_hour, tm->tm_min, tm->tm_sec,
+	       TM_YEAR_BASE + (long int) tm->tm_year);
+    }
+  return buf;
+}
+
 
 char *progname;
 
@@ -131,7 +161,7 @@ main (argc, argv)
 
   labels_saved = printing = header = FALSE;
   ltoday = time (0);
-  today = ctime (&ltoday);
+  today = string_from_time (ltoday);
   data.size = 200;
   data.buffer = xnew (200, char);
 
@@ -144,7 +174,7 @@ main (argc, argv)
       if (streq (data.buffer, "*** EOOH ***") && !printing)
 	{
 	  printing = header = TRUE;
-	  printf ("From \"Babyl to mail by %s\" %s", progname, today);
+	  printf ("From \"Babyl to mail by %s\" %s\n", progname, today);
 	  continue;
 	}
 
Index: lib-src/fakemail.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/fakemail.c,v
retrieving revision 1.35
diff -p -u -r1.35 fakemail.c
--- lib-src/fakemail.c	6 Feb 2006 11:28:28 -0000	1.35
+++ lib-src/fakemail.c	17 Mar 2006 05:22:01 -0000
@@ -53,6 +53,7 @@ main ()
 #include "ntlib.h"
 #endif
 
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 #include <ctype.h>
@@ -349,22 +350,47 @@ add_field (the_list, field, where)
   return where;
 }
 \f
+#define TM_YEAR_BASE 1900
+
+static char *
+string_from_time (time_t t)
+{
+  static char buf[sizeof "Sun Sep 16 01:03:52 "
+		  + sizeof (long int) * CHAR_BIT / 3];
+  struct tm const *tm = localtime (&t);
+  if (! tm)
+    sprintf (buf, "@%ld", (long int) t);
+  else
+    {
+      static char const wday[][4] =
+	{
+	  "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+	};
+      static char const mon[][4] =
+	{
+	  "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+	  "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+	};
+      sprintf (buf, "%s %s %2d %.2d:%.2d:%.2d %ld",
+	       wday[tm->tm_wday], mon[tm->tm_mon], tm->tm_mday,
+	       tm->tm_hour, tm->tm_min, tm->tm_sec,
+	       TM_YEAR_BASE + (long int) tm->tm_year);
+    }
+  return buf;
+}
+
 line_list
 make_file_preface ()
 {
   char *the_string, *temp;
-  long idiotic_interface;
   long prefix_length;
   long user_length;
   long date_length;
   line_list result;
 
   prefix_length = strlen (FROM_PREFIX);
-  time (&idiotic_interface);
-  the_date = ctime (&idiotic_interface);
-  /* the_date has an unwanted newline at the end */
-  date_length = strlen (the_date) - 1;
-  the_date[date_length] = '\0';
+  the_date = string_from_time (time (NULL));
+  date_length = strlen (the_date);
   temp = cuserid ((char *) NULL);
   user_length = strlen (temp);
   the_user = alloc_string (user_length + 1);
Index: src/editfns.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/editfns.c,v
retrieving revision 1.409
diff -p -u -r1.409 editfns.c
--- src/editfns.c	7 Feb 2006 09:08:53 -0000	1.409
+++ src/editfns.c	17 Mar 2006 05:22:01 -0000
@@ -49,6 +49,7 @@ Boston, MA 02110-1301, USA.  */
 #endif
 
 #include <ctype.h>
+#include <limits.h>
 
 #include "intervals.h"
 #include "buffer.h"
@@ -72,6 +73,8 @@ Boston, MA 02110-1301, USA.  */
 extern char **environ;
 #endif
 
+#define TM_YEAR_BASE 1900
+
 extern size_t emacs_strftimeu P_ ((char *, size_t, const char *,
 				   const struct tm *, int));
 static int tm_diff P_ ((struct tm *, struct tm *));
@@ -1722,7 +1725,7 @@ DOW and ZONE.)  */)
   XSETFASTINT (list_args[2], decoded_time->tm_hour);
   XSETFASTINT (list_args[3], decoded_time->tm_mday);
   XSETFASTINT (list_args[4], decoded_time->tm_mon + 1);
-  XSETINT (list_args[5], decoded_time->tm_year + 1900);
+  XSETINT (list_args[5], TM_YEAR_BASE + (EMACS_INT) decoded_time->tm_year);
   XSETFASTINT (list_args[6], decoded_time->tm_wday);
   list_args[7] = (decoded_time->tm_isdst)? Qt : Qnil;
 
@@ -1778,7 +1781,7 @@ usage: (encode-time SECOND MINUTE HOUR D
   tm.tm_hour = XINT (args[2]);
   tm.tm_mday = XINT (args[3]);
   tm.tm_mon = XINT (args[4]) - 1;
-  tm.tm_year = XINT (args[5]) - 1900;
+  tm.tm_year = XINT (args[5]) - TM_YEAR_BASE;
   tm.tm_isdst = -1;
 
   if (CONSP (zone))
@@ -1843,21 +1846,34 @@ but this is considered obsolete.  */)
      Lisp_Object specified_time;
 {
   time_t value;
-  char buf[30];
-  register char *tem;
+  char buf[sizeof "Sun Sep 16 01:03:52 1973"];
+  struct tm const *tm;
+  static char const wday[][4] =
+    {
+      "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+    };
+  static char const mon[][4] =
+    {
+      "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+      "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+    };
 
   if (! lisp_time_argument (specified_time, &value, NULL))
-    value = -1;
-  tem = (char *) ctime (&value);
+    error ("Invalid time specification");
 
-  strncpy (buf, tem, 24);
-  buf[24] = 0;
+  tm = localtime (&value);
+  if (! (tm
+	 && -999 - TM_YEAR_BASE <= tm->tm_year
+	 && tm->tm_year <= 9999 - TM_YEAR_BASE))
+    error ("Specified time is not representable");
 
+  sprintf (buf, "%s %s %2d %02d:%02d:%02d %04d",
+	   wday[tm->tm_wday], mon[tm->tm_mon], tm->tm_mday,
+	   tm->tm_hour, tm->tm_min, tm->tm_sec,
+	   TM_YEAR_BASE + tm->tm_year);
   return build_string (buf);
 }
 
-#define TM_YEAR_BASE 1900
-
 /* Yield A - B, measured in seconds.
    This function is copied from the GNU C Library.  */
 static int
Index: lib-src/ntlib.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/ntlib.c,v
retrieving revision 1.13
diff -p -u -r1.13 ntlib.c
--- lib-src/ntlib.c	6 Feb 2006 11:28:28 -0000	1.13
+++ lib-src/ntlib.c	17 Mar 2006 05:22:01 -0000
@@ -188,16 +188,6 @@ fchown (int fd, int uid, int gid)
   return 0;
 }
 
-/* Place a wrapper around the MSVC version of ctime.  It returns NULL
-   on network directories, so we handle that case here.
-   (Ulrich Leodolter, 1/11/95).  */
-char *
-sys_ctime (const time_t *t)
-{
-  char *str = (char *) ctime (t);
-  return (str ? str : "Sun Jan 01 00:00:00 1970");
-}
-
 FILE *
 sys_fopen(const char * path, const char * mode)
 {
Index: lib-src/ntlib.h
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/ntlib.h,v
retrieving revision 1.11
diff -p -u -r1.11 ntlib.h
--- lib-src/ntlib.h	6 Feb 2006 11:28:28 -0000	1.11
+++ lib-src/ntlib.h	17 Mar 2006 05:22:01 -0000
@@ -61,7 +61,6 @@ int fchown (int fd, int uid, int gid);
 #define close   _close
 #undef creat
 #define creat   _creat
-#undef ctime
 #undef dup
 #define dup     _dup
 #undef dup2
Index: src/w32.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/w32.c,v
retrieving revision 1.100
diff -p -u -r1.100 w32.c
--- src/w32.c	27 Feb 2006 02:07:37 -0000	1.100
+++ src/w32.c	17 Mar 2006 05:22:01 -0000
@@ -1325,16 +1325,6 @@ gettimeofday (struct timeval *tv, struct
 /* IO support and wrapper functions for W32 API. */
 /* ------------------------------------------------------------------------- */
 
-/* Place a wrapper around the MSVC version of ctime.  It returns NULL
-   on network directories, so we handle that case here.
-   (Ulrich Leodolter, 1/11/95).  */
-char *
-sys_ctime (const time_t *t)
-{
-  char *str = (char *) ctime (t);
-  return (str ? str : "Sun Jan 01 00:00:00 1970");
-}
-
 /* Emulate sleep...we could have done this with a define, but that
    would necessitate including windows.h in the files that used it.
    This is much easier.  */
Index: src/s/ms-w32.h
===================================================================
RCS file: /cvsroot/emacs/emacs/src/s/ms-w32.h,v
retrieving revision 1.37
diff -p -u -r1.37 ms-w32.h
--- src/s/ms-w32.h	6 Feb 2006 15:23:23 -0000	1.37
+++ src/s/ms-w32.h	17 Mar 2006 05:22:01 -0000
@@ -317,7 +317,6 @@ Boston, MA 02110-1301, USA.  */
 #define close   sys_close
 #undef creat
 #define creat   sys_creat
-#define ctime	sys_ctime
 #undef dup
 #define dup     sys_dup
 #undef dup2

Re: Emacs current-time-string core dump on 64-bit hosts

[Eli Zaretskii]

> From: Paul Eggert <eggert@CS.UCLA.EDU>
> Gcc: nnfolder+archive:outgo
> Date: Thu, 16 Mar 2006 21:58:09 -0800
> 
> 2006-03-16  Paul Eggert  <eggert@cs.ucla.edu>
> 
> 	Do not use ctime, since it has undefined behavior with
> 	out-of-range time stamps.  This fixes a bug where
> 	(current-time-string '(2814749767106 0)) would make Emacs dump
> 	core on 64-bit Solaris 8; the fix is to remove all uses of ctime
> 	from the Emacs source code.

Personally, I find it preposterous that we need to reinvent library
functions.  Isn't there a better (safer) library function, or a
combination thereof, that we could use instead of rolling our own?

> 	(Fdecode_time): Cast tm_year to EMACS_INT, to avoid overflow when
> 	int is narrow than EMACS_INT.

Can it happen that an int is narrower than EMACS_INT?  I thought it
was impossible, but perhaps I'm mistaken.

If this cannot happen, let's not clutter the code with unnecessary
kludges.

> 	* lib-src/ntlib.c (sys_ctime): Remove, since Emacs never calls
> 	ctime any more.
> 	* lib-src/ntlib.h (ctime): Likewise.
> 	* src/w32.c (sys_ctime): Likewise.
> 	* src/s/ms-w32.h (ctime): Likewise.

I wouldn't remove these: the functions are almost trivial wrappers
around the library version, and someone could try using ctime in the
future (in a different context), in which case they will hit the
Windows library bug again.

Re: Emacs current-time-string core dump on 64-bit hosts

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> Can it happen that an int is narrower than EMACS_INT?

Sure, on every LP64 host.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Emacs current-time-string core dump on 64-bit hosts

[Kevin Rodgers]

Eli Zaretskii wrote:
>>From: Paul Eggert <eggert@CS.UCLA.EDU>
>>Gcc: nnfolder+archive:outgo
>>Date: Thu, 16 Mar 2006 21:58:09 -0800
>>
>>2006-03-16  Paul Eggert  <eggert@cs.ucla.edu>
>>
>>	Do not use ctime, since it has undefined behavior with
>>	out-of-range time stamps.  This fixes a bug where
>>	(current-time-string '(2814749767106 0)) would make Emacs dump
>>	core on 64-bit Solaris 8; the fix is to remove all uses of ctime
>>	from the Emacs source code.
> 
> 
> Personally, I find it preposterous that we need to reinvent library
> functions.  Isn't there a better (safer) library function, or a
> combination thereof, that we could use instead of rolling our own?

Why not have Emacs test for an out-of-range time stamp before
calling ctime?

-- 
Kevin Rodgers

Re: Emacs current-time-string core dump on 64-bit hosts

[Andreas Schwab]

Paul Eggert <eggert@CS.UCLA.EDU> writes:

> 	* lib-src/b2m.c: Include <limits.h>, for CHAR_BIT.
> 	(TM_YEAR_BASE): New macro.
> 	(string_from_time): New function.
> 	(main): Use it instead of ctime, to avoid buffer overflows with
> 	outlandish time stamps.

Here ctime is only ever been called on the current wall time, if you set
that to some outlandish time stamp you already have enough problems
anyway.

> 	* lib-src/fakemail.c: Likewise.

Likewise.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Eli Zaretskii <eliz@gnu.org> writes:

> I find it preposterous that we need to reinvent library
> functions.  Isn't there a better (safer) library function, or a
> combination thereof, that we could use instead of rolling our own?

Not really.  The only plausible candidate library function is strftime,
but it's not suitable.

Emacs itself can't use strftime here, because of locale issues;
current-time-string wants the C locale, but strftime uses the LC_TIME
locale.

fakemail and b2m could potentially use strftime.  But strftime is such
a pain to use that its code won't be much simpler than just doing it
ourselves.  Also, strftime is a portability nightmare in its own
right.  Partly for the latter reason Emacs itself uses its own
implementation of strftime -- but Emacs's implementation is not easily
available to lib-src programs.

All in all it's simpler and more easier to maintain in this case, if
we simply roll our own code.  (If you don't believe me, try writing it
the other way....)


> Can it happen that an int is narrower than EMACS_INT?

Yes, and that occurs on the platform where Emacs had the core dump
(64-bit Solaris sparc).


> I wouldn't remove these [redefinitions of ctime under Microsoft
> Windows]: the functions are almost trivial wrappers around the
> library version, and someone could try using ctime in the future (in
> a different context),

But the whole point of the patch is that Emacs should not use ctime,
for the same reason that Emacs should not use gets.  Like gets, if you
can't control the input, ctime is inherently subject to buffer
overruns.  Like gets, we should remove all uses of ctime from Emacs,
and never use it again.

Come to think of it, we should also remove the ctime wrapper from the
Mac port.


Andreas Schwab <schwab@suse.de> writes:

> [In b2m] ctime is only ever been called on the current wall time, if
> you set that to some outlandish time stamp you already have enough
> problems anyway.

It has happened to me.  I didn't set the clock to an outlandish time;
the clock hardware did it to me.

Emacs utility programs shouldn't crash simply because the clock is set
to a large value.

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

    Personally, I find it preposterous that we need to reinvent library
    functions.  Isn't there a better (safer) library function, or a
    combination thereof, that we could use instead of rolling our own?

I agree.

Why not just check the input for being in a reasonable range?

Re: Emacs current-time-string core dump on 64-bit hosts

[Eli Zaretskii]

> From: Paul Eggert <eggert@CS.UCLA.EDU>
> Date: Fri, 17 Mar 2006 16:44:54 -0800
> 
> > I wouldn't remove these [redefinitions of ctime under Microsoft
> > Windows]: the functions are almost trivial wrappers around the
> > library version, and someone could try using ctime in the future (in
> > a different context),
> 
> But the whole point of the patch is that Emacs should not use ctime,
> for the same reason that Emacs should not use gets.

No, the point is that Emacs shouldn't use ctime _for_arguments_that_
_come_from_an_uncontrolled_source_.  But I still could imagine some
possible valid use in the future where the argument is generated by
code we control.  Having a 4-liner ready for such a situation is a
price I think we could manage to pay.

> Like gets, we should remove all uses of ctime from Emacs, and never
> use it again.

If there's a safer library function, I'd agree.  (There is one for
gets.)  But if the alternative is to spill the guts of ctime into
Emacs, I'd look very hard for other solutions.  Others suggested here
to test the argument for validity, for example.

> Emacs utility programs shouldn't crash simply because the clock is set
> to a large value.

No one is arguing that they should.  But IMHO the solution you suggest
is not the best one.  Time calculations are a very complex issue, as
I'm sure you know only too well; having that complex stuff inside
Emacs is a maintenance burden that we should avoid.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

>     Personally, I find it preposterous that we need to reinvent library
>     functions.  Isn't there a better (safer) library function, or a
>     combination thereof, that we could use instead of rolling our own?
>
> I agree.
>
> Why not just check the input for being in a reasonable range?

It's difficult to do an exact check on the input, since the input
range depends on the time zone and on leap second support.  Doing an
exact check would involve far more code than the solution already
proposed.

If you're willing to settle for an approximate check, it depends on
how approximate you are willing to settle for.

If the input check would be "approximately January 2, 1001 through
approximately December 30, 9998 AD" (to avoid time zone and leap
second issues), then it would be somewhat tricky to write the code
portably, since the numbers involved will exceed 2**32.  And this
would reject a few valid inputs.

If the input check would be "only 32-bit time_t values", then it's
pretty easy.  But that will eliminate most of the valid inputs on a
64-bit host; and the code would stop working in 2038.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Eli Zaretskii <eliz@gnu.org> writes:

> If there's a safer library function, I'd agree.  (There is one for
> gets.)  But if the alternative is to spill the guts of ctime into
> Emacs,

We're talking about 20 obvious lines of code here!  It's not that hard
to maintain.

Let's put this into a bit of perspective here.  Emacs already
maintains its own copy of strftime (about 1500 lines of
somewhat-tricky code), for similar portability reasons.  Is it
reasonable to worry about maintaining ctime when we're not worried
about strftime?


> Others suggested here to test the argument for validity, for
> example.

As I explained on bug-gnu-emacs just now, that particular suggestion
has problems as well.  It won't work as well as the code already
proposed.


> Time calculations are a very complex issue, as I'm sure you know
> only too well;

Sure, but this particular code isn't complex.  It's quite simple and
reliable.  Ugly perhaps, but simple.


> I still could imagine some possible valid use in the future where
> the [ctime] argument is generated by code we control.

I doubt that, but if such a thing ever occurs, we can easily resurrect
the wrapper from CVS.

In the meantime our best bet by far is to tell people this simple rule:

  Don't use ctime.

Such a rule is much easier to remember than this one:

  Don't use ctime unless you have sufficient control over its argument
  time stamps to know that they won't generate more than 26 bytes when
  one naively applies the algorithm on page 341 of the 1999 edition of
  the C Standard.

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

    If the input check would be "approximately January 2, 1001 through
    approximately December 30, 9998 AD" (to avoid time zone and leap
    second issues), then it would be somewhat tricky to write the code
    portably, since the numbers involved will exceed 2**32.  And this
    would reject a few valid inputs.

I think that is best.  It should be straightforward, too, if you first
use localtime to extract the year.


I think it is possible, even trivial, to fix ctime.  The largest year
value that can be represented in a 64-bit time-value is around 1
trillion, and that will fit in 12 digits.  Isn't this trivial to fix?
So why not just fix it in GNU libc, and report the bug for other
systems where it fails.

Fixing asctime as well, on systems where int is 64 bits, requires checking
for year numbers that are too big.  But it is not hard to do.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

>>    If the input check would be "approximately January 2, 1001 through
>>    approximately December 30, 9998 AD" (to avoid time zone and leap
>>    second issues), then it would be somewhat tricky to write the code
>>    portably, since the numbers involved will exceed 2**32.  And this
>>    would reject a few valid inputs.
>
> I think that is best.  It should be straightforward, too, if you first
> use localtime to extract the year.

OK, thanks, I've done that.  The revised patch is enclosed below.  The
check is equivalent to "-999 <= year <= 9999", and is done after
invoking localtime.

> I think it is possible, even trivial, to fix ctime.  The largest year
> value that can be represented in a 64-bit time-value is around 1
> trillion, and that will fit in 12 digits.  Isn't this trivial to fix?

No, because ctime is implemented in terms of localtime, localtime is
defined to use an 'int' to represent the year, and 'int' is only 32
bits on most hosts.  Fixing this would require some nontrivial
interface twiddling (presumably with a localtime variant).

> Fixing asctime as well, on systems where int is 64 bits, requires checking
> for year numbers that are too big.  But it is not hard to do.

Yes.  If the year fits into an 'int', then glibc ctime already does
the right thing, and outputs the full year without buffer overflow.
(Other C libraries crash, which causes the Emacs core dump.)

The problem is when the year does not fit into an 'int', as mentioned
above.  In this case glibc ctime reliably returns NULL, which is
certainly better than overrunning a buffer, but is not ideal.


Anyway, here's the revised fix for Emacs.

This fix attempts to avoid duplicated code, which I think was part of
the underlying objection to the earlier fix.  I am still reluctant to
use the system asctime and/or asctime_r to do the real work, since I
know of bugs on real-world systems with time values outside the
traditional range; so this patch still does its own implementation
instead of invoking asctime or asctime_r.  This substitute
implementation is only 15 lines of simple code, so I don't think it's
a big deal; but if you really want the code to invoke the system
asctime/asctime_r despite their bugs I can submit an alternate patch
that does that.


2006-03-19  Paul Eggert  <eggert@cs.ucla.edu>

	Do not use ctime, since it has undefined behavior with
	out-of-range time stamps.  This fixes a bug where
	(current-time-string '(2814749767106 0)) would make Emacs dump
	core on 64-bit Solaris 8; the fix is to remove all uses of ctime
	from the Emacs source code.  Please see
	<http://www.opengroup.org/austin/mailarchives/ag/msg09294.html>
	for more details about this portability problem.

	* src/ctime-r.h: New file.
	* lib-src/Makefile.in ($b2m${EXEEXT}, fakemail${EXEEXT}): Depend on it.
	* src/Makefile.in (editfns.o): Likewise.
	* lib-src/b2m.c: Include it.  Include <limits.h>, for CHAR_BIT.
	(main): Use emacs_ctime_r instead of ctime, to avoid buffer overflows
	with outlandish time stamps.  Report an error if an outlandish time
	stamp is discovered.
	* lib-src/fakemail.c: Likewise.
	* src/editfns.c: Include <limits.h>, for CHAR_BIT.
	Include ctime-r.h.
	(TM_YEAR_BASE): Move up, so changes can use it.
	(Fdecode_time, Fencode_time):
	Use TM_YEAR_BASE instead of hardwiring 1900.
	(Fdecode_time): Cast tm_year to EMACS_INT, to avoid overflow when
	int is narrow than EMACS_INT.
	(Fcurrent_time_string): Use emacs_ctime_r rather than ctime.
	As with Fformat_time_string, report an invalid time specification
	if the argument is invalid, and report an out-of-range time stamp
	if that problem occurs.  This is better than overrunning the buffer,
	and it preserves the historic behavior of always returning a
	fixed-size string.
	* lib-src/ntlib.c (sys_ctime): Remove, since Emacs never calls
	ctime any more.
	* lib-src/ntlib.h (ctime): Likewise.
	* src/w32.c (sys_ctime): Likewise.
	* src/s/ms-w32.h (ctime): Likewise.

*** /dev/null	Sat Sep 24 22:00:15 2005
--- src/ctime-r.h	Sun Mar 19 15:02:34 2006
***************
*** 0 ****
--- 1,56 ----
+ /* A safer replacement for ctime_r.
+ 
+    Copyright (C) 2006 Free Software Foundation, Inc.
+ 
+ This file is part of GNU Emacs.
+ 
+ GNU Emacs is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2, or (at your option)
+ any later version.
+ 
+ GNU Emacs is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU General Public License for more details.
+ 
+ You should have received a copy of the GNU General Public License
+ along with GNU Emacs; see the file COPYING.  If not, write to
+ the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ Boston, MA 02110-1301, USA.  */
+ 
+ #define TM_YEAR_BASE 1900
+ 
+ #define CTIME_R_BUFSIZE (sizeof "Sun Sep 16 01:03:52 1973")
+ 
+ /* A safer replacement for ctime_r.  This version reliably returns
+    NULL when given out-of-range values, instead of having undefined
+    behavior that possibly overruns buffers.  */
+ 
+ static char *
+ emacs_ctime_r (time_t const *t, char *buf)
+ {
+   struct tm const *tm = localtime (t);
+ 
+   if (tm
+       && -999 - TM_YEAR_BASE <= tm->tm_year
+       && tm->tm_year <= 9999 - TM_YEAR_BASE)
+     {
+       static char const wday[][4] =
+ 	{
+ 	  "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"
+ 	};
+       static char const mon[][4] =
+ 	{
+ 	  "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+ 	  "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
+ 	};
+       sprintf (buf, "%s %s %2d %02d:%02d:%02d %04d\n",
+ 	       wday[tm->tm_wday], mon[tm->tm_mon], tm->tm_mday,
+ 	       tm->tm_hour, tm->tm_min, tm->tm_sec,
+ 	       TM_YEAR_BASE + tm->tm_year);
+       return buf;
+     }
+ 
+   return NULL;
+ }
Index: lib-src/Makefile.in
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/Makefile.in,v
retrieving revision 1.148
diff -p -c -r1.148 Makefile.in
*** lib-src/Makefile.in	3 Mar 2006 12:02:31 -0000	1.148
--- lib-src/Makefile.in	19 Mar 2006 23:30:59 -0000
*************** digest-doc${EXEEXT}: ${srcdir}/digest-do
*** 430,436 ****
  sorted-doc${EXEEXT}: ${srcdir}/sorted-doc.c
  	$(CC) ${ALL_CFLAGS} ${srcdir}/sorted-doc.c $(LOADLIBES) -o sorted-doc
  
! b2m${EXEEXT}: ${srcdir}/b2m.c ../src/config.h $(GETOPTDEPS)
  	$(CC) ${ALL_CFLAGS} ${srcdir}/b2m.c  -DVERSION="\"${version}\"" \
  	   $(GETOPTOBJS) $(LOADLIBES) -o b2m
  
--- 430,437 ----
  sorted-doc${EXEEXT}: ${srcdir}/sorted-doc.c
  	$(CC) ${ALL_CFLAGS} ${srcdir}/sorted-doc.c $(LOADLIBES) -o sorted-doc
  
! b2m${EXEEXT}: ${srcdir}/b2m.c ${srcdir}/../src/ctime-r.h \
!   ../src/config.h $(GETOPTDEPS)
  	$(CC) ${ALL_CFLAGS} ${srcdir}/b2m.c  -DVERSION="\"${version}\"" \
  	   $(GETOPTOBJS) $(LOADLIBES) -o b2m
  
*************** pop.o: ${srcdir}/pop.c  ../src/config.h
*** 446,452 ****
  cvtmail${EXEEXT}: ${srcdir}/cvtmail.c
  	$(CC) ${ALL_CFLAGS} ${srcdir}/cvtmail.c $(LOADLIBES) -o cvtmail
  
! fakemail${EXEEXT}: ${srcdir}/fakemail.c ../src/config.h
  	$(CC) ${ALL_CFLAGS} ${srcdir}/fakemail.c $(LOADLIBES) -o fakemail
  
  yow${EXEEXT}: ${srcdir}/yow.c ../src/epaths.h
--- 447,454 ----
  cvtmail${EXEEXT}: ${srcdir}/cvtmail.c
  	$(CC) ${ALL_CFLAGS} ${srcdir}/cvtmail.c $(LOADLIBES) -o cvtmail
  
! fakemail${EXEEXT}: ${srcdir}/fakemail.c ${srcdir}/../src/ctime-r.h \
!   ../src/config.h
  	$(CC) ${ALL_CFLAGS} ${srcdir}/fakemail.c $(LOADLIBES) -o fakemail
  
  yow${EXEEXT}: ${srcdir}/yow.c ../src/epaths.h
Index: lib-src/b2m.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/b2m.c,v
retrieving revision 1.30
diff -p -c -r1.30 b2m.c
*** lib-src/b2m.c	7 May 2004 15:26:21 -0000	1.30
--- lib-src/b2m.c	19 Mar 2006 23:30:59 -0000
***************
*** 26,31 ****
--- 26,32 ----
  #undef static
  #endif
  
+ #include <limits.h>
  #include <stdio.h>
  #include <time.h>
  #include <sys/types.h>
***************
*** 34,39 ****
--- 35,42 ----
  #include <fcntl.h>
  #endif
  
+ #include "../src/ctime-r.h"
+ 
  #undef TRUE
  #define TRUE	1
  #undef FALSE
*************** main (argc, argv)
*** 89,94 ****
--- 92,98 ----
    time_t ltoday;
    char *labels, *p, *today;
    struct linebuffer data;
+   char buf[CTIME_R_BUFSIZE];
  
  #ifdef MSDOS
    _fmode = O_BINARY;		/* all of files are treated as binary files */
*************** main (argc, argv)
*** 131,137 ****
  
    labels_saved = printing = header = FALSE;
    ltoday = time (0);
!   today = ctime (&ltoday);
    data.size = 200;
    data.buffer = xnew (200, char);
  
--- 135,143 ----
  
    labels_saved = printing = header = FALSE;
    ltoday = time (0);
!   today = emacs_ctime_r (&ltoday, buf);
!   if (! today)
!     fatal ("current time is out of range");
    data.size = 200;
    data.buffer = xnew (200, char);
  
Index: lib-src/fakemail.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/fakemail.c,v
retrieving revision 1.35
diff -p -c -r1.35 fakemail.c
*** lib-src/fakemail.c	6 Feb 2006 11:28:28 -0000	1.35
--- lib-src/fakemail.c	19 Mar 2006 23:30:59 -0000
*************** main ()
*** 53,58 ****
--- 53,59 ----
  #include "ntlib.h"
  #endif
  
+ #include <limits.h>
  #include <stdio.h>
  #include <string.h>
  #include <ctype.h>
*************** main ()
*** 63,68 ****
--- 64,71 ----
  #ifdef HAVE_UNISTD_H
  #include <unistd.h>
  #endif
+ 
+ #include "../src/ctime-r.h"
  \f
  /* Type definitions */
  
*************** line_list
*** 353,359 ****
  make_file_preface ()
  {
    char *the_string, *temp;
!   long idiotic_interface;
    long prefix_length;
    long user_length;
    long date_length;
--- 356,363 ----
  make_file_preface ()
  {
    char *the_string, *temp;
!   char buf[CTIME_R_BUFSIZE];
!   time_t idiotic_interface;
    long prefix_length;
    long user_length;
    long date_length;
*************** make_file_preface ()
*** 361,367 ****
  
    prefix_length = strlen (FROM_PREFIX);
    time (&idiotic_interface);
!   the_date = ctime (&idiotic_interface);
    /* the_date has an unwanted newline at the end */
    date_length = strlen (the_date) - 1;
    the_date[date_length] = '\0';
--- 365,373 ----
  
    prefix_length = strlen (FROM_PREFIX);
    time (&idiotic_interface);
!   the_date = emacs_ctime_r (&idiotic_interface, buf);
!   if (! the_date)
!     fatal ("current time is out of range", 0);
    /* the_date has an unwanted newline at the end */
    date_length = strlen (the_date) - 1;
    the_date[date_length] = '\0';
Index: lib-src/ntlib.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/ntlib.c,v
retrieving revision 1.13
diff -p -c -r1.13 ntlib.c
*** lib-src/ntlib.c	6 Feb 2006 11:28:28 -0000	1.13
--- lib-src/ntlib.c	19 Mar 2006 23:31:00 -0000
*************** fchown (int fd, int uid, int gid)
*** 188,203 ****
    return 0;
  }
  
- /* Place a wrapper around the MSVC version of ctime.  It returns NULL
-    on network directories, so we handle that case here.
-    (Ulrich Leodolter, 1/11/95).  */
- char *
- sys_ctime (const time_t *t)
- {
-   char *str = (char *) ctime (t);
-   return (str ? str : "Sun Jan 01 00:00:00 1970");
- }
- 
  FILE *
  sys_fopen(const char * path, const char * mode)
  {
--- 188,193 ----
Index: lib-src/ntlib.h
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/ntlib.h,v
retrieving revision 1.11
diff -p -c -r1.11 ntlib.h
*** lib-src/ntlib.h	6 Feb 2006 11:28:28 -0000	1.11
--- lib-src/ntlib.h	19 Mar 2006 23:31:00 -0000
*************** int fchown (int fd, int uid, int gid);
*** 61,67 ****
  #define close   _close
  #undef creat
  #define creat   _creat
- #undef ctime
  #undef dup
  #define dup     _dup
  #undef dup2
--- 61,66 ----
Index: mac/inc/s-mac.h
===================================================================
RCS file: /cvsroot/emacs/emacs/mac/inc/s-mac.h,v
retrieving revision 1.10
diff -p -c -r1.10 s-mac.h
*** mac/inc/s-mac.h	6 Feb 2006 12:03:35 -0000	1.10
--- mac/inc/s-mac.h	19 Mar 2006 23:31:00 -0000
*************** Boston, MA 02110-1301, USA.  */
*** 293,299 ****
  
  #define gmtime sys_gmtime
  #define localtime sys_localtime
- #define ctime sys_ctime
  #define time sys_time
  
  #define index strchr
--- 293,298 ----
Index: src/Makefile.in
===================================================================
RCS file: /cvsroot/emacs/emacs/src/Makefile.in,v
retrieving revision 1.325
diff -p -c -r1.325 Makefile.in
*** src/Makefile.in	20 Feb 2006 22:16:00 -0000	1.325
--- src/Makefile.in	19 Mar 2006 23:31:00 -0000
*************** doc.o: doc.c $(config_h) epaths.h buffer
*** 1117,1124 ****
  doprnt.o: doprnt.c charset.h $(config_h)
  dosfns.o: buffer.h termchar.h termhooks.h frame.h blockinput.h window.h \
     msdos.h dosfns.h dispextern.h charset.h coding.h $(config_h)
! editfns.o: editfns.c window.h buffer.h systime.h $(INTERVAL_SRC) charset.h \
!    coding.h dispextern.h frame.h $(config_h)
  emacs.o: emacs.c commands.h systty.h syssignal.h blockinput.h process.h \
     termhooks.h buffer.h atimer.h systime.h $(INTERVAL_SRC) $(config_h) \
     window.h dispextern.h keyboard.h keymap.h
--- 1117,1124 ----
  doprnt.o: doprnt.c charset.h $(config_h)
  dosfns.o: buffer.h termchar.h termhooks.h frame.h blockinput.h window.h \
     msdos.h dosfns.h dispextern.h charset.h coding.h $(config_h)
! editfns.o: editfns.c ctime-r.h window.h buffer.h systime.h $(INTERVAL_SRC) \
!    charset.h coding.h dispextern.h frame.h $(config_h)
  emacs.o: emacs.c commands.h systty.h syssignal.h blockinput.h process.h \
     termhooks.h buffer.h atimer.h systime.h $(INTERVAL_SRC) $(config_h) \
     window.h dispextern.h keyboard.h keymap.h
Index: src/editfns.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/editfns.c,v
retrieving revision 1.409
diff -p -c -r1.409 editfns.c
*** src/editfns.c	7 Feb 2006 09:08:53 -0000	1.409
--- src/editfns.c	19 Mar 2006 23:31:00 -0000
*************** Boston, MA 02110-1301, USA.  */
*** 49,54 ****
--- 49,55 ----
  #endif
  
  #include <ctype.h>
+ #include <limits.h>
  
  #include "intervals.h"
  #include "buffer.h"
*************** Boston, MA 02110-1301, USA.  */
*** 64,69 ****
--- 65,72 ----
  #define MAX_10_EXP	310
  #endif
  
+ #include "ctime-r.h"
+ 
  #ifndef NULL
  #define NULL 0
  #endif
*************** Boston, MA 02110-1301, USA.  */
*** 72,77 ****
--- 75,82 ----
  extern char **environ;
  #endif
  
+ #define TM_YEAR_BASE 1900
+ 
  extern size_t emacs_strftimeu P_ ((char *, size_t, const char *,
  				   const struct tm *, int));
  static int tm_diff P_ ((struct tm *, struct tm *));
*************** DOW and ZONE.)  */)
*** 1722,1728 ****
    XSETFASTINT (list_args[2], decoded_time->tm_hour);
    XSETFASTINT (list_args[3], decoded_time->tm_mday);
    XSETFASTINT (list_args[4], decoded_time->tm_mon + 1);
!   XSETINT (list_args[5], decoded_time->tm_year + 1900);
    XSETFASTINT (list_args[6], decoded_time->tm_wday);
    list_args[7] = (decoded_time->tm_isdst)? Qt : Qnil;
  
--- 1727,1733 ----
    XSETFASTINT (list_args[2], decoded_time->tm_hour);
    XSETFASTINT (list_args[3], decoded_time->tm_mday);
    XSETFASTINT (list_args[4], decoded_time->tm_mon + 1);
!   XSETINT (list_args[5], TM_YEAR_BASE + (EMACS_INT) decoded_time->tm_year);
    XSETFASTINT (list_args[6], decoded_time->tm_wday);
    list_args[7] = (decoded_time->tm_isdst)? Qt : Qnil;
  
*************** usage: (encode-time SECOND MINUTE HOUR D
*** 1778,1784 ****
    tm.tm_hour = XINT (args[2]);
    tm.tm_mday = XINT (args[3]);
    tm.tm_mon = XINT (args[4]) - 1;
!   tm.tm_year = XINT (args[5]) - 1900;
    tm.tm_isdst = -1;
  
    if (CONSP (zone))
--- 1783,1789 ----
    tm.tm_hour = XINT (args[2]);
    tm.tm_mday = XINT (args[3]);
    tm.tm_mon = XINT (args[4]) - 1;
!   tm.tm_year = XINT (args[5]) - TM_YEAR_BASE;
    tm.tm_isdst = -1;
  
    if (CONSP (zone))
*************** but this is considered obsolete.  */)
*** 1843,1863 ****
       Lisp_Object specified_time;
  {
    time_t value;
!   char buf[30];
!   register char *tem;
  
    if (! lisp_time_argument (specified_time, &value, NULL))
!     value = -1;
!   tem = (char *) ctime (&value);
! 
!   strncpy (buf, tem, 24);
    buf[24] = 0;
  
    return build_string (buf);
  }
  
- #define TM_YEAR_BASE 1900
- 
  /* Yield A - B, measured in seconds.
     This function is copied from the GNU C Library.  */
  static int
--- 1848,1864 ----
       Lisp_Object specified_time;
  {
    time_t value;
!   char buf[CTIME_R_BUFSIZE];
  
    if (! lisp_time_argument (specified_time, &value, NULL))
!     error ("Invalid time specification");
!   if (! emacs_ctime_r (&value, buf))
!     error ("Specified time is not representable");
    buf[24] = 0;
  
    return build_string (buf);
  }
  
  /* Yield A - B, measured in seconds.
     This function is copied from the GNU C Library.  */
  static int
Index: src/mac.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/mac.c,v
retrieving revision 1.55
diff -p -c -r1.55 mac.c
*** src/mac.c	12 Mar 2006 08:19:42 -0000	1.55
--- src/mac.c	19 Mar 2006 23:31:00 -0000
*************** sys_localtime (const time_t *timer)
*** 2500,2520 ****
  }
  
  
- #undef ctime
- extern char *ctime (const time_t *);
- char *
- sys_ctime (const time_t *timer)
- {
- #if __MSL__ >= 0x6000
-   time_t unix_time = *timer;
- #else
-   time_t unix_time = *timer + CW_OR_MPW_UNIX_EPOCH_DIFF;
- #endif
- 
-   return ctime (&unix_time);
- }
- 
- 
  #undef time
  extern time_t time (time_t *);
  time_t
--- 2500,2505 ----
Index: src/w32.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/w32.c,v
retrieving revision 1.100
diff -p -c -r1.100 w32.c
*** src/w32.c	27 Feb 2006 02:07:37 -0000	1.100
--- src/w32.c	19 Mar 2006 23:31:00 -0000
*************** Boston, MA 02110-1301, USA.
*** 43,49 ****
  #undef chdir
  #undef chmod
  #undef creat
- #undef ctime
  #undef fopen
  #undef link
  #undef mkdir
--- 43,48 ----
*************** gettimeofday (struct timeval *tv, struct
*** 1325,1340 ****
  /* IO support and wrapper functions for W32 API. */
  /* ------------------------------------------------------------------------- */
  
- /* Place a wrapper around the MSVC version of ctime.  It returns NULL
-    on network directories, so we handle that case here.
-    (Ulrich Leodolter, 1/11/95).  */
- char *
- sys_ctime (const time_t *t)
- {
-   char *str = (char *) ctime (t);
-   return (str ? str : "Sun Jan 01 00:00:00 1970");
- }
- 
  /* Emulate sleep...we could have done this with a define, but that
     would necessitate including windows.h in the files that used it.
     This is much easier.  */
--- 1324,1329 ----
Index: src/s/ms-w32.h
===================================================================
RCS file: /cvsroot/emacs/emacs/src/s/ms-w32.h,v
retrieving revision 1.37
diff -p -c -r1.37 ms-w32.h
*** src/s/ms-w32.h	6 Feb 2006 15:23:23 -0000	1.37
--- src/s/ms-w32.h	19 Mar 2006 23:31:00 -0000
*************** Boston, MA 02110-1301, USA.  */
*** 317,323 ****
  #define close   sys_close
  #undef creat
  #define creat   sys_creat
- #define ctime	sys_ctime
  #undef dup
  #define dup     sys_dup
  #undef dup2
--- 317,322 ----

Re: Emacs current-time-string core dump on 64-bit hosts

[Eli Zaretskii]

> From: Paul Eggert <eggert@CS.UCLA.EDU>
> Date: Sun, 19 Mar 2006 15:44:25 -0800
> Cc: bug-gnu-emacs@gnu.org
> 
> 	* lib-src/ntlib.c (sys_ctime): Remove, since Emacs never calls
> 	ctime any more.
> 	* lib-src/ntlib.h (ctime): Likewise.
> 	* src/w32.c (sys_ctime): Likewise.
> 	* src/s/ms-w32.h (ctime): Likewise.

Please don't remove these from the w32 files (I explained earlier
why).

Thanks.

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

    > I think it is possible, even trivial, to fix ctime.  The largest year
    > value that can be represented in a 64-bit time-value is around 1
    > trillion, and that will fit in 12 digits.  Isn't this trivial to fix?

    No, because ctime is implemented in terms of localtime, localtime is
    defined to use an 'int' to represent the year, and 'int' is only 32
    bits on most hosts.

I do not follow you.  If conversion to `int' truncates the year, that
can cause incorrect results, but it can't cause the results to be any
further out of range.  The year in `ctime' will still fit in 12 digits.

Therefore, I still claim that allowing 12 digits for the year in
`ctime's output string will fix the problem of crashes.

However, it can't hurt to also change Emacs to reject extreme
values, until most systems have fixed their `ctime' functions.


The patch you've sent is unnecessarily complex.  The programs
in lib-src can call `ctime' safely because their argument
is the current time.  Only editfns.c needs to be changed.
Would you please put the new code directly into editfns.c?
That would avoid the need for new files, makefile changes, etc.

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

    In the meantime our best bet by far is to tell people this simple rule:

      Don't use ctime.

    Such a rule is much easier to remember than this one:

      Don't use ctime unless you have sufficient control over its argument
      time stamps to know that they won't generate more than 26 bytes when
      one naively applies the algorithm on page 341 of the 1999 edition of
      the C Standard.

I'd prefer just to fix ctime, using the method I told you.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

> Would you please put the new code directly into editfns.c?
> That would avoid the need for new files, makefile changes, etc.

OK, I've done this, and installed the following patch.  This patch is
simpler than what I submitted earlier, since it uses asctime rather
than formatting by hand.

2006-03-24  Paul Eggert  <eggert@cs.ucla.edu>

	* editfns.c: Do not use ctime, since it has undefined behavior
	with out-of-range time stamps.  This fixes a bug where
	(current-time-string '(2814749767106 0)) would make Emacs dump
	core on 64-bit Solaris 8.  The fix is to use localtime+asctime
	(checking for in-range results) instead of ctime.  Please see
	<http://www.opengroup.org/austin/mailarchives/ag/msg09294.html>
	for more details about this portability problem.
	(TM_YEAR_BASE): Move up, so the changes below can use it.
	(Fdecode_time, Fencode_time): Use TM_YEAR_BASE instead of 1900.
	(Fdecode_time): Cast tm_year to EMACS_INT, to avoid overflow when
	int is narrower than EMACS_INT.
	(Fcurrent_time_string): As with Fformat_time_string, report an
	invalid time specification if the argument is invalid.  Also,
	check for out-of-range time stamps; this prevents a buffer overrun
	that causes Emacs to dump core on 64-bit Solaris sparc, and it
	preserves the historic behavior of always returning a fixed-size
	string.

*** editfns.c	7 Feb 2006 09:08:53 -0000	1.409
--- editfns.c	24 Mar 2006 20:40:24 -0000	1.410
*************** Boston, MA 02110-1301, USA.  */
*** 72,77 ****
--- 72,79 ----
  extern char **environ;
  #endif
  
+ #define TM_YEAR_BASE 1900
+ 
  extern size_t emacs_strftimeu P_ ((char *, size_t, const char *,
  				   const struct tm *, int));
  static int tm_diff P_ ((struct tm *, struct tm *));
*************** DOW and ZONE.)  */)
*** 1722,1728 ****
    XSETFASTINT (list_args[2], decoded_time->tm_hour);
    XSETFASTINT (list_args[3], decoded_time->tm_mday);
    XSETFASTINT (list_args[4], decoded_time->tm_mon + 1);
!   XSETINT (list_args[5], decoded_time->tm_year + 1900);
    XSETFASTINT (list_args[6], decoded_time->tm_wday);
    list_args[7] = (decoded_time->tm_isdst)? Qt : Qnil;
  
--- 1724,1730 ----
    XSETFASTINT (list_args[2], decoded_time->tm_hour);
    XSETFASTINT (list_args[3], decoded_time->tm_mday);
    XSETFASTINT (list_args[4], decoded_time->tm_mon + 1);
!   XSETINT (list_args[5], TM_YEAR_BASE + (EMACS_INT) decoded_time->tm_year);
    XSETFASTINT (list_args[6], decoded_time->tm_wday);
    list_args[7] = (decoded_time->tm_isdst)? Qt : Qnil;
  
*************** usage: (encode-time SECOND MINUTE HOUR D
*** 1778,1784 ****
    tm.tm_hour = XINT (args[2]);
    tm.tm_mday = XINT (args[3]);
    tm.tm_mon = XINT (args[4]) - 1;
!   tm.tm_year = XINT (args[5]) - 1900;
    tm.tm_isdst = -1;
  
    if (CONSP (zone))
--- 1780,1786 ----
    tm.tm_hour = XINT (args[2]);
    tm.tm_mday = XINT (args[3]);
    tm.tm_mon = XINT (args[4]) - 1;
!   tm.tm_year = XINT (args[5]) - TM_YEAR_BASE;
    tm.tm_isdst = -1;
  
    if (CONSP (zone))
*************** but this is considered obsolete.  */)
*** 1844,1854 ****
  {
    time_t value;
    char buf[30];
    register char *tem;
  
    if (! lisp_time_argument (specified_time, &value, NULL))
!     value = -1;
!   tem = (char *) ctime (&value);
  
    strncpy (buf, tem, 24);
    buf[24] = 0;
--- 1846,1861 ----
  {
    time_t value;
    char buf[30];
+   struct tm *tm;
    register char *tem;
  
    if (! lisp_time_argument (specified_time, &value, NULL))
!     error ("Invalid time specification");
!   tm = localtime (&value);
!   if (! (tm && -999 - TM_YEAR_BASE <= tm->tm_year
! 	 && tm->tm_year <= 9999 - TM_YEAR_BASE))
!     error ("Specified time is not representable");
!   tem = asctime (tm);
  
    strncpy (buf, tem, 24);
    buf[24] = 0;
*************** but this is considered obsolete.  */)
*** 1856,1863 ****
    return build_string (buf);
  }
  
- #define TM_YEAR_BASE 1900
- 
  /* Yield A - B, measured in seconds.
     This function is copied from the GNU C Library.  */
  static int
--- 1863,1868 ----

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

>     > I think it is possible, even trivial, to fix ctime.  The largest year
>     > value that can be represented in a 64-bit time-value is around 1
>     > trillion, and that will fit in 12 digits.  Isn't this trivial to fix?
>
>     No, because ctime is implemented in terms of localtime, localtime is
>     defined to use an 'int' to represent the year, and 'int' is only 32
>     bits on most hosts.
>
> I do not follow you.  If conversion to `int' truncates the year, that
> can cause incorrect results, but it can't cause the results to be any
> further out of range.  The year in `ctime' will still fit in 12 digits.

True.

I had interpreted "fix ctime" to mean "ctime must always return a
string that contains the correct year, month, day, etc., assuming the
Gregorian calendar with no leap seconds".  But by "fix ctime" you
merely meant "ctime must never crash".  In that case, yes, a fix is
trivial, and the GNU C library's ctime implementation already does
that: its ctime always returns NULL (setting errno) with out-of-range
inputs.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

> The programs in lib-src can call `ctime' safely because their
> argument is the current time.

But the current time might be out of ctime's range for many reasons:

  * a hardware problem (the hardware clock jumped far in the future or past)
  * a software problem (a bug in the 'date' program's date parser, say)
  * a human error (someone set the date incorrectly).

All three of these things have happened to me.  Admittedly I deal with
time stamps a lot, but I expect similar problems do occur to others.

How about these changes to the lib-src programs instead?  They use
asctime instead of ctime, so the changes are trivial (four lines of
code each).

2006-03-24  Paul Eggert  <eggert@cs.ucla.edu>

	* b2m.c (main): Use localtime and asctime instead of ctime,
	and sanity-check localtime's results; this avoids a buffer
	overrun and/or dereferenced null pointer if the current time
	is out of range.
	* fakemail.c (make_file_preface): Likewise.

*** lib-src/b2m.c	7 May 2004 15:26:21 -0000	1.30
--- lib-src/b2m.c	24 Mar 2006 21:03:55 -0000
*************** main (argc, argv)
*** 87,92 ****
--- 87,93 ----
  {
    logical labels_saved, printing, header;
    time_t ltoday;
+   struct tm *tm;
    char *labels, *p, *today;
    struct linebuffer data;
  
*************** main (argc, argv)
*** 131,137 ****
  
    labels_saved = printing = header = FALSE;
    ltoday = time (0);
!   today = ctime (&ltoday);
    data.size = 200;
    data.buffer = xnew (200, char);
  
--- 132,141 ----
  
    labels_saved = printing = header = FALSE;
    ltoday = time (0);
!   tm = localtime (&ltoday);
!   if (! (tm && -999 - 1900 <= tm->tm_year && tm->tm_year <= 9999 - 1900))
!     fatal ("current time is out of range");
!   today = asctime (tm);
    data.size = 200;
    data.buffer = xnew (200, char);
  
*** lib-src/fakemail.c	6 Feb 2006 11:28:28 -0000	1.35
--- lib-src/fakemail.c	24 Mar 2006 21:03:55 -0000
*************** make_file_preface ()
*** 354,359 ****
--- 354,360 ----
  {
    char *the_string, *temp;
    long idiotic_interface;
+   struct tm *tm;
    long prefix_length;
    long user_length;
    long date_length;
*************** make_file_preface ()
*** 361,367 ****
  
    prefix_length = strlen (FROM_PREFIX);
    time (&idiotic_interface);
!   the_date = ctime (&idiotic_interface);
    /* the_date has an unwanted newline at the end */
    date_length = strlen (the_date) - 1;
    the_date[date_length] = '\0';
--- 362,371 ----
  
    prefix_length = strlen (FROM_PREFIX);
    time (&idiotic_interface);
!   tm = localtime (&idiotic_interface);
!   if (! (tm && -999 - 1900 <= tm->tm_year && tm->tm_year <= 9999 - 1900))
!     fatal ("current time is out of range", 0);
!   the_date = asctime (tm);
    /* the_date has an unwanted newline at the end */
    date_length = strlen (the_date) - 1;
    the_date[date_length] = '\0';

Re: Emacs current-time-string core dump on 64-bit hosts

[Eli Zaretskii]

> From: Paul Eggert <eggert@CS.UCLA.EDU>
> Date: Fri, 24 Mar 2006 12:45:02 -0800
> Cc: bug-gnu-emacs@gnu.org
> 
> Richard Stallman <rms@gnu.org> writes:
> 
> > Would you please put the new code directly into editfns.c?
> > That would avoid the need for new files, makefile changes, etc.
> 
> OK, I've done this, and installed the following patch.  This patch is
> simpler than what I submitted earlier, since it uses asctime rather
> than formatting by hand.

Thanks.

> 2006-03-24  Paul Eggert  <eggert@cs.ucla.edu>
> 
> 	* editfns.c: Do not use ctime, since it has undefined behavior
> 	with out-of-range time stamps.  This fixes a bug where
> 	(current-time-string '(2814749767106 0)) would make Emacs dump
> 	core on 64-bit Solaris 8.  The fix is to use localtime+asctime
> 	(checking for in-range results) instead of ctime.  Please see
> 	<http://www.opengroup.org/austin/mailarchives/ag/msg09294.html>
> 	for more details about this portability problem.
> 	(TM_YEAR_BASE): Move up, so the changes below can use it.
> 	(Fdecode_time, Fencode_time): Use TM_YEAR_BASE instead of 1900.
> 	(Fdecode_time): Cast tm_year to EMACS_INT, to avoid overflow when
> 	int is narrower than EMACS_INT.
> 	(Fcurrent_time_string): As with Fformat_time_string, report an
> 	invalid time specification if the argument is invalid.  Also,
> 	check for out-of-range time stamps; this prevents a buffer overrun
> 	that causes Emacs to dump core on 64-bit Solaris sparc, and it
> 	preserves the historic behavior of always returning a fixed-size
> 	string.

I took the liberty of moving the explanations in these entries to the
respective parts of the source.  I think such explanations belong in
the source, not in the logs.

>    if (! lisp_time_argument (specified_time, &value, NULL))
> -    value = -1;
> -  tem = (char *) ctime (&value);
> +    error ("Invalid time specification");
> +  tm = localtime (&value);
> +  if (! (tm && -999 - TM_YEAR_BASE <= tm->tm_year
> +	 && tm->tm_year <= 9999 - TM_YEAR_BASE))
> +    error ("Specified time is not representable");
> +  tem = asctime (tm);

I'm a bit worried about this change: previously current-time-string
never threw an error, while now it will.  In particular, invalid
values of the optional argument will now cause incompatible behavior:
where previously current-time-string would pass -1 to ctime, typically
causing "(null)" be the result, it now signals an error.  Wouldn't it
be better to simply produce some telltale string instead, and not
throw an error?

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

Please install this change.

But please add a comment saying that in a few years, when people
have had a chance to fix the ctime calls on modern systems
so that they won't crash and will handle 64 bit time values,
we should change it back to use ctime or at least remove the limits.

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

    But the current time might be out of ctime's range for many reasons:

      * a hardware problem (the hardware clock jumped far in the future or past)
      * a software problem (a bug in the 'date' program's date parser, say)
      * a human error (someone set the date incorrectly).

It seems unlikely to me that these things will be able to set the system's time
to a value the system can't handle.  More likely the time will get truncated
into the range the system can handle.  For instance, if someone specifies
an out-of-range date, it will probably be truncated into a reasonable date.
It will just be incorrect.

Meanwhile, the system will probably crash due to demons that use ctime.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Eli Zaretskii <eliz@gnu.org> writes:

> previously current-time-string never threw an error, while now it
> will.

Hmm, well, it'd be more precise to say something like this:

   Previously with out-of-range time stamps, current-time-string
   sometimes dumped core and sometimes had other (somewhat random)
   behavior.  Now it reliably throws an error.

I don't think this change in behavior will be a real problem in
practice.  The only time stamps affected are those before the year
-999 or after the year 9999.  Users with old programs won't
expect such time stamps to work, since historically those time stamps
never worked on 32-bit systems.


> In particular, invalid values of the optional argument will now
> cause incompatible behavior: where previously current-time-string
> would pass -1 to ctime, typically causing "(null)" be the result, it
> now signals an error.

Actually the typical result for -1 was probably more like "Wed Dec 31
23:59:59 1969", adjusted for the time zone.  However, strictly
speaking this was relying on undefined behavior, and implementations
were free to generate "(null)" or any other string they pleased, or to
overrun buffers for that matter.  (Admittedly I don't know of any that
did these awful things.)


> Wouldn't it be better to simply produce some telltale string
> instead, and not throw an error?

float-time, format-time-string, and decode-time all throw an error
when the time isn't representable, so there's good precedent for
current-time-string doing the same thing.  If we were to take the
telltale-string route, then we should probably modify float-time
etc. to be consistent.  I'd rather leave things be, though.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

> It seems unlikely to me that these things will be able to set the
> system's time to a value the system can't handle.  More likely the
> time will get truncated into the range the system can handle.

But I was able to temporarily set the time on a 64-bit Solaris 9 sparc
host (an UltraAX-i2 -- a several-year-old unit) to a date in the year
10001, using 64-bit GNU 'date' atop Solaris libc:

   # date -s '10001-01-01 00:00:00'; date
   Mon Jan  1 00:00:00 PST 10001
   Mon Jan  1 00:00:00 PST 10001

So there's at least one example of a host where the current time of
day can crash the 'ctime' implementation.  Admittedly, within a few
seconds the date automatically changed (to something that presumably
the time-of-day clock could represent):

   # sleep 2; date
   Sun Nov  2 00:00:01 PST 2064

However, I wouldn't be surprised if other hosts kept ticking right
along in the year 10001, even if it makes their 'ctime' crash.  The
kernel's time-of-day interface typically doesn't rely on ctime, so the
kernel's limits are unlikely to be the same as ctime's.

> Meanwhile, the system will probably crash due to demons that use ctime.

No daemons crashed in the above exercise.  I think few daemons use
ctime nowadays.  Even if some do, the overall system will probably
remain usable.

How about the following patch instead?  This one is like the earlier
one I proposed, but the code includes some comments about this stuff.

2006-03-25  Paul Eggert  <eggert@cs.ucla.edu>

	* b2m.c (main): Use localtime and asctime instead of ctime,
	and sanity-check localtime's results; this avoids a buffer
	overrun and/or dereferenced null pointer if the current time
	is out of range.
	* fakemail.c (make_file_preface): Likewise.

*** lib-src/b2m.c	7 May 2004 15:26:21 -0000	1.30
--- lib-src/b2m.c	26 Mar 2006 07:22:14 -0000
*************** main (argc, argv)
*** 87,92 ****
--- 87,93 ----
  {
    logical labels_saved, printing, header;
    time_t ltoday;
+   struct tm *tm;
    char *labels, *p, *today;
    struct linebuffer data;
  
*************** main (argc, argv)
*** 131,137 ****
  
    labels_saved = printing = header = FALSE;
    ltoday = time (0);
!   today = ctime (&ltoday);
    data.size = 200;
    data.buffer = xnew (200, char);
  
--- 132,143 ----
  
    labels_saved = printing = header = FALSE;
    ltoday = time (0);
!   tm = localtime (&ltoday);
!   /* Check for out-of-range dates.  Don't use 'ctime', as that might
!      dump core if the hardware clock is set to a bizarre value.  */
!   if (! (tm && -999 - 1900 <= tm->tm_year && tm->tm_year <= 9999 - 1900))
!     fatal ("current time is out of range");
!   today = asctime (tm);
    data.size = 200;
    data.buffer = xnew (200, char);
  
*** lib-src/fakemail.c	6 Feb 2006 11:28:28 -0000	1.35
--- lib-src/fakemail.c	26 Mar 2006 07:22:14 -0000
*************** make_file_preface ()
*** 354,359 ****
--- 354,360 ----
  {
    char *the_string, *temp;
    long idiotic_interface;
+   struct tm *tm;
    long prefix_length;
    long user_length;
    long date_length;
*************** make_file_preface ()
*** 361,367 ****
  
    prefix_length = strlen (FROM_PREFIX);
    time (&idiotic_interface);
!   the_date = ctime (&idiotic_interface);
    /* the_date has an unwanted newline at the end */
    date_length = strlen (the_date) - 1;
    the_date[date_length] = '\0';
--- 362,373 ----
  
    prefix_length = strlen (FROM_PREFIX);
    time (&idiotic_interface);
!   tm = localtime (&idiotic_interface);
!   /* Check for out-of-range dates.  Don't use 'ctime', as that might
!      dump core if the hardware clock is set to a bizarre value.  */
!   if (! (tm && -999 - 1900 <= tm->tm_year && tm->tm_year <= 9999 - 1900))
!     fatal ("current time is out of range", 0);
!   the_date = asctime (tm);
    /* the_date has an unwanted newline at the end */
    date_length = strlen (the_date) - 1;
    the_date[date_length] = '\0';

Re: Emacs current-time-string core dump on 64-bit hosts

[Eli Zaretskii]

> Cc: bug-gnu-emacs@gnu.org
> From: Paul Eggert <eggert@CS.UCLA.EDU>
> Date: Sat, 25 Mar 2006 21:25:39 -0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > previously current-time-string never threw an error, while now it
> > will.
> 
> Hmm, well, it'd be more precise to say something like this:
> 
>    Previously with out-of-range time stamps, current-time-string
>    sometimes dumped core and sometimes had other (somewhat random)
>    behavior.  Now it reliably throws an error.

The core dumps were not mentioned in the documentation, so I doubt
that some code was relying on that.  OTOH, it's quite possible that
some code _does_ rely on the fact that it never threw an error.

> I don't think this change in behavior will be a real problem in
> practice.  The only time stamps affected are those before the year
> -999 or after the year 9999.  Users with old programs won't
> expect such time stamps to work, since historically those time stamps
> never worked on 32-bit systems.

I don't like incompatible changes so close to release, but I'll let
Richard decide.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

> Ok, you can install this change.
>
> However, we don't want any of these changes to be operative on the GNU
> system because all they do is cause trouble.  On the GNU system, large
> year numbers work.  Why interfere with their use?
>
> So I think you should conditionalize them.

OK, I conditionalized them, and installed the following revised change.

2006-03-27  Paul Eggert  <eggert@cs.ucla.edu>

	* lib-src/b2m.c: Include <limits.h>.
	(TM_YEAR_IN_ASCTIME_RANGE): New macro.
	(main): Check for out-of-range time stamps.
	* lib-src/fakemail.c: Likewise.

--- b2m.c	7 May 2004 15:26:21 -0000	1.30
+++ b2m.c	27 Mar 2006 20:34:18 -0000
@@ -26,6 +26,7 @@
 #undef static
 #endif
 
+#include <limits.h>
 #include <stdio.h>
 #include <time.h>
 #include <sys/types.h>
@@ -44,6 +45,17 @@
 
 typedef int logical;
 
+/* True if TM_YEAR is a struct tm's tm_year value that is acceptable
+   to asctime.  Glibc asctime returns a useful string unless TM_YEAR
+   is nearly INT_MAX, but the C Standard lets C libraries overrun a
+   buffer if TM_YEAR needs more than 4 bytes.  */
+#ifdef __GLIBC__
+# define TM_YEAR_IN_ASCTIME_RANGE(tm_year) ((tm_year) <= INT_MAX - 1900)
+#else
+# define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
+    (-999 - 1900 <= (tm_year) && (tm_year) <= 9999 - 1900)
+#endif
+
 /*
  * A `struct linebuffer' is a structure which holds a line of text.
  * `readline' reads a line from a stream into a linebuffer and works
@@ -87,6 +99,7 @@ main (argc, argv)
 {
   logical labels_saved, printing, header;
   time_t ltoday;
+  struct tm *tm;
   char *labels, *p, *today;
   struct linebuffer data;
 
@@ -131,7 +144,13 @@ main (argc, argv)
 
   labels_saved = printing = header = FALSE;
   ltoday = time (0);
-  today = ctime (&ltoday);
+  /* Convert to a string, checking for out-of-range time stamps.
+     Don't use 'ctime', as that might dump core if the hardware clock
+     is set to a bizarre value.  */
+  tm = localtime (&ltoday);
+  if (! (tm && TM_YEAR_IN_ASCTIME_RANGE (tm->tm_year)))
+    fatal ("current time is out of range");
+  today = asctime (tm);
   data.size = 200;
   data.buffer = xnew (200, char);
 
--- fakemail.c	6 Feb 2006 11:28:28 -0000	1.35
+++ fakemail.c	27 Mar 2006 20:34:18 -0000
@@ -53,6 +53,7 @@ main ()
 #include "ntlib.h"
 #endif
 
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 #include <ctype.h>
@@ -70,6 +71,17 @@ main ()
 #define true 1
 #define false 0
 
+/* True if TM_YEAR is a struct tm's tm_year value that is acceptable
+   to asctime.  Glibc asctime returns a useful string unless TM_YEAR
+   is nearly INT_MAX, but the C Standard lets C libraries overrun a
+   buffer if TM_YEAR needs more than 4 bytes.  */
+#ifdef __GLIBC__
+# define TM_YEAR_IN_ASCTIME_RANGE(tm_year) ((tm_year) <= INT_MAX - 1900)
+#else
+# define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
+    (-999 - 1900 <= (tm_year) && (tm_year) <= 9999 - 1900)
+#endif
+
 /* Various lists */
 
 struct line_record
@@ -354,6 +366,7 @@ make_file_preface ()
 {
   char *the_string, *temp;
   long idiotic_interface;
+  struct tm *tm;
   long prefix_length;
   long user_length;
   long date_length;
@@ -361,7 +374,13 @@ make_file_preface ()
 
   prefix_length = strlen (FROM_PREFIX);
   time (&idiotic_interface);
-  the_date = ctime (&idiotic_interface);
+  /* Convert to a string, checking for out-of-range time stamps.
+     Don't use 'ctime', as that might dump core if the hardware clock
+     is set to a bizarre value.  */
+  tm = localtime (&idiotic_interface);
+  if (! (tm && TM_YEAR_IN_ASCTIME_RANGE (tm->tm_year)))
+    fatal ("current time is out of range", 0);
+  the_date = asctime (tm);
   /* the_date has an unwanted newline at the end */
   date_length = strlen (the_date) - 1;
   the_date[date_length] = '\0';

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Paul Eggert <eggert@CS.UCLA.EDU>
>> Date: Sun, 19 Mar 2006 15:44:25 -0800
>> 
>> 	* lib-src/ntlib.c (sys_ctime): Remove, since Emacs never calls
>> 	ctime any more.
>> 	* lib-src/ntlib.h (ctime): Likewise.
>> 	* src/w32.c (sys_ctime): Likewise.
>> 	* src/s/ms-w32.h (ctime): Likewise.
>
> Please don't remove these from the w32 files (I explained earlier why).

I've left them alone but I'd like to follow up on this, since a
comment or two would be helpful even if we don't change the code.

Here's your earlier explanation:

   I wouldn't remove these: the functions are almost trivial wrappers
   around the library version, and someone could try using ctime in the
   future (in a different context), in which case they will hit the
   Windows library bug again.

But this explanation is problematic, at least in the context of an
Emacs that is intended to be portable to 64-bit hosts.  Here's why.
The ntlib.c and w32 wrappers do this:

  char *str = (char *) ctime (t);
  return (str ? str : "Sun Jan 01 00:00:00 1970");

That is, the wrappers cause 'ctime' to always return a non-NULL pointer.
But any future Emacs code that uses ctime on an arbitrary time stamp
cannot expect ctime to always return a non-null pointer.  Even glibc
ctime (a "nice" ctime, which always has well-defined behavior) returns
NULL in some cases, e.g., if given a time stamp equal to 2**56.

Any future Emacs code that attempts to use ctime on an arbitrary time
stamp and an arbitrary POSIX platform will have to do something like
this, to be safe:

  struct tm *tm = localtime (&timestamp);
  if (! (tm && -999 - 1900 <= tm->tm_year && tm->tm_year <= 9999 - 1900))
    fatal ("current time is out of range");
  p = asctime (tm);

Even if Emacs can assume a "nice" ctime (a big assumption), it'd still
have to do something like this:

  p = ctime (&timestamp);
  if (! p)
    fatal ("current time is out of range");

But notice that neither code snippet needs the w32-style wrapper; so
the wrapper can be safely removed.

I suppose Emacs code might in theory want to invoke ctime in a context
where the time stamp is known to be in range, presumably because the
equivalent of the abovementioned tests have already been done.  But
this is pretty unlikely, because if the tests have already been done,
then the code already has the translated time.

If these arguments convince you, then let's please install the
above-mentioned change to the w32 files, which removes the ctime
wrappers.  If not, then I suggest the following change instead.  It
inserts a comment that attempts to explain the current situation, as I
understand it.  It also corrects a minor problem with the wrappers:
they return "Sun Jan 01 00:00:00 1970" for out-of-range time stamps,
which is wrong for two reasons.  First, 1970-01-01 was a Thursday, not
a Sunday.  Second, the day of the month should be space-padded, not
zero-padded.

There is a similar issue for src/mac.c and mac/inc/s-mac.h but I'd
like to get the W32 issues out of the way first.

2006-03-27  Paul Eggert  <eggert@cs.ucla.edu>

	* lib-src/ntlib.c (sys_ctime): Add a comment explaining why this
	wrapper is still here, even though Emacs doesn't use it.
	Correct the substitute string from "Sun Jan 01 00:00:00 1970"
	to "Thu Jan  1 00:00:00 1970".
	* src/w32.c (sys_ctime): Likewise.

*** lib-src/ntlib.c	6 Feb 2006 11:28:28 -0000	1.13
--- lib-src/ntlib.c	27 Mar 2006 21:47:52 -0000
*************** fchown (int fd, int uid, int gid)
*** 190,201 ****
  
  /* Place a wrapper around the MSVC version of ctime.  It returns NULL
     on network directories, so we handle that case here.
!    (Ulrich Leodolter, 1/11/95).  */
  char *
  sys_ctime (const time_t *t)
  {
    char *str = (char *) ctime (t);
!   return (str ? str : "Sun Jan 01 00:00:00 1970");
  }
  
  FILE *
--- 190,208 ----
  
  /* Place a wrapper around the MSVC version of ctime.  It returns NULL
     on network directories, so we handle that case here.
!    (Ulrich Leodolter, 1/11/95).
! 
!    ctime has undefined behavior with out-of-range time stamps so Emacs
!    no longer uses it.  However, a future version of Emacs might find a
!    use for ctime, in a context where time stamps are known to be in a
!    safe range for POSIX (i.e., from 1970 through 9999), but not for NT
!    (1970 through 3000), so this wrapper has not been removed from the
!    Emacs source code.  */
  char *
  sys_ctime (const time_t *t)
  {
    char *str = (char *) ctime (t);
!   return (str ? str : "Thu Jan  1 00:00:00 1970");
  }
  
  FILE *
*** src/w32.c	27 Feb 2006 02:07:37 -0000	1.100
--- src/w32.c	27 Mar 2006 21:47:52 -0000
*************** gettimeofday (struct timeval *tv, struct
*** 1327,1338 ****
  
  /* Place a wrapper around the MSVC version of ctime.  It returns NULL
     on network directories, so we handle that case here.
!    (Ulrich Leodolter, 1/11/95).  */
  char *
  sys_ctime (const time_t *t)
  {
    char *str = (char *) ctime (t);
!   return (str ? str : "Sun Jan 01 00:00:00 1970");
  }
  
  /* Emulate sleep...we could have done this with a define, but that
--- 1327,1345 ----
  
  /* Place a wrapper around the MSVC version of ctime.  It returns NULL
     on network directories, so we handle that case here.
!    (Ulrich Leodolter, 1/11/95).
! 
!    ctime has undefined behavior with out-of-range time stamps so Emacs
!    no longer uses it.  However, a future version of Emacs might find a
!    use for ctime, in a context where time stamps are known to be in a
!    safe range for POSIX (i.e., from 1970 through 9999), but not for W32
!    (1970 through 3000), so this wrapper has not been removed from the
!    Emacs source code.  */
  char *
  sys_ctime (const time_t *t)
  {
    char *str = (char *) ctime (t);
!   return (str ? str : "Thu Jan  1 00:00:00 1970");
  }
  
  /* Emulate sleep...we could have done this with a define, but that

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

    The core dumps were not mentioned in the documentation, so I doubt
    that some code was relying on that.  OTOH, it's quite possible that
    some code _does_ rely on the fact that it never threw an error.

To signal an error in a kind of case which formerly could sometimes
crash does not seem like a problem to me.

Re: Emacs current-time-string core dump on 64-bit hosts

[Eli Zaretskii]

> Cc: bug-gnu-emacs@gnu.org
> From: Paul Eggert <eggert@CS.UCLA.EDU>
> Date: Mon, 27 Mar 2006 14:00:15 -0800
> 
> Here's your earlier explanation:
> 
>    I wouldn't remove these: the functions are almost trivial wrappers
>    around the library version, and someone could try using ctime in the
>    future (in a different context), in which case they will hit the
>    Windows library bug again.
> 
> But this explanation is problematic, at least in the context of an
> Emacs that is intended to be portable to 64-bit hosts.  Here's why.
> The ntlib.c and w32 wrappers do this:
> 
>   char *str = (char *) ctime (t);
>   return (str ? str : "Sun Jan 01 00:00:00 1970");
> 
> That is, the wrappers cause 'ctime' to always return a non-NULL pointer.
> But any future Emacs code that uses ctime on an arbitrary time stamp
> cannot expect ctime to always return a non-null pointer.  Even glibc
> ctime (a "nice" ctime, which always has well-defined behavior) returns
> NULL in some cases, e.g., if given a time stamp equal to 2**56.

The problem is not with invalid time stamps; please read the comment
before the w32 wrapper.

> Even if Emacs can assume a "nice" ctime (a big assumption), it'd still
> have to do something like this:
> 
>   p = ctime (&timestamp);
>   if (! p)
>     fatal ("current time is out of range");
> 
> But notice that neither code snippet needs the w32-style wrapper; so
> the wrapper can be safely removed.

Paul, please re-read the comment in w32.c: it says that ctime returns
NULL when the current directory is on a networked drive.  So this has
nothing to do with invalid time stamps, and throwing a fatal error in
such cases would be a very bad idea, IMO.

I agree that the wrapper conceals the cases where ctime returns NULL
due to invalid time stamp, which is unfortunate.  But until someone
comes up with a simple enough solution that would differentiate
between the networked directory case and the invalid time case, I
think the wrapper is the lesser evil, especially since ``Emacs no
longer uses ctime'' for producing time strings (and thus the invalid
time stamp case is from now on much less important when ctime is
used).

> If these arguments convince you, then let's please install the
> above-mentioned change to the w32 files, which removes the ctime
> wrappers.  If not, then I suggest the following change instead.  It
> inserts a comment that attempts to explain the current situation, as I
> understand it.

I don't mind adding a comment, modulo the remarks below.

> It also corrects a minor problem with the wrappers:
> they return "Sun Jan 01 00:00:00 1970" for out-of-range time stamps,
> which is wrong for two reasons.  First, 1970-01-01 was a Thursday, not
> a Sunday.  Second, the day of the month should be space-padded, not
> zero-padded.

I guess "Sun Jan 01" was meant to emulate a zeroed out struct tm, so
I'm not so sure it's a problem.  But I have no objections to these
changes.

>   /* Place a wrapper around the MSVC version of ctime.  It returns NULL
>      on network directories, so we handle that case here.
> !    (Ulrich Leodolter, 1/11/95).
> ! 
> !    ctime has undefined behavior with out-of-range time stamps so Emacs
> !    no longer uses it.  However, a future version of Emacs might find a
> !    use for ctime, in a context where time stamps are known to be in a
> !    safe range for POSIX (i.e., from 1970 through 9999), but not for NT
> !    (1970 through 3000), so this wrapper has not been removed from the
> !    Emacs source code.  */

I do have an issue with this comment: it gives an impression that the
main reason for leaving the wrappers in place is the different range
of valid time stamps between Posix and MS-Windows systems.  But the
_real_ reason is the case of the network directories that is mentioned
above.  So how about the following comment instead:

  /* Place a wrapper around the MSVC version of ctime.  It returns NULL
     on network directories, so we handle that case here.
!    (Ulrich Leodolter, 1/11/95).
! 
!    (ctime has undefined behavior with out-of-range time stamps so Emacs
!    no longer uses it.  However, a future version of Emacs might find a
!    use for ctime, in a context where time stamps are known to be in a
!    safe range, so this wrapper has not been removed from the Emacs
!    source code, to avoid tripping on the network directory case.)  */

Note that I placed the whole addition in parentheses, since it really
is a kind of footnote for the original comment, which states the real
reason for having the wrapper.

Re: Emacs current-time-string core dump on 64-bit hosts

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> CC: eggert@CS.UCLA.EDU, bug-gnu-emacs@gnu.org
> Date: Mon, 27 Mar 2006 17:29:24 -0500
> 
>     The core dumps were not mentioned in the documentation, so I doubt
>     that some code was relying on that.  OTOH, it's quite possible that
>     some code _does_ rely on the fact that it never threw an error.
> 
> To signal an error in a kind of case which formerly could sometimes
> crash does not seem like a problem to me.

Do you think this change of behavior is worth mentioning in NEWS?  How
about mentioning the possibility of the signal in the doc string?

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

    +/* True if TM_YEAR is a struct tm's tm_year value that is acceptable
    +   to asctime.  Glibc asctime returns a useful string unless TM_YEAR
    +   is nearly INT_MAX, but the C Standard lets C libraries overrun a
    +   buffer if TM_YEAR needs more than 4 bytes.  */
    +#ifdef __GLIBC__
    +# define TM_YEAR_IN_ASCTIME_RANGE(tm_year) ((tm_year) <= INT_MAX - 1900)

In the future, we will want the condition to change.
So I think there should be a separate macro to control this.

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

    Do you think this change of behavior is worth mentioning in NEWS?

No, it is a minor issue.  We do not even document which functions can
or cannot signal errors; it is not worth space in the manual.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Eli Zaretskii <eliz@gnu.org> writes:

> Paul, please re-read the comment in w32.c: it says that ctime returns
> NULL when the current directory is on a networked drive.

Ah, sorry, I misunderstood that comment.  I thought the comment meant
that if code did this:

   stat (".", &sb);
   p = ctime (&sb.st_mtime);

then p is NULL when "." is a networked directory.  But from what you
write, the comment actually means that, given code like this:

   t = time (NULL);   
   p = ctime (&t);

then p is NULL when "." is a networked directory.  Am I understanding
you correctly now?

Hmm, but if that's the case, why can't w32.c and ntlib.c use this
wrapper instead?

   char *
   sys_ctime (const time_t *t)
   {
     return asctime (localtime (t));
   }

Is it possible that the actual bug with networked drives is in
localtime, not in ctime?  If so, shouldn't localtime be wrapped?

But localtime is used in a bunch of places, so I'd be a bit surprised
if it needed to be wrapped.

Or perhaps asctime needs to be wrapped?  That would be really strange;
I can't imagine why that would be.

(Can you tell that I'm somewhat at sea when reasoning about the
behavior of Microsoft Windows?  :-)

Anyway, I think your last draft comment sounds reasonable, though I'd
like to understand the bug better before worrying about the details
there.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

>     +#ifdef __GLIBC__
>     +# define TM_YEAR_IN_ASCTIME_RANGE(tm_year) ((tm_year) <= INT_MAX - 1900)
>
> In the future, we will want the condition to change.
> So I think there should be a separate macro to control this.

Something like this, without the #ifdef?

  #define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
    (ASCTIME_YEAR_MIN <= (tm_year) && (tm_year) <= ASCTIME_YEAR_MAX)

We would have Autoconf deduce ASCTIME_YEAR_MIN and ASCTIME_YEAR_MAX by
trying to run little test programs that crash, using binary search to
deduce the min and max values.

Yes, something like that should work, though it'd take some testing to
make sure that the little test programs crash reliably.  (I assume
it's not that high of a priority, though.)

Re: Emacs current-time-string core dump on 64-bit hosts

[Eli Zaretskii]

> Cc: bug-gnu-emacs@gnu.org
> From: Paul Eggert <eggert@CS.UCLA.EDU>
> Date: Wed, 29 Mar 2006 23:52:31 -0800
> 
> from what you write, the comment actually means that, given code
> like this:
> 
>    t = time (NULL);   
>    p = ctime (&t);
> 
> then p is NULL when "." is a networked directory.  Am I understanding
> you correctly now?

Yes, that's what I meant.  But I cannot prove that my understanding is
correct, since all I have is the comment text, and not further
evidence.

> Hmm, but if that's the case, why can't w32.c and ntlib.c use this
> wrapper instead?
> 
>    char *
>    sys_ctime (const time_t *t)
>    {
>      return asctime (localtime (t));
>    }
> 
> Is it possible that the actual bug with networked drives is in
> localtime, not in ctime?  If so, shouldn't localtime be wrapped?
> 
> But localtime is used in a bunch of places, so I'd be a bit surprised
> if it needed to be wrapped.
> 
> Or perhaps asctime needs to be wrapped?  That would be really strange;
> I can't imagine why that would be.
> 
> (Can you tell that I'm somewhat at sea when reasoning about the
> behavior of Microsoft Windows?  :-)

We all are at sea there.

Is there some Windows guru here who could resolve the issue?

> Anyway, I think your last draft comment sounds reasonable, though I'd
> like to understand the bug better before worrying about the details
> there.

Yep, me too.  But if no one responds, I think we should do as best as
we understand it now.

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

    Something like this, without the #ifdef?

      #define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
	(ASCTIME_YEAR_MIN <= (tm_year) && (tm_year) <= ASCTIME_YEAR_MAX)

    We would have Autoconf deduce ASCTIME_YEAR_MIN and ASCTIME_YEAR_MAX by
    trying to run little test programs that crash, using binary search to
    deduce the min and max values.

We could do that, but it is undesirable for configure to actually
run a program--it interferes with cross-compilation.

So I would prefer if we just have conditionals set for certain systems
by the s/ file.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

> So I would prefer if we just have conditionals set for certain systems
> by the s/ file.

OK.

I raised this issue on the Open Group mailing lists, and just now got
the bad news that the C standardization committee intends to change
the spec for ctime and asctime, by allowing implementations to crash
if the year is outside the range 1000-9999.  So we'd need to change
Emacs anyway, to be portable to future C standards.

I'm a little rusty with s/ files, but I assume that we could use
something like this in a generic s/ file somewhere:

   /* Safe values the tm_year members of arguments passed to asctime.
      C89 and C99 say the range is -999-1900 through 9999-1900, but
      <http://www.opengroup.org/austin/mailarchives/ag/msg09383.html>
      says the C committee will change this to 1000-1900 through
      9999-1900, so use the more-conservative range here.  */
   #ifndef TM_YEAR_IN_ASCTIME_RANGE
   # define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
        (1000 - 1900 <= (tm_year) && (tm_year) <= 9999 - 1900)
   #endif

and then in s/gnu-linux.h and src/s/gnu.h we'd put this:

   #define TM_YEAR_IN_ASCTIME_RANGE(tm_year) ((tm_year) <= INT_MAX - 1900)

and similarly for any other platform where we know the actual range.
Then b2m.c, fakemail.c, and editfns.c can use TM_YEAR_IN_ASCTIME_RANGE.

Is that the sort of thing you have in mind?  If so, can you please
suggest where the generic definition of TM_YEAR_IN_ASCTIME_RANGE
should go?

Re: Emacs current-time-string core dump on 64-bit hosts

[Richard Stallman]

       #ifndef TM_YEAR_IN_ASCTIME_RANGE
       # define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
	    (1000 - 1900 <= (tm_year) && (tm_year) <= 9999 - 1900)
       #endif

    and then in s/gnu-linux.h and src/s/gnu.h we'd put this:

       #define TM_YEAR_IN_ASCTIME_RANGE(tm_year) ((tm_year) <= INT_MAX - 1900)

I was thinking of something a little different, but this is fine too.

    Is that the sort of thing you have in mind?  If so, can you please
    suggest where the generic definition of TM_YEAR_IN_ASCTIME_RANGE
    should go?

Don't worry about it--just copy those 4 lines into each of the three
.c files.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

Richard Stallman <rms@gnu.org> writes:

> Don't worry about it--just copy those 4 lines into each of the three
> .c files.

OK, here's a patch that does this.  If I understand you correctly, you
want current-time-string to output a year that might contain more than
4 bytes (if it is greater than 9999, say) and might contain fewer (if
it is in the range 100-999, say).  I haven't installed this patch yet,
since I wanted to double-check this with you first.

With this patch, I assume that I should also put something like this line:

#define TM_YEAR_IN_ASCTIME_RANGE(tm_year) 1

into src/s/gnu-linux.h and into src/s/gnu.h, but I'm not sure exactly
where you'd prefer it.  At the end, perhaps?

2006-04-02  Paul Eggert  <eggert@cs.ucla.edu>

	* lib-src/b2m.c (main): Don't include <limits.h>.
	(TM_YEAR_BASE): New macro.
	(TM_YEAR_IN_ASCTIME_RANGE): Don't define if already defined, so
	that s/ files can override this.  Use the more-conservative range
	1000-9999.
	(main): Check for asctime returning NULL.
	* lib-src/fakemail.c: Likewise.
	* src/editfns.c (TM_YEAR_IN_ASCTIME_RANGE): New macro, identical to
	../lib-src/b2m.c and ../lib-src/editfns.c.
	(Fcurrent_time_string): Use it.
	Document that the year might not consume 4 columns if it's outside
	the range 1000-9999.
	Check for asctime failure.
	Don't assume that the output string length is always exactly 24.

Index: lib-src/b2m.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/b2m.c,v
retrieving revision 1.31
diff -p -c -r1.31 b2m.c
*** lib-src/b2m.c	27 Mar 2006 20:40:05 -0000	1.31
--- lib-src/b2m.c	3 Apr 2006 04:38:37 -0000
***************
*** 26,32 ****
  #undef static
  #endif
  
- #include <limits.h>
  #include <stdio.h>
  #include <time.h>
  #include <sys/types.h>
--- 26,31 ----
***************
*** 45,59 ****
  
  typedef int logical;
  
! /* True if TM_YEAR is a struct tm's tm_year value that is acceptable
!    to asctime.  Glibc asctime returns a useful string unless TM_YEAR
!    is nearly INT_MAX, but the C Standard lets C libraries overrun a
!    buffer if TM_YEAR needs more than 4 bytes.  */
! #ifdef __GLIBC__
! # define TM_YEAR_IN_ASCTIME_RANGE(tm_year) ((tm_year) <= INT_MAX - 1900)
! #else
  # define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
!     (-999 - 1900 <= (tm_year) && (tm_year) <= 9999 - 1900)
  #endif
  
  /*
--- 44,56 ----
  
  typedef int logical;
  
! #define TM_YEAR_BASE 1900
! 
! /* Nonzero if TM_YEAR is a struct tm's tm_year value that causes
!    asctime to have well-defined behavior.  */
! #ifndef TM_YEAR_IN_ASCTIME_RANGE
  # define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
!     (1000 - TM_YEAR_BASE <= (tm_year) && (tm_year) <= 9999 - TM_YEAR_BASE)
  #endif
  
  /*
*************** main (argc, argv)
*** 148,156 ****
       Don't use 'ctime', as that might dump core if the hardware clock
       is set to a bizarre value.  */
    tm = localtime (&ltoday);
!   if (! (tm && TM_YEAR_IN_ASCTIME_RANGE (tm->tm_year)))
      fatal ("current time is out of range");
-   today = asctime (tm);
    data.size = 200;
    data.buffer = xnew (200, char);
  
--- 145,153 ----
       Don't use 'ctime', as that might dump core if the hardware clock
       is set to a bizarre value.  */
    tm = localtime (&ltoday);
!   if (! (tm && TM_YEAR_IN_ASCTIME_RANGE (tm->tm_year)
! 	 && (today = asctime (tm))))
      fatal ("current time is out of range");
    data.size = 200;
    data.buffer = xnew (200, char);
  
Index: lib-src/fakemail.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/fakemail.c,v
retrieving revision 1.36
diff -p -c -r1.36 fakemail.c
*** lib-src/fakemail.c	27 Mar 2006 20:40:05 -0000	1.36
--- lib-src/fakemail.c	3 Apr 2006 04:38:37 -0000
*************** main ()
*** 53,59 ****
  #include "ntlib.h"
  #endif
  
- #include <limits.h>
  #include <stdio.h>
  #include <string.h>
  #include <ctype.h>
--- 53,58 ----
*************** main ()
*** 71,85 ****
  #define true 1
  #define false 0
  
! /* True if TM_YEAR is a struct tm's tm_year value that is acceptable
!    to asctime.  Glibc asctime returns a useful string unless TM_YEAR
!    is nearly INT_MAX, but the C Standard lets C libraries overrun a
!    buffer if TM_YEAR needs more than 4 bytes.  */
! #ifdef __GLIBC__
! # define TM_YEAR_IN_ASCTIME_RANGE(tm_year) ((tm_year) <= INT_MAX - 1900)
! #else
  # define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
!     (-999 - 1900 <= (tm_year) && (tm_year) <= 9999 - 1900)
  #endif
  
  /* Various lists */
--- 70,82 ----
  #define true 1
  #define false 0
  
! #define TM_YEAR_BASE 1900
! 
! /* Nonzero if TM_YEAR is a struct tm's tm_year value that causes
!    asctime to have well-defined behavior.  */
! #ifndef TM_YEAR_IN_ASCTIME_RANGE
  # define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
!     (1000 - TM_YEAR_BASE <= (tm_year) && (tm_year) <= 9999 - TM_YEAR_BASE)
  #endif
  
  /* Various lists */
*************** make_file_preface ()
*** 378,386 ****
       Don't use 'ctime', as that might dump core if the hardware clock
       is set to a bizarre value.  */
    tm = localtime (&idiotic_interface);
!   if (! (tm && TM_YEAR_IN_ASCTIME_RANGE (tm->tm_year)))
      fatal ("current time is out of range", 0);
-   the_date = asctime (tm);
    /* the_date has an unwanted newline at the end */
    date_length = strlen (the_date) - 1;
    the_date[date_length] = '\0';
--- 375,383 ----
       Don't use 'ctime', as that might dump core if the hardware clock
       is set to a bizarre value.  */
    tm = localtime (&idiotic_interface);
!   if (! (tm && TM_YEAR_IN_ASCTIME_RANGE (tm->tm_year)
! 	 && (the_date = asctime (tm))))
      fatal ("current time is out of range", 0);
    /* the_date has an unwanted newline at the end */
    date_length = strlen (the_date) - 1;
    the_date[date_length] = '\0';
Index: src/editfns.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/editfns.c,v
retrieving revision 1.411
diff -p -c -r1.411 editfns.c
*** src/editfns.c	25 Mar 2006 08:56:07 -0000	1.411
--- src/editfns.c	3 Apr 2006 04:38:37 -0000
*************** extern char **environ;
*** 74,79 ****
--- 74,86 ----
  
  #define TM_YEAR_BASE 1900
  
+ /* Nonzero if TM_YEAR is a struct tm's tm_year value that causes
+    asctime to have well-defined behavior.  */
+ #ifndef TM_YEAR_IN_ASCTIME_RANGE
+ # define TM_YEAR_IN_ASCTIME_RANGE(tm_year) \
+     (1000 - TM_YEAR_BASE <= (tm_year) && (tm_year) <= 9999 - TM_YEAR_BASE)
+ #endif
+ 
  extern size_t emacs_strftimeu P_ ((char *, size_t, const char *,
  				   const struct tm *, int));
  static int tm_diff P_ ((struct tm *, struct tm *));
*************** usage: (encode-time SECOND MINUTE HOUR D
*** 1833,1839 ****
  DEFUN ("current-time-string", Fcurrent_time_string, Scurrent_time_string, 0, 1, 0,
         doc: /* Return the current time, as a human-readable string.
  Programs can use this function to decode a time,
! since the number of columns in each field is fixed.
  The format is `Sun Sep 16 01:03:52 1973'.
  However, see also the functions `decode-time' and `format-time-string'
  which provide a much more powerful and general facility.
--- 1840,1847 ----
  DEFUN ("current-time-string", Fcurrent_time_string, Scurrent_time_string, 0, 1, 0,
         doc: /* Return the current time, as a human-readable string.
  Programs can use this function to decode a time,
! since the number of columns in each field is fixed
! if the year is in the range 1000-9999.
  The format is `Sun Sep 16 01:03:52 1973'.
  However, see also the functions `decode-time' and `format-time-string'
  which provide a much more powerful and general facility.
*************** but this is considered obsolete.  */)
*** 1847,1877 ****
       Lisp_Object specified_time;
  {
    time_t value;
-   char buf[30];
    struct tm *tm;
    register char *tem;
  
    if (! lisp_time_argument (specified_time, &value, NULL))
      error ("Invalid time specification");
!   /* Do not use ctime, since it has undefined behavior with
!      out-of-range time stamps.  This avoids a core dump triggered by
!      (current-time-string '(2814749767106 0)) on 64-bit Solaris 8. See
!      <http://www.opengroup.org/austin/mailarchives/ag/msg09294.html>
!      for more details about this portability problem.  */
    tm = localtime (&value);
!   /* Checking for out-of-range time stamps avoids buffer overruns that
!      cause core dump on some systems (e.g., 64-bit Solaris), and also
!      preserves the historic behavior of always returning a fixed-size
!      24-character string.  */
!   if (! (tm && -999 - TM_YEAR_BASE <= tm->tm_year
! 	 && tm->tm_year <= 9999 - TM_YEAR_BASE))
      error ("Specified time is not representable");
-   tem = asctime (tm);
  
!   strncpy (buf, tem, 24);
!   buf[24] = 0;
  
!   return build_string (buf);
  }
  
  /* Yield A - B, measured in seconds.
--- 1855,1877 ----
       Lisp_Object specified_time;
  {
    time_t value;
    struct tm *tm;
    register char *tem;
  
    if (! lisp_time_argument (specified_time, &value, NULL))
      error ("Invalid time specification");
! 
!   /* Convert to a string, checking for out-of-range time stamps.
!      Don't use 'ctime', as that might dump core if VALUE is out of
!      range.  */
    tm = localtime (&value);
!   if (! (tm && TM_YEAR_IN_ASCTIME_RANGE (tm->tm_year) && (tem = asctime (tm))))
      error ("Specified time is not representable");
  
!   /* Remove the trailing newline.  */
!   tem[strlen (tem) - 1] = '\0';
  
!   return build_string (tem);
  }
  
  /* Yield A - B, measured in seconds.

Re: Emacs current-time-string core dump on 64-bit hosts

[Paul Eggert]

I got a response via private email from Ulrich Leodolter about that
old comment of his in w32.c and ntlib.c.  He wrote:

> I cant remember the context, but i think know the reason for this
> workaround:
>
> time_t t = -1;
> char *str = ctime(&t);
>
> I verified that str is a null pointer when this code is compiled with
> MS Visual C/C++ 6.0, but it returns a time string on Linux/GNU.

So it appears that my analysis was correct.  That is, the problem was
unrelated to whether the working directory was a network drive.  The
problem (which is documented by Microsoft) is that, if a time stamp is
negative, ctime returns a null pointer.  This null pointer caused the
circa 1995 edition of Emacs to crash, since 1995 Emacs assumed that
ctime always returns a valid pointer.

If a future edition of Emacs uses ctime, it won't be able to assume
that ctime must return a nonnull pointer.  Even assuming reliable
implementations like glibc, a ctime-using Emacs would have to check
whether ctime's returned value is null; and to be portable to
less-reliable implementations like Solaris 8, Emacs would have to
check that ctime's argument is in an implementation-dependent range.
And either way, the w32 wrapper would not be needed.

With that in mind I'd like to re-propose the following minor cleanup
patch to the w32 code.  It doesn't fix any bugs, since Emacs no longer
uses ctime.  But it simplifies the Emacs code (e.g., it means we don't
have to fuss about the two typos in those "Sun Jan 01 00:00:00 1970"
strings :-).


2006-04-03  Paul Eggert  <eggert@cs.ucla.edu>

	* lib-src/ntlib.c (sys_ctime): Remove.  Emacs no longer uses ctime,
	and would not need to wrap w32 ctime even if Emacs used ctime,
	assuming Emacs used ctime safely (e.g., checking result against NULL).
	* lib-src/ntlib.h (ctime): Likewise.
	* src/w32.c (sys_ctime): Likewise.
	* src/s/ms-w32.h (ctime): Likewise.

Index: lib-src/ntlib.c
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/ntlib.c,v
retrieving revision 1.13
diff -p -c -r1.13 ntlib.c
*** lib-src/ntlib.c	6 Feb 2006 11:28:28 -0000	1.13
--- lib-src/ntlib.c	4 Apr 2006 04:32:17 -0000
*************** fchown (int fd, int uid, int gid)
*** 188,203 ****
    return 0;
  }
  
- /* Place a wrapper around the MSVC version of ctime.  It returns NULL
-    on network directories, so we handle that case here.
-    (Ulrich Leodolter, 1/11/95).  */
- char *
- sys_ctime (const time_t *t)
- {
-   char *str = (char *) ctime (t);
-   return (str ? str : "Sun Jan 01 00:00:00 1970");
- }
- 
  FILE *
  sys_fopen(const char * path, const char * mode)
  {
--- 188,193 ----
Index: lib-src/ntlib.h
===================================================================
RCS file: /cvsroot/emacs/emacs/lib-src/ntlib.h,v
retrieving revision 1.11
diff -p -c -r1.11 ntlib.h
*** lib-src/ntlib.h	6 Feb 2006 11:28:28 -0000	1.11
--- lib-src/ntlib.h	4 Apr 2006 04:32:17 -0000
*************** int fchown (int fd, int uid, int gid);
*** 61,67 ****
  #define close   _close
  #undef creat
  #define creat   _creat
- #undef ctime
  #undef dup
  #define dup     _dup
  #undef dup2
--- 61,66 ----
Index: src/w32.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/w32.c,v
retrieving revision 1.100
diff -p -c -r1.100 w32.c
*** src/w32.c	27 Feb 2006 02:07:37 -0000	1.100
--- src/w32.c	4 Apr 2006 04:32:17 -0000
*************** Boston, MA 02110-1301, USA.
*** 43,49 ****
  #undef chdir
  #undef chmod
  #undef creat
- #undef ctime
  #undef fopen
  #undef link
  #undef mkdir
--- 43,48 ----
*************** gettimeofday (struct timeval *tv, struct
*** 1325,1340 ****
  /* IO support and wrapper functions for W32 API. */
  /* ------------------------------------------------------------------------- */
  
- /* Place a wrapper around the MSVC version of ctime.  It returns NULL
-    on network directories, so we handle that case here.
-    (Ulrich Leodolter, 1/11/95).  */
- char *
- sys_ctime (const time_t *t)
- {
-   char *str = (char *) ctime (t);
-   return (str ? str : "Sun Jan 01 00:00:00 1970");
- }
- 
  /* Emulate sleep...we could have done this with a define, but that
     would necessitate including windows.h in the files that used it.
     This is much easier.  */
--- 1324,1329 ----
Index: src/s/ms-w32.h
===================================================================
RCS file: /cvsroot/emacs/emacs/src/s/ms-w32.h,v
retrieving revision 1.37
diff -p -c -r1.37 ms-w32.h
*** src/s/ms-w32.h	6 Feb 2006 15:23:23 -0000	1.37
--- src/s/ms-w32.h	4 Apr 2006 04:32:17 -0000
*************** Boston, MA 02110-1301, USA.  */
*** 317,323 ****
  #define close   sys_close
  #undef creat
  #define creat   sys_creat
- #define ctime	sys_ctime
  #undef dup
  #define dup     sys_dup
  #undef dup2
--- 317,322 ----

Re: Emacs current-time-string core dump on 64-bit hosts

[Eli Zaretskii]

> Cc: bug-gnu-emacs@gnu.org
> From: Paul Eggert <eggert@CS.UCLA.EDU>
> Date: Mon, 03 Apr 2006 21:57:04 -0700
> 
> So it appears that my analysis was correct.  That is, the problem was
> unrelated to whether the working directory was a network drive.  The
> problem (which is documented by Microsoft) is that, if a time stamp is
> negative, ctime returns a null pointer.  This null pointer caused the
> circa 1995 edition of Emacs to crash, since 1995 Emacs assumed that
> ctime always returns a valid pointer.

The comment clearly mentions ``network directories'', and he says he
doesn't remember the reason.  Even if the argument passed to ctime was
negative, as long as that is a result of legitimate sequence of system
calls, IMO it is a very bad idea to signal an error.

> With that in mind I'd like to re-propose the following minor cleanup
> patch to the w32 code.  It doesn't fix any bugs, since Emacs no longer
> uses ctime.  But it simplifies the Emacs code

The simplification is minimal, unlike the risks, so I'd like to leave
those functions alone.