[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: FW: 4698670: JDI spec?: some methods don't throw ClassNotLoadedException for non-loaded types
From: daniil.x.titov () oracle ! com
Date: 2018-04-23 4:56:43
Message-ID: 7d2a0f6e-4292-c542-d363-38e3984cd9f8 () oracle ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
[Attachment #4 (text/plain)]
Thank you, Serguei and Jerry,
I have a new version of the webrev, however I cannot upload it to
javaweb.us.oracle.com. Moreover, all previous webrevs uploaded there
(not only mine, just in case Mikael's webrev I attached to this email
are affected as well) are no longer available. Not sure is is some part
of the maintenance over a weekend or something else. Could you please
advise what other internal resource could be used to publish closed webrev?
Thanks!
Best regards,
Daniil
On 4/20/18 6:29 PM, serguei.spitsyn@oracle.com wrote:
> 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@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
>>>
>>>
>>
>
[Attachment #5 (multipart/related)]
[Attachment #7 (text/html)]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Thank you, Serguei and Jerry,</p>
<p>I have a new version of the webrev, however I cannot upload it to
javaweb.us.oracle.com. Moreover, all previous webrevs uploaded
there (not only mine, just in case Mikael's webrev I attached to
this email are affected as well) are no longer available. Not sure
is is some part of the maintenance over a weekend or something
else. Could you please advise what other internal resource could
be used to publish closed webrev?<br>
</p>
<p><br>
</p>
<p><img src="cid:part1.D9E612ED.902F9237@oracle.com" alt=""
width="1216" height="415"></p>
<p>Thanks!<br>
</p>
<p>Best regards,</p>
<p>Daniil</p>
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 4/20/18 6:29 PM,
<a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote:<br> \
</div> <blockquote type="cite"
cite="mid:14b44422-eab8-6b08-4d80-c083dde20b0e@oracle.com">Hi
Daniil,
<br>
<br>
I have the same question and also suggestion to remove the
referenced
<br>
debuggee fields dummyF and finDummyF as they have no other uses.
<br>
<br>
Also, a minor suggestion is to reorder the parameters of the
MockReferenceType
<br>
constructor the same as in the createObjectReference() method.
<br>
It looks more natural to me.
<br>
<br>
Hi Jerry,
<br>
We have to reply in the open.
<br>
I also tend to do this mistake.
<br>
Let's fix each other.
<br>
<br>
I've added the svc-dev list back.
<br>
<br>
Thanks,
<br>
Serguei
<br>
<br>
<br>
On 4/20/18 18:10, Gerald Thornbrugh wrote:
<br>
<blockquote type="cite">Hi Daniil,
<br>
<br>
Your changes look good but I have a small nit.
<br>
Since the code no longer uses the DEBUGGEE_METHODS[i][1]
elements in the
<br>
array due to your change, should they be removed?
<br>
<br>
Thanks for doing this!
<br>
<br>
Jerry
<br>
<br>
On 04/20/2018 04:47 PM, Daniil Titov wrote:
<br>
<blockquote type="cite">Hello,
<br>
<br>
Could you please help with reviewing this change? I need 2
reviewers.
<br>
<br>
Thanks!
<br>
<br>
Best regards,
<br>
Daniil
<br>
<br>
<br>
On 4/19/18, 4:52 PM, "Daniil Titov"
<a class="moz-txt-link-rfc2396E" \
href="mailto:daniil.x.titov@oracle.com"><daniil.x.titov@oracle.com></a> wrote: \
<br> <br>
Please review the change that affects two tests:
<br>
-
fromTonga/nsk/jdi/ClassType/invokeMethod/invokemethod009/TestDescription.java<br>
-
fromTonga/nsk/jdi/ObjectReference/invokeMethod/invokemethod006/TestDescription.java<br>
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.
<br>
39 static ValueImpl
prepareForAssignment(Value value,
<br>
40 ValueContainer destination)
<br>
41 throws InvalidTypeException,
ClassNotLoadedException {
<br>
42 if (value == null) {
<br>
43 /*
<br>
44 * TO DO: Centralize JNI signature
knowledge
<br>
45 */
<br>
46 if
(destination.signature().length() == 1) {
<br>
47 throw new
InvalidTypeException("Can't set a primitive type to null");
<br>
48 }
<br>
49 return null; // no further
checking or conversion necessary
<br>
50 } else {
<br>
51 return
((ValueImpl)value).prepareForAssignmentTo(destination);
<br>
52 }
<br>
53 }
<br>
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
<br>
279 } catch (ClassNotLoadedException
e) {
<br>
280 /*
<br>
281 * Since we got this exception,
<br>
282 * the field type must be a
reference type. The value
<br>
283 * we're trying to set is null, but
if the field's
<br>
284 * class has not yet been loaded
through the enclosing
<br>
285 * class loader, then setting to
null is essentially a
<br>
286 * no-op, and we should allow it
without an exception.
<br>
287 */
<br>
288 if (value != null) {
<br>
289 throw e;
<br>
290 }
<br>
291 }
<br>
292 }
<br>
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.
<br>
Bug:
<a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-4698670">https://bugs.openjdk.java.net/browse/JDK-4698670</a>
<br>
Webrev:
<a class="moz-txt-link-freetext" \
href="http://javaweb.us.oracle.com/~datitov/4698670/webrev.01">http://javaweb.us.oracle.com/~datitov/4698670/webrev.01</a>
<br>
Best regards,
<br>
Daniil
<br>
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>
["llniibcbnebkcpja.png" (image/png)]
["Attached Message" (message/rfc822)]
Please review the following change which addresses some build problems =
(warnings-as-errors) when building some former tonga tests with VS2017:
bug: https://bugs.openjdk.java.net/browse/JDK-8202053 =
<https://bugs.openjdk.java.net/browse/JDK-8202053>
webrev: =
http://javaweb.us.oracle.com/~mvidsted/webrevs/8202053/webrev.00/webrev/ =
<http://javaweb.us.oracle.com/~mvidsted/webrevs/8202053/webrev.00/webrev/>=
Cheers,
Mikael
[Attachment #12 (text/html)]
<html><head><meta http-equiv="Content-Type" content="text/html \
charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: \
space; -webkit-line-break: after-white-space;" class=""><div class=""><br \
class=""></div><div class="">Please review the following change which addresses some \
build problems (warnings-as-errors) when building some former tonga tests with \
VS2017:</div><div class=""><br class=""></div><div class="">bug: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8202053" \
class="">https://bugs.openjdk.java.net/browse/JDK-8202053</a></div><div \
class="">webrev: <a \
href="http://javaweb.us.oracle.com/~mvidsted/webrevs/8202053/webrev.00/webrev/" \
class="">http://javaweb.us.oracle.com/~mvidsted/webrevs/8202053/webrev.00/webrev/</a></div><div \
class=""><br class=""></div><div class="">Cheers,</div><div class="">Mikael</div><div \
class=""><br class=""></div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic