GNU bug report logs - #8298
'volatile' needed for local variables around vfork

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8298 in the body.
You can then email your comments to 8298 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: 'volatile' needed for local variables around vfork
Date: Sat, 19 Mar 2011 21:39:30 -0700

In three places, GNU Emacs assumes that a local variable
is preserved after a successful vfork, but on some architectures
these variables may be clobbered.  (I found this problem by
compiling with gcc -Wclobbered.)  I plan to address this problem
with the following patch:

2011-03-19  Paul Eggert  <eggert <at> cs.ucla.edu>
 
	* callproc.c (Fcall_process): Use 'volatile' to avoid vfork clobbering.
	* process.c (create_process): Likewise.
	* sysdep.c (sys_subshell): Likewise.
	Previously, the sys_subshell 'volatile' was incorrectly IF_LINTted out.

=== modified file 'src/callproc.c'
--- src/callproc.c	2011-03-18 04:45:18 +0000
+++ src/callproc.c	2011-03-18 05:56:46 +0000
@@ -180,7 +180,7 @@
   (int nargs, register Lisp_Object *args)
 {
   Lisp_Object infile, buffer, current_dir, path;
-  int display_p;
+  volatile int display_p_volatile;
   int fd[2];
   int filefd;
   register int pid;
@@ -190,6 +190,7 @@
   int bufsize = CALLPROC_BUFFER_SIZE_MIN;
   int count = SPECPDL_INDEX ();
 
+  const unsigned char **volatile new_argv_volatile;
   register const unsigned char **new_argv;
   /* File to use for stderr in the child.
      t means use same as standard output.  */
@@ -343,7 +344,7 @@
     UNGCPRO;
   }
 
-  display_p = INTERACTIVE && nargs >= 4 && !NILP (args[3]);
+  display_p_volatile = INTERACTIVE && nargs >= 4 && !NILP (args[3]);
 
   filefd = emacs_open (SSDATA (infile), O_RDONLY, 0);
   if (filefd < 0)
@@ -371,7 +372,7 @@
       && SREF (path, 1) == ':')
     path = Fsubstring (path, make_number (2), Qnil);
 
-  new_argv = (const unsigned char **)
+  new_argv_volatile = new_argv = (const unsigned char **)
     alloca (max (2, nargs - 2) * sizeof (char *));
   if (nargs > 4)
     {
@@ -542,6 +543,8 @@
 
     pid = vfork ();
 
+    new_argv = new_argv_volatile;
+
     if (pid == 0)
       {
 	if (fd[0] >= 0)
@@ -673,6 +676,7 @@
     int first = 1;
     EMACS_INT total_read = 0;
     int carryover = 0;
+    int display_p = display_p_volatile;
     int display_on_the_fly = display_p;
     struct coding_system saved_coding;
 

=== modified file 'src/process.c'
--- src/process.c	2011-03-20 02:48:50 +0000
+++ src/process.c	2011-03-20 03:07:54 +0000
@@ -1912,8 +1912,7 @@
     /* child_setup must clobber environ on systems with true vfork.
        Protect it from permanent change.  */
     char **save_environ = environ;
-
-    current_dir = ENCODE_FILE (current_dir);
+    volatile Lisp_Object encoded_current_dir = ENCODE_FILE (current_dir);
 
 #ifndef WINDOWSNT
     pid = vfork ();
@@ -2054,13 +2053,13 @@
 	  child_setup_tty (xforkout);
 #ifdef WINDOWSNT
 	pid = child_setup (xforkin, xforkout, xforkout,
-			   new_argv, 1, current_dir);
+			   new_argv, 1, encoded_current_dir);
 #else  /* not WINDOWSNT */
 #ifdef FD_CLOEXEC
 	emacs_close (wait_child_setup[0]);
 #endif
 	child_setup (xforkin, xforkout, xforkout,
-		     new_argv, 1, current_dir);
+		     new_argv, 1, encoded_current_dir);
 #endif /* not WINDOWSNT */
       }
     environ = save_environ;

=== modified file 'src/sysdep.c'
--- src/sysdep.c	2011-03-14 23:31:21 +0000
+++ src/sysdep.c	2011-03-18 05:50:40 +0000
@@ -488,7 +488,8 @@
   int pid;
   struct save_signal saved_handlers[5];
   Lisp_Object dir;
-  unsigned char * IF_LINT (volatile) str = 0;
+  unsigned char *volatile str_volatile = 0;
+  unsigned char *str;
   int len;
 
   saved_handlers[0].code = SIGINT;
@@ -512,7 +513,7 @@
     goto xyzzy;
 
   dir = expand_and_dir_to_file (Funhandled_file_name_directory (dir), Qnil);
-  str = (unsigned char *) alloca (SCHARS (dir) + 2);
+  str_volatile = str = (unsigned char *) alloca (SCHARS (dir) + 2);
   len = SCHARS (dir);
   memcpy (str, SDATA (dir), len);
   if (str[len - 1] != '/') str[len++] = '/';
@@ -544,6 +545,7 @@
 	sh = "sh";
 
       /* Use our buffer's default directory for the subshell.  */
+      str = str_volatile;
       if (str && chdir ((char *) str) != 0)
 	{
 #ifndef DOS_NT

Message #10 received at 8298-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 8310-done <at> debbugs.gnu.org, 8318-done <at> debbugs.gnu.org, 
	8306-done <at> debbugs.gnu.org, 8303-done <at> debbugs.gnu.org, 
	8277-done <at> debbugs.gnu.org, 8298-done <at> debbugs.gnu.org, 
	8290-done <at> debbugs.gnu.org, 8278-done <at> debbugs.gnu.org
Subject: fix merged to trunk
Date: Wed, 23 Mar 2011 15:06:46 -0700

I committed a fix to the trunk for this,
as part of a recent merge (bzr 103721).

Previous Next

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