[RFR] 8196969: JTreg Failure: serviceability/sa/ClhsdbJstack.java causes NPE

[Chris Plummer]

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