On Mon, 2018-11-19 at 15:09 +0000, Andrew Hughes wrote: > On Fri, 9 Nov 2018 at 16:02, Severin Gehwolf <sgehw...@redhat.com> wrote: > > > > Hi, > > > > Could somebody please review this 8u backport of 8210836 as I'd like to > > get 8210647 (opt for sa) backported to 8u as well? Unfortunately the > > change from JDK 12 doesn't apply cleanly so I've included select > > changes from 8140482 so that the backport remains minimal. If anything, > > this makes the code more robust I'd think. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8210836 > > webrev: > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210836/jdk8/webrev.01/ > > > > Testing: Manual testing of loading core file in SA on linux. Stepping > > through code in the debugger. Basic jsadebugd core file loading. > > > > Thoughts? > > > > Thanks, > > Severin > > > >
Thanks for the review! > Is there a reason not to just backport > https://bugs.openjdk.java.net/browse/JDK-8140482 > as a pre-requisite for this? It looks pretty uncontroversial (a lot of > it is indenting fixes), > makes the code a little more secure and would help any future > backports in these areas > if the code is in sync. Two reasons: (1) JDK-8140482 isn't a small change. (2) The patch doesn't apply cleanly[1] from JDK 9 so would be some work to get the pre-requisite in. It appears the code is already out of sync and the risk would be higher if we backported all of JDK-8140482 for a relatively small change of JDK-8210836. The aim was to keep the backport as minimal as possible. Thanks, Severin [1] I'm getting this: $ hg import ../../openjdk9-mainline/hotspot/JDK-8140482.jdk9.export.patch applying ../../openjdk9-mainline/hotspot/JDK-8140482.jdk9.export.patch patching file agent/src/os/linux/libproc_impl.c Hunk #1 FAILED at 37 Hunk #2 FAILED at 47 Hunk #3 FAILED at 69 3 out of 3 hunks FAILED -- saving rejects to file agent/src/os/linux/libproc_impl.c.rej patching file agent/src/os/linux/ps_core.c Hunk #1 FAILED at 773 1 out of 1 hunks FAILED -- saving rejects to file agent/src/os/linux/ps_core.c.rej patching file src/cpu/x86/vm/stubRoutines_x86.cpp Hunk #1 FAILED at 146 1 out of 1 hunks FAILED -- saving rejects to file src/cpu/x86/vm/stubRoutines_x86.cpp.rej unable to find 'src/cpu/x86/vm/templateTable_x86.cpp' for patching (use '--prefix' to apply patch relative to the current directory) 2 out of 2 hunks FAILED -- saving rejects to file src/cpu/x86/vm/templateTable_x86.cpp.rej patching file src/os/linux/vm/os_linux.cpp Hunk #2 FAILED at 5928 1 out of 2 hunks FAILED -- saving rejects to file src/os/linux/vm/os_linux.cpp.rej patching file src/share/vm/runtime/deoptimization.cpp Hunk #2 FAILED at 2082 Hunk #3 FAILED at 2178 2 out of 3 hunks FAILED -- saving rejects to file src/share/vm/runtime/deoptimization.cpp.rej patching file src/share/vm/runtime/task.cpp Hunk #1 succeeded at 122 with fuzz 1 (offset 4 lines). patching file src/share/vm/services/memoryService.cpp Hunk #1 succeeded at 546 with fuzz 2 (offset -2 lines). Hunk #2 FAILED at 558 1 out of 3 hunks FAILED -- saving rejects to file src/share/vm/services/memoryService.cpp.rej abort: patch failed to apply