From 7a64fbbb9292f4d65a6970206dec1a7d7645046b Mon Sep 17 00:00:00 2001 From: Simon Tooke Date: Thu, 17 Oct 2024 08:06:37 +0000 Subject: [PATCH] 8338851: Hoist os::Posix::realpath() to os::realpath() and implement on Windows Reviewed-by: dholmes, stuefe, jwaters --- src/hotspot/os/aix/os_aix.cpp | 6 +- src/hotspot/os/bsd/os_bsd.cpp | 6 +- src/hotspot/os/linux/os_linux.cpp | 6 +- src/hotspot/os/linux/os_perf_linux.cpp | 2 +- src/hotspot/os/posix/os_posix.cpp | 5 +- src/hotspot/os/posix/os_posix.hpp | 7 -- src/hotspot/os/windows/os_windows.cpp | 22 +++++ src/hotspot/share/runtime/os.hpp | 7 ++ .../share/services/diagnosticCommand.cpp | 4 +- .../share/utilities/globalDefinitions.hpp | 2 +- test/hotspot/gtest/runtime/test_os.cpp | 80 +++++++++++++++++++ 11 files changed, 123 insertions(+), 24 deletions(-) diff --git a/src/hotspot/os/aix/os_aix.cpp b/src/hotspot/os/aix/os_aix.cpp index 63aa53f0a2350..5022272d1b431 100644 --- a/src/hotspot/os/aix/os_aix.cpp +++ b/src/hotspot/os/aix/os_aix.cpp @@ -1293,7 +1293,7 @@ void os::jvm_path(char *buf, jint buflen) { Dl_info dlinfo; int ret = dladdr(CAST_FROM_FN_PTR(void *, os::jvm_path), &dlinfo); assert(ret != 0, "cannot locate libjvm"); - char* rp = os::Posix::realpath((char *)dlinfo.dli_fname, buf, buflen); + char* rp = os::realpath((char *)dlinfo.dli_fname, buf, buflen); assert(rp != nullptr, "error in realpath(): maybe the 'path' argument is too long?"); if (Arguments::sun_java_launcher_is_altjvm()) { @@ -1324,7 +1324,7 @@ void os::jvm_path(char *buf, jint buflen) { } assert(strstr(p, "/libjvm") == p, "invalid library name"); - rp = os::Posix::realpath(java_home_var, buf, buflen); + rp = os::realpath(java_home_var, buf, buflen); if (rp == nullptr) { return; } @@ -1345,7 +1345,7 @@ void os::jvm_path(char *buf, jint buflen) { snprintf(buf + len, buflen-len, "/hotspot/libjvm.so"); } else { // Go back to path of .so - rp = os::Posix::realpath((char *)dlinfo.dli_fname, buf, buflen); + rp = os::realpath((char *)dlinfo.dli_fname, buf, buflen); if (rp == nullptr) { return; } diff --git a/src/hotspot/os/bsd/os_bsd.cpp b/src/hotspot/os/bsd/os_bsd.cpp index 18818268c1f9a..6b3bf060d6f27 100644 --- a/src/hotspot/os/bsd/os_bsd.cpp +++ b/src/hotspot/os/bsd/os_bsd.cpp @@ -1509,7 +1509,7 @@ void os::jvm_path(char *buf, jint buflen) { assert(ret, "cannot locate libjvm"); char *rp = nullptr; if (ret && dli_fname[0] != '\0') { - rp = os::Posix::realpath(dli_fname, buf, buflen); + rp = os::realpath(dli_fname, buf, buflen); } if (rp == nullptr) { return; @@ -1541,7 +1541,7 @@ void os::jvm_path(char *buf, jint buflen) { p = strrchr(buf, '/'); assert(strstr(p, "/libjvm") == p, "invalid library name"); - rp = os::Posix::realpath(java_home_var, buf, buflen); + rp = os::realpath(java_home_var, buf, buflen); if (rp == nullptr) { return; } @@ -1575,7 +1575,7 @@ void os::jvm_path(char *buf, jint buflen) { snprintf(buf + len, buflen-len, "/libjvm%s", JNI_LIB_SUFFIX); } else { // Fall back to path of current library - rp = os::Posix::realpath(dli_fname, buf, buflen); + rp = os::realpath(dli_fname, buf, buflen); if (rp == nullptr) { return; } diff --git a/src/hotspot/os/linux/os_linux.cpp b/src/hotspot/os/linux/os_linux.cpp index c80663fec3d3a..514268a7400bc 100644 --- a/src/hotspot/os/linux/os_linux.cpp +++ b/src/hotspot/os/linux/os_linux.cpp @@ -2763,7 +2763,7 @@ void os::jvm_path(char *buf, jint buflen) { assert(ret, "cannot locate libjvm"); char *rp = nullptr; if (ret && dli_fname[0] != '\0') { - rp = os::Posix::realpath(dli_fname, buf, buflen); + rp = os::realpath(dli_fname, buf, buflen); } if (rp == nullptr) { return; @@ -2797,7 +2797,7 @@ void os::jvm_path(char *buf, jint buflen) { } assert(strstr(p, "/libjvm") == p, "invalid library name"); - rp = os::Posix::realpath(java_home_var, buf, buflen); + rp = os::realpath(java_home_var, buf, buflen); if (rp == nullptr) { return; } @@ -2818,7 +2818,7 @@ void os::jvm_path(char *buf, jint buflen) { snprintf(buf + len, buflen-len, "/hotspot/libjvm.so"); } else { // Go back to path of .so - rp = os::Posix::realpath(dli_fname, buf, buflen); + rp = os::realpath(dli_fname, buf, buflen); if (rp == nullptr) { return; } diff --git a/src/hotspot/os/linux/os_perf_linux.cpp b/src/hotspot/os/linux/os_perf_linux.cpp index a31a58e55af93..996f83611b048 100644 --- a/src/hotspot/os/linux/os_perf_linux.cpp +++ b/src/hotspot/os/linux/os_perf_linux.cpp @@ -789,7 +789,7 @@ char* SystemProcessInterface::SystemProcesses::ProcessIterator::get_exe_path() { jio_snprintf(buffer, PATH_MAX, "/proc/%s/exe", _entry->d_name); buffer[PATH_MAX - 1] = '\0'; - return os::Posix::realpath(buffer, _exePath, PATH_MAX); + return os::realpath(buffer, _exePath, PATH_MAX); } char* SystemProcessInterface::SystemProcesses::ProcessIterator::allocate_string(const char* str) const { diff --git a/src/hotspot/os/posix/os_posix.cpp b/src/hotspot/os/posix/os_posix.cpp index 78fd2a678886e..75253843593c6 100644 --- a/src/hotspot/os/posix/os_posix.cpp +++ b/src/hotspot/os/posix/os_posix.cpp @@ -1029,10 +1029,10 @@ char* os::Posix::describe_pthread_attr(char* buf, size_t buflen, const pthread_a return buf; } -char* os::Posix::realpath(const char* filename, char* outbuf, size_t outbuflen) { +char* os::realpath(const char* filename, char* outbuf, size_t outbuflen) { if (filename == nullptr || outbuf == nullptr || outbuflen < 1) { - assert(false, "os::Posix::realpath: invalid arguments."); + assert(false, "os::realpath: invalid arguments."); errno = EINVAL; return nullptr; } @@ -1067,7 +1067,6 @@ char* os::Posix::realpath(const char* filename, char* outbuf, size_t outbuflen) } } return result; - } int os::stat(const char *path, struct stat *sbuf) { diff --git a/src/hotspot/os/posix/os_posix.hpp b/src/hotspot/os/posix/os_posix.hpp index d872a6dae899e..cac5b250cdfa0 100644 --- a/src/hotspot/os/posix/os_posix.hpp +++ b/src/hotspot/os/posix/os_posix.hpp @@ -73,13 +73,6 @@ class os::Posix { // to buf with len buflen; buf is returned. static char* describe_pthread_attr(char* buf, size_t buflen, const pthread_attr_t* attr); - // A safe implementation of realpath which will not cause a buffer overflow if the resolved path - // is longer than PATH_MAX. - // On success, returns 'outbuf', which now contains the path. - // On error, it will return null and set errno. The content of 'outbuf' is undefined. - // On truncation error ('outbuf' too small), it will return null and set errno to ENAMETOOLONG. - static char* realpath(const char* filename, char* outbuf, size_t outbuflen); - // Returns true if given uid is root. static bool is_root(uid_t uid); diff --git a/src/hotspot/os/windows/os_windows.cpp b/src/hotspot/os/windows/os_windows.cpp index fe093aee116b6..66a6a92f5e078 100644 --- a/src/hotspot/os/windows/os_windows.cpp +++ b/src/hotspot/os/windows/os_windows.cpp @@ -5397,6 +5397,28 @@ void os::funlockfile(FILE* fp) { _unlock_file(fp); } +char* os::realpath(const char* filename, char* outbuf, size_t outbuflen) { + + if (filename == nullptr || outbuf == nullptr || outbuflen < 1) { + assert(false, "os::realpath: invalid arguments."); + errno = EINVAL; + return nullptr; + } + + char* result = nullptr; + ALLOW_C_FUNCTION(::_fullpath, char* p = ::_fullpath(nullptr, filename, 0);) + if (p != nullptr) { + if (strlen(p) < outbuflen) { + strcpy(outbuf, p); + result = outbuf; + } else { + errno = ENAMETOOLONG; + } + ALLOW_C_FUNCTION(::free, ::free(p);) // *not* os::free + } + return result; +} + // Map a block of memory. char* os::pd_map_memory(int fd, const char* file_name, size_t file_offset, char *addr, size_t bytes, bool read_only, diff --git a/src/hotspot/share/runtime/os.hpp b/src/hotspot/share/runtime/os.hpp index 65071769bc4cb..323b8d2df8dbe 100644 --- a/src/hotspot/share/runtime/os.hpp +++ b/src/hotspot/share/runtime/os.hpp @@ -668,6 +668,13 @@ class os: AllStatic { static void flockfile(FILE* fp); static void funlockfile(FILE* fp); + // A safe implementation of realpath which will not cause a buffer overflow if the resolved path + // is longer than PATH_MAX. + // On success, returns 'outbuf', which now contains the path. + // On error, it will return null and set errno. The content of 'outbuf' is undefined. + // On truncation error ('outbuf' too small), it will return null and set errno to ENAMETOOLONG. + static char* realpath(const char* filename, char* outbuf, size_t outbuflen); + static int compare_file_modified_times(const char* file1, const char* file2); static bool same_files(const char* file1, const char* file2); diff --git a/src/hotspot/share/services/diagnosticCommand.cpp b/src/hotspot/share/services/diagnosticCommand.cpp index 78c655a43fcf2..4263d2a1d4b56 100644 --- a/src/hotspot/share/services/diagnosticCommand.cpp +++ b/src/hotspot/share/services/diagnosticCommand.cpp @@ -1202,12 +1202,10 @@ void SystemDumpMapDCmd::execute(DCmdSource source, TRAPS) { output()->print_cr("(NMT is disabled, will not annotate mappings)."); } MemMapPrinter::print_all_mappings(&fs); -#ifndef _WIN64 // For the readers convenience, resolve path name. char tmp[JVM_MAXPATHLEN]; - const char* absname = os::Posix::realpath(name, tmp, sizeof(tmp)); + const char* absname = os::realpath(name, tmp, sizeof(tmp)); name = absname != nullptr ? absname : name; -#endif output()->print_cr("Memory map dumped to \"%s\".", name); } else { output()->print_cr("Failed to open \"%s\" for writing (%s).", name, os::strerror(errno)); diff --git a/src/hotspot/share/utilities/globalDefinitions.hpp b/src/hotspot/share/utilities/globalDefinitions.hpp index 6d385165b8b1b..25af626c6286b 100644 --- a/src/hotspot/share/utilities/globalDefinitions.hpp +++ b/src/hotspot/share/utilities/globalDefinitions.hpp @@ -210,7 +210,7 @@ FORBID_C_FUNCTION(char* strdup(const char *s), "use os::strdup"); FORBID_C_FUNCTION(char* strndup(const char *s, size_t n), "don't use"); FORBID_C_FUNCTION(int posix_memalign(void **memptr, size_t alignment, size_t size), "don't use"); FORBID_C_FUNCTION(void* aligned_alloc(size_t alignment, size_t size), "don't use"); -FORBID_C_FUNCTION(char* realpath(const char* path, char* resolved_path), "use os::Posix::realpath"); +FORBID_C_FUNCTION(char* realpath(const char* path, char* resolved_path), "use os::realpath"); FORBID_C_FUNCTION(char* get_current_dir_name(void), "use os::get_current_directory()"); FORBID_C_FUNCTION(char* getwd(char *buf), "use os::get_current_directory()"); FORBID_C_FUNCTION(wchar_t* wcsdup(const wchar_t *s), "don't use"); diff --git a/test/hotspot/gtest/runtime/test_os.cpp b/test/hotspot/gtest/runtime/test_os.cpp index 78e5212ab3709..fe3c634056c2f 100644 --- a/test/hotspot/gtest/runtime/test_os.cpp +++ b/test/hotspot/gtest/runtime/test_os.cpp @@ -405,6 +405,86 @@ TEST_VM(os, jio_snprintf) { test_snprintf(jio_snprintf, false); } +#ifndef MAX_PATH +#define MAX_PATH (2 * K) +#endif + +TEST_VM(os, realpath) { + // POSIX requires that the file exists; Windows tests for a valid drive letter + // but may or may not test if the file exists. */ + static const char* nosuchpath = "/1234567890123456789"; + static const char* tmppath = "/tmp"; + + char buffer[MAX_PATH]; + + // Test a non-existant path, but provide a short buffer. + errno = 0; + const char* returnedBuffer = os::realpath(nosuchpath, buffer, sizeof(nosuchpath) - 2); + // Reports ENOENT on Linux, ENAMETOOLONG on Windows. + EXPECT_TRUE(returnedBuffer == nullptr); +#ifdef _WINDOWS + EXPECT_TRUE(errno == ENAMETOOLONG); +#else + EXPECT_TRUE(errno == ENOENT); +#endif + + // Test a non-existant path, but provide an adequate buffer. + errno = 0; + buffer[0] = 0; + returnedBuffer = os::realpath(nosuchpath, buffer, sizeof(nosuchpath) + 3); + // Reports ENOENT on Linux, may return 0 (and report an error) or buffer on some versions of Windows. +#ifdef _WINDOWS + if (returnedBuffer != nullptr) { + EXPECT_TRUE(returnedBuffer == buffer); + } else { + EXPECT_TRUE(errno != 0); + } +#else + EXPECT_TRUE(returnedBuffer == nullptr); + EXPECT_TRUE(errno == ENOENT); +#endif + + // Test an existing path using a large buffer. + errno = 0; + returnedBuffer = os::realpath(tmppath, buffer, MAX_PATH); + EXPECT_TRUE(returnedBuffer == buffer); + + // Test an existing path using a buffer that is too small on a normal macOS install. + errno = 0; + returnedBuffer = os::realpath(tmppath, buffer, strlen(tmppath) + 3); + // On MacOS, /tmp is a symlink to /private/tmp, so doesn't fit in a small buffer. +#ifndef __APPLE__ + EXPECT_TRUE(returnedBuffer == buffer); +#else + EXPECT_TRUE(returnedBuffer == nullptr); + EXPECT_TRUE(errno == ENAMETOOLONG); +#endif + + // Test an existing path using a buffer that is too small. + errno = 0; + returnedBuffer = os::realpath(tmppath, buffer, strlen(tmppath) - 1); + EXPECT_TRUE(returnedBuffer == nullptr); + EXPECT_TRUE(errno == ENAMETOOLONG); + + // The following tests cause an assert inside os::realpath() in fastdebug mode: +#ifndef ASSERT + errno = 0; + returnedBuffer = os::realpath(nullptr, buffer, sizeof(buffer)); + EXPECT_TRUE(returnedBuffer == nullptr); + EXPECT_TRUE(errno == EINVAL); + + errno = 0; + returnedBuffer = os::realpath(tmppath, nullptr, sizeof(buffer)); + EXPECT_TRUE(returnedBuffer == nullptr); + EXPECT_TRUE(errno == EINVAL); + + errno = 0; + returnedBuffer = os::realpath(tmppath, buffer, 0); + EXPECT_TRUE(returnedBuffer == nullptr); + EXPECT_TRUE(errno == EINVAL); +#endif +} + #ifdef __APPLE__ // Not all macOS versions can use os::reserve_memory (i.e. anon_mmap) API // to reserve executable memory, so before attempting to use it,