RFR (xs) 8251209 [TESTBUG] CDS jvmti tests should use "@modules java.instrument"

[Ioi Lam]

On 8/6/20 5:58 PM, David Holmes wrote:
> Correction ...
>
> On 7/08/2020 7:52 am, David Holmes wrote:
>> Hi Ioi,
>>
>> On 7/08/2020 4:25 am, Ioi Lam wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8251209
>>> http://cr.openjdk.java.net/~iklam/jdk16/8251209-cds-jvmti-tests-modules-tag.v01/ 
>>>
>>>
>>> Summary -- changed the tests from (mis)using
>>>
>>>   * @requires vm.flavor != "minimal"
>>>
>>> to
>>>
>>>   * @modules java.instrument
>>>
>>> ... to be consistent with other jvmti tests.
>>
>> That seems like an invalid precondition to me. It would have been 
>> somewhat valid in the Compact Profiles world when we did not provide 
>> "java.instrument" in the profiles which supported MinimalVM, but you 
>> can define a minimal VM in a build that still has all modules 
>> available. I don't think building the minimal VM makes any changes to 
>> the supported modules.
>>
>> Also AIUI the @modules statement simply adds the necessary 
>> command-line args to use the java.instrument module (if present), it 
>> doesn't ensure that the listed module has to be present.
>
> It does in fact ensure that:
>
> "Otherwise, a test will not be run if the system being tested does not 
> contain all of the specified modules."
>
> http://openjdk.java.net/jtreg/tag-spec.html
>
> But as I said the module could be present in a JRE but you are still 
> using the MinimalVM.
>

Hi David,

As I mentioned above, I am following the same rule as other jvmti tests, 
which only use "@modules java.instrument" and do not check whether the 
VM is minimal. E.g.,

http://hg.openjdk.java.net/jdk/jdk/file/4d36e29a5410/test/hotspot/jtreg/serviceability/jvmti/GetObjectSizeClass.java

-------

If I understand correctly, you're saying someone can build a minimal JDK 
(configure --with-jvm-variants=minimal), and then try to add the 
java.instrument module to it. I.e., adding the following module to your 
JDK (with jlink, or by hand).


$ unzip -l ./jmods/java.instrument.jmod
   Length      Date    Time    Name
---------  ---------- -----   ----
       294  2020-08-04 17:03   classes/module-info.class
      1102  2020-08-04 17:03 
classes/sun/instrument/TransformerManager$TransformerInfo.class
      4294  2020-08-04 17:03 classes/sun/instrument/TransformerManager.class
       911  2020-08-04 17:03 
classes/sun/instrument/InstrumentationImpl$1.class
     16663  2020-08-04 17:03 
classes/sun/instrument/InstrumentationImpl.class
      1356  2020-08-04 17:03 
classes/java/lang/instrument/ClassFileTransformer.class
       554  2020-08-04 17:03 
classes/java/lang/instrument/IllegalClassFormatException.class
      1734  2020-08-04 17:03 
classes/java/lang/instrument/Instrumentation.class
       563  2020-08-04 17:03 
classes/java/lang/instrument/UnmodifiableModuleException.class
       970  2020-08-04 17:03 
classes/java/lang/instrument/ClassDefinition.class
       551  2020-08-04 17:03 
classes/java/lang/instrument/UnmodifiableClassException.class
      3244  2020-08-04 17:03   legal/COPYRIGHT
        44  2020-08-04 17:03   legal/LICENSE
     50920  2020-08-04 17:03   lib/libinstrument.so<<<<<<<<<

But this module has a native library, libinstrument.so, which requires 
JVMTI to be present in libjvm.so. E.g.:

     jvmtiEnv *
     retransformableEnvironment(JPLISAgent * agent) {
     ....
         jnierror = (*agent->mJVM)->GetEnv(  agent->mJVM,
                                    (void **) &retransformerEnv,
                                    JVMTI_VERSION_1_1);

So if you try to run the CDS JVMTI test cases, it will be executed 
(because your JDK says "I have java.instrument") and the test finds out 
that your JDK's java.instrument module isn't working properly. So the 
test is doing exactly what it's supposed to do.

I would argue that this is better than before (which would exclude the 
test when the libjvm.so is a minimal build, and would will not detect 
such a mis-configured java.instrument module.)


Thanks
- Ioi