Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
Volker, I’ve look at the following files: src/share/native/sun/management/DiagnosticCommandImpl.c: nit: “legel” - “legal” (two times) In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if you allow dcmd_info_array to become NULL, then jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to check that. src/solaris/native/sun/management/OperatingSystemImpl.c No comments. src/share/transport/socket/socketTransport.c No comments. src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider No comments. Thanks, /Staffan On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote: Hi, could you please review the following changes for the ppc-aix-port stage/stage-9 repositories (the changes are planned for integration into ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage): http://cr.openjdk.java.net/~simonis/webrevs/8031581/ I've build and smoke tested without any problems on Linux/x86_64 and PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64. With these changes (and together with the changes from 8028537: PPC64: Updated jdk/test scripts to understand the AIX os and environment and 8031134 : PPC64: implement printing on AIX) our port passes all but the following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 baseline from www.java.net/download/jdk8/testresults/testresults.html): java/net/Inet6Address/B6558853.java java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically) java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically) java/nio/channels/Selector/RacyDeregister.java sun/security/krb5/auto/Unreachable.java (only on IPv6) Thank you and best regards, Volker Following a detailed description of the various changes: src/share/native/java/util/zip/zip_util.c src/share/native/sun/management/DiagnosticCommandImpl.c According to ISO C it is perfectly legal for malloc to return zero if called with a zero argument. Fix various places where malloc can potentially correctly return zero because it was called with a zero argument. Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a compiler warning on Linux, but on AIX it prevents a VM crash later on because the return value of malloc() will be casted to int which is especially bad if that pointer was bigger than 32-bit. make/CompileJavaClasses.gmk Also use PollingWatchService on AIX. make/lib/NioLibraries.gmk src/aix/native/sun/nio/ch/AixNativeThread.c Put the implementation for the native methods of NativeThread into AixNativeThread.c on AIX. src/solaris/native/sun/nio/ch/PollArrayWrapper.c src/solaris/native/sun/nio/ch/Net.c src/aix/classes/sun/nio/ch/AixPollPort.java src/aix/native/sun/nio/ch/AixPollPort.c src/aix/native/java/net/aix_close.c On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT, ...) are defined to different values than on other operating systems. The problem is however, that these constants are hardcoded as public final static members of various, shared Java classes. We therefore have to map them from Java to native every time before calling one of the native poll functions and back to Java after the call on AIX in order to get the right semantics. src/share/classes/java/nio/file/CopyMoveHelper.java As discussed on the core-libs mailing list (see http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/024119.html) it is not necessary to call Files.getFileAttributeView() with any linkOptions because at that place we've already checked that the target file can not be a symbolic link. This change makes the implementation more robust on platforms which support symbolic links but do not support the O_NOFOLLOW flag to the open system call. It also makes the JDK pass the demo/zipfs/basic.sh test on AIX. src/share/classes/sun/nio/cs/ext/ExtendedCharsets.java Support compound text on AIX in the same way like on other Unix platforms. src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider Define the correct attach provider for AIX. src/solaris/native/java/net/net_util_md.h src/solaris/native/sun/nio/ch/FileDispatcherImpl.c src/solaris/native/sun/nio/ch/ServerSocketChannelImpl.c AIX needs a workaround for I/O cancellation (see: http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf1/close.htm). ..The close() subroutine is blocked until all subroutines which use the file descriptor return to usr space. For example, when a thread is calling close and another thread is calling select with the same file descriptor, the close subroutine does not return until the select call returns To fix this problem, we have to use the various NET_ wrappers which are declared in
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On 15/01/2014 06:24, David Holmes wrote: I'm not a fan of runtime checks of this kind though if it is only a very samll number of values it might be okay. Another option would be to make those classes into templates as done with Version.java.template and substitute the right values at build time. But I'll let Alan and net-dev folk come back with their preferred technique for this. I plan to spend time on Volker's webrev later in the week (just too busy with other things right now). For the translation issue then it's an oversight in the original implementation, it just hasn't come up before (to my knowledge anyway). The simplest solution here maybe to to just move them to sun.net.ch.Net and have them initialized to their native value. In general then I'm not too concerned about that one, it's the changes to support async close on AIX that are leaping out at me. -Alan
Re: JDK 9 RFR of 8030814: Long.parseUnsignedLong should throw exception on too large input
On Jan 14, 2014, at 8:56 PM, Brian Burkhalter brian.burkhal...@oracle.com wrote: Please see the updated webrev http://cr.openjdk.java.net/~bpb/8030814/webrev.3/. Hopefully my verbose description of the very clever overflow test is accurate and understandable. Also, I cleaned up the JTREG test a bit and added some tests based on the boundary values which split the range of the quantity guard. +1 a high comment to code ratio :-) I agree with Joe's comments. Paul.
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On Wed, Jan 15, 2014 at 10:03 AM, Alan Bateman alan.bate...@oracle.comwrote: On 15/01/2014 06:24, David Holmes wrote: I'm not a fan of runtime checks of this kind though if it is only a very samll number of values it might be okay. Another option would be to make those classes into templates as done with Version.java.template and substitute the right values at build time. But I'll let Alan and net-dev folk come back with their preferred technique for this. I plan to spend time on Volker's webrev later in the week (just too busy with other things right now). For the translation issue then it's an oversight in the original implementation, it just hasn't come up before (to my knowledge anyway). The simplest solution here maybe to to just move them to sun.net.ch.Net and have them initialized to their native value. Do you mean sun.nio.ch.Net right? Do you propose to completely remove the definitions of the POLL constants from: src/share/classes/sun/nio/ch/AbstractPollArrayWrapper.java src/solaris/classes/sun/nio/ch/Port.java and replace all their usages by Net.POLL* ? In general then I'm not too concerned about that one, it's the changes to support async close on AIX that are leaping out at me. -Alan
Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.
Hi Sherman, Thanks for comments, here are some answers: the answer to (1)-(3) is that i do whatever the JarOutputStream/ZipOutputStream of the current JDK does and for a reason - there was a request from couple of customers to make native unpack200 results binary identical to the results taken from Java version. The bits set in the general flags are reserved by PKWARE or Currently unused according to the Zip specification but ZipOutputStream sets them this way. Same goes to the extraction of crc/size/csize into separate header - just to be compatible with Java implementation. We indeed do not support large (4GB) files due to the limitations of in-memory unpacking so yes, i omitted the logic of creation of the extended Zip64 LOC and CEN entries because there will be no case when they might be called upon. The only Zip64 feature is the support of the archives with 64K files - the rest id the Java JarOutputStream compatibility tweaks. At this moment i don't think that supporting old JDK behavior for the jars with 64K files is required - the older unpack200 wouldn't handle the situation correctly anyways. If the request for such option will emerge then implementing it will be a whole new task. /Alex On 1/15/14 8:58, Xueming Shen wrote: Hi Alex, (1) what's the bits are you setting into the general flags fields as below? header[4] = ( store ) ? SWAP_BYTES(0x0800) : 0x0808; (2) why do you want to use extra data descriptor to set crc/size/csize? while I understand that Jar/ZipOutputStream does that, but that is because they don't have the choice given the nature of stream, which means they don't know the csize and crc value until all bits get compressed/crc-ed. In case of unpack, all bits should have already be compressed and the crc value has been calculated, I don't see any compelling reason we need to sue the extra data descriptor here. (3) to add the cafe jar magic is interesting. that is really an old implementation details of jar on solaris. any request to add this one into the unpack tool? (4) I dont think the changeset is tryin to support ZIP64 extra fields tag in LOC and CEN tables (I kinda understand why, given the unpack does all unpacking work in memory, it is practically unrealistic to have a 4G memory to hold all 4G data). It appears most of the changes other than the zip64 end table is really not related to support the new zip64 format (5) flag jdk.util.zip.inhibitZip64 is introduced in jdk8 to support a old behavior for 64K total entries. You guys might want to consider if it is also worth considering to have this flag supported in unpack200. -Sherman On 1/14/14 10:04 AM, Alexander Zuev wrote: Please review my fix for JDK-8029646: [pack200] should support the new zip64 format. The fix can be found at http://cr.openjdk.java.net/~kizune/8029646/webrev.00/ Bug description is: https://bugs.openjdk.java.net/browse/JDK-8029646 /Alex
Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.
Sherman et all, self-correction regarding the flags, i misread the specification so flags are: always support UTF-8 file encoding (bit 11) and using EOS marker for the compressed files(bit 4). /Alex On 1/15/14 18:26, Alexander Zuev wrote: Hi Sherman, Thanks for comments, here are some answers: the answer to (1)-(3) is that i do whatever the JarOutputStream/ZipOutputStream of the current JDK does and for a reason - there was a request from couple of customers to make native unpack200 results binary identical to the results taken from Java version. The bits set in the general flags are reserved by PKWARE or Currently unused according to the Zip specification but ZipOutputStream sets them this way. Same goes to the extraction of crc/size/csize into separate header - just to be compatible with Java implementation. We indeed do not support large (4GB) files due to the limitations of in-memory unpacking so yes, i omitted the logic of creation of the extended Zip64 LOC and CEN entries because there will be no case when they might be called upon. The only Zip64 feature is the support of the archives with 64K files - the rest id the Java JarOutputStream compatibility tweaks. At this moment i don't think that supporting old JDK behavior for the jars with 64K files is required - the older unpack200 wouldn't handle the situation correctly anyways. If the request for such option will emerge then implementing it will be a whole new task. /Alex On 1/15/14 8:58, Xueming Shen wrote: Hi Alex, (1) what's the bits are you setting into the general flags fields as below? header[4] = ( store ) ? SWAP_BYTES(0x0800) : 0x0808; (2) why do you want to use extra data descriptor to set crc/size/csize? while I understand that Jar/ZipOutputStream does that, but that is because they don't have the choice given the nature of stream, which means they don't know the csize and crc value until all bits get compressed/crc-ed. In case of unpack, all bits should have already be compressed and the crc value has been calculated, I don't see any compelling reason we need to sue the extra data descriptor here. (3) to add the cafe jar magic is interesting. that is really an old implementation details of jar on solaris. any request to add this one into the unpack tool? (4) I dont think the changeset is tryin to support ZIP64 extra fields tag in LOC and CEN tables (I kinda understand why, given the unpack does all unpacking work in memory, it is practically unrealistic to have a 4G memory to hold all 4G data). It appears most of the changes other than the zip64 end table is really not related to support the new zip64 format (5) flag jdk.util.zip.inhibitZip64 is introduced in jdk8 to support a old behavior for 64K total entries. You guys might want to consider if it is also worth considering to have this flag supported in unpack200. -Sherman On 1/14/14 10:04 AM, Alexander Zuev wrote: Please review my fix for JDK-8029646: [pack200] should support the new zip64 format. The fix can be found at http://cr.openjdk.java.net/~kizune/8029646/webrev.00/ Bug description is: https://bugs.openjdk.java.net/browse/JDK-8029646 /Alex
Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.
On 1/15/14 18:34, Alexander Zuev wrote: Sherman et all, self-correction regarding the flags, i misread the specification so flags are: always support UTF-8 file encoding (bit 11) and using EOS marker for the compressed files(bit 4). Damn my fast fingers - not bit 4, bit 3 and it tells we are separating crc/size/csize into the separate header record for deflated files. I will update the comments with the description. Sorry for the confusion. /Alex
Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.
Kumar, thanks for your findings. See my comments inline. On 1/15/14 2:10, Kumar Srinivasan wrote: Hi Alex, zip.cpp: (nit) I would keep the hex values to be in upper case just like the others for consistency, not a big deal. Fixed. typo: + // Zip64 END sugnature Also fixed. PackTestZip64.java: shouldn't we test a jar with 64K+ entries ? Do we really have to do so? Creating, repacking, packing, unpacking and comparing of such jar file would take a lot of testing time. Right now i'm just testing that we are producing binary identical jar after we normalized it. If you think creating of large jar file is necessary i can easily do so. /Alex Kumar On 1/14/2014 10:04 AM, Alexander Zuev wrote: Please review my fix for JDK-8029646: [pack200] should support the new zip64 format. The fix can be found at http://cr.openjdk.java.net/~kizune/8029646/webrev.00/ Bug description is: https://bugs.openjdk.java.net/browse/JDK-8029646 /Alex
Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.
Hello, the new webrev with all the typos and comments fixed can be found at http://cr.openjdk.java.net/~kizune/8029646/webrev.01/ /Alex
[JDK8] RFR (XS): JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
http://cr.openjdk.java.net/~vlivanov/8031502/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8031502 InvokeBytecodeGenerator can produce incorrect bytecode for a LambdaForm when invoking a method from Object declared in an interface. The problem is the following: (1) java.lang.CharSequence interface declares abstract method String toString(); (2) after 8014013 fix, VM resolves CharSequence::toString()/invokeInterface to CharSequence::toString()/invokeVirtual; (3) during LambdaForm compilation, CharSequence is considered statically invocable (see InvokeBytecodeGenerator::isStaticallyInvocable) and invokevirtual for CharSequence::toString() is issued, which is wrong (invokevirtual throws ICCE if it references an interface); The fix is straightforward: during LambdaForm compilation, switch back from invokevirtual to invokeinterface instruction when invoking a method on an interface. The fix is targeted for 8. Will be also integrated into 9. Testing: regression test, jdk/test/java/lang/invoke, vm.mlvm.testlist, nashorn, jruby. Thanks! Best regards, Vladimir Ivanov
Re: [JDK8] RFR (XS): JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
On 2014-01-15, at 10:31 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: The fix is targeted for 8. Will be also integrated into 9. Testing: regression test, jdk/test/java/lang/invoke, vm.mlvm.testlist, nashorn, jruby. Vladimir, Did you test also on defmeth, or has that been incorporated into one of the other test suites? David
Re: [JDK8] RFR (XS): JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
Looks good to me -Sundar On Wednesday 15 January 2014 09:01 PM, Vladimir Ivanov wrote: http://cr.openjdk.java.net/~vlivanov/8031502/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8031502 InvokeBytecodeGenerator can produce incorrect bytecode for a LambdaForm when invoking a method from Object declared in an interface. The problem is the following: (1) java.lang.CharSequence interface declares abstract method String toString(); (2) after 8014013 fix, VM resolves CharSequence::toString()/invokeInterface to CharSequence::toString()/invokeVirtual; (3) during LambdaForm compilation, CharSequence is considered statically invocable (see InvokeBytecodeGenerator::isStaticallyInvocable) and invokevirtual for CharSequence::toString() is issued, which is wrong (invokevirtual throws ICCE if it references an interface); The fix is straightforward: during LambdaForm compilation, switch back from invokevirtual to invokeinterface instruction when invoking a method on an interface. The fix is targeted for 8. Will be also integrated into 9. Testing: regression test, jdk/test/java/lang/invoke, vm.mlvm.testlist, nashorn, jruby. Thanks! Best regards, Vladimir Ivanov
Re: JDK 9 RFR of 8030814: Long.parseUnsignedLong should throw exception on too large input
Given the identified items are changed accordingly is this approved? Thanks, Brian - Original Message - From: joe.da...@oracle.com To: brian.burkhal...@oracle.com Cc: core-libs-dev@openjdk.java.net Sent: Tuesday, January 14, 2014 7:28:11 PM GMT -08:00 US/Canada Pacific Subject: Re: JDK 9 RFR of 8030814: Long.parseUnsignedLong should throw exception on too large input Hi Brian, Lots good other than a few quibbles: We usually use /* * */ for long multi-line comments rather than // // // In the test update, we don't usually include mention of the bug id other than the @bug line. Thanks, -Joe On 1/14/2014 11:56 AM, Brian Burkhalter wrote: Please see the updated webrev http://cr.openjdk.java.net/~bpb/8030814/webrev.3/. Hopefully my verbose description of the very clever overflow test is accurate and understandable. Also, I cleaned up the JTREG test a bit and added some tests based on the boundary values which split the range of the quantity guard. Thanks, Brian
Re: [JDK8] RFR (XS): JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
David, thanks for looking at the fix. Good suggestion. Just ran tests on default methods - all pass. Best regards, Vladimir Ivanov On 1/15/14 7:44 PM, David Chase wrote: On 2014-01-15, at 10:31 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: The fix is targeted for 8. Will be also integrated into 9. Testing: regression test, jdk/test/java/lang/invoke, vm.mlvm.testlist, nashorn, jruby. Vladimir, Did you test also on defmeth, or has that been incorporated into one of the other test suites? David ___ mlvm-dev mailing list mlvm-...@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: [JDK8] RFR (XS): JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
Sundar, thank you. Best regards, Vladimir Ivanov On 1/15/14 7:48 PM, A. Sundararajan wrote: Looks good to me -Sundar On Wednesday 15 January 2014 09:01 PM, Vladimir Ivanov wrote: http://cr.openjdk.java.net/~vlivanov/8031502/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8031502 InvokeBytecodeGenerator can produce incorrect bytecode for a LambdaForm when invoking a method from Object declared in an interface. The problem is the following: (1) java.lang.CharSequence interface declares abstract method String toString(); (2) after 8014013 fix, VM resolves CharSequence::toString()/invokeInterface to CharSequence::toString()/invokeVirtual; (3) during LambdaForm compilation, CharSequence is considered statically invocable (see InvokeBytecodeGenerator::isStaticallyInvocable) and invokevirtual for CharSequence::toString() is issued, which is wrong (invokevirtual throws ICCE if it references an interface); The fix is straightforward: during LambdaForm compilation, switch back from invokevirtual to invokeinterface instruction when invoking a method on an interface. The fix is targeted for 8. Will be also integrated into 9. Testing: regression test, jdk/test/java/lang/invoke, vm.mlvm.testlist, nashorn, jruby. Thanks! Best regards, Vladimir Ivanov
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
Hi Staffan, thanks for the review. Please find my comments inline: On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.comwrote: Volker, I’ve look at the following files: src/share/native/sun/management/DiagnosticCommandImpl.c: nit: “legel” - “legal” (two times) In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if you allow dcmd_info_array to become NULL, then jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to check that. Good catch. I actually had problems with malloc returning NULL in 'getDiagnosticCommandArgumentInfoArray()' and then changed all other potentially dangerous locations which used the same pattern. However I think if the 'dcmd_info_array' has zero length it would be perfectly fine to return a zero length array. So what about the following solution: dcmdInfoCls = (*env)-FindClass(env, sun/management/DiagnosticCommandInfo); num_commands = (*env)-GetArrayLength(env, commands); if (num_commands = 0) { result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL); if (result == NULL) { JNU_ThrowOutOfMemoryError(env, 0); } else { return result; } } dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo)); if (dcmd_info_array == NULL) { JNU_ThrowOutOfMemoryError(env, NULL); } jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array); result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); That seems easier and saves me from handling the exception. What do you think? src/solaris/native/sun/management/OperatingSystemImpl.c No comments. src/share/transport/socket/socketTransport.c No comments. src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider No comments. Thanks, /Staffan On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote: Hi, could you please review the following changes for the ppc-aix-port stage/stage-9 repositories (the changes are planned for integration into ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage): http://cr.openjdk.java.net/~simonis/webrevs/8031581/ I've build and smoke tested without any problems on Linux/x86_64 and PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64. With these changes (and together with the changes from 8028537: PPC64: Updated jdk/test scripts to understand the AIX os and environment and 8031134 : PPC64: implement printing on AIX) our port passes all but the following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 baseline from www.java.net/download/jdk8/testresults/testresults.html): java/net/Inet6Address/B6558853.java java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically) java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically) java/nio/channels/Selector/RacyDeregister.java sun/security/krb5/auto/Unreachable.java (only on IPv6) Thank you and best regards, Volker Following a detailed description of the various changes: src/share/native/java/util/zip/zip_util.c src/share/native/sun/management/DiagnosticCommandImpl.c - According to ISO C it is perfectly legal for malloc to return zero if called with a zero argument. Fix various places where malloc can potentially correctly return zero because it was called with a zero argument. - Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a compiler warning on Linux, but on AIX it prevents a VM crash later on because the return value of malloc() will be casted to int which is especially bad if that pointer was bigger than 32-bit. make/CompileJavaClasses.gmk - Also use PollingWatchService on AIX. make/lib/NioLibraries.gmk src/aix/native/sun/nio/ch/AixNativeThread.c - Put the implementation for the native methods of NativeThread into AixNativeThread.c on AIX. src/solaris/native/sun/nio/ch/PollArrayWrapper.c src/solaris/native/sun/nio/ch/Net.c src/aix/classes/sun/nio/ch/AixPollPort.java src/aix/native/sun/nio/ch/AixPollPort.c src/aix/native/java/net/aix_close.c - On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT, ...) are defined to different values than on other operating systems. The problem is however, that these constants are hardcoded as public final static members of various, shared Java classes. We therefore have to map them from Java to native every time before calling one of the native poll functions and back to Java after the call on AIX in order to get the right semantics. src/share/classes/java/nio/file/CopyMoveHelper.java - As discussed on the core-libs mailing list (see http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/024119.html) it is not necessary to call
Re: RFR(S/L): 8028537: PPC64: Updated the JDK regression tests to run on AIX
Hi Alan, thanks for the suggestion. That's fine for me. I've copied the empty SCTP stubs from the macosx to the aix directory as well and updated the make file accordingly (in the patch for 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests). Therefore, the changes to the three tests: test/com/sun/nio/sctp/SctpChannel/Util.java test/com/sun/nio/sctp/SctpMultiChannel/Util.java test/com/sun/nio/sctp/SctpServerChannel/Util.java can be considered obsolete. Regards, Volker On Tue, Jan 14, 2014 at 8:13 PM, Alan Bateman alan.bate...@oracle.comwrote: On 14/01/2014 16:57, Volker Simonis wrote: : test/com/sun/nio/sctp/SctpChannel/Util.java test/com/sun/nio/sctp/SctpMultiChannel/Util.java test/com/sun/nio/sctp/SctpServerChannel/Util.java - On AIX, we currently haven't implemented SCTP but we nevertheless compile the shared SCTP classes into the runtime class library. This way the AIX JDK can at least compile SCTP applications altough it can not run them. To support this scenario, the runtime check for the availability of SCTP has to be extended to catch UnsatisfiedLinkError and NoClassDefFoundError. UnsatisfiedLinkError will be thrown the first time when the class SctpChannelImpl will be loaded because it cannot load the its native support library in the static initialisation section. On the next load attempt of the class, a NoClassDefFoundError will be thrown because of the previously failed initialisation. OS X has the same issue and the solution used there are stub implementations that just throw UOE. Details in jdk/src/macosx/classes/sun/nio/ch/sctp and that maybe that would work for AIX too. -Alan.
Re: JDK 9 RFR of 8030814: Long.parseUnsignedLong should throw exception on too large input
Yes :-) -Joe On 01/15/2014 08:04 AM, Brian Burkhalter wrote: Given the identified items are changed accordingly is this approved? Thanks, Brian - Original Message - From: joe.da...@oracle.com To: brian.burkhal...@oracle.com Cc: core-libs-dev@openjdk.java.net Sent: Tuesday, January 14, 2014 7:28:11 PM GMT -08:00 US/Canada Pacific Subject: Re: JDK 9 RFR of 8030814: Long.parseUnsignedLong should throw exception on too large input Hi Brian, Lots good other than a few quibbles: We usually use /* * */ for long multi-line comments rather than // // // In the test update, we don't usually include mention of the bug id other than the @bug line. Thanks, -Joe On 1/14/2014 11:56 AM, Brian Burkhalter wrote: Please see the updated webrev http://cr.openjdk.java.net/~bpb/8030814/webrev.3/. Hopefully my verbose description of the very clever overflow test is accurate and understandable. Also, I cleaned up the JTREG test a bit and added some tests based on the boundary values which split the range of the quantity guard. Thanks, Brian
Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.
On 1/15/14 7:01 AM, Alexander Zuev wrote: Hello, the new webrev with all the typos and comments fixed can be found at http://cr.openjdk.java.net/~kizune/8029646/webrev.01/ /Alex (1) jarmagic can be just a static constant somewhere or a stack variable. not big deal though. (2) the test only tests EOF for s. there is possibility that the newly created has more extra bytes at the end...though in theory this should not happen, it might be better just add an extra line to check the sizes of two first first? (3) the rest of the change looks good, but I agreed with Kumar that you may need to add a regression test for a jar file with 64k entries. otherwise the code for zip64 end is not being tested. the code looks fine, but I would trust a regression test more than my eyes:-) Thanks! -Sherman
hg: jdk8/tl/jdk: 8031502: JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
Changeset: 9e91793fd516 Author:vlivanov Date: 2014-01-15 20:48 +0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/9e91793fd516 8031502: JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter Reviewed-by: sundar, lagergren, drchase ! src/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java + test/java/lang/invoke/ObjectMethodInInterfaceTest.java
Re: [JDK8] RFR (XS): JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
[I’m resending something I sent earlier today to Vladimir directly.] On Jan 15, 2014, at 7:31 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: http://cr.openjdk.java.net/~vlivanov/8031502/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8031502 InvokeBytecodeGenerator can produce incorrect bytecode for a LambdaForm when invoking a method from Object declared in an interface. The problem is the following: (1) java.lang.CharSequence interface declares abstract method String toString(); (2) after 8014013 fix, VM resolves CharSequence::toString()/invokeInterface to CharSequence::toString()/invokeVirtual; Without having looked at the changes of 8014013, why did the invoke type change? Is it an optimization that was added? (3) during LambdaForm compilation, CharSequence is considered statically invocable (see InvokeBytecodeGenerator::isStaticallyInvocable) and invokevirtual for CharSequence::toString() is issued, which is wrong (invokevirtual throws ICCE if it references an interface); The fix is straightforward: during LambdaForm compilation, switch back from invokevirtual to invokeinterface instruction when invoking a method on an interface. I find this risky. Right now MemberName is only used in a couple places in java.lang.invoke but in the future it might be used for other things (e.g. java.lang.reflect). The information MemberName contains should be correct and usable without changing. Otherwise all users have to patch the information the same way as you do in your patch. I would like to see the VM passing correct information (whatever the definition of correct is here). The fix is targeted for 8. Will be also integrated into 9. Testing: regression test, jdk/test/java/lang/invoke, vm.mlvm.testlist, nashorn, jruby. Thanks! Best regards, Vladimir Ivanov ___ mlvm-dev mailing list mlvm-...@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On Wed, Jan 15, 2014 at 6:27 PM, Volker Simonis volker.simo...@gmail.com wrote: On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis volker.simo...@gmail.com wrote: Hi Staffan, thanks for the review. Please find my comments inline: On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.com wrote: Volker, I’ve look at the following files: src/share/native/sun/management/DiagnosticCommandImpl.c: nit: “legel” - “legal” (two times) In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if you allow dcmd_info_array to become NULL, then jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to check that. Good catch. I actually had problems with malloc returning NULL in 'getDiagnosticCommandArgumentInfoArray()' and then changed all other potentially dangerous locations which used the same pattern. However I think if the 'dcmd_info_array' has zero length it would be perfectly fine to return a zero length array. So what about the following solution: Sorry for the noise - it seems I was a little indisposed during the last mails:) So this is the simple change I'd like to propose for Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo(): @@ -117,19 +119,23 @@ return NULL; } num_commands = (*env)-GetArrayLength(env, commands); - dcmd_info_array = (dcmdInfo*) malloc(num_commands * - sizeof(dcmdInfo)); + dcmdInfoCls = (*env)-FindClass(env, + sun/management/DiagnosticCommandInfo); + result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); + if (result == NULL) { + JNU_ThrowOutOfMemoryError(env, 0); + } + if (num_commands == 0) { + /* Handle the 'zero commands' case specially to avoid calling 'malloc()' */ + /* with a zero argument because that may legally return a NULL pointer. */ + return result; + } + dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo)); if (dcmd_info_array == NULL) { JNU_ThrowOutOfMemoryError(env, NULL); } jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array); - dcmdInfoCls = (*env)-FindClass(env, - sun/management/DiagnosticCommandInfo); - result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); - if (result == NULL) { - free(dcmd_info_array); - JNU_ThrowOutOfMemoryError(env, 0); - } for (i=0; inum_commands; i++) { args = getDiagnosticCommandArgumentInfoArray(env, (*env)-GetObjectArrayElement(env,commands,i), If the 'commands' input array is of zero length just return a zero length array. OK? dcmdInfoCls = (*env)-FindClass(env, sun/management/DiagnosticCommandInfo); num_commands = (*env)-GetArrayLength(env, commands); Sorry, of course I wanted to say if (num_commands == 0) here! if (num_commands = 0) { result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL); if (result == NULL) { JNU_ThrowOutOfMemoryError(env, 0); } else { return result; } } dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo)); if (dcmd_info_array == NULL) { JNU_ThrowOutOfMemoryError(env, NULL); } jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array); result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); That seems easier and saves me from handling the exception. What do you think? src/solaris/native/sun/management/OperatingSystemImpl.c No comments. src/share/transport/socket/socketTransport.c No comments. src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider No comments. Thanks, /Staffan On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote: Hi, could you please review the following changes for the ppc-aix-port stage/stage-9 repositories (the changes are planned for integration into ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage): http://cr.openjdk.java.net/~simonis/webrevs/8031581/ I've build and smoke tested without any problems on Linux/x86_64 and PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64. With these changes (and together with the changes from 8028537: PPC64: Updated jdk/test scripts to understand the AIX os and environment and 8031134 : PPC64: implement printing on AIX) our port passes all but the following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 baseline from www.java.net/download/jdk8/testresults/testresults.html): java/net/Inet6Address/B6558853.java java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically) java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically) java/nio/channels/Selector/RacyDeregister.java sun/security/krb5/auto/Unreachable.java (only on IPv6) Thank you and best regards,
Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.
On 1/15/14 21:34, Xueming Shen wrote: On 1/15/14 7:01 AM, Alexander Zuev wrote: Hello, the new webrev with all the typos and comments fixed can be found at http://cr.openjdk.java.net/~kizune/8029646/webrev.01/ /Alex (1) jarmagic can be just a static constant somewhere or a stack variable. not big deal though. Ok, i'll see to it. (2) the test only tests EOF for s. there is possibility that the newly created has more extra bytes at the end...though in theory this should not happen, it might be better just add an extra line to check the sizes of two first first? The test tests for EOF only for s because if s is -1 (EOF) and d is not (more extra bytes in newly created jar) due to the check at line 99 - if (s != d) the exception will be thrown for files content mismatch. (3) the rest of the change looks good, but I agreed with Kumar that you may need to add a regression test for a jar file with 64k entries. otherwise the code for zip64 end is not being tested. the code looks fine, but I would trust a regression test more than my eyes:-) All right, i'll modify the regression test. /Alex
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
Yes, that looks like a good solution. /Staffan On 15 jan 2014, at 17:34, Volker Simonis volker.simo...@gmail.com wrote: Hi Staffan, thanks for the review. Please find my comments inline: On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.com wrote: Volker, I’ve look at the following files: src/share/native/sun/management/DiagnosticCommandImpl.c: nit: “legel” - “legal” (two times) In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if you allow dcmd_info_array to become NULL, then jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to check that. Good catch. I actually had problems with malloc returning NULL in 'getDiagnosticCommandArgumentInfoArray()' and then changed all other potentially dangerous locations which used the same pattern. However I think if the 'dcmd_info_array' has zero length it would be perfectly fine to return a zero length array. So what about the following solution: dcmdInfoCls = (*env)-FindClass(env, sun/management/DiagnosticCommandInfo); num_commands = (*env)-GetArrayLength(env, commands); if (num_commands = 0) { result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL); if (result == NULL) { JNU_ThrowOutOfMemoryError(env, 0); } else { return result; } } dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo)); if (dcmd_info_array == NULL) { JNU_ThrowOutOfMemoryError(env, NULL); } jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array); result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); That seems easier and saves me from handling the exception. What do you think? src/solaris/native/sun/management/OperatingSystemImpl.c No comments. src/share/transport/socket/socketTransport.c No comments. src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider No comments. Thanks, /Staffan On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote: Hi, could you please review the following changes for the ppc-aix-port stage/stage-9 repositories (the changes are planned for integration into ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage): http://cr.openjdk.java.net/~simonis/webrevs/8031581/ I've build and smoke tested without any problems on Linux/x86_64 and PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64. With these changes (and together with the changes from 8028537: PPC64: Updated jdk/test scripts to understand the AIX os and environment and 8031134 : PPC64: implement printing on AIX) our port passes all but the following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 baseline from www.java.net/download/jdk8/testresults/testresults.html): java/net/Inet6Address/B6558853.java java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically) java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically) java/nio/channels/Selector/RacyDeregister.java sun/security/krb5/auto/Unreachable.java (only on IPv6) Thank you and best regards, Volker Following a detailed description of the various changes: src/share/native/java/util/zip/zip_util.c src/share/native/sun/management/DiagnosticCommandImpl.c According to ISO C it is perfectly legal for malloc to return zero if called with a zero argument. Fix various places where malloc can potentially correctly return zero because it was called with a zero argument. Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a compiler warning on Linux, but on AIX it prevents a VM crash later on because the return value of malloc() will be casted to int which is especially bad if that pointer was bigger than 32-bit. make/CompileJavaClasses.gmk Also use PollingWatchService on AIX. make/lib/NioLibraries.gmk src/aix/native/sun/nio/ch/AixNativeThread.c Put the implementation for the native methods of NativeThread into AixNativeThread.c on AIX. src/solaris/native/sun/nio/ch/PollArrayWrapper.c src/solaris/native/sun/nio/ch/Net.c src/aix/classes/sun/nio/ch/AixPollPort.java src/aix/native/sun/nio/ch/AixPollPort.c src/aix/native/java/net/aix_close.c On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT, ...) are defined to different values than on other operating systems. The problem is however, that these constants are hardcoded as public final static members of various, shared Java classes. We therefore have to map them from Java to native every time before calling one of the native poll functions and back to Java after the call on AIX in order to get the right semantics. src/share/classes/java/nio/file/CopyMoveHelper.java As discussed on the core-libs mailing list (see
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On 15 jan 2014, at 18:27, Volker Simonis volker.simo...@gmail.com wrote: On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis volker.simo...@gmail.com wrote: Hi Staffan, thanks for the review. Please find my comments inline: On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.com wrote: Volker, I’ve look at the following files: src/share/native/sun/management/DiagnosticCommandImpl.c: nit: “legel” - “legal” (two times) In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if you allow dcmd_info_array to become NULL, then jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to check that. Good catch. I actually had problems with malloc returning NULL in 'getDiagnosticCommandArgumentInfoArray()' and then changed all other potentially dangerous locations which used the same pattern. However I think if the 'dcmd_info_array' has zero length it would be perfectly fine to return a zero length array. So what about the following solution: dcmdInfoCls = (*env)-FindClass(env, sun/management/DiagnosticCommandInfo); num_commands = (*env)-GetArrayLength(env, commands); Sorry, of course I wanted to say if (num_commands == 0) here! I understood as much :-) if (num_commands = 0) { result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL); if (result == NULL) { JNU_ThrowOutOfMemoryError(env, 0); } else { return result; } } dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo)); if (dcmd_info_array == NULL) { JNU_ThrowOutOfMemoryError(env, NULL); } jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array); result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); That seems easier and saves me from handling the exception. What do you think? src/solaris/native/sun/management/OperatingSystemImpl.c No comments. src/share/transport/socket/socketTransport.c No comments. src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider No comments. Thanks, /Staffan On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote: Hi, could you please review the following changes for the ppc-aix-port stage/stage-9 repositories (the changes are planned for integration into ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage): http://cr.openjdk.java.net/~simonis/webrevs/8031581/ I've build and smoke tested without any problems on Linux/x86_64 and PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64. With these changes (and together with the changes from 8028537: PPC64: Updated jdk/test scripts to understand the AIX os and environment and 8031134 : PPC64: implement printing on AIX) our port passes all but the following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 baseline from www.java.net/download/jdk8/testresults/testresults.html): java/net/Inet6Address/B6558853.java java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically) java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically) java/nio/channels/Selector/RacyDeregister.java sun/security/krb5/auto/Unreachable.java (only on IPv6) Thank you and best regards, Volker Following a detailed description of the various changes: src/share/native/java/util/zip/zip_util.c src/share/native/sun/management/DiagnosticCommandImpl.c According to ISO C it is perfectly legal for malloc to return zero if called with a zero argument. Fix various places where malloc can potentially correctly return zero because it was called with a zero argument. Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a compiler warning on Linux, but on AIX it prevents a VM crash later on because the return value of malloc() will be casted to int which is especially bad if that pointer was bigger than 32-bit. make/CompileJavaClasses.gmk Also use PollingWatchService on AIX. make/lib/NioLibraries.gmk src/aix/native/sun/nio/ch/AixNativeThread.c Put the implementation for the native methods of NativeThread into AixNativeThread.c on AIX. src/solaris/native/sun/nio/ch/PollArrayWrapper.c src/solaris/native/sun/nio/ch/Net.c src/aix/classes/sun/nio/ch/AixPollPort.java src/aix/native/sun/nio/ch/AixPollPort.c src/aix/native/java/net/aix_close.c On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT, ...) are defined to different values than on other operating systems. The problem is however, that these constants are hardcoded as public final static members of various, shared Java classes. We therefore have to map them from Java to native every time before calling one of the native poll functions and back to Java after the call on AIX in order to get the right semantics.
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On 15 jan 2014, at 18:52, Volker Simonis volker.simo...@gmail.com wrote: On Wed, Jan 15, 2014 at 6:27 PM, Volker Simonis volker.simo...@gmail.com wrote: On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis volker.simo...@gmail.com wrote: Hi Staffan, thanks for the review. Please find my comments inline: On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.com wrote: Volker, I’ve look at the following files: src/share/native/sun/management/DiagnosticCommandImpl.c: nit: “legel” - “legal” (two times) In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if you allow dcmd_info_array to become NULL, then jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to check that. Good catch. I actually had problems with malloc returning NULL in 'getDiagnosticCommandArgumentInfoArray()' and then changed all other potentially dangerous locations which used the same pattern. However I think if the 'dcmd_info_array' has zero length it would be perfectly fine to return a zero length array. So what about the following solution: Sorry for the noise - it seems I was a little indisposed during the last mails:) So this is the simple change I'd like to propose for Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo(): @@ -117,19 +119,23 @@ return NULL; } num_commands = (*env)-GetArrayLength(env, commands); - dcmd_info_array = (dcmdInfo*) malloc(num_commands * - sizeof(dcmdInfo)); + dcmdInfoCls = (*env)-FindClass(env, + sun/management/DiagnosticCommandInfo); + result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); + if (result == NULL) { + JNU_ThrowOutOfMemoryError(env, 0); + } + if (num_commands == 0) { + /* Handle the 'zero commands' case specially to avoid calling 'malloc()' */ + /* with a zero argument because that may legally return a NULL pointer. */ + return result; + } + dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo)); if (dcmd_info_array == NULL) { JNU_ThrowOutOfMemoryError(env, NULL); } jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array); - dcmdInfoCls = (*env)-FindClass(env, - sun/management/DiagnosticCommandInfo); - result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); - if (result == NULL) { - free(dcmd_info_array); - JNU_ThrowOutOfMemoryError(env, 0); - } for (i=0; inum_commands; i++) { args = getDiagnosticCommandArgumentInfoArray(env, (*env)-GetObjectArrayElement(env,commands,i), If the 'commands' input array is of zero length just return a zero length array. OK? Yes, this looks good (still :-) ) /Staffan dcmdInfoCls = (*env)-FindClass(env, sun/management/DiagnosticCommandInfo); num_commands = (*env)-GetArrayLength(env, commands); Sorry, of course I wanted to say if (num_commands == 0) here! if (num_commands = 0) { result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL); if (result == NULL) { JNU_ThrowOutOfMemoryError(env, 0); } else { return result; } } dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo)); if (dcmd_info_array == NULL) { JNU_ThrowOutOfMemoryError(env, NULL); } jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array); result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL); That seems easier and saves me from handling the exception. What do you think? src/solaris/native/sun/management/OperatingSystemImpl.c No comments. src/share/transport/socket/socketTransport.c No comments. src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider No comments. Thanks, /Staffan On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote: Hi, could you please review the following changes for the ppc-aix-port stage/stage-9 repositories (the changes are planned for integration into ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage): http://cr.openjdk.java.net/~simonis/webrevs/8031581/ I've build and smoke tested without any problems on Linux/x86_64 and PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64. With these changes (and together with the changes from 8028537: PPC64: Updated jdk/test scripts to understand the AIX os and environment and 8031134 : PPC64: implement printing on AIX) our port passes all but the following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 baseline from www.java.net/download/jdk8/testresults/testresults.html):
Re: [JDK8] RFR (XS): JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
Chris, Thanks for looking into this. See my answers inline. Best regards, Vladimir Ivanov On 1/15/14 9:51 PM, Christian Thalinger wrote: [I’m resending something I sent earlier today to Vladimir directly.] On Jan 15, 2014, at 7:31 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: http://cr.openjdk.java.net/~vlivanov/8031502/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8031502 InvokeBytecodeGenerator can produce incorrect bytecode for a LambdaForm when invoking a method from Object declared in an interface. The problem is the following: (1) java.lang.CharSequence interface declares abstract method String toString(); (2) after 8014013 fix, VM resolves CharSequence::toString()/invokeInterface to CharSequence::toString()/invokeVirtual; Without having looked at the changes of 8014013, why did the invoke type change? Is it an optimization that was added? After 8014013, LinkResolver::resolve_interface_call returns virtual (_call_kind = CallInfo::vtable_call), instead of interface method and MethodHandles::init_method_MemberName uses it as is (without any fine tuning, which was done before). It's caused by the following: - LinkResolver::linktime_resolve_interface_method returns CharSequence::toString method, but it has vtable index, instead of itable index; - LinkResolver::runtime_resolve_interface_method looks at resolved method and sets _call_kind to vtable_call, since resolved method doesn't have itable index. - then MethodHandles::init_method_MemberName looks at CallInfo passed in and sets the reference kind flag to JVM_REF_invokeVirtual. That's how the conversion from invokeInterface to invokeVirtual occurs. I did a quick debugging session with pre-8014013 hotspot to check how it worked before, but don't remember all the details now. (3) during LambdaForm compilation, CharSequence is considered statically invocable (see InvokeBytecodeGenerator::isStaticallyInvocable) and invokevirtual for CharSequence::toString() is issued, which is wrong (invokevirtual throws ICCE if it references an interface); The fix is straightforward: during LambdaForm compilation, switch back from invokevirtual to invokeinterface instruction when invoking a method on an interface. I find this risky. Right now MemberName is only used in a couple places in java.lang.invoke but in the future it might be used for other things (e.g. java.lang.reflect). The information MemberName contains should be correct and usable without changing. Otherwise all users have to patch the information the same way as you do in your patch. I would like to see the VM passing correct information (whatever the definition of correct is here). It's an interesting question what kind of correctness is required for MemberName and I don't know the answer. Looking through the code, I got an impression MemberName isn't designed to provide information to be used as is for bytecode generation, because, at least: - MemberName::referenceKindIsConsistent already expects (clazz.isInterface() refKind == REF_invokeVirtual isObjectPublicMethod()) case; - InvokeBytecodeGenerator::emitStaticInvoke already has a special case for REF_invokeSpecial referenceKind, converting it to REF_invokeVirtual; Best regards, Vladimir Ivanov
Re: RFR: (8030875) Macros for checking and returning on exceptions
Hi Martin, A followup, what is the subtlety that makes it beneficial to wrap even a single statement in a do {} while (0)? It makes sense for macros with multiple statements but not for a single statement substitution of expressions. For a single statement (w/o ';'), it seems like a redundant overhead that does not solve an issue. Thanks, Roger On 1/14/2014 5:20 PM, Martin Buchholz wrote: On Tue, Jan 14, 2014 at 7:54 AM, roger riggs roger.ri...@oracle.com mailto:roger.ri...@oracle.com wrote: Hi David, The CHECK_RETURN macros have existed in java.net http://java.net for some time and I have not seen any empty statement warnings. The CHECK_EXCEPTION macros are new and does not have any uses yet. I don't plan to do any wholesale modification of current sources. The macros always produce a valid statement; (though my c/c++ may be a bit rusty). Macros should not generally be complete statements, including semicolons; then the source code looks like this: FOO() which looks (including to emacs) like someone forgot the trailing semicolon. Instead, these macros should be written using the well-known do ( ... ) while (0) idiom, e.g. #define CHECK_NULL(x) do { if ((x) == NULL) return; } while (0) (Note that it has been requested to rename the macros to include a JNU_prefix to be consistent with other macros in jni_util.h. I will propose a separate set of changes for that). Roger On 1/13/2014 9:51 PM, David Holmes wrote: Hi Roger, webrev: http://cr.openjdk.java.net/~rriggs/webrev-check-exception-8030875/ http://cr.openjdk.java.net/%7Erriggs/webrev-check-exception-8030875/ Do these macro definitions not cause empty statement warnings from the compiler? (Existing ones have the same problem I guess) Also I don't see any use of the CHECK_EXCEPTION macros? In fact the bulk of changes here seem completely unrelated to the exception aspect of this CR. Cheers, David [1] https://bugs.openjdk.java.net/browse/JDK-8030875
JDK 9 RFC on 6667086: Double.doubleToLongBits(final double value) contains inefficient test for NaN
Issue: https://bugs.openjdk.java.net/browse/JDK-6667086 Webrev: http://cr.openjdk.java.net/~bpb/6667086/webrev/ According to micro-benckmarks, there is no statistically significant performance change due to applying this patch but the code definitely looks cleaner. Thanks, Brian
Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.
Hi Alex, Kumar, thanks for your findings. See my comments inline. On 1/15/14 2:10, Kumar Srinivasan wrote: Hi Alex, zip.cpp: (nit) I would keep the hex values to be in upper case just like the others for consistency, not a big deal. Fixed. typo: + // Zip64 END sugnature Also fixed. PackTestZip64.java: shouldn't we test a jar with 64K+ entries ? Do we really have to do so? Creating, repacking, packing, unpacking and comparing of such jar file would take a lot of testing time. Right now i'm just testing that we are producing binary identical jar after we normalized it. If you think creating of large jar file is necessary i can easily do so. We must have a test!, it is not necessary that we should execute this all the time, but having this in place, will enable SQ to run this test periodically, and we can run it as well during development. You can copy most of the logic from here, and use the system property/strategy to enable the time consuming one. http://hg.openjdk.java.net/jdk9/dev/jdk/file/3b4ac8d1b76f/test/tools/launcher/BigJar.java I think it would be good to test jars created by the JDK (tools.jar or some suitable one), using the golden jar may not be appropriate, since it is a pre-canned binary. Also I am not sure if we should test jars created by JarOutputStream as well as JarFile, there are some nuances with these APIs and how they write out the jar file. Kumar /Alex Kumar On 1/14/2014 10:04 AM, Alexander Zuev wrote: Please review my fix for JDK-8029646: [pack200] should support the new zip64 format. The fix can be found at http://cr.openjdk.java.net/~kizune/8029646/webrev.00/ Bug description is: https://bugs.openjdk.java.net/browse/JDK-8029646 /Alex
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
Hi Peter/David, we could finally get a trace of exception with fastdebug build and ReferenceHandler modified (with runImpl() added and called from run()). The logs, disassembled code is available in JIRA https://bugs.openjdk.java.net/browse/JDK-8022321 as attachments. Observations from the log: Root Cause: 1) UncaughtException is being dispatched from Reference.java:143 141 ReferenceObject r; 142 synchronized (lock) { 143if (pending != null) { 144r = pending; 145pending = r.discovered; 146r.discovered = null; pending field in Reference is touched and updated by the collector, so at line 143 when the execution context is in Reference handler there might have been an Exception pending due to allocation done by collector which causes ReferenceHandler thread to die. Suggested fix: - As proposed earlier putting an outer guard(try-catch on OOME) in the ReferenceHandler will fix the issue, if ReferenceHandler is considered as part of the GC sub system then it should be alive even in the midst of an OOME so i feel that the additional guard should be allowed, however i might still be ignorant of vital implications. - Apart from the above changes, Peter's suggestion to create and call a private runImpl() from run() in ReferenceHandler makes sense to me. --- Thanks kalyan On 01/13/2014 03:57 PM, srikalyan wrote: On 1/11/14, 6:15 AM, Peter Levart wrote: On 01/10/2014 10:51 PM, srikalyan chandrashekar wrote: Hi Peter the version you provided ran indefinitely(i put a 10 minute timeout) and the program got interrupted(no error), Did you run it with or without fastedbug -XX:+TraceExceptions ? If with, it might be that fastdebug and/or -XX:+TraceExceptions changes the execution a bit so that we can no longer reproduce the wrong behaviour. With fastdebug -XX:TraceExceptions. I will try combination of possible options(i.e without -XX:TraceEception on debug build etc) soon. even if there were to be an error you cannot print the string of thread to console(these have been attempted earlier). ...it has been attempted to print toString in uncaught exception handler. At that time, the heap is still full. I'm printing it after the GC has cleared the heap. You can try that it works by commenting out the try { and corresponding } catch (OOME x) {} exception handler... Since there is a GC call prior to printing string i will give that a shot with non-debug build. - The test's running on interpreter mode, what i am watching for is one error with trace. Without fastdebug build and -XX:+TraceExceptions i am able to reproduce failure atleast 5 failures out of 1000 runs but with fastdebug+Trace no luck yet(already past few 1000 runs). It might be interesting to try with fastebug build but without the -XX:+TraceExceptions option to see what has an effect on it. It might also be interesting to try the modified ReferenceHandler (the one with private runImpl() method called from run()) and with normal non-fastdebug JDK. This info might be useful when one starts to inspect the exception handling code in interpreter... Regards, Peter -- Thanks kalyan Ph: (408)-585-8040 --- Thanks kalyan On 01/10/2014 02:57 AM, Peter Levart wrote: On 01/10/2014 09:31 AM, Peter Levart wrote: Since we suspect there's something wrong with exception handling in interpreter, I devised a hypothetical reproducer that tries to simulate ReferenceHandler in many aspects, but doesn't require to be a ReferenceHandler: http://cr.openjdk.java.net/~plevart/misc/OOME/OOMECatchingTest.java This is designed to run indefinitely and only terminate if/when thread dies. Could you run this program in the environment that causes the OOMEInReferenceHandler test to fail and see if it terminates? I forgot to mention that in order for this long-running program to exhibit interpreter behaviour, it should be run with -Xint option. So I suggest: -Xmx24M -XX:-UseTLAB -Xint Regards, Peter
Review request for JDK-6760902: inconsistent behavior in bootstrap class loader for classes and resources
There is an inconsistency in searching classes vs resources if bootclasspath contains an empty path. Empty path on bootclasspath is skipped by the bootstrap class loader when searching classes while it defaults to current working directory when searching resources as the application class loader. This fixes sun.misc.Launcher to skip empty path when constructing the paths from bootclasspath for resource lookup. Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6760902/webrev.00/ There is some incompatibility risk that may impact existing code depending on this behavior to search resources from the current working directory if empty path (rather than explicit) is set. I think most application using bootclasspath is to add their paths to load their classes and likely expect the resources are looked in the consistent way (i.e. skips the empty path). So I expect the incompatibility risk is low. thanks Mandy
Re: [JDK8] RFR (XS): JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
On Jan 15, 2014, at 11:41 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: Chris, Thanks for looking into this. See my answers inline. Best regards, Vladimir Ivanov On 1/15/14 9:51 PM, Christian Thalinger wrote: [I’m resending something I sent earlier today to Vladimir directly.] On Jan 15, 2014, at 7:31 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: http://cr.openjdk.java.net/~vlivanov/8031502/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8031502 InvokeBytecodeGenerator can produce incorrect bytecode for a LambdaForm when invoking a method from Object declared in an interface. The problem is the following: (1) java.lang.CharSequence interface declares abstract method String toString(); (2) after 8014013 fix, VM resolves CharSequence::toString()/invokeInterface to CharSequence::toString()/invokeVirtual; Without having looked at the changes of 8014013, why did the invoke type change? Is it an optimization that was added? After 8014013, LinkResolver::resolve_interface_call returns virtual (_call_kind = CallInfo::vtable_call), instead of interface method and MethodHandles::init_method_MemberName uses it as is (without any fine tuning, which was done before). It's caused by the following: - LinkResolver::linktime_resolve_interface_method returns CharSequence::toString method, but it has vtable index, instead of itable index; - LinkResolver::runtime_resolve_interface_method looks at resolved method and sets _call_kind to vtable_call, since resolved method doesn't have itable index. - then MethodHandles::init_method_MemberName looks at CallInfo passed in and sets the reference kind flag to JVM_REF_invokeVirtual. That's how the conversion from invokeInterface to invokeVirtual occurs. I did a quick debugging session with pre-8014013 hotspot to check how it worked before, but don't remember all the details now. (3) during LambdaForm compilation, CharSequence is considered statically invocable (see InvokeBytecodeGenerator::isStaticallyInvocable) and invokevirtual for CharSequence::toString() is issued, which is wrong (invokevirtual throws ICCE if it references an interface); The fix is straightforward: during LambdaForm compilation, switch back from invokevirtual to invokeinterface instruction when invoking a method on an interface. I find this risky. Right now MemberName is only used in a couple places in java.lang.invoke but in the future it might be used for other things (e.g. java.lang.reflect). The information MemberName contains should be correct and usable without changing. Otherwise all users have to patch the information the same way as you do in your patch. I would like to see the VM passing correct information (whatever the definition of correct is here). It's an interesting question what kind of correctness is required for MemberName and I don't know the answer. Looking through the code, I got an impression MemberName isn't designed to provide information to be used as is for bytecode generation, because, at least: - MemberName::referenceKindIsConsistent already expects (clazz.isInterface() refKind == REF_invokeVirtual isObjectPublicMethod()) case; - InvokeBytecodeGenerator::emitStaticInvoke already has a special case for REF_invokeSpecial referenceKind, converting it to REF_invokeVirtual; John would know the answer. Given this change should go into JDK 8 I think we should push for now. If we can come up with a better way to handle these situations we can push another change for 9 or 8u20. Best regards, Vladimir Ivanov ___ mlvm-dev mailing list mlvm-...@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
RFR 8031961 ProcessBuilder.Basic should use NIO Files to copy files
Please review this minor test improvement to replace spawning /bin/cp to copy files with NIO Files.copy. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-process-cp-8031961/ Thanks, Roger [1]https://bugs.openjdk.java.net/browse/JDK-8031961
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
On 16/01/2014 10:19 AM, srikalyan chandrashekar wrote: Hi Peter/David, we could finally get a trace of exception with fastdebug build and ReferenceHandler modified (with runImpl() added and called from run()). The logs, disassembled code is available in JIRA https://bugs.openjdk.java.net/browse/JDK-8022321 as attachments. All I can see is the log for the OOMECatchingTest program not one for the actual ReferenceHandler ?? Observations from the log: Root Cause: 1) UncaughtException is being dispatched from Reference.java:143 141 ReferenceObject r; 142 synchronized (lock) { 143if (pending != null) { 144r = pending; 145pending = r.discovered; 146r.discovered = null; pending field in Reference is touched and updated by the collector, so at line 143 when the execution context is in Reference handler there might have been an Exception pending due to allocation done by collector which causes ReferenceHandler thread to die. Sorry but the GC does not trigger asynchronous exceptions so this explanation does not make any sense to me. What part of the log led you to this conclusion? Suggested fix: - As proposed earlier putting an outer guard(try-catch on OOME) in the ReferenceHandler will fix the issue, if ReferenceHandler is considered as part of the GC sub system then it should be alive even in the midst of an OOME so i feel that the additional guard should be allowed, however i might still be ignorant of vital implications. - Apart from the above changes, Peter's suggestion to create and call a private runImpl() from run() in ReferenceHandler makes sense to me. Why would we need this? David - --- Thanks kalyan On 01/13/2014 03:57 PM, srikalyan wrote: On 1/11/14, 6:15 AM, Peter Levart wrote: On 01/10/2014 10:51 PM, srikalyan chandrashekar wrote: Hi Peter the version you provided ran indefinitely(i put a 10 minute timeout) and the program got interrupted(no error), Did you run it with or without fastedbug -XX:+TraceExceptions ? If with, it might be that fastdebug and/or -XX:+TraceExceptions changes the execution a bit so that we can no longer reproduce the wrong behaviour. With fastdebug -XX:TraceExceptions. I will try combination of possible options(i.e without -XX:TraceEception on debug build etc) soon. even if there were to be an error you cannot print the string of thread to console(these have been attempted earlier). ...it has been attempted to print toString in uncaught exception handler. At that time, the heap is still full. I'm printing it after the GC has cleared the heap. You can try that it works by commenting out the try { and corresponding } catch (OOME x) {} exception handler... Since there is a GC call prior to printing string i will give that a shot with non-debug build. - The test's running on interpreter mode, what i am watching for is one error with trace. Without fastdebug build and -XX:+TraceExceptions i am able to reproduce failure atleast 5 failures out of 1000 runs but with fastdebug+Trace no luck yet(already past few 1000 runs). It might be interesting to try with fastebug build but without the -XX:+TraceExceptions option to see what has an effect on it. It might also be interesting to try the modified ReferenceHandler (the one with private runImpl() method called from run()) and with normal non-fastdebug JDK. This info might be useful when one starts to inspect the exception handling code in interpreter... Regards, Peter -- Thanks kalyan Ph: (408)-585-8040 --- Thanks kalyan On 01/10/2014 02:57 AM, Peter Levart wrote: On 01/10/2014 09:31 AM, Peter Levart wrote: Since we suspect there's something wrong with exception handling in interpreter, I devised a hypothetical reproducer that tries to simulate ReferenceHandler in many aspects, but doesn't require to be a ReferenceHandler: http://cr.openjdk.java.net/~plevart/misc/OOME/OOMECatchingTest.java This is designed to run indefinitely and only terminate if/when thread dies. Could you run this program in the environment that causes the OOMEInReferenceHandler test to fail and see if it terminates? I forgot to mention that in order for this long-running program to exhibit interpreter behaviour, it should be run with -Xint option. So I suggest: -Xmx24M -XX:-UseTLAB -Xint Regards, Peter