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

[Chris Plummer]

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
>>
>