FW: 4698670: JDI spec?: some methods don't throw ClassNotLoadedException for non-loaded types

[serguei.spitsyn at oracle.com]

Hi Daniil,

I have the same question and also suggestion to remove the referenced
debuggee fields dummyF and finDummyF as they have no other uses.

Also, a minor suggestion is to reorder the parameters of the 
MockReferenceType
constructor the same as in the createObjectReference() method.
It looks more natural to me.

Hi Jerry,
We have to reply in the open.
I also tend to do this mistake.
Let's fix each other.

I've added the svc-dev list back.

Thanks,
Serguei


On 4/20/18 18:10, Gerald Thornbrugh wrote:
> Hi Daniil,
>
> Your changes look good but I have a small nit.
> Since the code no longer uses the DEBUGGEE_METHODS[i][1] elements in the
> array due to your change, should they be removed?
>
> Thanks for doing this!
>
> Jerry
>
> On 04/20/2018 04:47 PM, Daniil Titov wrote:
>> Hello,
>>
>> Could you please help with reviewing this change? I need 2 reviewers.
>>
>> Thanks!
>>
>> Best regards,
>> Daniil
>>
>>
>> On 4/19/18, 4:52 PM, "Daniil Titov" <daniil.x.titov at oracle.com> wrote:
>>
>>      Please review the change that affects two tests:
>>            - 
>> fromTonga/nsk/jdi/ClassType/invokeMethod/invokemethod009/TestDescription.java
>>       - 
>> fromTonga/nsk/jdi/ObjectReference/invokeMethod/invokemethod006/TestDescription.java
>>           Both tests try to emulate the case when the method with a 
>> parameter is invoked in the target VM via JDWP while the class 
>> corresponding to the parameter type is not yet loaded. The problem 
>> with these tests is that when they invoke the method in the target VM 
>> they pass null as an argument and in case of null 
>> com.sun.tools.jdi.ValueImpl.prepareForAssignment(Value, 
>> ValueContainer) method does a quick return without any additional check.
>>              39        static ValueImpl prepareForAssignment(Value 
>> value,
>>          40 ValueContainer destination)
>>          41                      throws InvalidTypeException, 
>> ClassNotLoadedException {
>>          42            if (value == null) {
>>          43                /*
>>          44                 * TO DO: Centralize JNI signature knowledge
>>          45                 */
>>          46                if (destination.signature().length() == 1) {
>>          47                    throw new InvalidTypeException("Can't 
>> set a primitive type to null");
>>          48                }
>>          49                return null;    // no further checking or 
>> conversion necessary
>>          50            } else {
>>          51                return 
>> ((ValueImpl)value).prepareForAssignmentTo(destination);
>>          52            }
>>          53        }
>>           This behavior is consistent with the code existing in 
>> open/src/jdk.jdi/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java, 
>> open/src/jdk.jdi/share/classes/com/sun/tools/jdi/ClassTypeImpl.java, 
>> and 
>> open/src/jdk.jdi/share/classes/com/sun/tools/jdi/ArrayReferenceImpl.java
>>              279            } catch (ClassNotLoadedException e) {
>>         280                /*
>>         281                 * Since we got this exception,
>>         282                 * the field type must be a reference 
>> type. The value
>>         283                 * we're trying to set is null, but if the 
>> field's
>>         284                 * class has not yet been loaded through 
>> the enclosing
>>         285                 * class loader, then setting to null is 
>> essentially a
>>         286                 * no-op, and we should allow it without 
>> an exception.
>>         287                 */
>>         288                if (value != null) {
>>         289                    throw e;
>>         290                }
>>         291            }
>>         292        }
>>           The fix corrects the tests to construct a mock value object 
>> with a required type and instead of null pass this mock value as an 
>> argument in the method invocation in the target VM.
>>           Bug: https://bugs.openjdk.java.net/browse/JDK-4698670
>>      Webrev: http://javaweb.us.oracle.com/~datitov/4698670/webrev.01
>>           Best regards,
>>      Daniil
>>
>>
>