On 8/6/20 11:01 PM, David Holmes wrote:
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.
You can disable JVMTI with "configure --disable-jvm-feature-jvmti". So
checking for vm.flavor != "minimal" is not sufficient.
Similarly, "@requires vm.cds" doesn't guarantee that JVMTI is supported.
Maybe we should have a new VM prop so we can do
@requires vm.jvmti
@modules java.instrument
Thanks
- Ioi
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
