bug#16986: Crash when idle

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

Thanks, Mitsuharu, for the diagnosis; I think you nailed it.

Dmitry, I spotted a couple of problems with that patch.  First, any 
compiler could do that optimization and I wouldn't be surprised if some 
version of GCC does it either now or in the future, so the fix shouldn't 
be conditionalized on __clang__.  Second, making 'directory' itself 
volatile would break the (uncommon) case where GC_MARK_STACK != 
GC_MAKE_GCPROS_NOOPS, because one can't portably assign the address of a 
volatile to a pointer that isn't volatile-qualified.  Plus, there's no 
need to have every access to that local be volatile; we need only ensure 
that the value is stored somewhere.

I installed the attached patch as emacs-24 bzr 117421, as a change that 
should not break anything and most likely fixes the bug.  Please let me 
know how it works in the Clang environment.

[-- Attachment #2: dired.patch --]
[-- Type: text/plain, Size: 1478 bytes --]

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2014-08-02 21:31:18 +0000
+++ src/ChangeLog	2014-08-02 22:57:13 +0000
@@ -1,5 +1,10 @@
 2014-08-02  Paul Eggert  <eggert@cs.ucla.edu>
 
+	Fix bug with clang + directory_files_internal + GC (Bug#16986).
+	* dired.c (directory_files_internal): Use a volatile variable
+	to prevent the compiler from optimizing away all copies of a local.
+	I wonder how many other GC-related bugs like this lurk elsewhere?
+
 	Avoid 100% CPU utilization on ssh session exit (Bug#17691).
 	* xterm.h (struct x_display_info): New member 'connection'.
 	* xterm.c (x_term_init, x_delete_terminal): Set and use it,

=== modified file 'src/dired.c'
--- src/dired.c	2014-04-16 13:27:28 +0000
+++ src/dired.c	2014-08-02 22:57:13 +0000
@@ -150,6 +150,12 @@
   Lisp_Object w32_save = Qnil;
 #endif
 
+  /* Don't let the compiler optimize away all copies of DIRECTORY,
+     which would break GC; see Bug#16986.  Although this is required
+     only in the common case where GC_MARK_STACK == GC_MAKE_GCPROS_NOOPS,
+     it shouldn't break anything in the other cases.  */
+  Lisp_Object volatile directory_volatile = directory;
+
   /* Because of file name handlers, these functions might call
      Ffuncall, and cause a GC.  */
   list = encoded_directory = dirfilename = Qnil;
@@ -325,6 +331,7 @@
     list = Fsort (Fnreverse (list),
 		  attrs ? Qfile_attributes_lessp : Qstring_lessp);
 
+  (void) directory_volatile;
   RETURN_UNGCPRO (list);
 }