[roland/dl-fileid] Factor file identity rules out of generic rtld code.

Message ID 20150213233547.4C0D92C3C27@topped-with-meat.com
State Committed
Headers

Commit Message

Roland McGrath Feb. 13, 2015, 11:35 p.m. UTC
  This is a not a real priority right now, but I think I'm going to want this
for my non-Linux port at some point.

Tested x86_64-linux-gnu.  (I also build-tested the new sysdeps/generic
version with out-of-tree hacks.)

This did increase the code size on x86_64 (with GCC 4.8.2) by 112 bytes in
ld.so, and by 96 bytes in libc.a.  I have not tried to investigate why in
any detail.  It's more than I expected, since the only real change not
inlined away is copying st_{dev,ino} into a separate stack struct.  (If the
compiler were inordinately clever it could realize that the struct stat64
and the struct r_file_id are never live at the same time and that the first
two fields of struct stat64 match the fields of struct r_file_id and so
just overlay them in the same stack space and not even spend the four
instructions it should take to copy the values.  But it ain't.)


Thanks,
Roland


2015-02-13  Roland McGrath  <roland@hack.frob.com>

	* sysdeps/generic/dl-fileid.h: New file.
	* sysdeps/posix/dl-fileid.h: New file.
	* include/link.h: Include <dl-fileid.h>.
	(struct link_map): Replace l_dev and l_ino with l_file_id.
	* elf/dl-load.c (_dl_map_object_from_fd): Use _dl_get_file_id rather
	than __fxstat64.  Use _dl_file_id_match_p rather than comparing l_dev
	and l_ino directly.  Initialize l_file_id rather than l_dev and l_ino.
  

Patch

--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -872,7 +872,6 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
   const ElfW(Phdr) *ph;
   size_t maplength;
   int type;
-  struct stat64 st;
   /* Initialize to keep the compiler happy.  */
   const char *errstring = NULL;
   int errval = 0;
@@ -880,7 +879,8 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
   bool make_consistent = false;
 
   /* Get file information.  */
-  if (__glibc_unlikely (__fxstat64 (_STAT_VER, fd, &st) < 0))
+  struct r_file_id id;
+  if (__glibc_unlikely (!_dl_get_file_id (fd, &id)))
     {
       errstring = N_("cannot stat shared object");
     call_lose_errno:
@@ -891,8 +891,8 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
     }
 
   /* Look again to see if the real name matched another already loaded.  */
-  for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next)
-    if (l->l_removed == 0 && l->l_ino == st.st_ino && l->l_dev == st.st_dev)
+  for (l = GL(dl_ns)[nsid]._ns_loaded; l != NULL; l = l->l_next)
+    if (!l->l_removed && _dl_file_id_match_p (&l->l_file_id, &id))
       {
 	/* The object is already loaded.
 	   Just bump its reference count and return it.  */
@@ -910,8 +910,7 @@  _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
   /* When loading into a namespace other than the base one we must
      avoid loading ld.so since there can only be one copy.  Ever.  */
   if (__glibc_unlikely (nsid != LM_ID_BASE)
-      && ((st.st_ino == GL(dl_rtld_map).l_ino
-	   && st.st_dev == GL(dl_rtld_map).l_dev)
+      && (_dl_file_id_match_p (&id, &GL(dl_rtld_map).l_file_id)
 	  || _dl_name_match_p (name, &GL(dl_rtld_map))))
     {
       /* This is indeed ld.so.  Create a new link_map which refers to
@@ -1390,8 +1389,7 @@  cannot enable executable stack as shared object requires");
     GL(dl_initfirst) = l;
 
   /* Finally the file information.  */
-  l->l_dev = st.st_dev;
-  l->l_ino = st.st_ino;
+  l->l_file_id = id;
 
   /* When we profile the SONAME might be needed for something else but
      loading.  Add it right away.  */
--- a/include/link.h
+++ b/include/link.h
@@ -40,6 +40,7 @@  extern unsigned int la_objopen (struct link_map *__map, Lmid_t __lmid,
 #include <stdint.h>
 #include <stddef.h>
 #include <bits/linkmap.h>
+#include <dl-fileid.h>
 #include <dl-lookupcfg.h>
 #include <tls.h>
 #include <bits/libc-lock.h>
@@ -235,8 +236,7 @@  struct link_map
 
     /* This information is kept to check for sure whether a shared
        object is the same as one already loaded.  */
-    dev_t l_dev;
-    ino64_t l_ino;
+    struct r_file_id l_file_id;
 
     /* Collected information about own RUNPATH directories.  */
     struct r_search_path_struct l_runpath_dirs;
--- /dev/null
+++ b/sysdeps/generic/dl-fileid.h
@@ -0,0 +1,46 @@ 
+/* File identity for the dynamic linker.  Stub version.
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdbool.h>
+
+/* This type stores whatever information is fetched by _dl_get_file_id
+   and compared by _dl_file_id_match_p.  */
+struct r_file_id
+  {
+    /* In the stub version, we don't record anything at all.  */
+  };
+
+/* Sample FD to fill in *ID.  Returns true on success.
+   On error, returns false, with errno set.  */
+static inline bool
+_dl_get_file_id (int fd __attribute__ ((unused)),
+                 struct r_file_id *id __attribute__ ((unused)))
+{
+  return true;
+}
+
+/* Compare two results from _dl_get_file_id for equality.
+   It's crucial that this never return false-positive matches.
+   It's ideal that it never return false-negative mismatches either,
+   but lack of matches is survivable.  */
+static inline bool
+_dl_file_id_match_p (const struct r_file_id *a __attribute__ ((unused)),
+                     const struct r_file_id *b __attribute__ ((unused)))
+{
+  return false;
+}
--- /dev/null
+++ b/sysdeps/posix/dl-fileid.h
@@ -0,0 +1,50 @@ 
+/* File identity for the dynamic linker.  Generic POSIX.1 version.
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdbool.h>
+#include <sys/stat.h>
+
+/* For POSIX.1 systems, the pair of st_dev and st_ino constitute
+   a unique identifier for a file.  */
+struct r_file_id
+  {
+    dev_t dev;
+    ino64_t ino;
+  };
+
+/* Sample FD to fill in *ID.  Returns true on success.
+   On error, returns false, with errno set.  */
+static inline bool
+_dl_get_file_id (int fd, struct r_file_id *id)
+{
+  struct stat64 st;
+
+  if (__glibc_unlikely (__fxstat64 (_STAT_VER, fd, &st) < 0))
+    return false;
+
+  id->dev = st.st_dev;
+  id->ino = st.st_ino;
+  return true;
+}
+
+/* Compare two results from _dl_get_file_id for equality.  */
+static inline bool
+_dl_file_id_match_p (const struct r_file_id *a, const struct r_file_id *b)
+{
+  return a->dev == b->dev && a->ino == b->ino;
+}