[9] RFR (M) 8054386: Allow Java debugging when CDS is enabled

[Mikhailo Seledtsov]

Changes look good to me.

Misha

On 6/2/2015 11:55 AM, Chris Plummer wrote:
> I'm going to have to separate out the ProcessTool.java changes into a 
> separate changeset (and CR). In the meantime, feel free to review what 
> I have below. The code won't be changing at all when I separate out 
> the ProcessTool.java changes.
>
> thanks,
>
> Chris
>
> On 6/2/15 12:36 AM, Chris Plummer wrote:
>> [Adding core-libs-dev at openjdk.java.net since this update includes 
>> changes to jdk/test library code]
>>
>> Please review the updated webrev:
>>
>> Webrev: http://cr.openjdk.java.net/~cjplummer/8054386/webrev.02/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8054386
>>
>> There were concerns about the new hotspot tests referencing jdk 
>> tests. One concern was that if the jdk tests change, they could break 
>> the hotspot tests, and this might initially go undetected. The other 
>> concern is that if the jdk and hotspot tests are placed in separate 
>> test bundles, then it would not be possible to run the hotspot tests.
>>
>> Because of these concerns, I moved the tests that were in 
>> hotspot/test/runtime/CDSJDITests to instead be in 
>> jdk/test/com/sun/jdi/CDSJDITests. There was a slight renaming of the 
>> tests in the process. Also, I had to update the jdk version of 
>> ProcessTool.java to include the createJavaProcessBuilder() variant 
>> that is in the hotspot version of ProcessTool.java.
>>
>> Lastly, in CDSJITTest.java I changed:
>>
>>     OutputAnalyzer output = new OutputAnalyzer(pb.start());
>>
>> to instead be:
>>
>>     OutputAnalyzer output = ProcessTools.executeProcess(pb);
>>
>> I had to do this since the jdk version of the OutputAnalyzer 
>> constructor is not public. The 1st version is what is commonly used 
>> in hostspot tests, and the 2nd version is what is commonly used in 
>> jdk tests. I decided to adopt the jdk way rather than make the 
>> OutputAnalyzer constructors public, although this will probably 
>> happen eventually when the two versions are unified.
>>
>> thanks,
>>
>> Chris
>>
>>
>> On 5/19/15 7:25 AM, Chris Plummer wrote:
>>> Hi,
>>>
>>> Please review the following changes for allowing java debugging when 
>>> CDS is enabled.
>>>
>>> Webrev:http://cr.openjdk.java.net/~cjplummer/8054386/webrev.01/
>>> Bug:https://bugs.openjdk.java.net/browse/JDK-8054386
>>>
>>> The VM changes are simple. I removed the check that prevents 
>>> debugging with CDS enabled, and added logic that will map the CDS 
>>> archive RW when debugging is enabled.
>>>
>>> The tests are a bit more complex. There are a bunch of existing JDI 
>>> tests for testing debugging support. Rather than start from scratch 
>>> or clone them, I instead just wrote wrapper tests that put the 
>>> relevant JDI test classes in the archive, and then invoke the JDI 
>>> test. I did this for 3 of the JDI tests. If you feel there are 
>>> others that would be good candidates, I'd be happy to add them. I'm 
>>> looking for ones that would result in modification of the RO class 
>>> metadata, such as setting a breakpoint (for which I already added 
>>> two tests).
>>>
>>> Testing done:
>>> -Using JPRT to run the new jtreg tests on all platforms.
>>> -Using JPRT to run all jtreg runtime tests on linux x86 and x_64.
>>> -Regular JPRT "-testset hotspot" run
>>> -Putting the JCK JVMTI tests in the archive and then running them.
>>> -Putting the nsk jdb, jdwp, jvmti, and jdi tests in the archive and 
>>> then running them.
>>> -Putting a simple test class in the archive and then setting a 
>>> breakpoint on it using jdb
>>>
>>> Some of the above testing resulted in the discovery of bugs that 
>>> still need to be addressed: JDK-8078644, JDK-8078730, and JDK-8079181.
>>>
>>> I also verified that without the change to map the archive RW, the 
>>> above testing resulted in a SEGV, which is what you would expect 
>>> (and actually want to see to prove that the testing is effective).
>>>
>>> thanks,
>>>
>>> Chris
>>>
>>
>