8186072: dll_build_name returns true even if file is missing.
authorgoetz
Thu, 17 Aug 2017 17:26:02 +0200
changeset 47089 27050b653624
parent 47087 2279556f90ec
child 47090 dafcd9eed3e9
8186072: dll_build_name returns true even if file is missing. Summary: Split dll_build_name into two functions and consolidate to os.cpp file. Reviewed-by: stuefe, dholmes
hotspot/src/os/aix/vm/os_aix.cpp
hotspot/src/os/bsd/vm/os_bsd.cpp
hotspot/src/os/linux/vm/os_linux.cpp
hotspot/src/os/solaris/vm/os_solaris.cpp
hotspot/src/os/windows/vm/os_windows.cpp
hotspot/src/share/vm/classfile/classLoader.cpp
hotspot/src/share/vm/prims/jvmtiExport.cpp
hotspot/src/share/vm/runtime/os.cpp
hotspot/src/share/vm/runtime/os.hpp
hotspot/src/share/vm/runtime/thread.cpp
--- a/hotspot/src/os/aix/vm/os_aix.cpp	Mon Aug 28 23:46:22 2017 +0000
+++ b/hotspot/src/os/aix/vm/os_aix.cpp	Thu Aug 17 17:26:02 2017 +0200
@@ -1185,62 +1185,6 @@
 // directory not the java application's temp directory, ala java.io.tmpdir.
 const char* os::get_temp_directory() { return "/tmp"; }
 
-static bool file_exists(const char* filename) {
-  struct stat statbuf;
-  if (filename == NULL || strlen(filename) == 0) {
-    return false;
-  }
-  return os::stat(filename, &statbuf) == 0;
-}
-
-bool os::dll_build_name(char* buffer, size_t buflen,
-                        const char* pname, const char* fname) {
-  bool retval = false;
-  // Copied from libhpi
-  const size_t pnamelen = pname ? strlen(pname) : 0;
-
-  // Return error on buffer overflow.
-  if (pnamelen + strlen(fname) + 10 > (size_t) buflen) {
-    *buffer = '\0';
-    return retval;
-  }
-
-  if (pnamelen == 0) {
-    snprintf(buffer, buflen, "lib%s.so", fname);
-    retval = true;
-  } else if (strchr(pname, *os::path_separator()) != NULL) {
-    int n;
-    char** pelements = split_path(pname, &n);
-    if (pelements == NULL) {
-      return false;
-    }
-    for (int i = 0; i < n; i++) {
-      // Really shouldn't be NULL, but check can't hurt
-      if (pelements[i] == NULL || strlen(pelements[i]) == 0) {
-        continue; // skip the empty path values
-      }
-      snprintf(buffer, buflen, "%s/lib%s.so", pelements[i], fname);
-      if (file_exists(buffer)) {
-        retval = true;
-        break;
-      }
-    }
-    // release the storage
-    for (int i = 0; i < n; i++) {
-      if (pelements[i] != NULL) {
-        FREE_C_HEAP_ARRAY(char, pelements[i]);
-      }
-    }
-    if (pelements != NULL) {
-      FREE_C_HEAP_ARRAY(char*, pelements);
-    }
-  } else {
-    snprintf(buffer, buflen, "%s/lib%s.so", pname, fname);
-    retval = true;
-  }
-  return retval;
-}
-
 // Check if addr is inside libjvm.so.
 bool os::address_is_in_vm(address addr) {
 
--- a/hotspot/src/os/bsd/vm/os_bsd.cpp	Mon Aug 28 23:46:22 2017 +0000
+++ b/hotspot/src/os/bsd/vm/os_bsd.cpp	Thu Aug 17 17:26:02 2017 +0200
@@ -1172,13 +1172,6 @@
 
 // DLL functions
 
-#define JNI_LIB_PREFIX "lib"
-#ifdef __APPLE__
-  #define JNI_LIB_SUFFIX ".dylib"
-#else
-  #define JNI_LIB_SUFFIX ".so"
-#endif
-
 const char* os::dll_file_extension() { return JNI_LIB_SUFFIX; }
 
 // This must be hard coded because it's the system's temporary
@@ -1201,62 +1194,6 @@
 const char* os::get_temp_directory() { return "/tmp"; }
 #endif // __APPLE__
 
-static bool file_exists(const char* filename) {
-  struct stat statbuf;
-  if (filename == NULL || strlen(filename) == 0) {
-    return false;
-  }
-  return os::stat(filename, &statbuf) == 0;
-}
-
-bool os::dll_build_name(char* buffer, size_t buflen,
-                        const char* pname, const char* fname) {
-  bool retval = false;
-  // Copied from libhpi
-  const size_t pnamelen = pname ? strlen(pname) : 0;
-
-  // Return error on buffer overflow.
-  if (pnamelen + strlen(fname) + strlen(JNI_LIB_PREFIX) + strlen(JNI_LIB_SUFFIX) + 2 > buflen) {
-    return retval;
-  }
-
-  if (pnamelen == 0) {
-    snprintf(buffer, buflen, JNI_LIB_PREFIX "%s" JNI_LIB_SUFFIX, fname);
-    retval = true;
-  } else if (strchr(pname, *os::path_separator()) != NULL) {
-    int n;
-    char** pelements = split_path(pname, &n);
-    if (pelements == NULL) {
-      return false;
-    }
-    for (int i = 0; i < n; i++) {
-      // Really shouldn't be NULL, but check can't hurt
-      if (pelements[i] == NULL || strlen(pelements[i]) == 0) {
-        continue; // skip the empty path values
-      }
-      snprintf(buffer, buflen, "%s/" JNI_LIB_PREFIX "%s" JNI_LIB_SUFFIX,
-               pelements[i], fname);
-      if (file_exists(buffer)) {
-        retval = true;
-        break;
-      }
-    }
-    // release the storage
-    for (int i = 0; i < n; i++) {
-      if (pelements[i] != NULL) {
-        FREE_C_HEAP_ARRAY(char, pelements[i]);
-      }
-    }
-    if (pelements != NULL) {
-      FREE_C_HEAP_ARRAY(char*, pelements);
-    }
-  } else {
-    snprintf(buffer, buflen, "%s/" JNI_LIB_PREFIX "%s" JNI_LIB_SUFFIX, pname, fname);
-    retval = true;
-  }
-  return retval;
-}
-
 // check if addr is inside libjvm.so
 bool os::address_is_in_vm(address addr) {
   static address libjvm_base_addr;
--- a/hotspot/src/os/linux/vm/os_linux.cpp	Mon Aug 28 23:46:22 2017 +0000
+++ b/hotspot/src/os/linux/vm/os_linux.cpp	Thu Aug 17 17:26:02 2017 +0200
@@ -1419,53 +1419,6 @@
   return os::stat(filename, &statbuf) == 0;
 }
 
-bool os::dll_build_name(char* buffer, size_t buflen,
-                        const char* pname, const char* fname) {
-  bool retval = false;
-  // Copied from libhpi
-  const size_t pnamelen = pname ? strlen(pname) : 0;
-
-  // Return error on buffer overflow.
-  if (pnamelen + strlen(fname) + 10 > (size_t) buflen) {
-    return retval;
-  }
-
-  if (pnamelen == 0) {
-    snprintf(buffer, buflen, "lib%s.so", fname);
-    retval = true;
-  } else if (strchr(pname, *os::path_separator()) != NULL) {
-    int n;
-    char** pelements = split_path(pname, &n);
-    if (pelements == NULL) {
-      return false;
-    }
-    for (int i = 0; i < n; i++) {
-      // Really shouldn't be NULL, but check can't hurt
-      if (pelements[i] == NULL || strlen(pelements[i]) == 0) {
-        continue; // skip the empty path values
-      }
-      snprintf(buffer, buflen, "%s/lib%s.so", pelements[i], fname);
-      if (file_exists(buffer)) {
-        retval = true;
-        break;
-      }
-    }
-    // release the storage
-    for (int i = 0; i < n; i++) {
-      if (pelements[i] != NULL) {
-        FREE_C_HEAP_ARRAY(char, pelements[i]);
-      }
-    }
-    if (pelements != NULL) {
-      FREE_C_HEAP_ARRAY(char*, pelements);
-    }
-  } else {
-    snprintf(buffer, buflen, "%s/lib%s.so", pname, fname);
-    retval = true;
-  }
-  return retval;
-}
-
 // check if addr is inside libjvm.so
 bool os::address_is_in_vm(address addr) {
   static address libjvm_base_addr;
--- a/hotspot/src/os/solaris/vm/os_solaris.cpp	Mon Aug 28 23:46:22 2017 +0000
+++ b/hotspot/src/os/solaris/vm/os_solaris.cpp	Thu Aug 17 17:26:02 2017 +0200
@@ -1356,60 +1356,6 @@
 // directory not the java application's temp directory, ala java.io.tmpdir.
 const char* os::get_temp_directory() { return "/tmp"; }
 
-static bool file_exists(const char* filename) {
-  struct stat statbuf;
-  if (filename == NULL || strlen(filename) == 0) {
-    return false;
-  }
-  return os::stat(filename, &statbuf) == 0;
-}
-
-bool os::dll_build_name(char* buffer, size_t buflen,
-                        const char* pname, const char* fname) {
-  bool retval = false;
-  const size_t pnamelen = pname ? strlen(pname) : 0;
-
-  // Return error on buffer overflow.
-  if (pnamelen + strlen(fname) + 10 > (size_t) buflen) {
-    return retval;
-  }
-
-  if (pnamelen == 0) {
-    snprintf(buffer, buflen, "lib%s.so", fname);
-    retval = true;
-  } else if (strchr(pname, *os::path_separator()) != NULL) {
-    int n;
-    char** pelements = split_path(pname, &n);
-    if (pelements == NULL) {
-      return false;
-    }
-    for (int i = 0; i < n; i++) {
-      // really shouldn't be NULL but what the heck, check can't hurt
-      if (pelements[i] == NULL || strlen(pelements[i]) == 0) {
-        continue; // skip the empty path values
-      }
-      snprintf(buffer, buflen, "%s/lib%s.so", pelements[i], fname);
-      if (file_exists(buffer)) {
-        retval = true;
-        break;
-      }
-    }
-    // release the storage
-    for (int i = 0; i < n; i++) {
-      if (pelements[i] != NULL) {
-        FREE_C_HEAP_ARRAY(char, pelements[i]);
-      }
-    }
-    if (pelements != NULL) {
-      FREE_C_HEAP_ARRAY(char*, pelements);
-    }
-  } else {
-    snprintf(buffer, buflen, "%s/lib%s.so", pname, fname);
-    retval = true;
-  }
-  return retval;
-}
-
 // check if addr is inside libjvm.so
 bool os::address_is_in_vm(address addr) {
   static address libjvm_base_addr;
--- a/hotspot/src/os/windows/vm/os_windows.cpp	Mon Aug 28 23:46:22 2017 +0000
+++ b/hotspot/src/os/windows/vm/os_windows.cpp	Thu Aug 17 17:26:02 2017 +0200
@@ -1177,70 +1177,6 @@
   }
 }
 
-static bool file_exists(const char* filename) {
-  if (filename == NULL || strlen(filename) == 0) {
-    return false;
-  }
-  return GetFileAttributes(filename) != INVALID_FILE_ATTRIBUTES;
-}
-
-bool os::dll_build_name(char *buffer, size_t buflen,
-                        const char* pname, const char* fname) {
-  bool retval = false;
-  const size_t pnamelen = pname ? strlen(pname) : 0;
-  const char c = (pnamelen > 0) ? pname[pnamelen-1] : 0;
-
-  // Return error on buffer overflow.
-  if (pnamelen + strlen(fname) + 10 > buflen) {
-    return retval;
-  }
-
-  if (pnamelen == 0) {
-    jio_snprintf(buffer, buflen, "%s.dll", fname);
-    retval = true;
-  } else if (c == ':' || c == '\\') {
-    jio_snprintf(buffer, buflen, "%s%s.dll", pname, fname);
-    retval = true;
-  } else if (strchr(pname, *os::path_separator()) != NULL) {
-    int n;
-    char** pelements = split_path(pname, &n);
-    if (pelements == NULL) {
-      return false;
-    }
-    for (int i = 0; i < n; i++) {
-      char* path = pelements[i];
-      // Really shouldn't be NULL, but check can't hurt
-      size_t plen = (path == NULL) ? 0 : strlen(path);
-      if (plen == 0) {
-        continue; // skip the empty path values
-      }
-      const char lastchar = path[plen - 1];
-      if (lastchar == ':' || lastchar == '\\') {
-        jio_snprintf(buffer, buflen, "%s%s.dll", path, fname);
-      } else {
-        jio_snprintf(buffer, buflen, "%s\\%s.dll", path, fname);
-      }
-      if (file_exists(buffer)) {
-        retval = true;
-        break;
-      }
-    }
-    // release the storage
-    for (int i = 0; i < n; i++) {
-      if (pelements[i] != NULL) {
-        FREE_C_HEAP_ARRAY(char, pelements[i]);
-      }
-    }
-    if (pelements != NULL) {
-      FREE_C_HEAP_ARRAY(char*, pelements);
-    }
-  } else {
-    jio_snprintf(buffer, buflen, "%s\\%s.dll", pname, fname);
-    retval = true;
-  }
-  return retval;
-}
-
 // Needs to be in os specific directory because windows requires another
 // header file <direct.h>
 const char* os::get_current_directory(char *buf, size_t buflen) {
--- a/hotspot/src/share/vm/classfile/classLoader.cpp	Mon Aug 28 23:46:22 2017 +0000
+++ b/hotspot/src/share/vm/classfile/classLoader.cpp	Thu Aug 17 17:26:02 2017 +0200
@@ -1066,7 +1066,7 @@
   char path[JVM_MAXPATHLEN];
   char ebuf[1024];
   void* handle = NULL;
-  if (os::dll_build_name(path, sizeof(path), Arguments::get_dll_dir(), "zip")) {
+  if (os::dll_locate_lib(path, sizeof(path), Arguments::get_dll_dir(), "zip")) {
     handle = os::dll_load(path, ebuf, sizeof ebuf);
   }
   if (handle == NULL) {
@@ -1104,7 +1104,7 @@
   char path[JVM_MAXPATHLEN];
   char ebuf[1024];
   void* handle = NULL;
-  if (os::dll_build_name(path, sizeof(path), Arguments::get_dll_dir(), "jimage")) {
+  if (os::dll_locate_lib(path, sizeof(path), Arguments::get_dll_dir(), "jimage")) {
     handle = os::dll_load(path, ebuf, sizeof ebuf);
   }
   if (handle == NULL) {
--- a/hotspot/src/share/vm/prims/jvmtiExport.cpp	Mon Aug 28 23:46:22 2017 +0000
+++ b/hotspot/src/share/vm/prims/jvmtiExport.cpp	Thu Aug 17 17:26:02 2017 +0200
@@ -2497,14 +2497,13 @@
       library = os::dll_load(agent, ebuf, sizeof ebuf);
     } else {
       // Try to load the agent from the standard dll directory
-      if (os::dll_build_name(buffer, sizeof(buffer), Arguments::get_dll_dir(),
+      if (os::dll_locate_lib(buffer, sizeof(buffer), Arguments::get_dll_dir(),
                              agent)) {
         library = os::dll_load(buffer, ebuf, sizeof ebuf);
       }
       if (library == NULL) {
-        // not found - try local path
-        char ns[1] = {0};
-        if (os::dll_build_name(buffer, sizeof(buffer), ns, agent)) {
+        // not found - try OS default library path
+        if (os::dll_build_name(buffer, sizeof(buffer), agent)) {
           library = os::dll_load(buffer, ebuf, sizeof ebuf);
         }
       }
--- a/hotspot/src/share/vm/runtime/os.cpp	Mon Aug 28 23:46:22 2017 +0000
+++ b/hotspot/src/share/vm/runtime/os.cpp	Thu Aug 17 17:26:02 2017 +0200
@@ -235,6 +235,82 @@
   return OS_OK;
 }
 
+bool os::dll_build_name(char* buffer, size_t size, const char* fname) {
+  int n = jio_snprintf(buffer, size, "%s%s%s", JNI_LIB_PREFIX, fname, JNI_LIB_SUFFIX);
+  return (n != -1);
+}
+
+// Helper for dll_locate_lib.
+// Pass buffer and printbuffer as we already printed the path to buffer
+// when we called get_current_directory. This way we avoid another buffer
+// of size MAX_PATH.
+static bool conc_path_file_and_check(char *buffer, char *printbuffer, size_t printbuflen,
+                                     const char* pname, char lastchar, const char* fname) {
+
+  // Concatenate path and file name, but don't print double path separators.
+  const char *filesep = (WINDOWS_ONLY(lastchar == ':' ||) lastchar == os::file_separator()[0]) ?
+                        "" : os::file_separator();
+  int ret = jio_snprintf(printbuffer, printbuflen, "%s%s%s", pname, filesep, fname);
+  // Check whether file exists.
+  if (ret != -1) {
+    struct stat statbuf;
+    return os::stat(buffer, &statbuf) == 0;
+  }
+  return false;
+}
+
+bool os::dll_locate_lib(char *buffer, size_t buflen,
+                        const char* pname, const char* fname) {
+  bool retval = false;
+
+  size_t fullfnamelen = strlen(JNI_LIB_PREFIX) + strlen(fname) + strlen(JNI_LIB_SUFFIX);
+  char* fullfname = (char*)NEW_C_HEAP_ARRAY(char, fullfnamelen + 1, mtInternal);
+  if (dll_build_name(fullfname, fullfnamelen + 1, fname)) {
+    const size_t pnamelen = pname ? strlen(pname) : 0;
+
+    if (pnamelen == 0) {
+      // If no path given, use current working directory.
+      const char* p = get_current_directory(buffer, buflen);
+      if (p != NULL) {
+        const size_t plen = strlen(buffer);
+        const char lastchar = buffer[plen - 1];
+        retval = conc_path_file_and_check(buffer, &buffer[plen], buflen - plen,
+                                          "", lastchar, fullfname);
+      }
+    } else if (strchr(pname, *os::path_separator()) != NULL) {
+      // A list of paths. Search for the path that contains the library.
+      int n;
+      char** pelements = split_path(pname, &n);
+      if (pelements != NULL) {
+        for (int i = 0; i < n; i++) {
+          char* path = pelements[i];
+          // Really shouldn't be NULL, but check can't hurt.
+          size_t plen = (path == NULL) ? 0 : strlen(path);
+          if (plen == 0) {
+            continue; // Skip the empty path values.
+          }
+          const char lastchar = path[plen - 1];
+          retval = conc_path_file_and_check(buffer, buffer, buflen, path, lastchar, fullfname);
+          if (retval) break;
+        }
+        // Release the storage allocated by split_path.
+        for (int i = 0; i < n; i++) {
+          if (pelements[i] != NULL) {
+            FREE_C_HEAP_ARRAY(char, pelements[i]);
+          }
+        }
+        FREE_C_HEAP_ARRAY(char*, pelements);
+      }
+    } else {
+      // A definite path.
+      const char lastchar = pname[pnamelen-1];
+      retval = conc_path_file_and_check(buffer, buffer, buflen, pname, lastchar, fullfname);
+    }
+  }
+
+  FREE_C_HEAP_ARRAY(char*, fullfname);
+  return retval;
+}
 
 // --------------------- sun.misc.Signal (optional) ---------------------
 
@@ -427,13 +503,13 @@
     // Try to load verify dll first. In 1.3 java dll depends on it and is not
     // always able to find it when the loading executable is outside the JDK.
     // In order to keep working with 1.2 we ignore any loading errors.
-    if (dll_build_name(buffer, sizeof(buffer), Arguments::get_dll_dir(),
+    if (dll_locate_lib(buffer, sizeof(buffer), Arguments::get_dll_dir(),
                        "verify")) {
       dll_load(buffer, ebuf, sizeof(ebuf));
     }
 
     // Load java dll
-    if (dll_build_name(buffer, sizeof(buffer), Arguments::get_dll_dir(),
+    if (dll_locate_lib(buffer, sizeof(buffer), Arguments::get_dll_dir(),
                        "java")) {
       _native_java_library = dll_load(buffer, ebuf, sizeof(ebuf));
     }
@@ -444,7 +520,7 @@
 #if defined(__OpenBSD__)
     // Work-around OpenBSD's lack of $ORIGIN support by pre-loading libnet.so
     // ignore errors
-    if (dll_build_name(buffer, sizeof(buffer), Arguments::get_dll_dir(),
+    if (dll_locate_lib(buffer, sizeof(buffer), Arguments::get_dll_dir(),
                        "net")) {
       dll_load(buffer, ebuf, sizeof(ebuf));
     }
--- a/hotspot/src/share/vm/runtime/os.hpp	Mon Aug 28 23:46:22 2017 +0000
+++ b/hotspot/src/share/vm/runtime/os.hpp	Thu Aug 17 17:26:02 2017 +0200
@@ -541,9 +541,16 @@
   static const char*    get_temp_directory();
   static const char*    get_current_directory(char *buf, size_t buflen);
 
-  // Builds a platform-specific full library path given a ld path and lib name
-  // Returns true if buffer contains full path to existing file, false otherwise
+  // Builds the platform-specific name of a library.
+  // Returns false if the buffer is too small.
   static bool           dll_build_name(char* buffer, size_t size,
+                                       const char* fname);
+
+  // Builds a platform-specific full library path given an ld path and
+  // unadorned library name. Returns true if the buffer contains a full
+  // path to an existing file, false otherwise. If pathname is empty,
+  // uses the path to the current directory.
+  static bool           dll_locate_lib(char* buffer, size_t size,
                                        const char* pathname, const char* fname);
 
   // Symbol lookup, find nearest function name; basically it implements
--- a/hotspot/src/share/vm/runtime/thread.cpp	Mon Aug 28 23:46:22 2017 +0000
+++ b/hotspot/src/share/vm/runtime/thread.cpp	Thu Aug 17 17:26:02 2017 +0200
@@ -3925,13 +3925,12 @@
       }
     } else {
       // Try to load the agent from the standard dll directory
-      if (os::dll_build_name(buffer, sizeof(buffer), Arguments::get_dll_dir(),
+      if (os::dll_locate_lib(buffer, sizeof(buffer), Arguments::get_dll_dir(),
                              name)) {
         library = os::dll_load(buffer, ebuf, sizeof ebuf);
       }
-      if (library == NULL) { // Try the local directory
-        char ns[1] = {0};
-        if (os::dll_build_name(buffer, sizeof(buffer), ns, name)) {
+      if (library == NULL) { // Try the library path directory.
+        if (os::dll_build_name(buffer, sizeof(buffer), name)) {
           library = os::dll_load(buffer, ebuf, sizeof ebuf);
         }
         if (library == NULL) {