RFR(M): 8172970: TESTBUG: need test coverage for the JVMTI functions allowed in the start phase

[David Holmes]

Hi Serguei,

On 28/04/2017 3:14 PM, serguei.spitsyn at oracle.com wrote:
> Please, review the jdk 10 fix for the test enhancement:
>   https://bugs.openjdk.java.net/browse/JDK-8172970
>
>
> Webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8172970-start-phase.1/

Sorry but I can't quite figure out exactly what this test is doing. What 
is the overall call structure here? I was expecting to see a difference 
between things that can be called at early-start and those that can not 
- or are these all expected to work okay in either case?

A few comments:

  44 #define TranslateError(err) "JVMTI error"

I don't see the point of the above.

---

  99 static long get_thread_local(jvmtiEnv *jvmti, jthread thread) {

The thread local functions use "long" as the datatype but that will only 
be 32-bit on 64-bit Windows. I think you need to use intptr_t for 
complete portability.

---

  277     printf("    Filed declaring");

typo: filed -> field

---

All your little wrapper functions make the JVMTI call and then call 
check_jvmti_error - but all that does is record if it passed or failed. 
If it failed you still continue with the next operation even if it 
relies on the success of the first one eg:

  378         set_thread_local(jvmti, thread, exp_val);
  379         act_val = get_thread_local(jvmti, cur_thread);

and the sequences in print_method_info:

  228     err = (*jvmti)->IsMethodNative(jvmti, method, &is_native);
  229     check_jvmti_error(jvmti, "IsMethodNative", err);
  230     printf("    Method is native: %d\n", is_native);
  231
  232     if (is_native == JNI_FALSE) {
  233         err = (*jvmti)->GetMaxLocals(jvmti, method, &locals_max);

The call at #233 may not be valid because the method actually is native 
but the IsMethodNative call failed for some reason.

Thanks,
David
-----

>
>
> Summary:
>   The task was to provide a test coverage for the JVMTI functions
> allowed during the start phase.
>   It includes both enabling and disabling the can_generate_early_vmstart
> capability.
>   Testing the JVMTI functions allowed in any case has not been targeted
> by this fix.
>
> Testing:
>   New test is passed.
>
>
> Thanks,
> Serguei