Re: RFR: 8150758: [TESTBUG] need jvmti tests for module aware agents

2016-09-19 Thread serguei.spit...@oracle.com
Hi Dmitry, Thanks a lot for this additional test coverage and discovering new bug: https://bugs.openjdk.java.net/browse/JDK-8165681 The tests look pretty good to me. A couple of minor comments. Dots are missed in the .c files comments.

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

2016-09-19 Thread David Holmes
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

2016-09-19 Thread David Holmes
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

2016-09-19 Thread David Holmes
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

2016-09-19 Thread harold seigel
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

2016-09-19 Thread Daniel D. Daugherty
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

2016-09-19 Thread Daniel D. Daugherty
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

2016-09-19 Thread harold seigel
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

2016-09-19 Thread Dmitry Samersoff
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

2016-09-19 Thread harold seigel
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

2016-09-19 Thread serguei.spit...@oracle.com
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: JDK-8166202: Tracefile gensrc cannot handle closed src dir in different location

2016-09-19 Thread Tim Bell
Erik: The xml/xsl source files used to generate sources for trace in hotspot have hard coded relative paths in them that make assumptions on where the oracle closed files are located. The source files should not make these kind of assumptions since that makes it difficult to change the

Re: ThreadMXBean.getThreadAllocatedBytes() allocates memory

2016-09-19 Thread Bengt Rutisson
Hi Claes and David, Thanks for the quick responses! Yes, a pre-allocated array is probably not the way to go. Letting the user pass a result array is much better or just specialising the "long getThreadAllocatedBytes(long id)" to avoid allocations instead of re-using the "long[]

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

2016-09-19 Thread harold seigel
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

RFR: 8150758: [TESTBUG] need jvmti tests for module aware agents

2016-09-19 Thread Dmitry Dmitriev
Hello, Please review new tests for module aware agents. There are 3 tests: 1) MAAClassFileLoadHook.java - verifies ClassFileLoadHook event with different combinations of can_generate_early_vmstart and can_generate_early_class_hook_events JVMTI capabilities. Expects to find(or not) class from

Re: RFR: JDK-8166202: Tracefile gensrc cannot handle closed src dir in different location

2016-09-19 Thread Erik Gahlin
Looks good. Erik The xml/xsl source files used to generate sources for trace in hotspot have hard coded relative paths in them that make assumptions on where the oracle closed files are located. The source files should not make these kind of assumptions since that makes it difficult to

Re: RFR: JDK-8166202: Tracefile gensrc cannot handle closed src dir in different location

2016-09-19 Thread David Holmes
Hi Erik, On 17/09/2016 12:51 AM, Erik Joelsson wrote: The xml/xsl source files used to generate sources for trace in hotspot have hard coded relative paths in them that make assumptions on where the oracle closed files are located. The source files should not make these kind of assumptions