Hi Egor, Thank you for finding this problem. I created a new Jira issue [1] for that.
[1] https://bugs.openjdk.java.net/browse/JDK-8252933 Best regards, Daniil On 9/8/20, 8:31 AM, "Egor Ushakov" <egor.usha...@jetbrains.com> wrote: Hi Daniil, we have some tests checking the number of jdwp packets and they've detected a regression here: com.jetbrains.jdi.ObjectReferenceImpl#validateAssignment now always requests referenceType (requiring one more jdwp packet if value is not yet cached), previously it happened only for arrays. Simple fix like this solves the issue: =================================================================== --- src/main/java/com/jetbrains/jdi/ObjectReferenceImpl.java (revision d96cab0ec2eeb791cceb7884341c56496cc026b9) +++ src/main/java/com/jetbrains/jdi/ObjectReferenceImpl.java (revision 10a78a742d17338778e0226990ad0029a71cbf50) @@ -647,11 +647,10 @@ */ JNITypeParser destSig = new JNITypeParser(destination.signature()); - JNITypeParser sourceSig = new JNITypeParser(type().signature()); if (destSig.isPrimitive()) { throw new InvalidTypeException("Can't assign object value to primitive"); } - if (destSig.isArray() && !sourceSig.isArray()) { + if (destSig.isArray() && !new JNITypeParser(type().signature()).isArray()) { throw new InvalidTypeException("Can't assign non-array value to an array"); } if (destSig.isVoid()) { If possible - please push the fix. Thanks, Egor On 19-May-20 19:32, Daniil Titov wrote: > Hi Chris and Serguei, > > Thank you for reviewing this change. > > Best regards, > Daniil > > On 5/18/20, 12:41 PM, "Chris Plummer" <chris.plum...@oracle.com> wrote: > > Looks good. > > thanks, > > Chris > > On 5/14/20 1:33 PM, Daniil Titov wrote: > > Hi Serguei and Chris, > > > > Please review a new version of the change [1] that addresses your comments. > > > > Testing: Mach5 tier1-tier5 tests successfully passed. > > > > Regarding CR for the JDWP spec issues related to missing type information in some of the protocol packet descriptions [3], as Chris has just noticed > > we really don't need it, so I withdrew this CR. > > > > [1] http://cr.openjdk.java.net/~dtitov/8241080/webrev.02 > > [2] https://bugs.openjdk.java.net/browse/JDK-8241080 > > [3] https://bugs.openjdk.java.net/browse/JDK-8245057 > > > > Thank you, > > Daniil > > > > > > From: "serguei.spit...@oracle.com" <serguei.spit...@oracle.com> > > Date: Monday, May 11, 2020 at 11:53 AM > > To: Daniil Titov <daniil.x.ti...@oracle.com>, serviceability-dev <serviceability-dev@openjdk.java.net> > > Subject: Re: RFR: 8241080: Consolidate signature parsing code in serviceability tools > > > > Hi Daniil, > > > > It looks pretty good in general. > > A couple of nits below. > > > > http://cr.openjdk.java.net/~dtitov/8241080/webrev.01/src/jdk.jdwp.agent/share/native/libjdwp/invoker.c.udiff.html > > + void *cursor; > > + jbyte argumentTag; > > + jint argIndex = 0; > > + jvalue *argument = request->arguments;; > > . . . > > void *cursor; > > jint argIndex = 0; > > + jbyte argumentTag; > > jvalue *argument = request->arguments; > > > > It is better if the local variables 'cursor' and 'argumentTag' get initialized. > > There is double semicolon: "arguments;;" > > > > > > http://cr.openjdk.java.net/~dtitov/8241080/webrev.01/src/jdk.jdwp.agent/share/native/libjdwp/signature.h.html > > 43 static inline jbyte basicType(const char *signature) { > > > > It'd be nice to rename it to basicTypeTag() to get it unified with other functions below. > > > > It is more safe to run tier5 as well. > > > > Thanks, > > Serguei > > > > > > On 5/9/20 09:29, Daniil Titov wrote: > > Please review a change[1] that centralizes the signature processing in serviceability tools to make it capable of being easily extensible in the future. > > > > Testing: Mach5 tier1-tier3 tests successfully passed. > > > > [1] http://cr.openjdk.java.net/~dtitov/8241080/webrev.01 > > [2] https://bugs.openjdk.java.net/browse/JDK-8241080 > > > > Thank you, > > Daniil > > > > > > > > > > > > > > > -- Egor Ushakov Software Developer JetBrains https://urldefense.com/v3/__http://www.jetbrains.com__;!!GqivPVa7Brio!JOhCc9k54KkUPFPbiviF1MS9YfERc-yqYJLt1n1eVlGWDpapPTdxtFv6f9a6PBgP129w$ The Drive to Develop
