Is it possible to find PDB (windows debugging info) for published jvm.dll?
Hello, I'm trying to debug a native crash that regularly happens for one of our customers. It crashes somewhere amidst java interpreter code, with corrupted value in CPU's RIP register. So it's quite complicated. It would certainly help me a lot if I had access to jvm.pdb files for published JRE's... I'm mostly after 10.0.1+10 and 8.0_144-b01 at the moment. According to the published binaries, the PDB's are generated here on the build server: t:\workspace\build\windows-x64\support\modules_libs\java.base\server\jvm.pdb I believe they are saved somewhere after building, at least internally? Is there some way to obtain those pdb's myself? If not, can someone send me the PDB's, at least for 10.0.1+10? I understand that I can build OpenJDK myself to obtain the PDB's, but unfortunately they will not match dumps provided by the client.
Re: 8194734 : Handle to jimage file inherited into child processes (win)
Alan, thanks a lot for applying my patch! Now, do I need to try and remove the magic from Unix process creation code? Or you'd rather have it untouched?
Re: 8194734 : Handle to jimage file inherited into child processes (win)
I'm still puzzled by the Unix change. I completely agree with Martin about CLOSEXEC but there is something fishy here that we need to get a handle on. When we fork then we close >= 3 in the child so there shouldn't be any magic with fd 3. The current code closes >= 4. FD 3 is a special one that is intended to be error pipe back to parent. But after jimage was implemented, FD 3 is occupied by it instead. As far as I understand it, there's no need for hardcoded FD numbers at all. I can remove all magic from this code if someone promises me he'll get the patch applied (after review, of course!) Aside from removing magic from code, the future patch will also fix communicating errors back to parent, because with current code it obviously can't work.
Re: 8194734 : Handle to jimage file inherited into child processes (win)
Please get Windows change applied. I'm mostly interested in this part, because bug is currently hidden on Unix. I'm still puzzled by the Unix change. I completely agree with Martin about CLOSEXEC but there is something fishy here that we need to get a handle on. When we fork then we close >= 3 in the child so there shouldn't be any magic with fd 3.
Re: 8194734 : Handle to jimage file inherited into child processes (win)
I still need someone to get this patch applied. Please? At this moment, it seems that patch is validated: > Martin Buchholz > But regardless of that, we should try to set the CLOEXEC bit on all > our file descriptor > Alan Bateman > The update to osSupport_Windows.cpp looks okay.
Re: 8194734 : Handle to jimage file inherited into child processes (win)
> I'm hoping that JDK core libraries overall do a better job of reusing > the code in e.g. > src/hotspot/os/linux/os_linux:os::open > somehow, that actually gets the CLOEXEC bit right. os::open is exactly what was used in libjimage before the bug was introduced. I already described it in my first mail. I'm rarther new with all this. I understand it that hotspot is some sort of staging grounds for alpha features? If so, I can understand why libjimage no longer uses it. Also, it felt reasonable to me that someone wanted a library with dependencies minimzed.
Re: 8194734 : Handle to jimage file inherited into child processes (win)
I'm still only half paying attention, but I agree with Alexander that the low-level infrastructure in libjimage is fishy. The rule should be that file descriptors created by the JDK should have the CLOEXEC bit on. But even if the CLOEXEC bit was forgotten, the subprocess code should still succeed in closing that fd and use FAIL_FILENO for the error reporting pipe in childproc.c. That's two independent problems and I suggest adressing them one at a time. I anyone willing to get my patch applied? I do not have commiter rights yet, but my OCA is already processed.
Re: 8194734 : Handle to jimage file inherited into child processes (win)
> The only surprise is that it doesn't close as it shouldn't need the > file descriptor after it mmapped. Indeed, both on Windows and Unix file descriptor can be closed after mapping. On the other hand, ImageFileReader is not prepared for that. See http://hg.openjdk.java.net/jdk/jdk/file/8bdf2b5f472d/src/java.base/share/native/libjimage/imageFile.cpp ImageFileReader::get_resource() uses ImageFileReader::read_at() if resource is not compressed. Furthermore, ImageFileReader::get_resource() uses ImageFileReader::read_at() if not entire jimage is mapped. This happens if architecture is not 64-bit. Even if these code paths are changed, I still think that closing file descriptor can be unexpected and will provoke future bugs.
Re: 8194734 : Handle to jimage file inherited into child processes (win)
> STDERR_FILENO is 2 so I'm curious which error reporting pipe is bring > discussed here. We're discussing line 365 of childproc.c http://hg.openjdk.java.net/jdk/jdk/file/8bdf2b5f472d/src/java.base/unix/native/libjava/childproc.c
Re: 8194734 : Handle to jimage file inherited into child processes (win)
> The update to osSupport_Windows.cpp looks okay. I think it would be > useful to discuss the osSupport_unix.cpp change further as the > childproc code is supported to close the file descriptors. childproc code uses some dark magic to figure which files to close. At the moment, closeDescriptors() will close all files which has descriptors bigger then from_fd (4). However, jimage ends up having descriptor 3, so it's not closed. On the other hand, childproc expects descriptor 3 to be occupied by a pipe to parent (FAIL_FILENO). This means that currently, dark magic is already broken. The only reason for jimage to not go into child process is being mistaken with FAIL_FILENO. One way of fixing bug in childproc could be increasing FAIL_FILENO to match its actual value of file descriptor. This way, jimage will no longer be mistaken with FAIL_FILENO and therefore, it will NOT be closed. With all this in mind, I believe it wouldn't hurt to explicitly say that jimage must not be inherited, without relying on current implementation of childproc. That makes a sturdy and clean code. When this patch is accepted, I can also work on fixing bugs in childproc, if that is considered worthy.
8194734 : Handle to jimage file inherited into child processes (win)
The problem in this bug is that jimage file is mistakenly opened with
"inherit by child processes" flag. In our case, the child process is
started with Runtime.exec() and serves as updater that can also replace
embedded JRE. However, due to jimage ("lib/modules") file handle being
inherited, embedded JRE can not be replaced.
The bug was introduced in commits "8087181: Move native jimage code to
its own library (maybe libjimage)"
Before this commit, os::open() was used in libjimage code and it handles
file inheritance correctly, see:
/hotspot/src/share/vm/classfile/imageFile.cpp
* ImageFileReader::open() uses os::open(_name, 0, O_RDONLY)
/hotspot/src/os/windows/vm/os_windows.cpp
* os::open() calls to ::open(pathbuf, oflag | O_BINARY | O_NOINHERIT, mode);
/hotspot/src/os/linux/vm/os_linux.cpp
* os::open() uses both O_CLOEXEC and FD_CLOEXEC
After this commit, a new osSupport::openReadOnly() is implemented and it
does not handle file inheritance properly.
Please find patch attached.
As far as I understand from OpenJDK's "How to contribute" page, I need a
"sponsor" who will get this patch applied.
Windows - compiled and tested:
Bug fixed.
Ubuntu - compiled and tested:
Bug did not occur before my patch due to another bug in childProcess()
function (src/java.base/unix/native/libjava/childproc.c) According to my
debugging, FD_CLOEXEC is applied to jimage file there because
childProcess() thinks that it's a error pipe (FAIL_FILENO). Should the
bug in childProcess() be fixed, it can unearth currently fixed bug.
# HG changeset patch
# User Alexander Miloslavskiy <alexandr.miloslavs...@gmail.com>
# Date 1522703926 -10800
# Tue Apr 03 00:18:46 2018 +0300
# Node ID 87954e967cc67cab4b480a4ec7ff54a334e0f4ce
# Parent de0fd2c8a401a564f06d42fac15559154c42358a
8194734: jimage file descriptor was inherited into processes started with
Runtime.exec()
Summary: Prevented inheritance with FD_CLOEXEC on unix, O_NOINHERIT on windows.
Refactoring: added O_RDONLY (equals 0)
Refactoring: added O_BINARY on windows (currently ignored by osSupport::read
and osSupport::map_memory)
diff -r de0fd2c8a401 -r 87954e967cc6
src/java.base/unix/native/libjimage/osSupport_unix.cpp
--- a/src/java.base/unix/native/libjimage/osSupport_unix.cppFri Mar 30
14:36:18 2018 -0700
+++ b/src/java.base/unix/native/libjimage/osSupport_unix.cppTue Apr 03
00:18:46 2018 +0300
@@ -38,7 +38,16 @@
* Return the file descriptor.
*/
jint osSupport::openReadOnly(const char *path) {
-return ::open(path, 0);
+int fd = ::open(path, O_RDONLY);
+
+// jimage file descriptors must not be inherited by child processes.
+// This is especially true for child processes started with Runtime.exec
(see 8194734)
+#ifdef FD_CLOEXEC
+int flags = ::fcntl(fd, F_GETFD);
+::fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
+#endif
+
+return fd;
}
/**
diff -r de0fd2c8a401 -r 87954e967cc6
src/java.base/windows/native/libjimage/osSupport_windows.cpp
--- a/src/java.base/windows/native/libjimage/osSupport_windows.cpp Fri Mar
30 14:36:18 2018 -0700
+++ b/src/java.base/windows/native/libjimage/osSupport_windows.cpp Tue Apr
03 00:18:46 2018 +0300
@@ -38,7 +38,9 @@
* Return the file descriptor.
*/
jint osSupport::openReadOnly(const char *path) {
-return ::open(path, 0, 0);
+// jimage file descriptors must not be inherited by child processes.
+// This is especially true for child processes started with Runtime.exec
(see 8194734)
+return ::open(path, O_BINARY | O_NOINHERIT, O_RDONLY);
}
/**
