Hi Leonid,

On 6/26/20 7:51 PM, Leonid Mesnik wrote:
Hi

The idea basically looks good. I think it just make a sense to polish it a little bit to 
hide "sh" usage from test and get core from OutputAnalyzer.
Ok, I'll look into both of those.
Also, there is a 'CrashApp' in ClhsdbCDSCore.java. Makes it sense to unify it with LingeredApp crasher? Currently, it uses Unsafe to crash application.
Yes, I purposely didn't not make that change. My main goal with the LingeredApp changes is to make it easier to make existing LingeredApp SA tests run on both a live process and on a core, and my main goal with ClhsdbCDSCore and TestJmapCore was to move the core finding code and ulimit code to a common location that could be reused by other tests.

Keep in mind that ClhsdbLauncher and LingeredApp are independent of each other. You can have a LingeredApp tests that use or don't use ClhsdbLauncher, and you can have a non-LingeredApp tests that use or don't use ClhsdbLauncher. So I didn't want to go down the path of changing ClhsdbCDSCore (a non LingeredApp test) to use LingeredApp. Likewise I did not change TestJmapCore to use LingeredApp or ClhsdbLauncher. Possibly there is good reason to convert some of the tests to start using LingeredApp and/or ClhsdbLauncher, but that should be done under a separate RFE.


Also, crashes are used in other tests, I see some implementations in
open/test/hotspot/jtreg/vmTestbase/vm/share/vmcrasher
I don't see vmcrasher being used by any tests. In any case, my first attempt went down the Unsafe path to produce a crash. The issue is that it forces every user of LingeredApp to include the @module for Unsafe. I also tried using a WhiteBox API. That was worse, also requiring every user of LingeredApp to include an @module, plus the tests that actually want to cause a crash need to @build WhiteBox.java and then do the classfile install. It also required additional module related hacks in LingeredApp. The issue with my current solution is how to get libLingeredApp.c to compile has not been settled on. I'm still waiting for an answer from the build team.

So it would be nice to have some common way to crash hotspot.
I can see possibly moving the crashing code out of LingeredApp and into a native lib that non-LingeredApp tests can use, although that really is just a very small part of the changes to LingeredApp. For the most part the changes would look the same except you would call a different API to cause the crash.

Leonid

Thanks for having a look.

Chris
On Jun 25, 2020, at 2:41 PM, Chris Plummer <[email protected]> wrote:

Hello,

Please help with a preliminary review of changes to add better support for 
writing SA tests that work on core files:

https://bugs.openjdk.java.net/browse/JDK-8248194
http://cr.openjdk.java.net/~cjplummer/8248194/webrev.00/index.html

As pointed out, this is a preliminary review. I suspect there will be some 
feedback for changes/improvements. Also, I still need to work out a final 
solution for how to get LingeredApp to produce a crash. What I currently have 
works but is somewhat of a hack w.r.t. the makefile change, so you can ignore 
the makefiile change for now. I'm working on a more proper solution with the 
build team.

As outlined in the CR, these are the 3 main goals of this CR:

1. SATestUtils should include support for finding the core file. This includes 
parsing the output of the crashed process to locate where the core file was 
saved, and returning this location to the user.

2. SATestUtils should include support for adding the "ulimit -c unlimited" 
prefix to the command that will produce the core file, allowing the overriding of any 
lower limit so we can be sure the core file will be produced.

3. LingeredApp should include support for producing a core file.

As proof of concept for these 3 changes in test library support, I'm updating 
the following 3 tests:

ClhsdbCDSCore.java: Use the SATestUtils support listed above. This test does 
not use LingeredApp, so those improvements don't apply.

TestJmapCore.java: Use the SATestUtils support listed above. This test does not 
use LingeredApp, so those improvements don't apply.

ClhsdbFindPC.java: Use all the above features, including having LingeredApp 
produce a core file. This is the only test modified to start testing on core 
files that didn't previously do so. It still also tests on a live process.

In the future more Clhsdb tests will be converted to work on core files in a 
manner similar to ClhsdbFindPC.

The new SATestUtils code is borrowed from (more like ripped out of) ClhsdbCDSCore.java and 
TestJmapCore.java. They both had a lot of code dedicated to finding the core file and also applying 
"ulimit -c unlimitted" if necessary, but didn't do so in quite the same way. Now both 
these tests share code in SATestUtils.java. One thing I did drop is TestJmapCore.java use of 
":KILLED_PID" in the output to help find the core file. It's no longer necessary based on 
the smarter core locating code I pulled from ClhsdbCDSCore.java.

thanks,

Chris

Reply via email to