Fix file-attributes race on GNU hosts

* doc/lispref/files.texi (File Attributes):
Document file-attributes atomicity.
* etc/NEWS: Document the fix.
* src/dired.c (file_attributes): New args DIRNAME and FILENAME,
for diagnostics.  All callers changed.  On platforms like
GNU/Linux that support O_PATH, fix a race condition in
file-attributes and similar functions, so that these functions do
not return nonsense if a directory entry is replaced while getting
its attributes.  On non-GNU platforms, do a better (though not
perfect) job of detecting the race, and return nil if detected.

doc/lispref/files.texi

@@ -1249,6 +1249,13 @@ the default, but we plan to change that, so you should specify a
 non-@code{nil} value for @var{id-format} if you use the returned
 @acronym{UID} or @acronym{GID}.
 
+On GNU platforms when operating on a local file, this function is
+atomic: if the filesystem is simultaneously being changed by some
+other process, this function returns the file's attributes either
+before or after the change.  Otherwise this function is not atomic,
+and might return @code{nil} it detects the race condition, or might
+return a hodgepodge of the previous and current file attributes.
+
 Accessor functions are provided to access the elements in this list.
 The accessors are mentioned along with the descriptions of the
 elements below.

etc/NEWS

@@ -1427,6 +1427,12 @@ job of signaling list cycles instead of looping indefinitely.
 ** The new functions 'make-nearby-temp-file' and 'temporary-file-directory'
 can be used for creation of temporary files of remote or mounted directories.
 
++++
+** On GNU platforms when operating on a local file, 'file-attributes'
+no longer suffers from a race when called while another process is
+altering the filesystem.  On non-GNU platforms 'file-attributes'
+attempts to detect the race, and returns nil if it does so.
+
 +++
 ** The new function 'file-local-name' can be used to specify arguments
 of remote processes.

src/dired.c

@@ -51,7 +51,8 @@ extern int is_slow_fs (const char *);
 #endif
 
 static ptrdiff_t scmp (const char *, const char *, ptrdiff_t);
-static Lisp_Object file_attributes (int, char const *, Lisp_Object);
+static Lisp_Object file_attributes (int, char const *, Lisp_Object,
+				    Lisp_Object, Lisp_Object);
 
 /* Return the number of bytes in DP's name.  */
 static ptrdiff_t
@@ -161,7 +162,7 @@ read_dirent (DIR *dir, Lisp_Object dirname)
 /* Function shared by Fdirectory_files and Fdirectory_files_and_attributes.
    If not ATTRS, return a list of directory filenames;
    if ATTRS, return a list of directory filenames and their attributes.
-   In the latter case, ID_FORMAT is passed to Ffile_attributes.  */
+   In the latter case, pass ID_FORMAT to file_attributes.  */
 
 Lisp_Object
 directory_files_internal (Lisp_Object directory, Lisp_Object full,
@@ -225,7 +226,7 @@ directory_files_internal (Lisp_Object directory, Lisp_Object full,
   if (attrs)
     {
       /* Do this only once to avoid doing it (in w32.c:stat) for each
-	 file in the directory, when we call Ffile_attributes below.  */
+	 file in the directory, when we call file_attributes below.  */
       record_unwind_protect (directory_files_internal_w32_unwind,
 			     Vw32_get_true_file_attributes);
       w32_save = Vw32_get_true_file_attributes;
@@ -304,7 +305,7 @@ directory_files_internal (Lisp_Object directory, Lisp_Object full,
 	  if (attrs)
 	    {
 	      Lisp_Object fileattrs
-		= file_attributes (fd, dp->d_name, id_format);
+		= file_attributes (fd, dp->d_name, directory, name, id_format);
 	      list = Fcons (Fcons (finalname, fileattrs), list);
 	    }
 	  else
@@ -351,7 +352,7 @@ If NOSORT is non-nil, the list is not sorted--its order is unpredictable.
     return call5 (handler, Qdirectory_files, directory,
                   full, match, nosort);
 
-  return directory_files_internal (directory, full, match, nosort, 0, Qnil);
+  return directory_files_internal (directory, full, match, nosort, false, Qnil);
 }
 
 DEFUN ("directory-files-and-attributes", Fdirectory_files_and_attributes,
@@ -379,7 +380,8 @@ which see.  */)
     return call6 (handler, Qdirectory_files_and_attributes,
                   directory, full, match, nosort, id_format);
 
-  return directory_files_internal (directory, full, match, nosort, 1, id_format);
+  return directory_files_internal (directory, full, match, nosort,
+				   true, id_format);
 }
 
 
@@ -923,14 +925,17 @@ so last access time will always be midnight of that day.  */)
     }
 
   encoded = ENCODE_FILE (filename);
-  return file_attributes (AT_FDCWD, SSDATA (encoded), id_format);
+  return file_attributes (AT_FDCWD, SSDATA (encoded), Qnil, filename,
+			  id_format);
 }
 
 static Lisp_Object
-file_attributes (int fd, char const *name, Lisp_Object id_format)
+file_attributes (int fd, char const *name,
+		 Lisp_Object dirname, Lisp_Object filename,
+		 Lisp_Object id_format)
 {
+  ptrdiff_t count = SPECPDL_INDEX ();
   struct stat s;
-  int lstat_result;
 
   /* An array to hold the mode string generated by filemodestring,
      including its terminating space and null byte.  */
@@ -938,22 +943,60 @@ file_attributes (int fd, char const *name, Lisp_Object id_format)
 
   char *uname = NULL, *gname = NULL;
 
-#ifdef WINDOWSNT
-  /* We usually don't request accurate owner and group info, because
-     it can be very expensive on Windows to get that, and most callers
-     of 'lstat' don't need that.  But here we do want that information
-     to be accurate.  */
-  w32_stat_get_owner_group = 1;
+  int err = EINVAL;
+
+#ifdef O_PATH
+  int namefd = openat (fd, name, O_PATH | O_CLOEXEC | O_NOFOLLOW);
+  if (namefd < 0)
+    err = errno;
+  else
+    {
+      record_unwind_protect_int (close_file_unwind, namefd);
+      if (fstat (namefd, &s) != 0)
+	err = errno;
+      else
+	{
+	  err = 0;
+	  fd = namefd;
+	  name = "";
+	}
+    }
 #endif
 
-  lstat_result = fstatat (fd, name, &s, AT_SYMLINK_NOFOLLOW);
-
+  if (err == EINVAL)
+    {
 #ifdef WINDOWSNT
-  w32_stat_get_owner_group = 0;
+      /* We usually don't request accurate owner and group info,
+	 because it can be expensive on Windows to get that, and most
+	 callers of 'lstat' don't need that.  But here we do want that
+	 information to be accurate.  */
+      w32_stat_get_owner_group = 1;
 #endif
+      if (fstatat (fd, name, &s, AT_SYMLINK_NOFOLLOW) == 0)
+	err = 0;
+#ifdef WINDOWSNT
+      w32_stat_get_owner_group = 0;
+#endif
+    }
 
-  if (lstat_result < 0)
-    return Qnil;
+  if (err != 0)
+    return unbind_to (count, Qnil);
+
+  Lisp_Object file_type;
+  if (S_ISLNK (s.st_mode))
+    {
+      /* On systems lacking O_PATH support there is a race if the
+	 symlink is replaced between the call to fstatat and the call
+	 to emacs_readlinkat.  Detect this race unless the replacement
+	 is also a symlink.  */
+      file_type = emacs_readlinkat (fd, name);
+      if (NILP (file_type))
+	return unbind_to (count, Qnil);
+    }
+  else
+    file_type = S_ISDIR (s.st_mode) ? Qt : Qnil;
+
+  unbind_to (count, Qnil);
 
   if (!(NILP (id_format) || EQ (id_format, Qinteger)))
     {
@@ -964,8 +1007,7 @@ file_attributes (int fd, char const *name, Lisp_Object id_format)
   filemodestring (&s, modes);
 
   return CALLN (Flist,
-		(S_ISLNK (s.st_mode) ? emacs_readlinkat (fd, name)
-		 : S_ISDIR (s.st_mode) ? Qt : Qnil),
+		file_type,
 		make_number (s.st_nlink),
 		(uname
 		 ? DECODE_SYSTEM (build_unibyte_string (uname))

src/kqueue.c

@@ -130,7 +130,7 @@ kqueue_compare_dir_list (Lisp_Object watch_object)
     return;
   }
   new_directory_files =
-    directory_files_internal (dir, Qnil, Qnil, Qnil, 1, Qnil);
+    directory_files_internal (dir, Qnil, Qnil, Qnil, true, Qnil);
   new_dl = kqueue_directory_listing (new_directory_files);
 
   /* Parse through the old list.  */
@@ -453,7 +453,7 @@ only when the upper directory of the renamed file is watched.  */)
   if (NILP (Ffile_directory_p (file)))
     watch_object = list4 (watch_descriptor, file, flags, callback);
   else {
-    dir_list = directory_files_internal (file, Qnil, Qnil, Qnil, 1, Qnil);
+    dir_list = directory_files_internal (file, Qnil, Qnil, Qnil, true, Qnil);
     watch_object = list5 (watch_descriptor, file, flags, callback, dir_list);
   }
   watch_list = Fcons (watch_object, watch_list);

src/sysdep.c

@@ -2930,7 +2930,7 @@ list_system_processes (void)
      process.  */
   procdir = build_string ("/proc");
   match = build_string ("[0-9]+");
-  proclist = directory_files_internal (procdir, Qnil, match, Qt, 0, Qnil);
+  proclist = directory_files_internal (procdir, Qnil, match, Qt, false, Qnil);
 
   /* `proclist' gives process IDs as strings.  Destructively convert
      each string into a number.  */