RFR (xs) 8251209 [TESTBUG] CDS jvmti tests should use "@modules java.instrument"
Ioi Lam
ioi.lam at oracle.com
Fri Aug 7 06:13:59 UTC 2020
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
>>
>>
More information about the hotspot-runtime-dev
mailing list