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


Reply via email to