On 7/08/2020 2:41 pm, Ioi Lam wrote:
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
Sure but I contend those tests are wrong and the tests you are changing
are right (or more right given common test configurations).
-------
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).
Just build a JDK with multiple VMs present.
$ 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.
The whole point of the @requires is to not waste time and resources
running a test on a platform that cannot run the test successfully.
So the fully correct solution could be to have both settings:
@requires vm.flavor != "minimal"
@modules java.instrument
if you require both a VM that supports JVM TI and you need a JRE that
includes the java.instrument module. But that assumes your test does
need java.instrument. Not all JVM TI tests need java.instrument, but all
instrumentation tests depend on JVM TI. Just looking at the first three
of tests in your webrev I don't see any dependency on java.instrument -
they are CDS only tests as far as I can see and so require a VM with CDS
which means not a Minimal VM - though perhaps it is sufficient to have the
@requires vm.cds
in those cases?
For the other JVM TI related tests using -javaagent they probably need
both @requires and @module.
David
-----
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
