On 10/2/19 1:39 AM, Severin Gehwolf wrote:
Hi Chris,
On Tue, 2019-10-01 at 12:51 -0700, Chris Plummer wrote:
Hi Severin,
Sorry, this is not an area that I have any expertise in. However, I did
confirm that it fixes the NPE I was seeing with JShellHeapDumpTest.java,
which brings up a question. You said this happens with -Xcomp, but I was
never using -Xcomp. Might it also be triggered without -Xcomp?
Yes, as soon as C1/C2 kick in you might be seeing the issue. In fact, I
had one reproducer which triggered the issue quite reliably which
didn't use -Xcomp explicitly. Nevertheless, certain methods got JIT
compiled soon and the issue surfaced. You should be able to verify by
adding -XX:+PrintCompilation to the host program and/or comparing it to
-Xint runs.
TLDR; -Xcomp helps reproducing the issue on smaller test cases, but
isn't essential in the general case.
Does that make sense?
Yes, thanks for the explanation.
Chris
Thanks,
Severin
thanks,
Chris
On 10/1/19 1:58 AM, Severin Gehwolf wrote:
Anyone? Chris maybe?
On Fri, 2019-09-27 at 14:12 +0200, Severin Gehwolf wrote:
Hi,
Could I please get reviews for this SA fix? The issue only happens
intermittently and with -Xcomp. The new regression test reproduces the
issue somewhat reliably. I got 10/10 fails for unpatched, but I've seen
it pass as well.
When the issue happens, PCDesc's getScopeDecodeOffset() returs 0
(DebugInformationRecorder.SERIALIZED_NULL). The current SA code doesn't
handle this case and goes on and tries to read ScopeDesc from the
DebugInfoReadStream at the bogus offset. From then on, bad things
happen. A NPE in StackTrace could be one symptom.
The same code in hotspot deals with serialized null differently. It
doesn't read from the debug info stream, and manually sets up a
reasonable frame. Note decode_body is called from ScopeDesc's
constructor where decode_offset might have been set to 0:
void ScopeDesc::decode_body() {
if (decode_offset() == DebugInformationRecorder::serialized_null) {
// This is a sentinel record, which is only relevant to
// approximate queries. Decode a reasonable frame.
_sender_decode_offset = DebugInformationRecorder::serialized_null;
_method = _code->method();
_bci = InvocationEntryBci;
_locals_decode_offset = DebugInformationRecorder::serialized_null;
_expressions_decode_offset = DebugInformationRecorder::serialized_null;
_monitors_decode_offset = DebugInformationRecorder::serialized_null;
} else {
// decode header
DebugInfoReadStream* stream = stream_at(decode_offset());
_sender_decode_offset = stream->read_int();
_method = stream->read_method();
_bci = stream->read_bci();
// decode offsets for body and sender
_locals_decode_offset = stream->read_int();
_expressions_decode_offset = stream->read_int();
_monitors_decode_offset = stream->read_int();
}
}
The proposed patch handles serialized null scopes similar to the
hotspot side of things, by returning a null scope. CompiledVFrame
already deals with null scopes when in debugging mode.
Bug: https://bugs.openjdk.java.net/browse/JDK-8196969
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8196969/03/webrev/
Testing: tier 1 tests on Linux x86_64 (release/fastdebug). jdk-submit
and ran various reproducer tests including 1000 interations of the
added regression test. All pass.
Thoughts?
Thanks,
Severin
