Re: [teststabilization] RFR: 8141685: com/sun/jndi/ldap/InvalidLdapFilters.java initializes context failed

2019-12-06 Thread Vyom Tiwari
Hi Aleks, Changes looks OK to me, one minor bit you need to update @bug tag before pushing. Thanks, Vyom On Fri, Dec 6, 2019 at 11:41 PM Daniel Fuchs wrote: > Looks good to me Aleksei! > > best regards, > > -- daniel > > On 06/12/2019 17:33, Aleks Efimov wrote: > > Hi, > > > >

Re: RFR: JDK-8235453: tools/jpackage/junit/junit.java failed due to "module not found: jdk.incubator.jpackage"

2019-12-06 Thread Phil Race
Well we could deprecate and remove the solaris port :-) But until that is done this is the only way I know of. we could require all jpackage tests to include some helper code which decides if it is applicable but that will be more work upfront and many jpackage tests are already platform

Re: RFR: 8235482: [TESTBUG] PackageType LINUX_RPM should be excluded for jpackage tests on debian systems

2019-12-06 Thread Jie Fu
Hi Alexey, Thanks for your review and comments. I have two questions:  - 1) Have you ever tried the rpm tests on a debian system?  - 2) How to decide whether the rpm tests are supported on a debian system? Thanks a lot. Best regards, Jie On 2019/12/7 上午1:26, Alexey Semenyuk wrote: This fix

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Maurizio Cimadamore
On 06/12/2019 18:29, Raffaello Giulietti wrote: Hello, great job! I think that the doc of MemoryAddress.copy() should be explicit about the direction of the copying, so it should either: Thanks! -  I'll rectify the doc to specify lower-to-higher. Maurizio * explicitly specify a

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Maurizio Cimadamore
Thanks Paul, I'll do a pass and fix the issues you found. Some comments inline: On 06/12/2019 21:04, Paul Sandoz wrote: I mostly looked at the API and implementation and not the tests. s/offset/add or plus ? add ‘l’ to the offset of this memory address the result of which is the offset of

Re: [EXTERNAL] JDK-8234076 bug fix candidate

2019-12-06 Thread Henry Jen
Thanks for the work, the test case covers that when —module= is used before other options, which is good. But we need another to make sure that when —module= is put in an argument file or environment variable, that should not be allowed. The fix is simple, we need to add that to isTerminalOp()

Re: RFR: JDK-8235453: tools/jpackage/junit/junit.java failed due to "module not found: jdk.incubator.jpackage"

2019-12-06 Thread Alexander Matveev
Looks good, but is there better way to exclude tests on Solaris? I do not like idea adding @requires for all tests. Thanks, Alexander On 12/6/2019 10:35 AM, Alexey Semenyuk wrote: Looks good. - Alexey On 12/6/2019 1:33 PM, Andy Herrick wrote: Please review this jpackager test fix for bug

Re: JDK 14 RFR of JDK-8235514: Update record serialization tests to not use hard coded source versions

2019-12-06 Thread Lance Andersen
+1 > On Dec 6, 2019, at 4:15 PM, Joe Darcy wrote: > > Hello, > > Please review the patch below to update to the records serialization tests to > avoid hard-coded values for the -source argument during programmatic > invocations of the compiler. > > Thanks, > > -Joe > > diff -r

JDK 14 RFR of JDK-8235514: Update record serialization tests to not use hard coded source versions

2019-12-06 Thread Joe Darcy
Hello, Please review the patch below to update to the records serialization tests to avoid hard-coded values for the -source argument during programmatic invocations of the compiler. Thanks, -Joe diff -r 3b9efbac1b50 test/jdk/java/io/Serializable/records/BadCanonicalCtrTest.java ---

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Paul Sandoz
I mostly looked at the API and implementation and not the tests. s/offset/add or plus ? add ‘l’ to the offset of this memory address the result of which is the offset of the returned memory address. If we ever have controlled operator overloading that’s how I would like to express it :-)

Re: RFR: JDK-8233270: Add support to jtreg helpers to unpack packages

2019-12-06 Thread Phil Race
On 12/6/19 12:51 PM, Alexey Semenyuk wrote: On 12/5/2019 8:44 PM, Alexander Matveev wrote: Hi Alexey, 1) Remove this file: test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JavaTool.java.rej Done. webrev updated in place. 2) Agree with Phil, they probably should be pushed as two

Re: RFR: JDK-8233270: Add support to jtreg helpers to unpack packages

2019-12-06 Thread Alexey Semenyuk
On 12/5/2019 8:44 PM, Alexander Matveev wrote: Hi Alexey, 1) Remove this file: test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JavaTool.java.rej Done. webrev updated in place. 2) Agree with Phil, they probably should be pushed as two separate bugs. Decoupling is not straightforward

RE: [EXTERNAL] JDK-8234076 bug fix candidate

2019-12-06 Thread Nikola Grcevski
Thank you Henry! I have modified the fix in checkArg to use JLI_StrCCmp as suggested below and I have extended the BasicTest.java (in test/jdk/tools/launcher/modules/basic) with few additional tests. I don't have author status yet, therefore I'm attaching the patch right after my signature. I

Re: RFR: JDK-8233270: Add support to jtreg helpers to unpack packages

2019-12-06 Thread Andy Herrick
The changes look good to me. /Andy On 12/6/2019 12:55 PM, Alexey Semenyuk wrote: Initially the work was focused on [2] issue to fix icons assignment for launchers created by jpackage. In the process of working on the fix for [2] it turned out jpackage test framework was lacking functionality

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Raffaello Giulietti
Hi, MemoryLayouts exposes BITS_8_BE and BITS_8_LE. Is there a reason to have both or is just love for symmetries (which I share)? Greetings Raffaello > Hi, > as part of the effort to upstream the changes related to JEP 370 (foreign memory access API) [1], I'd like to ask for a code

Re: RFR: JDK-8235453: tools/jpackage/junit/junit.java failed due to "module not found: jdk.incubator.jpackage"

2019-12-06 Thread Alexey Semenyuk
Looks good. - Alexey On 12/6/2019 1:33 PM, Andy Herrick wrote: Please review this jpackager test fix for bug [1] at [2]. the fix adds "@requires (os.family == "linux") | (os.family == "mac") | (os.family == "windows")" to all jpackage tests that were not already filtered with "@requires

RFR: JDK-8235453: tools/jpackage/junit/junit.java failed due to "module not found: jdk.incubator.jpackage"

2019-12-06 Thread Andy Herrick
Please review this jpackager test fix for bug [1] at [2]. the fix adds "@requires (os.family == "linux") | (os.family == "mac") | (os.family == "windows")" to all jpackage tests that were not already filtered with "@requires (os.family == "XXX")" [1]

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Raffaello Giulietti
Hello, great job! I think that the doc of MemoryAddress.copy() should be explicit about the direction of the copying, so it should either: * explicitly specify a direction, e.g., lower-to-higher addresses * or specify that in the case of an overlap the copying is smart enough to not

Re: [teststabilization] RFR: 8141685: com/sun/jndi/ldap/InvalidLdapFilters.java initializes context failed

2019-12-06 Thread Daniel Fuchs
Looks good to me Aleksei! best regards, -- daniel On 06/12/2019 17:33, Aleks Efimov wrote: Hi, InvalidLdapFilters test has been seen failing intermittently with timeouts. The cause of the observed failures could be the binding of test server to the wildcard address. Proposed fix binds the

Re: RFR: JDK-8233270: Add support to jtreg helpers to unpack packages

2019-12-06 Thread Alexey Semenyuk
Initially the work was focused on [2] issue to fix icons assignment for launchers created by jpackage. In the process of working on the fix for [2] it turned out jpackage test framework was lacking functionality to cover the fix in an environment where installation of packages produced by

[teststabilization] RFR: 8141685: com/sun/jndi/ldap/InvalidLdapFilters.java initializes context failed

2019-12-06 Thread Aleks Efimov
Hi, InvalidLdapFilters test has been seen failing intermittently with timeouts. The cause of the observed failures could be the binding of test server to the wildcard address. Proposed fix binds the test server to the loopback address and uses the test library to construct LDAP provider URL.

Re: RFR: 8235482: [TESTBUG] PackageType LINUX_RPM should be excluded for jpackage tests on debian systems

2019-12-06 Thread Alexey Semenyuk
This fix will disable running rpm tests on Debian systems that support it. This is not a good idea. If you need to exclude some packaging from jpackage tests, just run jtreg with "jpackage.test.disabledPackagers" system property set to the list of corresponding package types. E.g.:

Re: RFR: 8235482: [TESTBUG] PackageType LINUX_RPM should be excluded for jpackage tests on debian systems

2019-12-06 Thread Andy Herrick
looks good to me. /Andy On 12/6/2019 7:51 AM, Jie Fu wrote: Hi all, May I get reviews for the small fix? JBS:    https://bugs.openjdk.java.net/browse/JDK-8235482 Webrev: http://cr.openjdk.java.net/~jiefu/8235482/webrev.00/ Thanks a lot. Best regards, Jie

Re: RFR: JEP 367: Remove the Pack200 Tools and API

2019-12-06 Thread Erik Joelsson
Sounds good and looks good. /Erik On 2019-12-05 20:18, Henry Jen wrote: OK, so I created an issue[1] for follow up for Windows build and reverted the change in flags-cflags.m4, if nothing else, I’ll push without another webrev pinging that I get an +1 from someone in build-de, Erik? [1]

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-06 Thread Daniel D. Daugherty
On 12/6/19 3:04 AM, Langer, Christoph wrote: Thanks, David. I'll run the final change once again through jdk-submit befor pushing. Alan, Dan, may I consider this reviewed by either of you? Sorry I haven't done a full review on this change. I just happened to see the typo fly by... Dan

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Maurizio Cimadamore
On 06/12/2019 12:28, Vladimir Ivanov wrote: * ciField.cpp - this one is to trust final fields in the foreign memory access implementation (otherwise VM doesn't trust memory segment bounds) New packages aren't part of java.base. Considering the implementation resides in an incubator

RE: RFR(S): 8220348: [ntintel] asserts about copying unalinged array

2019-12-06 Thread Langer, Christoph
Hi Martin, ok, you are the author – so I won’t insist.  Best regards Christoph From: Doerr, Martin Sent: Freitag, 6. Dezember 2019 12:22 To: Langer, Christoph Cc: core-libs-dev@openjdk.java.net; security-dev ; Lindenmaier, Goetz ; Thomas Stüfe Subject: RE: RFR(S): 8220348: [ntintel]

RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-06 Thread Langer, Christoph
Thanks, Alan. Then I'll push it now. A final jdk-submit run succeeded (mach5-one-clanger-JDK-8234185-4-20191206-0819-7283662). > -Original Message- > From: Alan Bateman > Sent: Freitag, 6. Dezember 2019 13:17 > To: Langer, Christoph ; David Holmes > ; daniel.daughe...@

RFR: 8235482: [TESTBUG] PackageType LINUX_RPM should be excluded for jpackage tests on debian systems

2019-12-06 Thread Jie Fu
Hi all, May I get reviews for the small fix? JBS:    https://bugs.openjdk.java.net/browse/JDK-8235482 Webrev: http://cr.openjdk.java.net/~jiefu/8235482/webrev.00/ Thanks a lot. Best regards, Jie

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Vladimir Ivanov
* ciField.cpp - this one is to trust final fields in the foreign memory access implementation (otherwise VM doesn't trust memory segment bounds) New packages aren't part of java.base. Considering the implementation resides in an incubator module, is it enough to consider them as trusted

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-06 Thread Alan Bateman
On 06/12/2019 08:04, Langer, Christoph wrote: Thanks, David. I'll run the final change once again through jdk-submit befor pushing. Alan, Dan, may I consider this reviewed by either of you? Yes, I think this looks okay. -Alan

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Maurizio Cimadamore
On 06/12/2019 11:53, Jorn Vernee wrote: > * drop offset() - but then add an overload of MemorySegment::asSlice which takes an address instead of a plain long offset This sounds good to me, since it fits with what we're doing with makeUncheckedSegment(MemoryAddress, length), and we added the

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Jorn Vernee
> * drop offset() - but then add an overload of MemorySegment::asSlice which takes an address instead of a plain long offset This sounds good to me, since it fits with what we're doing with makeUncheckedSegment(MemoryAddress, length), and we added the offset() accessor to support slicing. I

Re: RFR JDK-8235351: Lookup::unreflect should bind with the original caller independent of Method's accessible flag

2019-12-06 Thread Alan Bateman
On 06/12/2019 03:17, Mandy Chung wrote: When unreflecting on a Method for caller-sensitive method, the produced MethodHandle should bind with the original caller lookup independent of the accessible flag. Webrev:   http://cr.openjdk.java.net/~mchung/jdk14/8235351/webrev.00/ This looks good to

RE: RFR(S): 8220348: [ntintel] asserts about copying unalinged array

2019-12-06 Thread Doerr, Martin
Hi Christoph, that would work, but I don’t want to pollute this file with compiler specific defines. In addition, I don’t like introducing a macro which works on some platforms and does nothing on other ones (which is the case for hotspot’s ATTRIBUTE_ALIGNED). Because Windows 32 bit is the

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Maurizio Cimadamore
Hi, here's an updated version of the patch: http://cr.openjdk.java.net/~mcimadamore/panama/8234049_v2/ And a delta of the changes since last version here: http://cr.openjdk.java.net/~mcimadamore/panama/8234049_v2_delta/ The javadoc has been updated inline here:

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Maurizio Cimadamore
On 05/12/2019 23:27, Maurizio Cimadamore wrote: This gives same performance as with -XX:+TrustFinalNonStaticFields - if we remove these changes, then memory segment bounds are never inlined. I'm happy to change this if you have other suggestions on how to get there, of course (I can run some

Re: RFR JDK-8234049: Implementation of Memory Access API (Incubator)

2019-12-06 Thread Maurizio Cimadamore
Couple of naming ideas: MemorySegment.isAccessible() is a very overloaded term, isOwnByCurrentThread() is maybe better ? One solution here would be to, instead, have an accessor called ownerThread() - then clients can test doing ownerThread() != Thread.currentThread (or some other thread).

RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-06 Thread Langer, Christoph
Thanks, David. I'll run the final change once again through jdk-submit befor pushing. Alan, Dan, may I consider this reviewed by either of you? Thanks Christoph > -Original Message- > From: David Holmes > Sent: Freitag, 6. Dezember 2019 08:02 > To: Langer, Christoph ; >