October 2017 - core-libs-dev

RFR(xs): (aix but affects shared code too) 8186665: buffer overflow in Java_java_nio_MappedByteBuffer_isLoaded0
by Thomas Stüfe 19 Oct '17

19 Oct '17

Hi all, may I please have reviews for the following patch. Issue: https://bugs.openjdk.java.net/browse/JDK-8186665 Webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8186665-buffer-overflow-in-minco… Issue text for your convenience: -- In Java_java_nio_MappedByteBuffer_isLoaded0, we call mincore(2) to retrieve the paging status of an address range. The size of the output buffer for mincore(2) depends on the number of pages in *system page size* in the given memory range (this is spelled out more or less explicitly on AIX and Linux, nothing is said on BSD/OSX, but I assume the same). The number of pages in the memory range is calculated by MappedByteBuffer.isLoaded() and handed down to Java_java_nio_MappedByteBuffer_isLoaded0() together with the memory range to test. MappedByteBuffer.isLoaded() calculates this number of pages based on jjdk.internal.misc.Unsafe.pagesize(), which ultimately comes down to os::vm_page_size(). For AIX, os::vm_page_size() may return a page size larger than the system page size of 4K. The reason for this is that on AIX, memory can be backed by different page sizes, usually either 4K or 64K - e.g. posix thread stacks may have 4K pages, java heap (system V shared memory) with 64K pages, but mmap memory is always 4K page backed... But as the OpenJDK code base generally assumes one homogeneous page size for everything - which is usually synonymous with os::vm_page_size() - a decision had to be made which page size to assume as a global system page size, and this may be a larger page size than the 4K system page size mincore(2) assumes. This usually is no problem, but with mincore(2) it is: as the size of the output buffer depends on the number of pages, calculating with a too-large page size causes the output buffer to be too small and hence the buffer overflows. The solution must be to base the size of the mincore output buffer on the system page size. -- Fix: I now re-calculate, just on AIX, the number of pages in the memory range based on sysconf(_SC_PAGESIZE) right before the call to mincore(2). Note that this would be a good and valid solution for all platforms, but I kept the change AIX only for now. I also added - and this is the shared part - an assert on all platforms which gets triggered on a buffer overflow. Thank you, Kind Regards, Thomas

4 14

Random.nextLong(long bound)?
by Steven Schlansker 19 Oct '17

19 Oct '17

My coworker is implementing an algorithm for which he needs a bounded random long. He needs to be in full control of the seed and not share the instance (i.e. not use ThreadLocalRandom). Random helpfully provides a nextInt(int bound) but nextLong has no such overload. The code to do so is not super involved, but you only have to briefly peruse the wasteland of StackOverflow answers to see what happens when you get developers to solve this problem themselves: https://stackoverflow.com/questions/2546078/java-random-long-number-in-0-x-… Seems like a nextLong(bound) would be a trivial addition with outsize benefits. Any reason not to add it? PS -- nextLong says "Returns the next pseudorandom, uniformly distributed {@code long} value from this random number generator's sequence." and "Because class {@code Random} uses a seed with only 48 bits, this algorithm will not return all possible {@code long} values." How can this be true? Given a long L1 which can be generated and L2 which cannot, it seems trivial that P(L1) != P(L2) and therefore it is not a uniformly distributed long value. Am I misunderstanding this concept? Thanks! Steven

5 5

Review Request JDK-8187089: StringConcatFactory.makeConcat & makeConcatWithConstants should throw StringConcatException if parameter slot count is over 200
by mandy chung 19 Oct '17

19 Oct '17

StringConcatFactory::makeConcat and makeConcatWithConstants APIs are specified to accept concatType to have a maximum of 200 parameter slots. If violated, it should throw StringConcatException. The implementation mistakenly checks the parameter count. The javadoc of these methods states "parameter count" rather than "parameter slots" whereas @apiNote in the class spec correctly says "parameter slots". Webrev: http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8187089/webrev.00/ CSR: https://bugs.openjdk.java.net/browse/JDK-8189634 Thanks Mandy

3 4

AssertionError in WildcardTypeImpl.getUpperBoundASTs
by Dawid Weiss 19 Oct '17

19 Oct '17

Hi all, We are observing very infrequent assertion errors originating in getUpperBoundASTs, mostly during concurrent builds on our build machine, when it's under a heavy load. This has been happening from time to time with various Java versions; most recently with 1.8.0_131. I don't see anything in Jira about it, but even a quick Google search yields some hits [1]. The stack trace starts with (I include the jackson stack frame which calls it): java.lang.AssertionError: null at sun.reflect.generics.reflectiveObjects.WildcardTypeImpl.getUpperBoundASTs(WildcardTypeImpl.java:86) ~[?:1.8.0_131] at sun.reflect.generics.reflectiveObjects.WildcardTypeImpl.getUpperBounds(WildcardTypeImpl.java:122) ~[?:1.8.0_131] at com.fasterxml.jackson.databind.type.TypeFactory._fromWildcard(TypeFactory.java:1424) ~[jackson-databind-2.8.2.jar:2.8.2] The problem cannot be easily reproduced, but I see a comment mentioned on the core-libs-dev a while ago [2] by Joel Borggrén-Franck, so perhaps his question is a valid one and worth investigating: > Btw, has anyone seen the assert for upper/lower bounds == null fail in the wild? Dawid [1] https://www.google.com/search?q=getUpperBoundASTs+AssertionError&filter=0&b… (note the "[JENKINS] Lucene-olr-master-Linux" hit). [2] http://markmail.org/message/4phfnh7dj3qdlo6b

2 4

RFR (JDK10/JAXP) 8181150: Fix lint warnings in JAXP repo: rawtypes and unchecked
by Joe Wang 18 Oct '17

18 Oct '17

Hi, Please review a cleanup of rawtypes and unchecked warnings from JAXP sources. The patch involved a good number of classes (278). However, the majority of the changes are straight-forward replacement, e.g. s/Vector/List[ArrayList] and their relevant methods e.g. s/elementAt/get and etc. In a few cases, ArrayIndexOutOfBoundsException was substituted with IndexOutOfBoundsException as they were caught. A few cleanups unrelated to rawtypes and unchecked were made before Roger's suggestion to stay within the scope of the request, that is, a couple of StringBuffer substitution, other than that this changeset is almost rawtypes and unchecked cleanup only. JBS: https://bugs.openjdk.java.net/browse/JDK-8181150 webrevs: http://cr.openjdk.java.net/~joehw/jdk10/8181150/webrev/ Thanks, Joe

3 4

Re: [PATCH] Unnecessary Amount Of Internal Class Conversion
by Ben Walsh 18 Oct '17

18 Oct '17

Thank you for the suggestion Alan. I'll continue further discussion on that mailing list. Regards, Ben Ben Walsh From: Alan Bateman <Alan.Bateman(a)oracle.com> To: Ben Walsh <ben_walsh(a)uk.ibm.com>, core-libs-dev(a)openjdk.java.net Date: 18/10/2017 12:00 Subject: Re: [PATCH] Unnecessary Amount Of Internal Class Conversion On 18/10/2017 11:35, Ben Walsh wrote: > I have observed a problem where an unnecessary amount of internal class > conversion is occurring. > > I have a patch which I would like to contribute which represents a > performance optimisation for this problem in the java.instrument module > implementation. > > It avoids having to convert all classes from a JVM's internal class format > to the .class file format, in order to call the ClassFileLoadHook, when no > java transformer installed. > > I would like to pair with a sponsor to contribute this patch. > The java.lang.instrument API is maintained by serviceability-dev so that would be the best place to bring that specific issue. -Alan Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

1 0

[PATCH] Unnecessary Amount Of Internal Class Conversion
by Ben Walsh 18 Oct '17

18 Oct '17

I have observed a problem where an unnecessary amount of internal class conversion is occurring. I have a patch which I would like to contribute which represents a performance optimisation for this problem in the java.instrument module implementation. It avoids having to convert all classes from a JVM's internal class format to the .class file format, in order to call the ClassFileLoadHook, when no java transformer installed. I would like to pair with a sponsor to contribute this patch. Regards, Ben Walsh Hursley Labs IBM United Kingdom Limited Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

2 1

RFR 8187772 : JVM crash when currency set on MacOS 10.10 and earlier
by Brent Christian 16 Oct '17

16 Oct '17

Hi, Please review my change to prevent a startup crash on earlier versions of MacOS. Bug: https://bugs.openjdk.java.net/browse/JDK-8187772 Webrev: http://cr.openjdk.java.net/~bchristi/8187772/webrev.00/ When a non-default currency is set in the Language & Region control panel, it's reflected as a variant in the locale identifier string. For example, "en_US@currency=RUB" for U.S. English using the Russian rubles currency. convertToPOSIXLocale() is not expecting such variants, nor such a long string. The result is a SEGV from memmove() on line 161. (Additional details are in the bug report.) The fix truncates the string passed to convertToPOSIXLocale() before any '@'-denoted variant tags. FWIW, the crash only happens on MacOS 10.10 and earlier because later versions of MacOS always return a region along with the language (line 65), so we don't need to query the locale identifier (line 84) to determine the region (hyphenPos is never NULL on line 82). Thanks, -Brent

3 4

RFR 8189262: jdk.jlink module-info.java javadoc comment refers to the non-existent jmage tool doc
by Sundararajan Athijegannathan 16 Oct '17

16 Oct '17

Please review: Webrev: http://cr.openjdk.java.net/~sundar/8189262/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189262 Thanks, -Sundar

3 3

Review Request JDK-8189202: (jdeps) Need jdeps output format easy for jlink --add-modules to use
by mandy chung 16 Oct '17

16 Oct '17

http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8189202/webrev.00/ This proposes to add a new jdeps --print-module-deps option to print a comma-separated list of modules that the analyzed classes depend on and such output can be taken by jlink --add-modules option. This will make it easy for users to run jlink to create a custom image. As Paul and Mikael suggest one use is doing something like: $ jlink --module-path jmods --add-modules $(jdeps --print-module-deps myapp.jar) --output image jdeps emits warnings if myapp.jar has split packages with JDK modules and you can use -q to suppress the warnings. jdeps --list-deps and --list-reduced-deps provide similiar information but it also lists dependences to JDK internal APIs. It is intended to print the dependences to make it easy to convert to @modules in jtreg tests and also can be used to set up --add-exports option if needed. This patch also changes these options not to show "not found" and "unnamed module" which are not specific for module dependencies. thanks Mandy

2 1