Re: RFR(S) 8161225: Assert failure in JVMTI GetNamedModule at JPLISAgent.c line: 792

2016-09-20 Thread David Holmes
Hi Chris, On 21/09/2016 3:07 PM, Chris Plummer wrote: Hello, Please help review the following: https://bugs.openjdk.java.net/browse/JDK-8161225 http://cr.openjdk.java.net/~cjplummer/8161225/webrev.00/ Looks fine. Only comment:

RFR(S) 8161225: Assert failure in JVMTI GetNamedModule at JPLISAgent.c line: 792

2016-09-20 Thread Chris Plummer
Hello, Please help review the following: https://bugs.openjdk.java.net/browse/JDK-8161225 http://cr.openjdk.java.net/~cjplummer/8161225/webrev.00/ The main fix is in JPLISAgent.c, which is to no longer call jplis_assert_msg() when there is a PHASE error, and also remove the test from

Re: RFR (S): 8147943 jvmti.h generated with GPL header

2016-09-20 Thread Alan Bateman
On 20/09/2016 11:33, serguei.spit...@oracle.com wrote: Please, review the fix for: https://bugs.openjdk.java.net/browse/JDK-8147943 Webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8147943-jvmti-header.hs1/ Summary: The problem is that the

Re: RFR (S): 8147943 jvmti.h generated with GPL header

2016-09-20 Thread serguei.spit...@oracle.com
On 9/20/16 15:21, Daniel D. Daugherty wrote: On 9/20/16 12:33 PM, serguei.spit...@oracle.com wrote: Please, review the fix for: https://bugs.openjdk.java.net/browse/JDK-8147943 Webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8147943-jvmti-header.hs1/

Re: RFR (S): 8147943 jvmti.h generated with GPL header

2016-09-20 Thread Daniel D. Daugherty
On 9/20/16 12:33 PM, serguei.spit...@oracle.com wrote: Please, review the fix for: https://bugs.openjdk.java.net/browse/JDK-8147943 Webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8147943-jvmti-header.hs1/ src/share/vm/prims/jvmti.xml No comments.

RFR (S): 8147943 jvmti.h generated with GPL header

2016-09-20 Thread serguei.spit...@oracle.com
Please, review the fix for: https://bugs.openjdk.java.net/browse/JDK-8147943 Webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8147943-jvmti-header.hs1/ Summary: The problem is that the build/*/hotspot/variant-server/gensrc/jvmvtifiles/jvmti.h is currently generated

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

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

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

2016-09-20 Thread serguei.spit...@oracle.com
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

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

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

2016-09-20 Thread harold seigel
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: 8150758: [TESTBUG] need jvmti tests for module aware agents

2016-09-20 Thread Dmitry Dmitriev
Dmitry, Sergeui, thank you for the review! For the record, webrev.02: http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.02/ Dmitry On 20.09.2016 13:39, Dmitry Samersoff wrote: Dmitry, Looks good for me! Minor formatting

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

2016-09-20 Thread serguei.spit...@oracle.com
Hi Dmitry, The fix looks good, thank you for the update. Thumbs up. Thanks, Serguei On 9/20/16 03:17, Dmitry Dmitriev wrote: Serguei, Dmitry, Thank you for the feedback! Here is an updated webrev.01; http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.01/

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

2016-09-20 Thread Dmitry Samersoff
Dmitry, Looks good for me! Minor formatting nits: libMAAClassFileLoadHook.c 103: missed space after , libMAAClassFileLoadHook.c: 266: extra semicolon -Dmitry On 2016-09-20 13:17, Dmitry Dmitriev wrote: > Serguei, Dmitry, > > Thank you for the feedback! Here is an updated webrev.01; >

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

2016-09-20 Thread Dmitry Dmitriev
Serguei, Dmitry, Thank you for the feedback! Here is an updated webrev.01; http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.01/ Dmitry On 20.09.2016 12:48, Dmitry Samersoff wrote: Dmitry, Please, also change !strcmp(...) to

Re: RFR(S): JDK-8165500: TestJpsJar shouldn't jar all test.classpath directories

2016-09-20 Thread Dmitry Samersoff
Dmitry, After some debugging: 1. We have to explicitly @build JpsBase to put it into jar file. 2. Couple of testlibrary classes are build because of JpsBase.java dependency. Jtreg put it to ./JTwork/classes/sun/tools/jps/jdk/testlibrary/ i.e. we have incomplete jdk.testlibrary package. 3. If

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

2016-09-20 Thread Dmitry Samersoff
Dmitry, Please, also change !strcmp(...) to strcmp(...) == 0 because semantically strcmp result is not a boolean false. -Dmitry On 2016-09-20 06:38, serguei.spit...@oracle.com wrote: > Hi Dmitry, > > > Thanks a lot for this additional test coverage and discovering new bug: >