GNU bug report logs - #8545
issues with recent doprnt-related changes

Previous Next

Full log

Message #126 received at 8545 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 8545 <at> debbugs.gnu.org
Subject: Re: issues with recent doprnt-related changes
Date: Wed, 04 May 2011 00:28:18 -0700

On 04/29/11 04:16, Eli Zaretskii wrote:
> I guess so, yes.  I would like to have other opinions, though, so I
> will start a new thread on emacs-devel about that.

It seems from that discussion that strings can contain MOST_POSITIVE_FIXNUM bytes.
Also, that va_arg bug really needs fixing.  So I plan to install the following
patch after some more testing.  This assumes va_copy exists, which may affect
the Windows port, but a one-line macro should suffice if it doesn't have
va_copy already.

=== modified file 'ChangeLog'
--- ChangeLog	2011-05-04 06:11:49 +0000
+++ ChangeLog	2011-05-04 07:19:21 +0000
@@ -1,5 +1,9 @@
 2011-05-04  Paul Eggert  <eggert <at> cs.ucla.edu>
 
+	Use C99's va_copy to avoid undefined behavior on x86-64 GNU/Linux.
+	* Makefile.in (GNULIB_MODULES): Add stdarg, for va_copy.
+	* lib/stdarg.in.h, m4/stdarg.m4: New files, from gnulib.
+
 	* Makefile.in (GNULIB_TOOL_FLAG): Add --conditional-dependencies.
 	This new gnulib-tool option saves 'configure' the trouble of
 	checking for strtoull when strtoumax exists.

=== modified file 'Makefile.in'
--- Makefile.in	2011-05-04 06:11:49 +0000
+++ Makefile.in	2011-05-04 07:19:21 +0000
@@ -333,7 +333,7 @@
 GNULIB_MODULES = \
   careadlinkat crypto/md5 dtoastr filemode getloadavg getopt-gnu \
   ignore-value intprops lstat mktime readlink \
-  socklen stdio strftime strtoumax symlink sys_stat
+  socklen stdarg stdio strftime strtoumax symlink sys_stat
 GNULIB_TOOL_FLAGS = \
  --conditional-dependencies --import --no-changelog --no-vc-files \
  --makefile-name=gnulib.mk

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2011-05-04 06:13:23 +0000
+++ src/ChangeLog	2011-05-04 07:20:46 +0000
@@ -1,5 +1,16 @@
 2011-05-04  Paul Eggert  <eggert <at> cs.ucla.edu>
 
+	* term.c (vfatal): Remove stray call to va_end.
+	It's not needed and the C Standard doesn't allow it here anyway.
+
+	Use C99's va_copy to avoid undefined behavior on x86-64 GNU/Linux.
+	* eval.c (verror): doprnt a copy of ap, not the original.  (Bug#8545)
+
+	* eval.c (verror): OK to create a string of up to MOST_POSITIVE_FIXNUM
+	bytes.
+
+	* term.c: Don't include <stdarg.h>, as <lisp.h> does that.
+
 	Arithmetic overflows now return float rather than wrapping around.
 	(Bug#8611).
 	* data.c: Include <intprops.h>.

=== modified file 'src/eval.c'
--- src/eval.c	2011-04-30 19:00:39 +0000
+++ src/eval.c	2011-05-04 07:19:21 +0000
@@ -1994,7 +1994,7 @@
 {
   char buf[4000];
   size_t size = sizeof buf;
-  size_t size_max = min (MOST_POSITIVE_FIXNUM, SIZE_MAX);
+  size_t size_max = min (MOST_POSITIVE_FIXNUM + 1, SIZE_MAX);
   size_t mlen = strlen (m);
   char *buffer = buf;
   size_t used;
@@ -2002,7 +2002,10 @@
 
   while (1)
     {
-      used = doprnt (buffer, size, m, m + mlen, ap);
+      va_list ap_copy;
+      va_copy (ap_copy, ap);
+      used = doprnt (buffer, size, m, m + mlen, ap_copy);
+      va_end (ap_copy);
 
       /* Note: the -1 below is because `doprnt' returns the number of bytes
 	 excluding the terminating null byte, and it always terminates with a

=== modified file 'src/term.c'
--- src/term.c	2011-04-24 09:00:03 +0000
+++ src/term.c	2011-05-04 07:20:46 +0000
@@ -26,7 +26,6 @@
 #include <sys/file.h>
 #include <unistd.h>
 #include <signal.h>
-#include <stdarg.h>
 #include <setjmp.h>
 
 #include "lisp.h"
@@ -3619,7 +3618,6 @@
   vfprintf (stderr, str, ap);
   if (!(strlen (str) > 0 && str[strlen (str) - 1] == '\n'))
     fprintf (stderr, "\n");
-  va_end (ap);
   fflush (stderr);
   exit (1);
 }

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.