Re: RFR: 8280642: IllegalAccessError thrown by ObjectInputStream.resolveProxyClass is not handled

2022-01-28 Thread Mandy Chung
On Fri, 28 Jan 2022 21:02:23 GMT, Roger Riggs  wrote:

> During deserialization of a serialized data stream that contains a proxy 
> descriptor with non-public interfaces
> `java.io.ObjectInputStream` checks that the interfaces can be loaded from a 
> single classloader in `ObjectInputStream.resolveProxyClass`.
> If the interfaces cannot be loaded from a single classloader, an 
> `IllegalAccessError` is thrown.
> When `ObjectInputStream.readObject` encounters this case, it reflects an 
> incompatibility
> between the classloaders of the source of the serialized stream and the 
> classloader being used for deserialization.
> When a proxy object cannot be created from the interfaces, 
> `ObjectInputStream.readObject` should catch
> the `InvalidAccessError` and throw `InvalidObjectException` with the 
> `InvalidAccessError` as the cause.
> This allows the application to handle the exception consistently with other 
> errors during deserialization.

`readProxyDesc` throws a mix of `InvalidClassException` and 
`InvalidObjectException`.   I'm not close to the spec of this area and let 
others to comment which one would be appropriate for this case.In general, 
wrapping IAE with an existing exception is a reasonable solution.

IMO,  it'd be helpful to clarify this in the javadoc and document this specific 
exception.

-

PR: https://git.openjdk.java.net/jdk/pull/7274


Re: RFR: 8280889: java/lang/instrument/GetObjectSizeIntrinsicsTest.java fails with -XX:-UseCompressedOops

2022-01-28 Thread Serguei Spitsyn
On Fri, 28 Jan 2022 15:37:59 GMT, Aleksey Shipilev  wrote:

> Recent test regression after adding new cases in the test. Without compressed 
> oops, ~1G elements `Object[]` array takes >8G of memory, which fails the 
> test. The fix cuts it down to 512M when reference size is 8 bytes. 
> Additionally, `ObjectAlignmentInBytes=32` is excessive for new test configs.
> 
> Additional testing: 
>  - [x] Linux x86_64 fastdebug, default, affected test still passes
>  - [x] Linux x86_32 fastdebug, default, affected test still passes
>  - [x] Linux x86_64 fastdebug, `-XX:-UseCompressedOops`, affected test now 
> passes

Looks good to me.
Thanks,
Serguei

-

Marked as reviewed by sspitsyn (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7269


Re: RFR: 8280642: IllegalAccessError thrown by ObjectInputStream.resolveProxyClass is not handled

2022-01-28 Thread Roger Riggs
On Fri, 28 Jan 2022 23:54:54 GMT, Mandy Chung  wrote:

> `ObjectInputStream::readObject` does not specify that 
> `InvalidObjectException` may be thrown. 
> Have you considered throwing `InvalidClassException` be an alternative? i.e. 
> extend `InvalidClassException` to report failure in creating the proxy class 
> during deserialization.

There are other checks related to the Proxy interfaces that throw 
InvalidObjectException.
InvalidClassException was a close second.
The throwing of `IllegalAccessException` is not described for `readObject` and 
`InvalidObjectException` is a subclass of `IOException` that acts as a 
catch-all.

-

PR: https://git.openjdk.java.net/jdk/pull/7274


Re: RFR: 8280642: IllegalAccessError thrown by ObjectInputStream.resolveProxyClass is not handled

2022-01-28 Thread Mandy Chung
On Fri, 28 Jan 2022 21:02:23 GMT, Roger Riggs  wrote:

> During deserialization of a serialized data stream that contains a proxy 
> descriptor with non-public interfaces
> `java.io.ObjectInputStream` checks that the interfaces can be loaded from a 
> single classloader in `ObjectInputStream.resolveProxyClass`.
> If the interfaces cannot be loaded from a single classloader, an 
> `IllegalAccessError` is thrown.
> When `ObjectInputStream.readObject` encounters this case, it reflects an 
> incompatibility
> between the classloaders of the source of the serialized stream and the 
> classloader being used for deserialization.
> When a proxy object cannot be created from the interfaces, 
> `ObjectInputStream.readObject` should catch
> the `InvalidAccessError` and throw `InvalidObjectException` with the 
> `InvalidAccessError` as the cause.
> This allows the application to handle the exception consistently with other 
> errors during deserialization.

`ObjectInputStream::readObject` does not specify that `InvalidObjectException` 
may be thrown.   Have you considered throwing `InvalidClassException` be an 
alternative?i.e. extend `InvalidClassException` to report failure in 
creating the proxy class during deserialization.

-

PR: https://git.openjdk.java.net/jdk/pull/7274


Re: RFR: 8280642: IllegalAccessError thrown by ObjectInputStream.resolveProxyClass is not handled

2022-01-28 Thread Naoto Sato
On Fri, 28 Jan 2022 21:02:23 GMT, Roger Riggs  wrote:

> During deserialization of a serialized data stream that contains a proxy 
> descriptor with non-public interfaces
> `java.io.ObjectInputStream` checks that the interfaces can be loaded from a 
> single classloader in `ObjectInputStream.resolveProxyClass`.
> If the interfaces cannot be loaded from a single classloader, an 
> `IllegalAccessError` is thrown.
> When `ObjectInputStream.readObject` encounters this case, it reflects an 
> incompatibility
> between the classloaders of the source of the serialized stream and the 
> classloader being used for deserialization.
> When a proxy object cannot be created from the interfaces, 
> `ObjectInputStream.readObject` should catch
> the `InvalidAccessError` and throw `InvalidObjectException` with the 
> `InvalidAccessError` as the cause.
> This allows the application to handle the exception consistently with other 
> errors during deserialization.

Looks good.
In the CSR, I see some `InvalidAccessError` which should be 
`IllegalAccessError`.

src/java.base/share/classes/java/io/ObjectInputStream.java line 1999:

> 1997: resolveEx = ex;
> 1998: } catch (IllegalAccessError err) {
> 1999: IOException ice = new 
> InvalidObjectException(err.getMessage());

Would the variable be `ioe`?

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7274


RFR: 8280642: IllegalAccessError thrown by ObjectInputStream.resolveProxyClass is not handled

2022-01-28 Thread Roger Riggs
During deserialization of a serialized data stream that contains a proxy 
descriptor with non-public interfaces
`java.io.ObjectInputStream` checks that the interfaces can be loaded from a 
single classloader in `ObjectInputStream.resolveProxyClass`.
If the interfaces cannot be loaded from a single classloader, an 
`IllegalAccessError` is thrown.
When `ObjectInputStream.readObject` encounters this case, it reflects an 
incompatibility
between the classloaders of the source of the serialized stream and the 
classloader being used for deserialization.
When a proxy object cannot be created from the interfaces, 
`ObjectInputStream.readObject` should catch
the `InvalidAccessError` and throw `InvalidObjectException` with the 
`InvalidAccessError` as the cause.
This allows the application to handle the exception consistently with other 
errors during deserialization.

-

Commit messages:
 - 8280642: IllegalAccessError thrown by ObjectInputStream.resolveProxyClass is 
not handled

Changes: https://git.openjdk.java.net/jdk/pull/7274/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7274=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8280642
  Stats: 13 lines in 2 files changed: 5 ins; 0 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7274.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7274/head:pull/7274

PR: https://git.openjdk.java.net/jdk/pull/7274


Re: Proposed JEP: Safer Process Launch by ProcessBuilder and Runtime.exec

2022-01-28 Thread Roger Riggs

Hi Raffaello,

My mistake, the problem with empty args was fixed in 17 as well as some 
of the problems with
escaping of double-quotes.  The default legacy mode does not check for 
unbalanced quotes
possible merging of arguments.  The simplest case are for .exe 
execution, in which the

argument parsing by applications allows a more reliable encoding.
The command parsing by cmd.exe is less flexible and has more issues.

Setting the system property 
jdk.lang.Process.allowAmbiguousCommands=false applies some
additional checks. But it is not the default. It is a goal to reduce the 
number

of modes and simplify the code in the Windows ProcessImpl.

I'm all in favor of solving the problem on Windows, suggestions welcome.
But also a consideration is not breaking (too many) existing applications.

Thanks, Roger


On 1/28/22 2:14 PM, Raffaello Giulietti wrote:

Hi Roger,

I'm trying the following (ugly) code on JDK 17/Win, where Args.exe 
does nothing else than writing out its argv[], redirecting to a log file.


    public static void main(String[] args) throws IOException, 
InterruptedException {

    String[] command = {
    "C:\\Users\\alpha\\Args.exe",
    "",
    "a",
    "",
    "b",
    "",
    };
    var processBuilder = new ProcessBuilder(command);
    processBuilder.redirectOutput(new 
File("C:\\Users\\alpha\\my.log"));

    var process = processBuilder.start();
    Thread.sleep(2_000);
    System.out.println("process.exitValue() = " + 
process.exitValue());

    }


Here's the log file

argv[0] = [C:\Users\alpha\Args.exe]
argv[1] = []
argv[2] = [a]
argv[3] = []
argv[4] = [b]
argv[5] = []

so empty args seem to work correctly, at least in this plain example.

Have you specific examples that behave incorrectly?
I'm asking because I'd like to setup a simple set of rules to solve 
the issue on Windows altogether.






On 2022-01-28 16:48, Roger Riggs wrote:

Hi Raffaello,

For .exe executables, one example is an empty string in the list of 
arguments to ProcessBuilder.
The empty string is not visible in the generated command line. For 
position sensitive commands, it appears the argument is dropped.
An argument in ProcessBuilder with mismatched quotes can cause the 
argument to be joined with the next in the generated command line.
A stray "\" at the end of an argument can cause the following 
character to be quoted, possibly joining the argument with the next.


For .cmd executables, cmd.exe interprets more characters as argument 
separators and will split arguments.
For example, an argument with a semi-colon or comma, (unquoted) will 
be split into two arguments when parsed by cmd.exe.
The goal is to improve the integrity and robustness of the command 
encoding.


Thanks, Roger


On 1/28/22 4:07 AM, Raffaello Giulietti wrote:

Hello,

if I understand correctly, the issue addressed here (on Windows) is 
how to assemble a single command string from an array of argument 
strings to pass to CreateProcess() in a way that the individual 
argument strings can be fully recovered in the invoked program.

Similarly when the command string is passed to an instance of cmd.exe.

Are there known (non security critical) examples that do not work 
correctly JDK 18 or earlier?



Greetings
Raffaello


On 2022-01-20 19:05, Roger Riggs wrote:
A JEP to Improve safety of process launch by ProcessBuilder and 
Runtime.exec on Windows[1].


Argument encoding errors have been problematic on Windows systems 
due to

improperly quoted command arguments.

The idea is to tighten up quoting and encoding of command line 
arguments.


Comments appreciated,  Roger

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






Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v2]

2022-01-28 Thread Stuart Marks
On Fri, 28 Jan 2022 01:06:47 GMT, liach  wrote:

>> Commited the change on df78e05e3e692e2189c9d318fbd4892a4b96a55f
>
> Will we gradually phase out the `Random` class? If yes, we should put this 
> conversion method in `Random` so that we don't break the newer API shall 
> `Random` be retired.

I don't think Random will ever be removed, so this probably isn't an issue.

There is the overall question of which factoring is preferable: adding a 
default method on RandomGenerator, as is done here, or an alternative of adding 
a static utility to Random, something like:

// Random.java
public static Random from(RandomGenerator generator) { ... }

This makes the call site a bit longer and adds a level of nesting:

Collections.shuffle(list, 
Random.from(RandomGenerator.of("L64X128MixRandom")));

compared to the default method approach:

Collections.shuffle(list, 
RandomGenerator.of("L64X128MixRandom").asRandom()); 

I believe this allows the wrapper class to be placed in java.util, or even as a 
nested class of Random, instead of having to be a new top-level class in 
jdk.internal.

Another consideration is whether the adapter should be on the "new" thing or on 
the "old" thing. There's a little bit of precedent to put adapters on the "old" 
thing so that the "new" thing doesn't get polluted with the old. For example, 
consider Enumeration::asIterator. (Even though the adaptation is going the 
other way in that case, there are still alternative factorings that place the 
adapter on the new instead of the old.)

I don't think any of this is compelling but it's worth a discussion.

Regardless of where the adapter ends up, the "other" class should have a note 
and a link that refers to the adapter.

-

PR: https://git.openjdk.java.net/jdk/pull/7001


Re: Proposed JEP: Safer Process Launch by ProcessBuilder and Runtime.exec

2022-01-28 Thread Raffaello Giulietti

Hi Roger,

I'm trying the following (ugly) code on JDK 17/Win, where Args.exe does 
nothing else than writing out its argv[], redirecting to a log file.


public static void main(String[] args) throws IOException, 
InterruptedException {

String[] command = {
"C:\\Users\\alpha\\Args.exe",
"",
"a",
"",
"b",
"",
};
var processBuilder = new ProcessBuilder(command);
processBuilder.redirectOutput(new 
File("C:\\Users\\alpha\\my.log"));

var process = processBuilder.start();
Thread.sleep(2_000);
System.out.println("process.exitValue() = " + process.exitValue());
}


Here's the log file

argv[0] = [C:\Users\alpha\Args.exe]
argv[1] = []
argv[2] = [a]
argv[3] = []
argv[4] = [b]
argv[5] = []

so empty args seem to work correctly, at least in this plain example.

Have you specific examples that behave incorrectly?
I'm asking because I'd like to setup a simple set of rules to solve the 
issue on Windows altogether.






On 2022-01-28 16:48, Roger Riggs wrote:

Hi Raffaello,

For .exe executables, one example is an empty string in the list of 
arguments to ProcessBuilder.
The empty string is not visible in the generated command line. For 
position sensitive commands, it appears the argument is dropped.
An argument in ProcessBuilder with mismatched quotes can cause the 
argument to be joined with the next in the generated command line.
A stray "\" at the end of an argument can cause the following character 
to be quoted, possibly joining the argument with the next.


For .cmd executables, cmd.exe interprets more characters as argument 
separators and will split arguments.
For example, an argument with a semi-colon or comma, (unquoted) will be 
split into two arguments when parsed by cmd.exe.
The goal is to improve the integrity and robustness of the command 
encoding.


Thanks, Roger


On 1/28/22 4:07 AM, Raffaello Giulietti wrote:

Hello,

if I understand correctly, the issue addressed here (on Windows) is 
how to assemble a single command string from an array of argument 
strings to pass to CreateProcess() in a way that the individual 
argument strings can be fully recovered in the invoked program.

Similarly when the command string is passed to an instance of cmd.exe.

Are there known (non security critical) examples that do not work 
correctly JDK 18 or earlier?



Greetings
Raffaello


On 2022-01-20 19:05, Roger Riggs wrote:
A JEP to Improve safety of process launch by ProcessBuilder and 
Runtime.exec on Windows[1].


Argument encoding errors have been problematic on Windows systems due to
improperly quoted command arguments.

The idea is to tighten up quoting and encoding of command line 
arguments.


Comments appreciated,  Roger

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




Re: Fix proposal for bug JDK-8221642

2022-01-28 Thread Mandy Chung
Your proposal is essentially for all JNI code with no caller frame to 
default to java.base, which gets all permissions.  It means that it 
could break encapsulation to access any members.  Arguably one could 
consider JNI have superpower.  In addition, default to java.base may not 
make sense for some Java APIs, ResouroceBundle::getBundle(String 
bundlename) is one example.  It uses the caller class's loader to load 
the resource bundle.   Default to java.base means it defaults to the 
bootstrap loader which can't find the resource bundle on the class path 
for example.   For the ResourceBundle case, it seems that the unnamed 
module defined by the system class loader might be an appropriate default.


The proper way is to examine each caller-sensitive method and 
investigate what makes sense when invoked by JNI code with no caller 
frame.  JDK-8177155 is the RFE for such task. System::getLogger, 
Logger::getLogger, and core reflection API are looked at but more to 
follow up.


I created https://bugs.openjdk.java.net/browse/JDK-8280902 to follow up 
the ResourceBundle::getBundle issue.


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

On 1/28/22 3:15 AM, Andreas Rosenberg wrote:


Hi Mandy,

thanks for looking at my problem. Yes, "setAccessible" is one of the 
problems,


but our main issue is related to "ResourceBundle".

I've created a small example that shows the 
problem:https://github.com/anrose00/JniSensitiveCaller 



Any comments on my proposal would be great.

Andreas

*From:*Mandy Chung 
*Sent:* Freitag, 28. Januar 2022 02:54
*To:* Andreas Rosenberg 
*Cc:* hotspot-...@openjdk.java.net; 'core-libs-dev' 


*Subject:* Re: Fix proposal for bug JDK-8221642

I see how NPE is thrown (from `AccessibleObject::setAccessible` and 
`trySetAccessible`). The proper fix should follow the rule as the 
access check that it can set the accessible flag only on public 
members of a public type that is exported unconditionally.


The fix is straight forward but involves spec change.  I'll post PR soon.

Mandy

On 1/27/22 8:45 AM, Mandy Chung wrote:

Hi Andreas,

What methods are you calling that throws NPE?  Do you have the
stack trace to share?

The spec of AccessibleObject was updated for JDK-8221530 if there
is no caller frame when calling from JNI:

"The check when invoked by JNI code with no Java class on the
stack only succeeds if the member and the declaring class are
public, and the class is in a package that is exported to all
modules."

I think AccessibleObject::canAccess, setAccessible,
trySetAccessible should follow the same rule.

Mandy

On 1/27/22 2:19 AM, Andreas Rosenberg wrote:

Hi,

this is my first posting regarding to JDK contribution, so this may be 
the wrong place to ask.

Please point me in the right direction in this case.

We are using Java rather heavily via JNI on a custom application. For a 
long time we did stick to JRE 1.8

for various reasons. My task is to plan an upgrade to a more recent JDK 
version and while doing some

test I encountered bugs related to this: JDK-8227491(JNI - caller 
sensitive methods).

We are parsing Java class files to auto gen the JNI code for our 
application, and are also using reflection.

The workaround given is clumsy and needs manual intervention, so I was 
looking for a more elegant solution.

The problem is: a caller sensitive method wants to determine the caller 
class for security checks. In case of

a JNI call no Java stack frame exists, so the JVM function "jclass 
JVM_GetCallerClass(JNIEnv* env)" answers NULL

which leads to NPEs.

My idea is this: create an internal proxy class inside "java.base" that 
reflects this case

(e.g. "java.lang.NativeCall" or "java.lang.NativeCode").

This class is final and implements nothing.

Then "jclass JVM_GetCallerClass(JNIEnv* env)" (jvm.cpp) could be 
modified and instead of answering NULL

in case of a JNI call, it should do this to answer the class proxy:

return JVM_FindClassFromBootLoader(env, "java/lang/NativeCall");

This would have the following advantages:

- JNI code could again simply call "caller sensitive methods" without 
the need to make an additional wrapper class

- it would be more a expressive way on the Java side to detect "the callee 
is native code" than checking for null

- it would fit better into the framework

I already applied this fix on my own copy of the JDK 17 sources and it 
works pretty well for us.

As there are probably security considerations involved, advice from 
experts is required.

But from my understanding the Java security model is designed for the 

Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger [v9]

2022-01-28 Thread kabutz
On Fri, 28 Jan 2022 18:59:56 GMT, kabutz  wrote:

>> BigInteger currently uses three different algorithms for multiply. The 
>> simple quadratic algorithm, then the slightly better Karatsuba if we exceed 
>> a bit count and then Toom Cook 3 once we go into the several thousands of 
>> bits. Since Toom Cook 3 is a recursive algorithm, it is trivial to 
>> parallelize it. I have demonstrated this several times in conference talks. 
>> In order to be consistent with other classes such as Arrays and Collection, 
>> I have added a parallelMultiply() method. Internally we have added a 
>> parameter to the private multiply method to indicate whether the calculation 
>> should be done in parallel.
>> 
>> The performance improvements are as should be expected. Fibonacci of 100 
>> million (using a single-threaded Dijkstra's sum of squares version) 
>> completes in 9.2 seconds with the parallelMultiply() vs 25.3 seconds with 
>> the sequential multiply() method. This is on my 1-8-2 laptop. The final 
>> multiplications are with very large numbers, which then benefit from the 
>> parallelization of Toom-Cook 3. Fibonacci 100 million is a 347084 bit number.
>> 
>> We have also parallelized the private square() method. Internally, the 
>> square() method defaults to be sequential.
>> 
>> Some benchmark results, run on my 1-6-2 server:
>> 
>> 
>> Benchmark  (n)  Mode  Cnt  Score 
>>  Error  Units
>> BigIntegerParallelMultiply.multiply100ss4 51.707 
>> ±   11.194  ms/op
>> BigIntegerParallelMultiply.multiply   1000ss4988.302 
>> ±  235.977  ms/op
>> BigIntegerParallelMultiply.multiply  1ss4  24662.063 
>> ± 1123.329  ms/op
>> BigIntegerParallelMultiply.parallelMultiply100ss4 49.337 
>> ±   26.611  ms/op
>> BigIntegerParallelMultiply.parallelMultiply   1000ss4527.560 
>> ±  268.903  ms/op
>> BigIntegerParallelMultiply.parallelMultiply  1ss4   9076.551 
>> ± 1899.444  ms/op
>> 
>> 
>> We can see that for larger calculations (fib 100m), the execution is 2.7x 
>> faster in parallel. For medium size (fib 10m) it is 1.873x faster. And for 
>> small (fib 1m) it is roughly the same. Considering that the fibonacci 
>> algorithm that we used was in itself sequential, and that the last 3 
>> calculations would dominate, 2.7x faster should probably be considered quite 
>> good on a 1-6-2 machine.
>
> kabutz has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   Benchmark for testing the effectiveness of BigInteger.parallelMultiply()

I have added a benchmark for checking performance difference between sequential 
and parallel multiply of very large Mersenne primes using BigInteger. We want 
to measure real time, user time, system time and the amount of memory 
allocated. To calculate this, we create our own thread factory for the common 
ForkJoinPool and then use that to measure user time, cpu time and bytes 
allocated.

We use reflection to discover all methods that match "*ultiply", and use them 
to multiply two very large Mersenne primes together.

### Results on a 1-6-2 machine running Ubuntu linux

Memory allocation increased from 83.9GB to 84GB, for both the sequential and 
parallel versions. This is an increase of just 0.1%. On this machine, the 
parallel version was 3.8x faster in latency (real time), but it used 2.7x more 
CPU resources.

Testing multiplying Mersenne primes of 2^57885161-1 and 2^82589933-1

 openjdk version "18-internal" 2022-03-15

BigInteger.parallelMultiply()
real  0m6.288s
user  1m3.010s
sys   0m0.027s
mem   84.0GB
BigInteger.multiply()
real  0m23.682s
user  0m23.530s
sys   0m0.004s
mem   84.0GB


 openjdk version "1.8.0_302"

BigInteger.multiply()
real  0m25.657s
user  0m25.390s
sys   0m0.001s
mem   83.9GB


 openjdk version "9.0.7.1"

BigInteger.multiply()
real  0m24.907s
user  0m24.700s
sys   0m0.001s
mem   83.9GB


 openjdk version "10.0.2" 2018-07-17

BigInteger.multiply()
real  0m24.632s
user  0m24.380s
sys   0m0.004s
mem   83.9GB


 openjdk version "11.0.12" 2021-07-20 LTS

BigInteger.multiply()
real  0m22.114s
user  0m21.930s
sys   0m0.001s
mem   83.9GB


 openjdk version "12.0.2" 2019-07-16

BigInteger.multiply()
real  0m23.015s
user  0m22.830s
sys   0m0.000s
mem   83.9GB


 openjdk version "13.0.9" 2021-10-19

BigInteger.multiply()
real  0m23.548s
user  0m23.350s
sys   0m0.005s
mem   83.9GB


 openjdk version "14.0.2" 2020-07-14

BigInteger.multiply()
real  0m22.918s
user  0m22.530s
sys   0m0.131s
mem   83.9GB



 openjdk version "15.0.5" 2021-10-19

BigInteger.multiply()
real  0m22.038s
user  0m21.750s
sys   0m0.003s
mem   83.9GB


 openjdk version "16.0.2" 2021-07-20

BigInteger.multiply()
real  0m23.049s
user  0m22.760s
sys   0m0.006s
mem   83.9GB


 openjdk version "17" 2021-09-14

BigInteger.multiply()
real  0m22.580s
user 

Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger [v9]

2022-01-28 Thread kabutz
> BigInteger currently uses three different algorithms for multiply. The simple 
> quadratic algorithm, then the slightly better Karatsuba if we exceed a bit 
> count and then Toom Cook 3 once we go into the several thousands of bits. 
> Since Toom Cook 3 is a recursive algorithm, it is trivial to parallelize it. 
> I have demonstrated this several times in conference talks. In order to be 
> consistent with other classes such as Arrays and Collection, I have added a 
> parallelMultiply() method. Internally we have added a parameter to the 
> private multiply method to indicate whether the calculation should be done in 
> parallel.
> 
> The performance improvements are as should be expected. Fibonacci of 100 
> million (using a single-threaded Dijkstra's sum of squares version) completes 
> in 9.2 seconds with the parallelMultiply() vs 25.3 seconds with the 
> sequential multiply() method. This is on my 1-8-2 laptop. The final 
> multiplications are with very large numbers, which then benefit from the 
> parallelization of Toom-Cook 3. Fibonacci 100 million is a 347084 bit number.
> 
> We have also parallelized the private square() method. Internally, the 
> square() method defaults to be sequential.
> 
> Some benchmark results, run on my 1-6-2 server:
> 
> 
> Benchmark  (n)  Mode  Cnt  Score  
> Error  Units
> BigIntegerParallelMultiply.multiply100ss4 51.707 
> ±   11.194  ms/op
> BigIntegerParallelMultiply.multiply   1000ss4988.302 
> ±  235.977  ms/op
> BigIntegerParallelMultiply.multiply  1ss4  24662.063 
> ± 1123.329  ms/op
> BigIntegerParallelMultiply.parallelMultiply100ss4 49.337 
> ±   26.611  ms/op
> BigIntegerParallelMultiply.parallelMultiply   1000ss4527.560 
> ±  268.903  ms/op
> BigIntegerParallelMultiply.parallelMultiply  1ss4   9076.551 
> ± 1899.444  ms/op
> 
> 
> We can see that for larger calculations (fib 100m), the execution is 2.7x 
> faster in parallel. For medium size (fib 10m) it is 1.873x faster. And for 
> small (fib 1m) it is roughly the same. Considering that the fibonacci 
> algorithm that we used was in itself sequential, and that the last 3 
> calculations would dominate, 2.7x faster should probably be considered quite 
> good on a 1-6-2 machine.

kabutz has updated the pull request incrementally with one additional commit 
since the last revision:

  Benchmark for testing the effectiveness of BigInteger.parallelMultiply()

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6409/files
  - new: https://git.openjdk.java.net/jdk/pull/6409/files/25e8c082..fc7b844a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6409=08
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6409=07-08

  Stats: 322 lines in 1 file changed: 322 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6409.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6409/head:pull/6409

PR: https://git.openjdk.java.net/jdk/pull/6409


Re: Additional Date-Time Formats

2022-01-28 Thread Naoto Sato

Updated the CSR based on your inputs.

On 1/27/22 2:34 PM, Stephen Colebourne wrote:

On Thu, 27 Jan 2022 at 19:23, Naoto Sato  wrote:

Now come to think of it, I came up with the draft based on `ofPattern()`
methods. One of them is a overload method that takes a Locale argument.
Why is it so?


There is a case for the Locale on the static method (as a convenience,
and to remind callers that the locale matters). But there is no case
for it on the builder.


Eliminated the overload with locale. Instead, added a static method 
DateTimeFormatterBuilder.getLocalizedDateTimePattern() that take 
"requestedTemplate". Also, renamed `appendLocalizedPattern` to 
`appendLocalized` to make it an overload method to the existing one.





The spec Javadoc doesn't explain what repeating the pattern letter
actually does/means. Is "M" the same as ""?


That depends on the locale and the availability of the formats. For
example, 'M' translates into these patterns in each locale with
Gregorian calendar:

https://unicode-org.github.io/cldr-staging/charts/40/by_type/date_&_time.gregorian.html#959cbb42bb2962f


As things stand, the Javadoc isn't a standalone spec. I don't know how
much that matters, but I think there should be some indication in
Javadoc as to why the letter should be repeated.


Added a description that explains the number of pattern letters. They 
follow the same presentation rule as in the pattern chart.


Naoto


Re: RFR: JDK-8280534: Enable compile-time doclint reference checking [v2]

2022-01-28 Thread Iris Clark
On Fri, 28 Jan 2022 02:15:59 GMT, Joe Darcy  wrote:

>> The changes in this PR on top of the out-for-review changes in 
>> https://git.openjdk.java.net/jdk/pull/7222 allow compile-time doclint 
>> checking to be enabled in all JDK modules.
>> 
>> Typically, a @SuppressWarnings("doclint:refernce") annotation is added to 
>> declaration with javadoc blocks that have already had distinguished 
>> cross-module links added (JDK-8280492).
>> 
>> One exception is in src/java.base/share/classes/java/net/package-info.java 
>> where the cross-module link was (for now) removed. Currently the 
>> SuppressWarnings annotation type is not declared to allow its annotations to 
>> be applied to package declarations. I'll look into amending that, but in the 
>> mean time, I think it is beneficial for the JDK build, and the base module 
>> in particular, to have compile-time doclint protections turned on.
>
> Joe Darcy has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains four additional commits since 
> the last revision:
> 
>  - Use capabilities of JDK-8280744.
>  - Merge branch 'master' into JDK-8280534
>  - Cover java.base.
>  - JDK-8280534: Enable compile-time doclint reference checking

Marked as reviewed by iris (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7237


RFR: JDK-8221642: AccessibleObject::setAccessible throws NPE when invoked by JNI code with no java frame on stack

2022-01-28 Thread Mandy Chung
`AccessibleObject::setAccessible` and `trySetAccessible` methods should only 
allow access to public member of a public type that is unconditionally exported 
consistent with the access check as described in the class specification, when 
invoked by JNI code with no Java class on the stack.   The current 
implementation throws NPE when finding the module of the caller class as the 
caller class is null.

The specification of `canAccess`, `setAccessible` and `trySetAccessible` are 
updated to specify the behavior when the caller class is null.   I consider 
this spec update as a clarification as the class specification covers this case.

-

Commit messages:
 - Fix typo
 - 8221642: AccessibleObject::setAccessible throws NPE when invoked by JNI code 
with no java frame on stack

Changes: https://git.openjdk.java.net/jdk/pull/7271/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7271=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8221642
  Stats: 193 lines in 3 files changed: 166 ins; 19 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7271.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7271/head:pull/7271

PR: https://git.openjdk.java.net/jdk/pull/7271


Re: RFR: 8278753: Runtime crashes with access violation during JNI_CreateJavaVM call

2022-01-28 Thread Yumin Qi
On Wed, 26 Jan 2022 08:59:49 GMT, Alan Bateman  wrote:

>> Please review,
>>   When jlink with --compress=2, zip is used to compress the files while 
>> doing copy. The user case failed to load zip.dll, since zip.dll is not set 
>> in PATH. This failure is after we get NULL from GetModuleHandle("zip.dll"), 
>> then do LoadLibrary("zip.dll") will have same result.
>>   The fix is calling load_zip_library of ClassLoader first --- if zip 
>> library already loaded just return the cached handle for following usage, if 
>> not, load zip library and cached the handle.
>> 
>>   Tests: tier1,4,7 in test
>>Manually tested user case, and checked output of jimage list  
>> for jlinked files using --compress=2.
>> 
>> Thanks
>> Yumin
>
> I think this looks okay but I think @JimLaskey and/or @sundararajana should 
> look at this because it creates a dependency on a JVM_* function. I'm trying 
> to think if there are any interop issues when using jrtfs. Jim/Sundar can 
> correct me but I think we are okay there because a tool on say JDK 8 (or 11 
> or 17) that accesses a JDK 19 run-time image will use the BasicImageReader 
> and won't use libjimage in the target VM.

Thanks to @AlanBateman, @JimLaskey or @sundararajana Could you have a look and 
comment? 

Thanks.

-

PR: https://git.openjdk.java.net/jdk/pull/7206


RE: Pull Request: 7013: AIX: InetAddress.getByName(addr) does not work as expected

2022-01-28 Thread Michael Felt
Thanks Alan - for opening the ticket. (Open my eyes).

Still need to know if I am expected to repond to the ticket, and if so - how.


-Original Message-
From: Alan Bateman  
Sent: Saturday, 22 January 2022 11:00
To: Thomas Stüfe ; Michael Felt 

Cc: ppc-aix-port-dev ; Java Core Libs 

Subject: Re: Pull Request: 7013: AIX: InetAddress.getByName(addr) does not work 
as expected

On 22/01/2022 08:40, Thomas Stüfe wrote:
> Hi Micheal,
>
> welcome, and thank you for your contribution!
>
> I opened a bug id for you to track this:
> https://bugs.openjdk.java.net/browse/JDK-8280498 . I can sponsor this 
> patch for you.
>
> You need one reviewer, I think. I don't have the cycles right now to 
> think this through. I cc core-libs dev since this is a networking 
> issue and one of their tests.
>
The networking tests are maintained on net-dev so if there is a PR then it 
should go to that list.

-Alan




RE: Fix proposal for bug JDK-8221642

2022-01-28 Thread Andreas Rosenberg
Hi Mandy,

thanks for looking at my problem. Yes, "setAccessible" is one of the problems,
but our main issue is related to "ResourceBundle".

I've created a small example that shows the problem:  
https://github.com/anrose00/JniSensitiveCaller

Any comments on my proposal would be great.

Andreas


From: Mandy Chung 
Sent: Freitag, 28. Januar 2022 02:54
To: Andreas Rosenberg 
Cc: hotspot-...@openjdk.java.net; 'core-libs-dev' 

Subject: Re: Fix proposal for bug JDK-8221642

I see how NPE is thrown (from `AccessibleObject::setAccessible` and 
`trySetAccessible`).  The proper fix should follow the rule as the access check 
that it can set the accessible flag only on public members of a public type 
that is exported unconditionally.

The fix is straight forward but involves spec change.  I'll post PR soon.

Mandy
On 1/27/22 8:45 AM, Mandy Chung wrote:
Hi Andreas,

What methods are you calling that throws NPE?  Do you have the stack trace to 
share?

The spec of AccessibleObject was updated for JDK-8221530 if there is no caller 
frame when calling from JNI:

"The check when invoked by JNI code with no Java class on the stack only 
succeeds if the member and the declaring class are public, and the class is in 
a package that is exported to all modules."

I think AccessibleObject::canAccess, setAccessible, trySetAccessible should 
follow the same rule.

Mandy
On 1/27/22 2:19 AM, Andreas Rosenberg wrote:

Hi,



this is my first posting regarding to JDK contribution, so this may be the 
wrong place to ask.

Please point me in the right direction in this case.



We are using Java rather heavily via JNI on a custom application. For a long 
time we did stick to JRE 1.8

for various reasons. My task is to plan an upgrade to a more recent JDK version 
and while doing some

test I encountered bugs related to this: JDK-8227491  (JNI - caller sensitive 
methods).



We are parsing Java class files to auto gen the JNI code for our application, 
and are also using reflection.

The workaround given is clumsy and needs manual intervention, so I was looking 
for a more elegant solution.



The problem is: a caller sensitive method wants to determine the caller class 
for security checks. In case of

a JNI call no Java stack frame exists, so the JVM function "jclass 
JVM_GetCallerClass(JNIEnv* env)" answers NULL

which leads to NPEs.



My idea is this: create an internal proxy class inside "java.base" that 
reflects this case

(e.g. "java.lang.NativeCall" or "java.lang.NativeCode").

This class is final and implements nothing.



Then "jclass JVM_GetCallerClass(JNIEnv* env)" (jvm.cpp) could be modified and 
instead of answering NULL

in case of a JNI call, it should do this to answer the class proxy:



return JVM_FindClassFromBootLoader(env, "java/lang/NativeCall");



This would have the following advantages:

- JNI code could again simply call "caller sensitive methods" without the need 
to make an additional wrapper class

- it would be more a expressive way on the Java side to detect "the callee is 
native code" than checking for null

- it would fit better into the framework



I already applied this fix on my own copy of the JDK 17 sources and it works 
pretty well for us.



As there are probably security considerations involved, advice from experts is 
required.

But from my understanding the Java security model is designed for the main app 
being writing in Java.

In this case there are always Java stacks frames available as parents for 
caller sensitive methods, so

the proposed fix would not affect the behavior. This assumes that 
"GetCallerClass" only answers

NULL for the JNI case. This needs verification.



If the main app is native code which uses JNI, the Java security model can only 
affect the Java part and

as soon as an additional Java stack frame has been generated a regular Java 
class will be found and

the "standard behavior" should apply again.



Comments appreciated.



It this fix looks reasonable, what are the steps to get it implemented and 
integrated into the official

source tree?



Best regards,

Andy







Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v13]

2022-01-28 Thread Daniel Fuchs
On Fri, 28 Jan 2022 16:58:55 GMT, Michael McMahon  wrote:

>> Hi,
>> 
>> This change adds Channel Binding Token (CBT) support to HTTPS 
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, 
>> Kerberos) authentication scheme. When enabled, the implementation 
>> preemptively includes a CBT with authentication requests over Kerberos. The 
>> feature is enabled as follows:
>> 
>> A system property "jdk.spnego.cbt" is defined which can have the values 
>> "never" (default), which means the feature is disabled, "always", which 
>> means the CBT is included for all https Negotiate authentications, or it can 
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of 
>> domains/hosts where the feature is enabled, and disabled everywhere else. In 
>> the given example, the CBT would be included in authentication requests for 
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its 
>> sub-domains.
>> 
>> A test will be added separately to the implementation.
>> 
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>> 
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   spec update from CSR

Marked as reviewed by dfuchs (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7065


Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v13]

2022-01-28 Thread Michael McMahon
> Hi,
> 
> This change adds Channel Binding Token (CBT) support to HTTPS 
> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, Kerberos) 
> authentication scheme. When enabled, the implementation preemptively includes 
> a CBT with authentication requests over Kerberos. The feature is enabled as 
> follows:
> 
> A system property "jdk.spnego.cbt" is defined which can have the values 
> "never" (default), which means the feature is disabled, "always", which means 
> the CBT is included for all https Negotiate authentications, or it can take 
> the form "domain:a,b.c,*.d.com" which is a comma separated list of 
> domains/hosts where the feature is enabled, and disabled everywhere else. In 
> the given example, the CBT would be included in authentication requests for 
> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its 
> sub-domains.
> 
> A test will be added separately to the implementation.
> 
> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
> 
> Thanks,
> Michael

Michael McMahon has updated the pull request incrementally with one additional 
commit since the last revision:

  spec update from CSR

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7065/files
  - new: https://git.openjdk.java.net/jdk/pull/7065/files/59f703da..468e5345

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7065=12
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7065=11-12

  Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7065.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7065/head:pull/7065

PR: https://git.openjdk.java.net/jdk/pull/7065


Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v2]

2022-01-28 Thread Roger Riggs
On Fri, 28 Jan 2022 00:36:01 GMT, Yasser Bazzi  wrote:

>> Hi, could i get a review on this implementation proposed by Stuart Marks, i 
>> decided to use the 
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/random/RandomGenerator.html
>>  interface to create the default method `asRandom()` that wraps around the 
>> newer algorithms to be used on classes that do not accept the new interface.
>> 
>> Some things to note as proposed by the bug report, the protected method 
>> next(int bits) is not overrided and setSeed() method if left blank up to 
>> discussion on what to do with it.
>> 
>> Small test done on 
>> https://gist.github.com/YShow/da678561419cda8e32fccf3a27a649d4
>
> Yasser Bazzi has updated the pull request incrementally with four additional 
> commits since the last revision:
> 
>  - make sure setseed its initialized and throw
>  - remove tabs
>  - Change name of function from wrapRandom to wrap
>  - Change variable name and wording in javadocs

(For future reference: As tempting as it is to make other small improvements to 
the code such as missing but optional @ Overrides, it obscures the changes that 
are the subject of the PR.).

src/java.base/share/classes/jdk/internal/util/random/RandomWrapper.java line 36:

> 34: /**
> 35:  * Class used to wrap a {@link java.util.random.RandomGenerator} to
> 36:  * {@link java.util.Random}

Missing period at end of sentence.

src/java.base/share/classes/jdk/internal/util/random/RandomWrapper.java line 42:

> 40: public class RandomWrapper extends Random implements RandomGenerator {
> 41: private final RandomGenerator generator;
> 42: private boolean initialized;

"final" would be appropriate here.

src/java.base/share/classes/jdk/internal/util/random/RandomWrapper.java line 59:

> 57: /**
> 58:  * setSeed does not exist in {@link java.util.random.RandomGenerator} 
> so can't
> 59:  * use it

Missing period.

-

PR: https://git.openjdk.java.net/jdk/pull/7001


Re: Proposed JEP: Safer Process Launch by ProcessBuilder and Runtime.exec

2022-01-28 Thread Roger Riggs

Hi Raffaello,

For .exe executables, one example is an empty string in the list of 
arguments to ProcessBuilder.
The empty string is not visible in the generated command line. For 
position sensitive commands, it appears the argument is dropped.
An argument in ProcessBuilder with mismatched quotes can cause the 
argument to be joined with the next in the generated command line.
A stray "\" at the end of an argument can cause the following character 
to be quoted, possibly joining the argument with the next.


For .cmd executables, cmd.exe interprets more characters as argument 
separators and will split arguments.
For example, an argument with a semi-colon or comma, (unquoted) will be 
split into two arguments when parsed by cmd.exe.

The goal is to improve the integrity and robustness of the command encoding.

Thanks, Roger


On 1/28/22 4:07 AM, Raffaello Giulietti wrote:

Hello,

if I understand correctly, the issue addressed here (on Windows) is 
how to assemble a single command string from an array of argument 
strings to pass to CreateProcess() in a way that the individual 
argument strings can be fully recovered in the invoked program.

Similarly when the command string is passed to an instance of cmd.exe.

Are there known (non security critical) examples that do not work 
correctly JDK 18 or earlier?



Greetings
Raffaello


On 2022-01-20 19:05, Roger Riggs wrote:
A JEP to Improve safety of process launch by ProcessBuilder and 
Runtime.exec on Windows[1].


Argument encoding errors have been problematic on Windows systems due to
improperly quoted command arguments.

The idea is to tighten up quoting and encoding of command line 
arguments.


Comments appreciated,  Roger

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




RFR: 8280889: java/lang/instrument/GetObjectSizeIntrinsicsTest.java fails with -XX:-UseCompressedOops

2022-01-28 Thread Aleksey Shipilev
Recent test regression after adding new cases in the test. Without compressed 
oops, ~1G elements `Object[]` array takes >8G of memory, which fails the test. 
The fix cuts it down to 512M when reference size is 8 bytes. Additionally, 
`ObjectAlignmentInBytes=32` is excessive for new test configs.

Additional testing: 
 - [x] Linux x86_64 fastdebug, default, affected test still passes
 - [x] Linux x86_32 fastdebug, default, affected test still passes
 - [x] Linux x86_64 fastdebug, `-XX:-UseCompressedOops`, affected test now 
passes

-

Commit messages:
 - Fix

Changes: https://git.openjdk.java.net/jdk/pull/7269/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7269=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8280889
  Stats: 10 lines in 1 file changed: 1 ins; 3 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7269.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7269/head:pull/7269

PR: https://git.openjdk.java.net/jdk/pull/7269


Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger [v7]

2022-01-28 Thread kabutz
On Fri, 14 Jan 2022 09:15:37 GMT, kabutz  wrote:

>>> > embarrassingly parallelizable
>>> 
>>> Having looked at [embarrassingly 
>>> parallel](https://en.wikipedia.org/wiki/Embarrassingly_parallel), I'm not 
>>> certain that this particular problem would qualify. The algorithm is easy 
>>> to parallelize, but in the end we still have some rather large numbers, so 
>>> memory will be our primary dominator. I'd expect to see a linear speedup if 
>>> it was "perfectly parallel", but this does not come close to that.
>> 
>> Ok, fair point, to avoid possible confusion I have removed "embarrassingly". 
>> I don't think we need to refer to other algorithms.
>
> Hi @PaulSandoz is there anything else that we need to do? Or is this in the 
> hopper for Java 19 already?

> @kabutz please see comments from Joe on the 
> [CSR](https://bugs.openjdk.java.net/browse/JDK-8278886), which should be easy 
> to address (i can update the CSR after you make changes).

I'm working on some results for the question by Joe about the latency vs CPU 
usage for the parallelMultiply() vs multiply() methods. It wasn't so easy, 
because measuring a single thread is easier than all of the FJP threads. But I 
have a nice benchmark that I'm running now. I had to write my own harness and 
not use JMH, because I don't think that JMH can test at that level. I'm also 
measuring object allocation.

Furthermore, I'm testing against all Java versions going back to Java 8, to 
make sure that we don't get any surprises. Here is my version:


BigInteger.multiply()
real  0m22.616s
user  0m22.470s
sys   0m0.008s
mem   84.0GB
BigInteger.parallelMultiply()
real  0m6.283s
user  1m3.200s
sys   0m0.004s
mem   84.0GB


I will upload the results for all the Java versions later, and will also submit 
the benchmark.

-

PR: https://git.openjdk.java.net/jdk/pull/6409


Re: RFR: JDK-8280534: Enable compile-time doclint reference checking [v2]

2022-01-28 Thread Lance Andersen
On Fri, 28 Jan 2022 02:15:59 GMT, Joe Darcy  wrote:

>> The changes in this PR on top of the out-for-review changes in 
>> https://git.openjdk.java.net/jdk/pull/7222 allow compile-time doclint 
>> checking to be enabled in all JDK modules.
>> 
>> Typically, a @SuppressWarnings("doclint:refernce") annotation is added to 
>> declaration with javadoc blocks that have already had distinguished 
>> cross-module links added (JDK-8280492).
>> 
>> One exception is in src/java.base/share/classes/java/net/package-info.java 
>> where the cross-module link was (for now) removed. Currently the 
>> SuppressWarnings annotation type is not declared to allow its annotations to 
>> be applied to package declarations. I'll look into amending that, but in the 
>> mean time, I think it is beneficial for the JDK build, and the base module 
>> in particular, to have compile-time doclint protections turned on.
>
> Joe Darcy has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains four additional commits since 
> the last revision:
> 
>  - Use capabilities of JDK-8280744.
>  - Merge branch 'master' into JDK-8280534
>  - Cover java.base.
>  - JDK-8280534: Enable compile-time doclint reference checking

Marked as reviewed by lancea (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7237


Re: Proposed JEP: Safer Process Launch by ProcessBuilder and Runtime.exec

2022-01-28 Thread Raffaello Giulietti

Hello,

if I understand correctly, the issue addressed here (on Windows) is how 
to assemble a single command string from an array of argument strings to 
pass to CreateProcess() in a way that the individual argument strings 
can be fully recovered in the invoked program.

Similarly when the command string is passed to an instance of cmd.exe.

Are there known (non security critical) examples that do not work 
correctly JDK 18 or earlier?



Greetings
Raffaello


On 2022-01-20 19:05, Roger Riggs wrote:
A JEP to Improve safety of process launch by ProcessBuilder and 
Runtime.exec on Windows[1].


Argument encoding errors have been problematic on Windows systems due to
improperly quoted command arguments.

The idea is to tighten up quoting and encoding of command line arguments.

Comments appreciated,  Roger

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