Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi David, Thanks again. I included Sergeui's suggestions before pushing the fix. I also created an enhancement bug to improve the test: https://bugs.openjdk.java.net/browse/JDK-8166453 Harold On 9/21/2016 2:37 AM, David Holmes wrote: Hi Harold, On 21/09/2016 12:11 AM, harold seigel

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Harold, On 21/09/2016 12:11 AM, harold seigel wrote: Hi David, Thanks for the review. Please review this updated webrev containing the comment modifications that you suggested: http://cr.openjdk.java.net/~hseigel/bug_8160987.3/ No further comments - the suggestions from Sergeui seem

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Dan, Thanks for looking at it again. See comment in-line. Thanks, Harold On 9/20/2016 11:25 AM, Daniel D. Daugherty wrote: On 9/20/16 8:11 AM, harold seigel wrote: Hi David, Thanks for the review. Please review this updated webrev containing the comment modifications that you

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Serguei, Thanks for the suggestions. Harold On 9/20/2016 1:01 PM, serguei.spit...@oracle.com wrote: Hi Harold, Looks good. http://cr.openjdk.java.net/~hseigel/bug_8160987.3/test/com/sun/jdi/InterfaceMethodsTest.java.frames.html 197 // invoke interface static method A 198

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Harold, Looks good. http://cr.openjdk.java.net/~hseigel/bug_8160987.3/test/com/sun/jdi/InterfaceMethodsTest.java.frames.html 197 // invoke interface static method A 198 testInvokePos(ifaceClass, null, "staticMethodA", "()I", vm().mirrorOf(RESULT_A)); 199 200 //

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

On 9/20/16 8:11 AM, harold seigel wrote: Hi David, Thanks for the review. Please review this updated webrev containing the comment modifications that you suggested: http://cr.openjdk.java.net/~hseigel/bug_8160987.3/ src/jdk.jdwp.agent/share/native/libjdwp/invoker.c No comments.

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Serguei, Thanks for checking on this. Harold On 9/19/2016 10:13 PM, serguei.spit...@oracle.com wrote: On 9/19/16 11:03, Daniel D. Daugherty wrote: On 9/19/16 6:51 AM, harold seigel wrote: Hi, Please review this updated webrev for fixing JDK-8160987

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi David, Thanks for the review. Please review this updated webrev containing the comment modifications that you suggested: http://cr.openjdk.java.net/~hseigel/bug_8160987.3/ Please also see comments in-line. On 9/19/2016 10:50 PM, David Holmes wrote: Hi Harold, I'm having a lot of

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Harold, I'm having a lot of issues with the code and testing here. Please bear with me. On 19/09/2016 10:51 PM, harold seigel wrote: Hi, Please review this updated webrev for fixing JDK-8160987 :

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Never mind ... On 20/09/2016 11:42 AM, David Holmes wrote: I'm missing something here. Why are you considering it an error to find the method in a super-interface? InvokeMethod Command (3) Invokes a static method. The method must be member of the class type or one of its superclasses,

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

I'm missing something here. Why are you considering it an error to find the method in a super-interface? InvokeMethod Command (3) Invokes a static method. The method must be member of the class type or one of its superclasses, superinterfaces, or implemented interfaces. Access control is not

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Dan, Thanks for the review! For now, I'll leave out the blanks following the commas. If people feel that the blanks are important then I will enter an RFE to add blanks after all the commas in the JNI calls. Thanks, Harold On 9/19/2016 2:05 PM, Daniel D. Daugherty wrote: Just FYI.

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Just FYI. The prevailing style in that file is for JNI calls to not have a space after that comma. I don't like it either, but that appears to be how the original code was written... Dan On 9/19/16 11:46 AM, harold seigel wrote: Will do. Thanks! Harold On 9/19/2016 1:26 PM, Dmitry

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

On 9/19/16 6:51 AM, harold seigel wrote: Hi, Please review this updated webrev for fixing JDK-8160987 : http://cr.openjdk.java.net/~hseigel/bug_8160987.2/ src/jdk.jdwp.agent/share/native/libjdwp/invoker.c L356: error =

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Will do. Thanks! Harold On 9/19/2016 1:26 PM, Dmitry Samersoff wrote: Harold, Please, add space after comma at 356 and 362 $0.2 -Dmitry On 2016-09-19 20:07, harold seigel wrote: Thanks Serguei. I'll make that change before checking in the fix. Harold On 9/19/2016 12:55 PM,

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Harold, Please, add space after comma at 356 and 362 $0.2 -Dmitry On 2016-09-19 20:07, harold seigel wrote: > Thanks Serguei. I'll make that change before checking in the fix. > > Harold > > > On 9/19/2016 12:55 PM, serguei.spit...@oracle.com wrote: >> Hi Harold, >> >> 351 static

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Thanks Serguei. I'll make that change before checking in the fix. Harold On 9/19/2016 12:55 PM, serguei.spit...@oracle.com wrote: Hi Harold, 351 static jvmtiError check_methodClass(JNIEnv *env, jclass clazz, jmethodID method) 352 { 353 jclass containing_class; 354 jvmtiError error; 355

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Harold, 351 static jvmtiError check_methodClass(JNIEnv *env, jclass clazz, jmethodID method) 352 { 353 jclass containing_class; 354 jvmtiError error; 355 356 error = JVMTI_FUNC_PTR(gdata->jvmti,GetMethodDeclaringClass) 357 (gdata->jvmti, method, _class); It is better to initialize

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi, Please review this updated webrev for fixing JDK-8160987 : http://cr.openjdk.java.net/~hseigel/bug_8160987.2/ It provides a more efficient implementation and fixes a test problem. This fix was tested as described below and with the

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

This is just normal way of thinking. :) Thanks, Serguei On 9/16/16 07:37, Daniel D. Daugherty wrote: Going down the JNI rabbit hole is my fault. When I mumbled on 2016-09-02 about how this bug might be fixed, I was too focused on the fact that the JNI layer didn't do what we want and I

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Dan, Thanks, but your comments were very helpful. Harold On 9/16/2016 10:37 AM, Daniel D. Daugherty wrote: Going down the JNI rabbit hole is my fault. When I mumbled on 2016-09-02 about how this bug might be fixed, I was too focused on the fact that the JNI layer didn't do what we want

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Going down the JNI rabbit hole is my fault. When I mumbled on 2016-09-02 about how this bug might be fixed, I was too focused on the fact that the JNI layer didn't do what we want and I talked about how to fix it using JNI functions. Harold, my apologies for wasting your time. Dan On

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Serguei, Thanks for the suggestion! That provides a much cleaner implementation. Harold On 9/15/2016 11:28 PM, serguei.spit...@oracle.com wrote: On 9/15/16 19:13, David Holmes wrote: On 16/09/2016 8:52 AM, serguei.spit...@oracle.com wrote: Hi Harold, I did not got deep into the fix

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

On 9/15/16 19:13, David Holmes wrote: On 16/09/2016 8:52 AM, serguei.spit...@oracle.com wrote: Hi Harold, I did not got deep into the fix yet but wonder why the JVMTI function is My copy-paste failed. I wanted to list the JVMTI function name: GetMethodDeclaringClass. Thanks, Serguei not

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

On 16/09/2016 8:52 AM, serguei.spit...@oracle.com wrote: Hi Harold, I did not got deep into the fix yet but wonder why the JVMTI function is not used. I was wondering a similar thing. It seems very heavyweight to use Java level reflection from inside native code to validate the native

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Harold, I did not got deep into the fix yet but wonder why the JVMTI function is not used. Thanks, Serguei On 9/15/16 13:05, harold seigel wrote: Hi Dan, One could argue that a spec compliant JNI implementation wouldn't need this change in the first place... Regardless, I'm

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

On 9/15/16 2:05 PM, harold seigel wrote: Hi Dan, One could argue that a spec compliant JNI implementation wouldn't need this change in the first place... That's a completely different discussion. And we don't want to go down that rat hole again... :-) Regardless, I'm withdrawing this

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

Hi Dan, One could argue that a spec compliant JNI implementation wouldn't need this change in the first place... Regardless, I'm withdrawing this change because I found that it fails a com/sun/jdi JTreg test involving static methods in interfaces. Thanks, Harold On 9/15/2016 3:37 PM,

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

On 9/15/16 12:10 PM, harold seigel wrote: (Adding hotspot-runtime) Hi Dan, Thanks for looking at this. I could pass NULL instead of clazz to ToReflectMethod() to ensure that the method object isn't being obtained from clazz. I don't think that would be a JNI spec compliant use of the JNI

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

(Adding hotspot-runtime) Hi Dan, Thanks for looking at this. I could pass NULL instead of clazz to ToReflectMethod() to ensure that the method object isn't being obtained from clazz. Harold On 9/15/2016 1:09 PM, Daniel D. Daugherty wrote: On 9/15/16 9:31 AM, harold seigel wrote: Hi,

Re: RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

On 9/15/16 9:31 AM, harold seigel wrote: Hi, Please review this small fix for JDK-8160987. The JDWP InvokeStatic() method was depending on the JNI function that it called to enforce the requirement that the specified method must be a member of the specified class or one of its super