Re: RFR [14/java.xml] 8230094: CCE in createXMLEventWriter(Result) over an arbitrary XMLStreamWriter

[Joe Wang]

On 8/28/19 8:12 AM, Lance Andersen wrote:

Hi Joe,

The change and test look OK


Thanks Lance!


On Aug 27, 2019, at 1:20 PM, Joe Wang > wrote:


Please review a patch for an issue where a custom impl of 
XMLStreamWriter caused a CCE. The problem was that our implementation 
was changed to an internally extended interface. We could consider 
making it external, but for now, we'll do with a check.


Note that there's no material change in XMLOutputFactoryImpl.java 
except a minor one at line 160.


took a moment to spot the change here :-)


It's hidden in there ;-)

Best,
Joe



Best
Lance


JBS: https://bugs.openjdk.java.net/browse/JDK-8230094
webrev: http://cr.openjdk.java.net/~joehw/jdk14/8230094/webrev/

Thanks,
Joe




Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: JDK 14 RFR of JDK-8230074: Improve specification for {Math, StrictMath}.negateExact

[Brian Burkhalter]

Hi Joe,

Looks fine.

Brian

> On Aug 28, 2019, at 9:38 AM, Joe Darcy  wrote:
> 
> Please review the small amendment to make the specifications for some of the 
> "exact" methods in Math and StrictMath more explicit in their exceptional 
> behavior:
> 
> JDK-8230074: Improve specification for {Math, StrictMath}.negateExact
> http://cr.openjdk.java.net/~darcy/8230074.0/ 
> 
> 
> Patch below.

Re: 8229333: java/io/File/SetLastModified.java timed out

[Lance Andersen]

Looks fine Brian, fingers crossed the little bugger does not come back :-)

> On Aug 27, 2019, at 2:00 PM, Brian Burkhalter  
> wrote:
> 
> The failure reported in [1] was observed to occur on two Mac minis with HDDs. 
> Given the large file access it is possibly due to slow disk access. The 
> suggested fix is to increase the timeout [2]. If it is still observed at a 
> later date a new issue can be filed.
> 
> Thanks,
> 
> Brian
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8229333
> [2] diff:
> 
> @@ -22,11 +22,13 @@
>  */
> 
> /* @test
> -   @bug 4091757 6652379 8177809
> -   @summary Basic test for setLastModified method
> + * @bug 4091757 6652379 8177809
> + * @summary Basic test for setLastModified method
> + * @run main/timeout=180 SetLastModified
>  */
> 
> -import java.io.*;
> +import java.io.File;
> +import java.io.FileOutputStream;
> import java.nio.ByteBuffer;
> import java.nio.channels.FileChannel;

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR: 8230302: GenerateJLIClassesPlugin can generate invalid DirectMethodHandle methods

[Claes Redestad]

Hi Mandy,

On 2019-08-28 19:11, Mandy Chung wrote:



On 8/28/19 7:38 AM, Claes Redestad wrote:

Hi,

we currently spin code for a non-sensical invokeVirtual DMH, so let's
remove that.

Such methods are benign since we won't ever attempt to resolve such
nonsensical forms, but surely a waste of space. This patch tightens
checks conservatively (could possibly apply the same restriction to
invokeSpecial?) and adds a few negative tests using inputs that jlink
should refuse.



I also think same check (first parameter is the receiver) can be applied 
to invokespecial for DMH.


While tightening this up would be good, I also want to make absolutely
certain there aren't any edge cases that we're missing where omitting
the receiver would actually be fine. I need to study the specification a
bit more (and add more tests), so for now I think I'd like to leave
things as is.

Again, while it might be possible to make the plugin generate a LF that
doesn't make sense, it's AFAICT not possible to construct a DMH at
runtime that would pick that up, so making things stricter is mainly
about good hygiene.




Webrev: http://cr.openjdk.java.net/~redestad/8230302/webrev.00/



This looks okay.   


Thanks!

It'd be good to move away from hardcoded defaults as 
tracked by JDK-8230301.


Yes, I have that lined up, running tests etc. I just need to ensure
there are no regressions, but since we now have build-time profile
guidance on what to pregenerate it seems most of the things in the
defaults are now either redundant or dead weight.

/Claes

Re: RFR: 8230302: GenerateJLIClassesPlugin can generate invalid DirectMethodHandle methods

[Mandy Chung]

On 8/28/19 7:38 AM, Claes Redestad wrote:

Hi,

we currently spin code for a non-sensical invokeVirtual DMH, so let's
remove that.

Such methods are benign since we won't ever attempt to resolve such
nonsensical forms, but surely a waste of space. This patch tightens
checks conservatively (could possibly apply the same restriction to
invokeSpecial?) and adds a few negative tests using inputs that jlink
should refuse.



I also think same check (first parameter is the receiver) can be applied 
to invokespecial for DMH.



Webrev: http://cr.openjdk.java.net/~redestad/8230302/webrev.00/



This looks okay.    It'd be good to move away from hardcoded defaults as 
tracked by JDK-8230301.


Mandy

Re: RFR: JDK-8229979: jpackage cleanup src files, help text, and javadoc

[Alexey Semenyuk]

Understood. Thank you for explanation.

- Alexey

On 8/28/2019 12:43 PM, Andy Herrick wrote:


On 8/28/2019 11:47 AM, Alexey Semenyuk wrote:

Looks good.

Some files in the review contain no diffs though, like 
http://cr.openjdk.java.net/~herrick/8229979/webrev.01/test/jdk/tools/jpackage/windows/exe/WinUpgradeUUIDTest.java.sdiff.html. 
Why are they in the review?


I ran a script to remove extraneous trailing spaces and fix file 
permission.  The result is that the file changed, but webrev has 
nothing to show.  I built the webrev with a file list so all changed 
files are these, even if there is no visible change.


/ANdy



- Alexey

On 8/28/2019 9:36 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


[1] https://bugs.openjdk.java.net/browse/JDK-8229979

[2] http://cr.openjdk.java.net/~herrick/8229979/

Thanks,
Andy

Re: JDK 14 RFR of JDK-8229997: Apply java.io.Serial annotations in java.base

[Roger Riggs]

Hi Joe,

Looks fine,

Roger


On 8/27/19 9:28 PM, Joe Darcy wrote:

Hello,

Recent work for JDK-8202385: "Annotation to mark serial-related fields 
and methods" added the java.io.Serial annotation type to the platform. 
The intention of this new annotation type is to allow 
serialization-related fields and methods to be marked as documentation 
and to allow stricter compile-time checking, analogous to the checking 
done for @Override. Implementing those stricter serialization-related 
checks will be done under JDK-8202056.


Please review the application of java.io.Serial to types in the base 
module other than security types:


    JDK-8229997: Apply java.io.Serial annotations in java.base
    http://cr.openjdk.java.net/~darcy/8229997.0/

(Security-related types in the base module are being handled under 
JDK-822.)


As a reminder, the 5 serialization-related methods and 2 fields are:

    * private void writeObject(java.io.ObjectOutputStream stream) 
throws IOException
    * private void readObject(java.io.ObjectInputStream stream) throws 
IOException, ClassNotFoundException

    * private void readObjectNoData() throws ObjectStreamException
    * ANY-ACCESS-MODIFIER Object writeReplace() throws 
ObjectStreamException
    * ANY-ACCESS-MODIFIER Object readResolve() throws 
ObjectStreamException

    * private static final ObjectStreamField[] serialPersistentFields
    * private static final long serialVersionUID

Due to the separate source code management policies for 
java.util.concurrent, I did not include that package in this exercise. 
Likewise, applying the annotation to some of the time-zone related 
files caused build failures so I excluded those types as well.



Thanks,

-Joe

Re: RFR: JDK-8229979: jpackage cleanup src files, help text, and javadoc

[Andy Herrick]

On 8/28/2019 11:47 AM, Alexey Semenyuk wrote:

Looks good.

Some files in the review contain no diffs though, like 
http://cr.openjdk.java.net/~herrick/8229979/webrev.01/test/jdk/tools/jpackage/windows/exe/WinUpgradeUUIDTest.java.sdiff.html. 
Why are they in the review?


I ran a script to remove extraneous trailing spaces and fix file 
permission.  The result is that the file changed, but webrev has nothing 
to show.  I built the webrev with a file list so all changed files are 
these, even if there is no visible change.


/ANdy



- Alexey

On 8/28/2019 9:36 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


[1] https://bugs.openjdk.java.net/browse/JDK-8229979

[2] http://cr.openjdk.java.net/~herrick/8229979/

Thanks,
Andy

JDK 14 RFR of JDK-8230074: Improve specification for {Math, StrictMath}.negateExact

[Joe Darcy]

Hello,

Please review the small amendment to make the specifications for some of 
the "exact" methods in Math and StrictMath more explicit in their 
exceptional behavior:


    JDK-8230074: Improve specification for {Math, StrictMath}.negateExact
    http://cr.openjdk.java.net/~darcy/8230074.0/

Patch below.

Thanks,

-Joe

--- old/src/java.base/share/classes/java/lang/Math.java 2019-08-28 
09:31:35.64099 -0700
+++ new/src/java.base/share/classes/java/lang/Math.java 2019-08-28 
09:31:35.50099 -0700

@@ -952,6 +952,7 @@
 /**
  * Returns the argument incremented by one, throwing an exception 
if the

  * result overflows an {@code int}.
+ * The overflow only occurs for {@linkplain Integer#MAX_VALUE the 
maximum value}.

  *
  * @param a the value to increment
  * @return the result
@@ -970,6 +971,7 @@
 /**
  * Returns the argument incremented by one, throwing an exception 
if the

  * result overflows a {@code long}.
+ * The overflow only occurs for {@linkplain Long#MAX_VALUE the 
maximum value}.

  *
  * @param a the value to increment
  * @return the result
@@ -988,6 +990,7 @@
 /**
  * Returns the argument decremented by one, throwing an exception 
if the

  * result overflows an {@code int}.
+ * The overflow only occurs for {@linkplain Integer#MIN_VALUE the 
minimum value}.

  *
  * @param a the value to decrement
  * @return the result
@@ -1006,6 +1009,7 @@
 /**
  * Returns the argument decremented by one, throwing an exception 
if the

  * result overflows a {@code long}.
+ * The overflow only occurs for {@linkplain Long#MIN_VALUE the 
minimum value}.

  *
  * @param a the value to decrement
  * @return the result
@@ -1024,6 +1028,7 @@
 /**
  * Returns the negation of the argument, throwing an exception if the
  * result overflows an {@code int}.
+ * The overflow only occurs for {@linkplain Integer#MIN_VALUE the 
minimum value}.

  *
  * @param a the value to negate
  * @return the result
@@ -1042,6 +1047,7 @@
 /**
  * Returns the negation of the argument, throwing an exception if the
  * result overflows a {@code long}.
+ * The overflow only occurs for {@linkplain Long#MIN_VALUE the 
minimum value}.

  *
  * @param a the value to negate
  * @return the result
--- old/src/java.base/share/classes/java/lang/StrictMath.java 2019-08-28 
09:31:35.99699 -0700
+++ new/src/java.base/share/classes/java/lang/StrictMath.java 2019-08-28 
09:31:35.85699 -0700

@@ -837,6 +837,7 @@
 /**
  * Returns the argument incremented by one,
  * throwing an exception if the result overflows an {@code int}.
+ * The overflow only occurs for {@linkplain Integer#MAX_VALUE the 
maximum value}.

  *
  * @param a the value to increment
  * @return the result
@@ -851,6 +852,7 @@
 /**
  * Returns the argument incremented by one,
  * throwing an exception if the result overflows a {@code long}.
+ * The overflow only occurs for {@linkplain Long#MAX_VALUE the 
maximum value}.

  *
  * @param a the value to increment
  * @return the result
@@ -865,6 +867,7 @@
 /**
  * Returns the argument decremented by one,
  * throwing an exception if the result overflows an {@code int}.
+ * The overflow only occurs for {@linkplain Integer#MIN_VALUE the 
minimum value}.

  *
  * @param a the value to decrement
  * @return the result
@@ -879,6 +882,7 @@
 /**
  * Returns the argument decremented by one,
  * throwing an exception if the result overflows a {@code long}.
+ * The overflow only occurs for {@linkplain Long#MIN_VALUE the 
minimum value}.

  *
  * @param a the value to decrement
  * @return the result
@@ -893,6 +897,7 @@
 /**
  * Returns the negation of the argument,
  * throwing an exception if the result overflows an {@code int}.
+ * The overflow only occurs for {@linkplain Integer#MIN_VALUE the 
minimum value}.

  *
  * @param a the value to negate
  * @return the result
@@ -907,6 +912,7 @@
 /**
  * Returns the negation of the argument,
  * throwing an exception if the result overflows a {@code long}.
+ * The overflow only occurs for {@linkplain Long#MIN_VALUE the 
minimum value}.

  *
  * @param a the value to negate
  * @return the result

Re: JDK 14 RFR of JDK-8229997: Apply java.io.Serial annotations in java.base

[Alan Bateman]

On 28/08/2019 17:08, Joe Darcy wrote:
The intention here is to be "mostly exhaustive" rather than "entirely 
exhaustive" in applying the annotation type so I'd prefer to defer 
updating generated exceptions to future work.

Okay, although all you need is:

$ hg diff make
diff -r e17f768b3b71 make/scripts/genExceptions.sh
--- a/make/scripts/genExceptions.sh    Wed Aug 28 10:56:50 2019 -0400
+++ b/make/scripts/genExceptions.sh    Wed Aug 28 17:22:10 2019 +0100
@@ -60,6 +60,7 @@
 extends ${SUPER}
 {

+    @java.io.Serial
 private static final long serialVersionUID = $SVUID;
 __END__

Re: JDK 14 RFR of JDK-8229997: Apply java.io.Serial annotations in java.base

[Martin Buchholz]

Should Serial.java have an @since 14 ?

Re: JDK 14 RFR of JDK-8229997: Apply java.io.Serial annotations in java.base

[Joe Darcy]

Hi Alan,

On 8/27/2019 11:39 PM, Alan Bateman wrote:

On 28/08/2019 02:28, Joe Darcy wrote:

Hello,

Recent work for JDK-8202385: "Annotation to mark serial-related 
fields and methods" added the java.io.Serial annotation type to the 
platform. The intention of this new annotation type is to allow 
serialization-related fields and methods to be marked as 
documentation and to allow stricter compile-time checking, analogous 
to the checking done for @Override. Implementing those stricter 
serialization-related checks will be done under JDK-8202056.


Please review the application of java.io.Serial to types in the base 
module other than security types:


    JDK-8229997: Apply java.io.Serial annotations in java.base
    http://cr.openjdk.java.net/~darcy/8229997.0/
There are exception classes in java.base that are generated at build 
time so I think you'll need to update make/scripts/genExceptions.sh to 
ensure that the serialVersionUID in those classes are annotated.


The intention here is to be "mostly exhaustive" rather than "entirely 
exhaustive" in applying the annotation type so I'd prefer to defer 
updating generated exceptions to future work.


From some off-list discussions with Stuart and Roger, the envisioned 
nature of the compile-time checking is that the full suite of new checks 
would only be enabled if the annotations are present. Some additional 
checking would occur without the annotation, such as an ineffectual 
serialVersionUID in an enum type.


Thanks,

-Joe

Re: RFR: JDK-8229979: jpackage cleanup src files, help text, and javadoc

[Alexey Semenyuk]

Looks good.

Some files in the review contain no diffs though, like 
http://cr.openjdk.java.net/~herrick/8229979/webrev.01/test/jdk/tools/jpackage/windows/exe/WinUpgradeUUIDTest.java.sdiff.html. 
Why are they in the review?


- Alexey

On 8/28/2019 9:36 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


[1] https://bugs.openjdk.java.net/browse/JDK-8229979

[2] http://cr.openjdk.java.net/~herrick/8229979/

Thanks,
Andy

Re: RFR [14/java.xml] 8230094: CCE in createXMLEventWriter(Result) over an arbitrary XMLStreamWriter

[Lance Andersen]

Hi Joe,

The change and test look OK

> On Aug 27, 2019, at 1:20 PM, Joe Wang  wrote:
> 
> Please review a patch for an issue where a custom impl of XMLStreamWriter 
> caused a CCE. The problem was that our implementation was changed to an 
> internally extended interface. We could consider making it external, but for 
> now, we'll do with a check.
> 
> Note that there's no material change in XMLOutputFactoryImpl.java except a 
> minor one at line 160.

took a moment to spot the change here :-)

Best
Lance
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8230094
> webrev: http://cr.openjdk.java.net/~joehw/jdk14/8230094/webrev/
> 
> Thanks,
> Joe

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR: JDK-8229979: jpackage cleanup src files, help text, and javadoc

[Kevin Rushforth]

Looks good.

-- Kevin

On 8/28/2019 6:36 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


[1] https://bugs.openjdk.java.net/browse/JDK-8229979

[2] http://cr.openjdk.java.net/~herrick/8229979/

Thanks,
Andy

RFR: 8230302: GenerateJLIClassesPlugin can generate invalid DirectMethodHandle methods

[Claes Redestad]

Hi,

we currently spin code for a non-sensical invokeVirtual DMH, so let's
remove that.

Such methods are benign since we won't ever attempt to resolve such
nonsensical forms, but surely a waste of space. This patch tightens
checks conservatively (could possibly apply the same restriction to
invokeSpecial?) and adds a few negative tests using inputs that jlink
should refuse.

Webrev: http://cr.openjdk.java.net/~redestad/8230302/webrev.00/

Testing: tier1+2

Thanks!

/Claes

RFR: JDK-8229979: jpackage cleanup src files, help text, and javadoc

[Andy Herrick]

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


[1] https://bugs.openjdk.java.net/browse/JDK-8229979

[2] http://cr.openjdk.java.net/~herrick/8229979/

Thanks,
Andy

Re: RFR: JDK-8224833: jpackages differences between platforms

[Andy Herrick]

This looks fine.

/Andy

On 8/27/2019 7:25 PM, Alexey Semenyuk wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


[1] https://bugs.openjdk.java.net/browse/JDK-8224833

[2] http://cr.openjdk.java.net/~asemenyuk/8224833/webrev.00/

Thanks,
Alexey

Re: RFC: DMH lambda form arguments by jlink

[Claes Redestad]

Filed:

 https://bugs.openjdk.java.net/browse/JDK-8230302 to deal with the bug
 that the plugin can and does generate some invalid DMHs

 https://bugs.openjdk.java.net/browse/JDK-8230301 to re-examine the
 ability to hardcode default sets of DMHs, invokers etc. This can and
 should be controlled by generating and providing a property file to
 jlink, which we currently do at build time.

/Claes

On 2019-08-25 21:45, Claes Redestad wrote:

(bcc jdk-dev, add core-libs-dev)

Hi Andrey,

I think you might be right that L_L of invoke virtual is non-sensical
and should be removed. I vaguely recall that it at one point have been
coded so that the receiver was implied for these forms when translating.

Also worth noting that the intent was to remove the hardcoded "default"
signatures as the technique to record the set of LFs generated by
typical applications matured. I think we might be at that point now, and
should evaluate if all these "defaults" in GenerateJLIClassesPlugin can
be removed.

Thanks!

/Claes

On 2019-08-23 19:28, Andrey Petushkov wrote:

Dear JDK developers,

(I'm apologising if posted to wrong alias, please forward as appropriate)

The question is about lambda form cache, created by jlink. More precisely
by jdk/tools/jlink/internal/plugins/GenerateJLIClassesPlugin.java.
It appears to me that it has a bug in set of signatures, specifically in
attempt to create LF for invocation of invokevirtual type with 
signature of

"L_L" [1]. According to documentation [2] and supported by the code the
arguments to lambda form must be the arguments of the target method
prepended by DMH itself. Hence the invokevirtual linkage should always be
supplied with at least two oop arguments coming first (DMH and receiver)

If I'm correct this bug is totally harmless, since such lambda form
although buggy (the native wrapper for respective MethodHandle intrinsic
have register clash between receiver (receiver_reg) and MemberName
(member_reg) at [3]) it will never be actually used, because the 
actual MH

invoke will not have such signature. But for the sake of consistency I'd
rather remove the signature in question.

The problem is actually found by jtreg tests on aarch32 platform [4]
(please don't confuse with arm port) where it happen to exist assert
checking for mentioned register clash [5]

Will be happy if someone could confirm if this is indeed correct
description of what's happening or point to mistake in analysis, as
appropriate

Thanks,
Andrey

[1]
http://hg.openjdk.java.net/jdk/jdk/file/00bf1e66de11/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/GenerateJLIClassesPlugin.java#l151 


[2] https://wiki.openjdk.java.net/display/HotSpot/Direct+method+handles
[3]
http://hg.openjdk.java.net/jdk/jdk/file/00bf1e66de11/src/hotspot/cpu/x86/methodHandles_x86.cpp#l293 


[4]
https://mail.openjdk.java.net/pipermail/aarch32-port-dev/2018-September/001093.html 


[5]
http://cr.openjdk.java.net/~apetushkov/aarch32jdk11%2b28/src/hotspot/cpu/aarch32/methodHandles_aarch32.cpp.html 


line
269

Re: JDK 14 RFR of JDK-8229997: Apply java.io.Serial annotations in java.base

[Alan Bateman]

On 28/08/2019 02:28, Joe Darcy wrote:

Hello,

Recent work for JDK-8202385: "Annotation to mark serial-related fields 
and methods" added the java.io.Serial annotation type to the platform. 
The intention of this new annotation type is to allow 
serialization-related fields and methods to be marked as documentation 
and to allow stricter compile-time checking, analogous to the checking 
done for @Override. Implementing those stricter serialization-related 
checks will be done under JDK-8202056.


Please review the application of java.io.Serial to types in the base 
module other than security types:


    JDK-8229997: Apply java.io.Serial annotations in java.base
    http://cr.openjdk.java.net/~darcy/8229997.0/
There are exception classes in java.base that are generated at build 
time so I think you'll need to update make/scripts/genExceptions.sh to 
ensure that the serialVersionUID in those classes are annotated.


-Alan