Re: RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking

2022-03-17 Thread Jiangli Zhou
On Thu, 17 Mar 2022 23:44:32 GMT, Liam Miller-Cushon  wrote:

> This change removes support for `-Dsun.misc.URLClassPath.disableJarChecking`. 
> As discussed in the bug the feature doesn't current work, and only ever 
> supported scenarios where a security manager is installed, so it seems safe 
> to remove.

src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 806:

> 804: 
> 805: /* Throws if the given jar file is does not start with the 
> correct LOC */
> 806: @SuppressWarnings("removal")

I noticed the @SuppressWarnings("removal") when looking at the PR. It is added 
by 
https://github.com/openjdk/jdk/commit/6765f902505fbdd02f25b599f942437cd805cad1 
for [JDK-8266459](https://bugs.openjdk.java.net/browse/JDK-8266459). So looks 
like the method has been targeted for deprcating/removal.

-

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


Integrated: 8282407: Missing ')' in MacResources.properties

2022-03-17 Thread Alexander Matveev
On Sat, 12 Mar 2022 03:12:30 GMT, Alexander Matveev  
wrote:

> - Fixed by adding missing ']'.
>  - I changed '()' to '[]', since other error messages use '[]' and not '()'.

This pull request has now been integrated.

Changeset: d83cee98
Author:Alexander Matveev 
URL:   
https://git.openjdk.java.net/jdk/commit/d83cee98b5e6628f19f1b5dea11038079dd0c758
Stats: 6 lines in 3 files changed: 0 ins; 0 del; 6 mod

8282407: Missing ')' in MacResources.properties

Reviewed-by: naoto

-

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2 [v2]

2022-03-17 Thread David Holmes
On Fri, 18 Mar 2022 02:39:23 GMT, Mikael Vidstedt  wrote:

>> Note: this PR replaces the one I messed up earlier.
>> 
>> Background, from JBS:
>> 
>> src/java.base/share/native/libverify/check_code.c: In function 
>> 'read_all_code':
>> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' 
>> may be used uninitialized [-Werror=maybe-uninitialized]
>> 942 | check_and_push(context, lengths, VM_MALLOC_BLK);
>> | ^~~
>> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
>> 2 of type 'const void *' to 'check_and_push' declared here
>> 4145 | static void check_and_push(context_type *context, const void *ptr, 
>> int kind)
>> | ^~
>> 
>> Because the second argument of check_and_push is "const void*" GCC assumes 
>> that the malloc:ed data, which has not yet been initialized, will not be/can 
>> not be modified later which in turn suggests it may be used without ever 
>> being initialized.
>> 
>> The same general issue was addressed in JDK-8266168, presumably for GCC 11.1.
>> 
>> Details:
>> 
>> Instead of sprinkling more calloc calls around or using pragmas/gcc 
>> attributes I chose to change the check_and_push function to take a 
>> (non-const) void* argument, and provide a new wrapper function 
>> check_and_push_const which handles the const argument case. For the 
>> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
>> const conversion.
>> 
>> To avoid having multiple ways of solving the same problem I also chose to 
>> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
>> call.
>> 
>> Testing:
>> 
>> tier1 + builds-tier{2,3,4,5}
>
> Mikael Vidstedt has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Use const char for check_and_push_string_utf

Looks good!

-

Marked as reviewed by dholmes (Reviewer).

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2 [v2]

2022-03-17 Thread Mikael Vidstedt
On Fri, 18 Mar 2022 02:10:32 GMT, David Holmes  wrote:

>> Mikael Vidstedt has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Use const char for check_and_push_string_utf
>
> src/java.base/share/native/libverify/check_code.c line 472:
> 
>> 470: 
>> 471: static void check_and_push_malloc_block(context_type *context, void 
>> *ptr);
>> 472: static void check_and_push_string_utf(context_type *context, const void 
>> *ptr);
> 
> Can't this be:
> 
> `static void check_and_push_string_utf(context_type *context, const char* 
> str);`

Indeed it can. Fixed.

-

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2 [v2]

2022-03-17 Thread Mikael Vidstedt
> Note: this PR replaces the one I messed up earlier.
> 
> Background, from JBS:
> 
> src/java.base/share/native/libverify/check_code.c: In function 
> 'read_all_code':
> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' may 
> be used uninitialized [-Werror=maybe-uninitialized]
> 942 | check_and_push(context, lengths, VM_MALLOC_BLK);
> | ^~~
> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
> 2 of type 'const void *' to 'check_and_push' declared here
> 4145 | static void check_and_push(context_type *context, const void *ptr, int 
> kind)
> | ^~
> 
> Because the second argument of check_and_push is "const void*" GCC assumes 
> that the malloc:ed data, which has not yet been initialized, will not be/can 
> not be modified later which in turn suggests it may be used without ever 
> being initialized.
> 
> The same general issue was addressed in JDK-8266168, presumably for GCC 11.1.
> 
> Details:
> 
> Instead of sprinkling more calloc calls around or using pragmas/gcc 
> attributes I chose to change the check_and_push function to take a 
> (non-const) void* argument, and provide a new wrapper function 
> check_and_push_const which handles the const argument case. For the 
> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
> const conversion.
> 
> To avoid having multiple ways of solving the same problem I also chose to 
> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
> call.
> 
> Testing:
> 
> tier1 + builds-tier{2,3,4,5}

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

  Use const char for check_and_push_string_utf

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7859/files
  - new: https://git.openjdk.java.net/jdk/pull/7859/files/c5c60cb2..9f412d0b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7859=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7859=00-01

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

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


Re: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread David Holmes

On 18/03/2022 10:51 am, Cheng Jin wrote:

Hi Raffaello,

My concern is why the verification happens even without initialization in the 
test without mh.invoke() in the main(), which I don't think is covered or 
explained in the JVM Spec.
Put it in another way, my understanding is, when the class gets loaded, it is 
verified which doesn't necessarily lead to initialization, am I correct?


From Section 5.4 of the JVMS:

This specification allows an implementation flexibility as to when 
linking activities (and, because of recursion, loading) take place, 
provided that all of the following properties are maintained:


• A class or interface is completely loaded before it is linked.
• A class or interface is completely verified and prepared before it is 
initialized.


---

Hotspot does loading, verification and preparing upfront/eagerly.

HTH.

David
-


Best Regards
Cheng

-Original Message-
From: core-libs-dev  On Behalf Of 
Raffaello Giulietti
Sent: March 17, 2022 8:35 PM
To: core-libs-dev@openjdk.java.net
Subject: [EXTERNAL] Re: When to initialize the method's class for 
MethodHandles.Lookup.findStatic()?

Cheng,

initialization is the last thing that happens because it's where user provided 
code gets executed.

This has always been this way, as long as I can remember. See the JVMS for the 
gory details.


Greetings
Raffaello


On 2022-03-18 01:21, Cheng Jin wrote:

Hi David,

1) for the test with mh.invoke() in main(),  the log shows:
[0.262s][info][class,init] Start class verification for: Test_1
[0.262s][info][class,init] End class verification for: Test_1
[0.263s][info][class,init] 282 Initializing 'Test_1' (0x000800c00800)
[0.263s][info][class,init] Start class verification for: Test_2
[0.263s][info][class,init] End class verification for: Test_2
[0.272s][info][class,init] 366 Initializing 'Test_2' (0x000800c00a08) 
<--

2) for the test without  mh.invoke() in main(),  the log shows:
[0.296s][info][class,init] Start class verification for: Test_1
[0.296s][info][class,init] End class verification for: Test_1
[0.297s][info][class,init] 282 Initializing 'Test_1' (0x000800c00800)
[0.297s][info][class,init] Start class verification for: Test_2
[0.297s][info][class,init] End class verification for: Test_2
(Test_2 was verified but didn't get initialized)

The comparison above literally surprised me that the bytecode verification 
happened prior to the class initialization, which means
the class got verified at first even without initialization coz I previously 
thought the initialization should trigger the verification rather than in the 
reversed order.

Could you explain a little more about why it goes in this way?

Best Regards
Cheng


-Original Message-
From: core-libs-dev  On Behalf Of David 
Holmes
Sent: March 17, 2022 7:46 PM
To: Raffaello Giulietti ; 
core-libs-dev@openjdk.java.net
Subject: [EXTERNAL] Re: When to initialize the method's class for 
MethodHandles.Lookup.findStatic()?

Run with -Xlog:class+init=info to see the classes that get initialized and in 
what order.

David

On 18/03/2022 5:53 am, Raffaello Giulietti wrote:

Hi again,

here's code that shows that initialization doesn't happen during
lookup but only upon invoking the method handle. (I'm on Java 17.)

As long as the 2nd line in main() is commented, you don't see the
message "Test_2 initialized", which shows that the lookup doesn't
initialize Test_2.
When you uncomment the line in main(), the message will appear.

So, as advertised, it's the invocation of the method handle that can
trigger initialization, not the lookup.


HTH
Raffaello



import java.lang.invoke.*;

public class Test_1 {

       static MethodHandle mh;

       static {
       try {
       mh = MethodHandles.lookup().findStatic(Test_2.class,
"testMethod", MethodType.methodType(int.class, int.class));
       } catch (NoSuchMethodException | IllegalAccessException e) {
       e.printStackTrace();
       }
       }

       public static void main(String[] args) throws Throwable {
       System.out.println(mh);
       // System.out.println(Test_1.mh.invoke(0));
       }

}




public class Test_2 {

       static {
       System.out.println("Test_2 initialized");
       }

       static int testMethod(int value) { return (value + 1); }

}




On 2022-03-17 20:38, Raffaello Giulietti wrote:

Hi,

as far as I can see, the code should not even compile, as there's a
static field in Test_1 which is initialized with an expression that
throws checked exceptions (findStatic(..)).

In addition, it seems to me that there's nothing in your code that
reveals whether Test_2 has been initialized during the lookup. How
can you tell?

Finally, the method handle invocation in Test_1 will throw, as you
don't pass any argument to a handle that expects one.

Can you perhaps add more details?


Greetings
Raffaello



On 2022-03-17 17:42, Cheng Jin wrote:

Hi there,

The document of
INVALID URI REMOVED

Re: RFR: 8283225: ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 (aix) [v4]

2022-03-17 Thread David Holmes
On Thu, 17 Mar 2022 16:23:14 GMT, Tyler Steele  wrote:

>> As described in the linked issue, NullClassBytesTest fails due an 
>> OutOfMemoryError produced on AIX when the test calls defineClass with a byte 
>> array of size of 0. The native implementation of defineClass then calls  
>> malloc with a size of 0. On AIX malloc(0) returns NULL, while on other 
>> platforms it return a valid address. When NULL is produced by malloc for 
>> this reason, ClassLoader.c incorrectly interprets this as a failure due to a 
>> lack of memory.
>> 
>> ~~This PR modifies ClassLoader.c to produce an OutOfMemoryError only when 
>> `errno == ENOMEM` and to produce a ClassFormatError with the message 
>> "ClassLoader internal allocation failure" in all other cases (in which 
>> malloc returns NULL).~~ [edit: The above no longer describes the PR's 
>> proposed fix. See discussion below]
>> 
>> In addition, I performed some minor tidy-up work in ClassLoader.c by 
>> changing instances of `return 0` to `return NULL`, and `if (some_ptr == 0)` 
>> to `if (some_ptr == NULL)`. This was done to improve the clarity of the code 
>> in ClassLoader.c, but didn't feel worthy of opening a separate issue.
>> 
>> ### Alternatives
>> 
>> It would be possible to address this failure by modifying the test to accept 
>> the OutOfMemoryError on AIX. I thought it was a better solution to modify 
>> ClassLoader.c to produce an OutOfMemoryError only when the system is 
>> actually out of memory.
>> 
>> ### Testing
>> 
>> This change has been tested on AIX and Linux/x86.
>
> Tyler Steele has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Improve comment
>   
>   - Reword to avoid double use of malloc(X)
>   - Remove bug id

Update looks good. Sorry for the AIX_ONLY misdirect.

Thanks,
David

-

Marked as reviewed by dholmes (Reviewer).

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2

2022-03-17 Thread David Holmes
On Thu, 17 Mar 2022 20:56:34 GMT, Mikael Vidstedt  wrote:

> Note: this PR replaces the one I messed up earlier.
> 
> Background, from JBS:
> 
> src/java.base/share/native/libverify/check_code.c: In function 
> 'read_all_code':
> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' may 
> be used uninitialized [-Werror=maybe-uninitialized]
> 942 | check_and_push(context, lengths, VM_MALLOC_BLK);
> | ^~~
> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
> 2 of type 'const void *' to 'check_and_push' declared here
> 4145 | static void check_and_push(context_type *context, const void *ptr, int 
> kind)
> | ^~
> 
> Because the second argument of check_and_push is "const void*" GCC assumes 
> that the malloc:ed data, which has not yet been initialized, will not be/can 
> not be modified later which in turn suggests it may be used without ever 
> being initialized.
> 
> The same general issue was addressed in JDK-8266168, presumably for GCC 11.1.
> 
> Details:
> 
> Instead of sprinkling more calloc calls around or using pragmas/gcc 
> attributes I chose to change the check_and_push function to take a 
> (non-const) void* argument, and provide a new wrapper function 
> check_and_push_const which handles the const argument case. For the 
> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
> const conversion.
> 
> To avoid having multiple ways of solving the same problem I also chose to 
> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
> call.
> 
> Testing:
> 
> tier1 + builds-tier{2,3,4,5}

Changes requested by dholmes (Reviewer).

src/java.base/share/native/libverify/check_code.c line 472:

> 470: 
> 471: static void check_and_push_malloc_block(context_type *context, void 
> *ptr);
> 472: static void check_and_push_string_utf(context_type *context, const void 
> *ptr);

Can't this be:

`static void check_and_push_string_utf(context_type *context, const char* str);`

-

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


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

2022-03-17 Thread Joe Darcy
On Wed, 16 Mar 2022 14:54:41 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 one additional 
> commit since the last revision:
> 
>   add since in javadoc

> The current API docs on `Random.setSeed` look like:
> 
> ```java
> /**
>  * Sets the seed of this random number generator using a single
>  * {@code long} seed. The general contract of {@code setSeed} is
>  * that it alters the state of this random number generator object
>  * so as to be in exactly the same state as if it had just been
>  * created with the argument {@code seed} as a seed. The method
>  * {@code setSeed} is implemented by class {@code Random} by
>  * atomically updating the seed to
>  *  {@code (seed ^ 0x5DEECE66DL) & ((1L << 48) - 1)}
>  * and clearing the {@code haveNextNextGaussian} flag used by {@link
>  * #nextGaussian}.
>  *
>  * The implementation of {@code setSeed} by class {@code Random}
>  * happens to use only 48 bits of the given seed. In general, however,
>  * an overriding method may use all 64 bits of the {@code long}
>  * argument as a seed value.
>  *
>  * @param seed the initial seed
>  */
> ```
> 
> An updated version would possibly look like:
> 
> ```java
> /**
>  * Sets the seed of this random number generator using a single
>  * {@code long} seed (optional operation).
>  * 
>  * @implSpec The general contract of {@code setSeed} is
>  * that it alters the state of this random number generator object
>  * so as to be in exactly the same state as if it had just been
>  * created with the argument {@code seed} as a seed.
>  * 
>  * The method {@code setSeed} is implemented by class
>  * {@code Random} by atomically updating the seed to
>  *  {@code (seed ^ 0x5DEECE66DL) & ((1L << 48) - 1)}
>  * and clearing the {@code haveNextNextGaussian} flag used by {@link
>  * #nextGaussian}.
>  *
>  * The implementation of {@code setSeed} by class {@code Random}
>  * happens to use only 48 bits of the given seed. In general, however,
>  * an overriding method may use all 64 bits of the {@code long}
>  * argument as a seed value.
>  *
>  * @param seed the initial seed
>  * @throws UnsupportedOperationException if the {@code setSeed}
>  * operation is not supported by this random number generator
>  */
> ```
> 
> This moves pretty much everything besides the first sentence to `@implSpec`, 
> as they are only applicable to the `Random` class, but not its subclasses. 
> The rest is an emulation of `List.add` javadoc on optional operations.

Please also update the Random.setSeed spec to accommodate the behavior of 
SecureRandom.setSeed:

"Reseeds this random object, using the eight bytes contained in the given long 
seed. The given seed supplements, rather than replaces, the existing seed. 
Thus, repeated calls are guaranteed never to reduce randomness.

This method is defined for compatibility with java.util.Random."

https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/security/SecureRandom.html#setSeed(long)

Thanks.

-

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


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

2022-03-17 Thread liach
On Wed, 16 Mar 2022 14:54:41 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 one additional 
> commit since the last revision:
> 
>   add since in javadoc

The current API docs on `Random.setSeed` look like:

/**
 * Sets the seed of this random number generator using a single
 * {@code long} seed. The general contract of {@code setSeed} is
 * that it alters the state of this random number generator object
 * so as to be in exactly the same state as if it had just been
 * created with the argument {@code seed} as a seed. The method
 * {@code setSeed} is implemented by class {@code Random} by
 * atomically updating the seed to
 *  {@code (seed ^ 0x5DEECE66DL) & ((1L << 48) - 1)}
 * and clearing the {@code haveNextNextGaussian} flag used by {@link
 * #nextGaussian}.
 *
 * The implementation of {@code setSeed} by class {@code Random}
 * happens to use only 48 bits of the given seed. In general, however,
 * an overriding method may use all 64 bits of the {@code long}
 * argument as a seed value.
 *
 * @param seed the initial seed
 */


An updated version would possibly look like:

/**
 * Sets the seed of this random number generator using a single
 * {@code long} seed (optional operation).
 * 
 * @implSpec The general contract of {@code setSeed} is
 * that it alters the state of this random number generator object
 * so as to be in exactly the same state as if it had just been
 * created with the argument {@code seed} as a seed.
 * 
 * The method {@code setSeed} is implemented by class
 * {@code Random} by atomically updating the seed to
 *  {@code (seed ^ 0x5DEECE66DL) & ((1L << 48) - 1)}
 * and clearing the {@code haveNextNextGaussian} flag used by {@link
 * #nextGaussian}.
 *
 * The implementation of {@code setSeed} by class {@code Random}
 * happens to use only 48 bits of the given seed. In general, however,
 * an overriding method may use all 64 bits of the {@code long}
 * argument as a seed value.
 *
 * @param seed the initial seed
 * @throws UnsupportedOperationException if the {@code setSeed}
 * operation is not supported by this random number generator
 */

This moves pretty much everything besides the first sentence to `@implSpec`, as 
they are only applicable to the `Random` class, but not its subclasses. The 
rest is an emulation of `List.add` javadoc on optional operations.

-

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


Re: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Raffaello Giulietti
OK, from what I read so far from you is that you were surprised about 
the order, not about why verification happens even without initialization.


Verification is part of linking, which surely must happen before user 
code (e.g., initialization) is executed, which explains the order.


But once linked (thus, verified), there's no pressure to have 
initialization code (user code) executed. This can wait until the very 
last moment, when some other code of the class is invoked, or some field 
is accessed, or an instance is created, etc. It's a lazy execution 
strategy, which the JVM spec allows and which HotSpot implements this way.


HTH
Raffaello


On 2022-03-18 01:51, Cheng Jin wrote:

Hi Raffaello,

My concern is why the verification happens even without initialization in the 
test without mh.invoke() in the main(), which I don't think is covered or 
explained in the JVM Spec.
Put it in another way, my understanding is, when the class gets loaded, it is 
verified which doesn't necessarily lead to initialization, am I correct?

Best Regards
Cheng

-Original Message-
From: core-libs-dev  On Behalf Of 
Raffaello Giulietti
Sent: March 17, 2022 8:35 PM
To: core-libs-dev@openjdk.java.net
Subject: [EXTERNAL] Re: When to initialize the method's class for 
MethodHandles.Lookup.findStatic()?

Cheng,

initialization is the last thing that happens because it's where user provided 
code gets executed.

This has always been this way, as long as I can remember. See the JVMS for the 
gory details.


Greetings
Raffaello


On 2022-03-18 01:21, Cheng Jin wrote:

Hi David,

1) for the test with mh.invoke() in main(),  the log shows:
[0.262s][info][class,init] Start class verification for: Test_1
[0.262s][info][class,init] End class verification for: Test_1
[0.263s][info][class,init] 282 Initializing 'Test_1' (0x000800c00800)
[0.263s][info][class,init] Start class verification for: Test_2
[0.263s][info][class,init] End class verification for: Test_2
[0.272s][info][class,init] 366 Initializing 'Test_2' (0x000800c00a08) 
<--

2) for the test without  mh.invoke() in main(),  the log shows:
[0.296s][info][class,init] Start class verification for: Test_1
[0.296s][info][class,init] End class verification for: Test_1
[0.297s][info][class,init] 282 Initializing 'Test_1' (0x000800c00800)
[0.297s][info][class,init] Start class verification for: Test_2
[0.297s][info][class,init] End class verification for: Test_2
(Test_2 was verified but didn't get initialized)

The comparison above literally surprised me that the bytecode verification 
happened prior to the class initialization, which means
the class got verified at first even without initialization coz I previously 
thought the initialization should trigger the verification rather than in the 
reversed order.

Could you explain a little more about why it goes in this way?

Best Regards
Cheng


-Original Message-
From: core-libs-dev  On Behalf Of David 
Holmes
Sent: March 17, 2022 7:46 PM
To: Raffaello Giulietti ; 
core-libs-dev@openjdk.java.net
Subject: [EXTERNAL] Re: When to initialize the method's class for 
MethodHandles.Lookup.findStatic()?

Run with -Xlog:class+init=info to see the classes that get initialized and in 
what order.

David

On 18/03/2022 5:53 am, Raffaello Giulietti wrote:

Hi again,

here's code that shows that initialization doesn't happen during
lookup but only upon invoking the method handle. (I'm on Java 17.)

As long as the 2nd line in main() is commented, you don't see the
message "Test_2 initialized", which shows that the lookup doesn't
initialize Test_2.
When you uncomment the line in main(), the message will appear.

So, as advertised, it's the invocation of the method handle that can
trigger initialization, not the lookup.


HTH
Raffaello



import java.lang.invoke.*;

public class Test_1 {

       static MethodHandle mh;

       static {
       try {
       mh = MethodHandles.lookup().findStatic(Test_2.class,
"testMethod", MethodType.methodType(int.class, int.class));
       } catch (NoSuchMethodException | IllegalAccessException e) {
       e.printStackTrace();
       }
       }

       public static void main(String[] args) throws Throwable {
       System.out.println(mh);
       // System.out.println(Test_1.mh.invoke(0));
       }

}




public class Test_2 {

       static {
       System.out.println("Test_2 initialized");
       }

       static int testMethod(int value) { return (value + 1); }

}




On 2022-03-17 20:38, Raffaello Giulietti wrote:

Hi,

as far as I can see, the code should not even compile, as there's a
static field in Test_1 which is initialized with an expression that
throws checked exceptions (findStatic(..)).

In addition, it seems to me that there's nothing in your code that
reveals whether Test_2 has been initialized during the lookup. How
can you tell?

Finally, the method handle invocation in Test_1 will throw, as you
don't pass any argument to a 

RE: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Cheng Jin
Hi Raffaello,

My concern is why the verification happens even without initialization in the 
test without mh.invoke() in the main(), which I don't think is covered or 
explained in the JVM Spec.
Put it in another way, my understanding is, when the class gets loaded, it is 
verified which doesn't necessarily lead to initialization, am I correct?

Best Regards
Cheng

-Original Message-
From: core-libs-dev  On Behalf Of 
Raffaello Giulietti
Sent: March 17, 2022 8:35 PM
To: core-libs-dev@openjdk.java.net
Subject: [EXTERNAL] Re: When to initialize the method's class for 
MethodHandles.Lookup.findStatic()?

Cheng,

initialization is the last thing that happens because it's where user provided 
code gets executed.

This has always been this way, as long as I can remember. See the JVMS for the 
gory details.


Greetings
Raffaello


On 2022-03-18 01:21, Cheng Jin wrote:
> Hi David,
> 
> 1) for the test with mh.invoke() in main(),  the log shows:
> [0.262s][info][class,init] Start class verification for: Test_1
> [0.262s][info][class,init] End class verification for: Test_1
> [0.263s][info][class,init] 282 Initializing 'Test_1' (0x000800c00800)
> [0.263s][info][class,init] Start class verification for: Test_2
> [0.263s][info][class,init] End class verification for: Test_2
> [0.272s][info][class,init] 366 Initializing 'Test_2' (0x000800c00a08) 
> <--
> 
> 2) for the test without  mh.invoke() in main(),  the log shows:
> [0.296s][info][class,init] Start class verification for: Test_1
> [0.296s][info][class,init] End class verification for: Test_1
> [0.297s][info][class,init] 282 Initializing 'Test_1' (0x000800c00800)
> [0.297s][info][class,init] Start class verification for: Test_2
> [0.297s][info][class,init] End class verification for: Test_2
> (Test_2 was verified but didn't get initialized)
> 
> The comparison above literally surprised me that the bytecode verification 
> happened prior to the class initialization, which means
> the class got verified at first even without initialization coz I previously 
> thought the initialization should trigger the verification rather than in the 
> reversed order.
> 
> Could you explain a little more about why it goes in this way?
> 
> Best Regards
> Cheng
> 
> 
> -Original Message-
> From: core-libs-dev  On Behalf Of David 
> Holmes
> Sent: March 17, 2022 7:46 PM
> To: Raffaello Giulietti ; 
> core-libs-dev@openjdk.java.net
> Subject: [EXTERNAL] Re: When to initialize the method's class for 
> MethodHandles.Lookup.findStatic()?
> 
> Run with -Xlog:class+init=info to see the classes that get initialized and in 
> what order.
> 
> David
> 
> On 18/03/2022 5:53 am, Raffaello Giulietti wrote:
>> Hi again,
>>
>> here's code that shows that initialization doesn't happen during
>> lookup but only upon invoking the method handle. (I'm on Java 17.)
>>
>> As long as the 2nd line in main() is commented, you don't see the
>> message "Test_2 initialized", which shows that the lookup doesn't
>> initialize Test_2.
>> When you uncomment the line in main(), the message will appear.
>>
>> So, as advertised, it's the invocation of the method handle that can
>> trigger initialization, not the lookup.
>>
>>
>> HTH
>> Raffaello
>>
>> 
>>
>> import java.lang.invoke.*;
>>
>> public class Test_1 {
>>
>>       static MethodHandle mh;
>>
>>       static {
>>       try {
>>       mh = MethodHandles.lookup().findStatic(Test_2.class,
>> "testMethod", MethodType.methodType(int.class, int.class));
>>       } catch (NoSuchMethodException | IllegalAccessException e) {
>>       e.printStackTrace();
>>       }
>>       }
>>
>>       public static void main(String[] args) throws Throwable {
>>       System.out.println(mh);
>>       // System.out.println(Test_1.mh.invoke(0));
>>       }
>>
>> }
>>
>>
>>
>>
>> public class Test_2 {
>>
>>       static {
>>       System.out.println("Test_2 initialized");
>>       }
>>
>>       static int testMethod(int value) { return (value + 1); }
>>
>> }
>>
>>
>>
>>
>> On 2022-03-17 20:38, Raffaello Giulietti wrote:
>>> Hi,
>>>
>>> as far as I can see, the code should not even compile, as there's a
>>> static field in Test_1 which is initialized with an expression that
>>> throws checked exceptions (findStatic(..)).
>>>
>>> In addition, it seems to me that there's nothing in your code that
>>> reveals whether Test_2 has been initialized during the lookup. How
>>> can you tell?
>>>
>>> Finally, the method handle invocation in Test_1 will throw, as you
>>> don't pass any argument to a handle that expects one.
>>>
>>> Can you perhaps add more details?
>>>
>>>
>>> Greetings
>>> Raffaello
>>>
>>>
>>>
>>> On 2022-03-17 17:42, Cheng Jin wrote:
 Hi there,

 The document of
 INVALID URI REMOVED
 _en_java_javase_17_docs_api_java.base_java_lang_invoke_MethodHandles
 .Lookup.html-23findStatic-28java.lang.Class-2Cjava.lang.String-2Cjav
 

Re: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Raffaello Giulietti

Cheng,

initialization is the last thing that happens because it's where user 
provided code gets executed.


This has always been this way, as long as I can remember. See the JVMS 
for the gory details.



Greetings
Raffaello


On 2022-03-18 01:21, Cheng Jin wrote:

Hi David,

1) for the test with mh.invoke() in main(),  the log shows:
[0.262s][info][class,init] Start class verification for: Test_1
[0.262s][info][class,init] End class verification for: Test_1
[0.263s][info][class,init] 282 Initializing 'Test_1' (0x000800c00800)
[0.263s][info][class,init] Start class verification for: Test_2
[0.263s][info][class,init] End class verification for: Test_2
[0.272s][info][class,init] 366 Initializing 'Test_2' (0x000800c00a08) 
<--

2) for the test without  mh.invoke() in main(),  the log shows:
[0.296s][info][class,init] Start class verification for: Test_1
[0.296s][info][class,init] End class verification for: Test_1
[0.297s][info][class,init] 282 Initializing 'Test_1' (0x000800c00800)
[0.297s][info][class,init] Start class verification for: Test_2
[0.297s][info][class,init] End class verification for: Test_2
(Test_2 was verified but didn't get initialized)

The comparison above literally surprised me that the bytecode verification 
happened prior to the class initialization, which means
the class got verified at first even without initialization coz I previously 
thought the initialization should trigger the verification rather than in the 
reversed order.

Could you explain a little more about why it goes in this way?

Best Regards
Cheng


-Original Message-
From: core-libs-dev  On Behalf Of David 
Holmes
Sent: March 17, 2022 7:46 PM
To: Raffaello Giulietti ; 
core-libs-dev@openjdk.java.net
Subject: [EXTERNAL] Re: When to initialize the method's class for 
MethodHandles.Lookup.findStatic()?

Run with -Xlog:class+init=info to see the classes that get initialized and in 
what order.

David

On 18/03/2022 5:53 am, Raffaello Giulietti wrote:

Hi again,

here's code that shows that initialization doesn't happen during
lookup but only upon invoking the method handle. (I'm on Java 17.)

As long as the 2nd line in main() is commented, you don't see the
message "Test_2 initialized", which shows that the lookup doesn't
initialize Test_2.
When you uncomment the line in main(), the message will appear.

So, as advertised, it's the invocation of the method handle that can
trigger initialization, not the lookup.


HTH
Raffaello



import java.lang.invoke.*;

public class Test_1 {

      static MethodHandle mh;

      static {
      try {
      mh = MethodHandles.lookup().findStatic(Test_2.class,
"testMethod", MethodType.methodType(int.class, int.class));
      } catch (NoSuchMethodException | IllegalAccessException e) {
      e.printStackTrace();
      }
      }

      public static void main(String[] args) throws Throwable {
      System.out.println(mh);
      // System.out.println(Test_1.mh.invoke(0));
      }

}




public class Test_2 {

      static {
      System.out.println("Test_2 initialized");
      }

      static int testMethod(int value) { return (value + 1); }

}




On 2022-03-17 20:38, Raffaello Giulietti wrote:

Hi,

as far as I can see, the code should not even compile, as there's a
static field in Test_1 which is initialized with an expression that
throws checked exceptions (findStatic(..)).

In addition, it seems to me that there's nothing in your code that
reveals whether Test_2 has been initialized during the lookup. How
can you tell?

Finally, the method handle invocation in Test_1 will throw, as you
don't pass any argument to a handle that expects one.

Can you perhaps add more details?


Greetings
Raffaello



On 2022-03-17 17:42, Cheng Jin wrote:

Hi there,

The document of
INVALID URI REMOVED
_en_java_javase_17_docs_api_java.base_java_lang_invoke_MethodHandles
.Lookup.html-23findStatic-28java.lang.Class-2Cjava.lang.String-2Cjav
a.lang.invoke.MethodType-29=DwIDaQ=jf_iaSHvJObTbx-siA1ZOg=X90f
3XIRXAH8hbNam6bIUlWfF_qUAezL9ue7M7bFuPQ=RvhguidNJ90V-HK-3Ctl-kUZE5
cIfo_nt3_r8VZ0Fcc=tw_ph6oUkS0eCvzITWi9zEkarss5yNeHDrAIfvd3s3g=
in the Java API is ambiguous in terms of when to initialize the
method's class as follows (the same description as in other OpenJDK
versions)

If the returned method handle is invoked, the method's class will be
initialized, if it has not already been initialized.


It occurs to me that the method's class should be initialized when
invoking the method handle but OpenJDK actually chooses to do the
initialization in lookup.findStatic() rather than in mh.invoke()
e.g.
import java.lang.invoke.*;

public class Test_1 {
  static MethodHandle mh =
MethodHandles.lookup().findStatic(Test_2.class, "testMethod",
MethodType.methodType(int.class, int.class)); <---
Test_2.class gets initialized and verified.

  public static void main(String[] args) throws Throwable {
  Test_1.mh.invoke();
  }
}


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

2022-03-17 Thread Yasser Bazzi
On Wed, 16 Mar 2022 14:54:41 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 one additional 
> commit since the last revision:
> 
>   add since in javadoc

The toString method of both classes do not implement anything so i dont think 
we should override it, also as per request of Joe Darcy how should we proceed 
with the annotation of implSpec?

-

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


Re: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Raffaello Giulietti
Sure, which again shows that Test_2 is not initialized by the lookup of 
the mh but only upon its invocation.


Raffaello


On 2022-03-18 00:46, David Holmes wrote:
Run with -Xlog:class+init=info to see the classes that get initialized 
and in what order.


David

On 18/03/2022 5:53 am, Raffaello Giulietti wrote:

Hi again,

here's code that shows that initialization doesn't happen during 
lookup but only upon invoking the method handle. (I'm on Java 17.)


As long as the 2nd line in main() is commented, you don't see the 
message "Test_2 initialized", which shows that the lookup doesn't 
initialize Test_2.

When you uncomment the line in main(), the message will appear.

So, as advertised, it's the invocation of the method handle that can 
trigger initialization, not the lookup.



HTH
Raffaello



import java.lang.invoke.*;

public class Test_1 {

 static MethodHandle mh;

 static {
 try {
 mh = MethodHandles.lookup().findStatic(Test_2.class, 
"testMethod", MethodType.methodType(int.class, int.class));

 } catch (NoSuchMethodException | IllegalAccessException e) {
 e.printStackTrace();
 }
 }

 public static void main(String[] args) throws Throwable {
 System.out.println(mh);
 // System.out.println(Test_1.mh.invoke(0));
 }

}




public class Test_2 {

 static {
 System.out.println("Test_2 initialized");
 }

 static int testMethod(int value) { return (value + 1); }

}




On 2022-03-17 20:38, Raffaello Giulietti wrote:

Hi,

as far as I can see, the code should not even compile, as there's a 
static field in Test_1 which is initialized with an expression that 
throws checked exceptions (findStatic(..)).


In addition, it seems to me that there's nothing in your code that 
reveals whether Test_2 has been initialized during the lookup. How 
can you tell?


Finally, the method handle invocation in Test_1 will throw, as you 
don't pass any argument to a handle that expects one.


Can you perhaps add more details?


Greetings
Raffaello



On 2022-03-17 17:42, Cheng Jin wrote:

Hi there,

The document of 
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#findStatic(java.lang.Class,java.lang.String,java.lang.invoke.MethodType) 
in the Java API is ambiguous in terms of when to initialize the 
method's class as follows (the same description as in other OpenJDK 
versions)


If the returned method handle is invoked, the method's class will be 
initialized, if it has not already been initialized.



It occurs to me that the method's class should be initialized when 
invoking the method handle but OpenJDK actually chooses to do the 
initialization in lookup.findStatic() rather than in mh.invoke()

e.g.
import java.lang.invoke.*;

public class Test_1 {
 static MethodHandle mh = 
MethodHandles.lookup().findStatic(Test_2.class, "testMethod", 
MethodType.methodType(int.class, int.class)); <--- 
Test_2.class gets initialized and verified.


 public static void main(String[] args) throws Throwable {
 Test_1.mh.invoke();
 }
}

public class Test_2 {
 static int testMethod(int value) { return (value + 1); }
}

So there should be more clear explanation what is the correct or 
expected behaviour at this point and why OpenJDK doesn't comply with 
the document to delay the initialization of the method's class to 
mh.invoke().


Best Regards
Cheng Jin


RE: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Cheng Jin
Hi David,

1) for the test with mh.invoke() in main(),  the log shows:
[0.262s][info][class,init] Start class verification for: Test_1
[0.262s][info][class,init] End class verification for: Test_1
[0.263s][info][class,init] 282 Initializing 'Test_1' (0x000800c00800)
[0.263s][info][class,init] Start class verification for: Test_2
[0.263s][info][class,init] End class verification for: Test_2
[0.272s][info][class,init] 366 Initializing 'Test_2' (0x000800c00a08) 
<--

2) for the test without  mh.invoke() in main(),  the log shows:
[0.296s][info][class,init] Start class verification for: Test_1
[0.296s][info][class,init] End class verification for: Test_1
[0.297s][info][class,init] 282 Initializing 'Test_1' (0x000800c00800)
[0.297s][info][class,init] Start class verification for: Test_2
[0.297s][info][class,init] End class verification for: Test_2
(Test_2 was verified but didn't get initialized)

The comparison above literally surprised me that the bytecode verification 
happened prior to the class initialization, which means
the class got verified at first even without initialization coz I previously 
thought the initialization should trigger the verification rather than in the 
reversed order.

Could you explain a little more about why it goes in this way?

Best Regards
Cheng


-Original Message-
From: core-libs-dev  On Behalf Of David 
Holmes
Sent: March 17, 2022 7:46 PM
To: Raffaello Giulietti ; 
core-libs-dev@openjdk.java.net
Subject: [EXTERNAL] Re: When to initialize the method's class for 
MethodHandles.Lookup.findStatic()?

Run with -Xlog:class+init=info to see the classes that get initialized and in 
what order.

David

On 18/03/2022 5:53 am, Raffaello Giulietti wrote:
> Hi again,
> 
> here's code that shows that initialization doesn't happen during 
> lookup but only upon invoking the method handle. (I'm on Java 17.)
> 
> As long as the 2nd line in main() is commented, you don't see the 
> message "Test_2 initialized", which shows that the lookup doesn't 
> initialize Test_2.
> When you uncomment the line in main(), the message will appear.
> 
> So, as advertised, it's the invocation of the method handle that can 
> trigger initialization, not the lookup.
> 
> 
> HTH
> Raffaello
> 
> 
> 
> import java.lang.invoke.*;
> 
> public class Test_1 {
> 
>      static MethodHandle mh;
> 
>      static {
>      try {
>      mh = MethodHandles.lookup().findStatic(Test_2.class,
> "testMethod", MethodType.methodType(int.class, int.class));
>      } catch (NoSuchMethodException | IllegalAccessException e) {
>      e.printStackTrace();
>      }
>      }
> 
>      public static void main(String[] args) throws Throwable {
>      System.out.println(mh);
>      // System.out.println(Test_1.mh.invoke(0));
>      }
> 
> }
> 
> 
> 
> 
> public class Test_2 {
> 
>      static {
>      System.out.println("Test_2 initialized");
>      }
> 
>      static int testMethod(int value) { return (value + 1); }
> 
> }
> 
> 
> 
> 
> On 2022-03-17 20:38, Raffaello Giulietti wrote:
>> Hi,
>>
>> as far as I can see, the code should not even compile, as there's a 
>> static field in Test_1 which is initialized with an expression that 
>> throws checked exceptions (findStatic(..)).
>>
>> In addition, it seems to me that there's nothing in your code that 
>> reveals whether Test_2 has been initialized during the lookup. How 
>> can you tell?
>>
>> Finally, the method handle invocation in Test_1 will throw, as you 
>> don't pass any argument to a handle that expects one.
>>
>> Can you perhaps add more details?
>>
>>
>> Greetings
>> Raffaello
>>
>>
>>
>> On 2022-03-17 17:42, Cheng Jin wrote:
>>> Hi there,
>>>
>>> The document of
>>> INVALID URI REMOVED
>>> _en_java_javase_17_docs_api_java.base_java_lang_invoke_MethodHandles
>>> .Lookup.html-23findStatic-28java.lang.Class-2Cjava.lang.String-2Cjav
>>> a.lang.invoke.MethodType-29=DwIDaQ=jf_iaSHvJObTbx-siA1ZOg=X90f
>>> 3XIRXAH8hbNam6bIUlWfF_qUAezL9ue7M7bFuPQ=RvhguidNJ90V-HK-3Ctl-kUZE5
>>> cIfo_nt3_r8VZ0Fcc=tw_ph6oUkS0eCvzITWi9zEkarss5yNeHDrAIfvd3s3g=
>>> in the Java API is ambiguous in terms of when to initialize the 
>>> method's class as follows (the same description as in other OpenJDK
>>> versions)
>>>
>>> If the returned method handle is invoked, the method's class will be 
>>> initialized, if it has not already been initialized.
>>>
>>>
>>> It occurs to me that the method's class should be initialized when 
>>> invoking the method handle but OpenJDK actually chooses to do the 
>>> initialization in lookup.findStatic() rather than in mh.invoke() 
>>> e.g.
>>> import java.lang.invoke.*;
>>>
>>> public class Test_1 {
>>>  static MethodHandle mh =
>>> MethodHandles.lookup().findStatic(Test_2.class, "testMethod", 
>>> MethodType.methodType(int.class, int.class)); <--- 
>>> Test_2.class gets initialized and verified.
>>>
>>>  public static void main(String[] args) throws Throwable {
>>>  

Re: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Raffaello Giulietti

Hi Cheng,

I'm not sure I understand your point. By class initialization I mean 
what's specified in the JVMS [1].


Are you concerned with initialization or with verification (which 
precedes initialization)?



Raffaello



[1] https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-5.html#jvms-5.5



On 2022-03-18 00:02, Cheng Jin wrote:

Hi Raffaello,

The code snippet I posted previously was not the original test I verified on 
Java 17 (which was generated via asmtools to trigger verification)

Here's the pretty much the test I used:

Test_1.java
import java.lang.invoke.*;

public class Test_1 {

  static MethodHandle mh;

  static {
  try {
  mh = MethodHandles.lookup().findStatic(Test_2.class,
"testMethod", MethodType.methodType(int[].class));
  } catch (NoSuchMethodException | IllegalAccessException e) {
  e.printStackTrace();
  }
  }

  public static void main(String[] args) throws Throwable {
  //Test_1.mh.invoke();
  }

}

Test_2.jasm
super public class Test_2
version 52:0
{
   static Method "":"()V"
stack 5 locals 12
   {
bipush  -2;
 istore 7;
iload   7;
sipush  997;
if_icmplt   L127;
iconst_0;
istore  8;
L127:   stack_frame_type full;
locals_map int, class "[B", class "[B", class "[Ljava/lang/Class;", class "[I", 
class "[I", class "[I", int, bogus, bogus, float, float;
iload   8;
 istore 8;
 return;
   }
   public Method "":"()V"
stack 1 locals 1
   {
aload_0;
invokespecial   Method java/lang/Object."":"()V";
return;
   }

   static Method testMethod:"()[I"
stack 6 locals 1
   {
getstatic   Field testArray:"[I";
areturn;
   }

} // end Class Test_2

Jdk17/bin/java  -jar asmtools.jar jasm  Test_2.jasm   // create the .class file 
of Test_2
Jdk17/bin/java  Test_1
java.lang.IllegalAccessException: no such method: 
Test_2.testMethod()int[]/invokeStatic
 at 
java.base/java.lang.invoke.MemberName.makeAccessException(MemberName.java:972)
 at 
java.base/java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1117)
 at 
java.base/java.lang.invoke.MethodHandles$Lookup.resolveOrFail(MethodHandles.java:3649)
 at 
java.base/java.lang.invoke.MethodHandles$Lookup.findStatic(MethodHandles.java:2588)
 at Test_1.(Test_1.java:9)
Caused by: java.lang.VerifyError: Inconsistent stackmap frames at branch target 
15
Exception Details:
   Location:
 Test_2.()V @15: iload
   Reason:
 Type top (current frame, locals[0]) is not assignable to integer (stack 
map, locals[0])
   Current Frame:
 bci: @9
 flags: { }
 locals: { top, top, top, top, top, top, top, integer }
 stack: { integer, integer }
   Stackmap Frame:
 bci: @15
 flags: { }
 locals: { integer, '[B', '[B', '[Ljava/lang/Class;', '[I', '[I', '[I', 
integer, top, top, float, float }
 stack: { }
   Bytecode:
 000: 10fe 3607 1507 1103 e5a1 0006 0336 0815
 010: 0836 08b1
   Stackmap Table:
 
full_frame(@15,{Integer,Object[#11],Object[#11],Object[#20],Object[#5],Object[#5],Object[#5],Integer,Top,Top,Float,Float},{})

 at java.base/java.lang.invoke.MethodHandleNatives.resolve(Native 
Method)
 at 
java.base/java.lang.invoke.MemberName$Factory.resolve(MemberName.java:1085)
 at 
java.base/java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1114)
 ... 3 more

Test_2 was intentionally modified to throw VerifyError if initialized.
As you see above,  mh.inovke() was commented out in Test_1.main() but the 
Test_2 was still verified in the test.
So it is impossible to verify Test_2 if it is not initialized, which only means 
Test_2 is initialized during the lookup.findstatic rather than mh.invoke().

Best Regards
Cheng

- Original Message -

From: "Cheng Jin" 
To: "core-libs-dev" 
Sent: Thursday, March 17, 2022 5:42:57 PM
Subject: When to initialize the method's class for 
MethodHandles.Lookup.findStatic()?



Hi there,

The document of
INVALID URI REMOVED
n_java_javase_17_docs_api_java.base_java_lang_invoke_MethodHandles.Loo
kup.html-23findStatic-28java.lang.Class-2Cjava.lang.String-2Cjava.lang
.invoke.MethodType-29=DwIFaQ=jf_iaSHvJObTbx-siA1ZOg=X90f3XIRXAH8
hbNam6bIUlWfF_qUAezL9ue7M7bFuPQ=Xt-10pHYoPWnY6dByKowR4yeLtEs7kZkKUgt
bxKUGvM=dPAMGMxphLLXT9N4ZdukiNvWyvRPAGkcGCBLTy_sm0c=
in the Java API is ambiguous in terms of when to initialize the
method's class as follows (the same description as in other OpenJDK
versions)

If the returned method handle is invoked, the method's class will be
initialized, if it has not already been initialized.


It occurs to me that the method's class should be initialized when
invoking the method handle but 

RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking

2022-03-17 Thread Liam Miller-Cushon
This change removes support for `-Dsun.misc.URLClassPath.disableJarChecking`. 
As discussed in the bug the feature doesn't current work, and only ever 
supported scenarios where a security manager is installed, so it seems safe to 
remove.

-

Commit messages:
 - 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking

Changes: https://git.openjdk.java.net/jdk/pull/7861/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7861=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8283340
  Stats: 25 lines in 1 file changed: 0 ins; 20 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7861.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7861/head:pull/7861

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


Re: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread David Holmes
Run with -Xlog:class+init=info to see the classes that get initialized 
and in what order.


David

On 18/03/2022 5:53 am, Raffaello Giulietti wrote:

Hi again,

here's code that shows that initialization doesn't happen during lookup 
but only upon invoking the method handle. (I'm on Java 17.)


As long as the 2nd line in main() is commented, you don't see the 
message "Test_2 initialized", which shows that the lookup doesn't 
initialize Test_2.

When you uncomment the line in main(), the message will appear.

So, as advertised, it's the invocation of the method handle that can 
trigger initialization, not the lookup.



HTH
Raffaello



import java.lang.invoke.*;

public class Test_1 {

     static MethodHandle mh;

     static {
     try {
     mh = MethodHandles.lookup().findStatic(Test_2.class, 
"testMethod", MethodType.methodType(int.class, int.class));

     } catch (NoSuchMethodException | IllegalAccessException e) {
     e.printStackTrace();
     }
     }

     public static void main(String[] args) throws Throwable {
     System.out.println(mh);
     // System.out.println(Test_1.mh.invoke(0));
     }

}




public class Test_2 {

     static {
     System.out.println("Test_2 initialized");
     }

     static int testMethod(int value) { return (value + 1); }

}




On 2022-03-17 20:38, Raffaello Giulietti wrote:

Hi,

as far as I can see, the code should not even compile, as there's a 
static field in Test_1 which is initialized with an expression that 
throws checked exceptions (findStatic(..)).


In addition, it seems to me that there's nothing in your code that 
reveals whether Test_2 has been initialized during the lookup. How can 
you tell?


Finally, the method handle invocation in Test_1 will throw, as you 
don't pass any argument to a handle that expects one.


Can you perhaps add more details?


Greetings
Raffaello



On 2022-03-17 17:42, Cheng Jin wrote:

Hi there,

The document of 
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#findStatic(java.lang.Class,java.lang.String,java.lang.invoke.MethodType) 
in the Java API is ambiguous in terms of when to initialize the 
method's class as follows (the same description as in other OpenJDK 
versions)


If the returned method handle is invoked, the method's class will be 
initialized, if it has not already been initialized.



It occurs to me that the method's class should be initialized when 
invoking the method handle but OpenJDK actually chooses to do the 
initialization in lookup.findStatic() rather than in mh.invoke()

e.g.
import java.lang.invoke.*;

public class Test_1 {
 static MethodHandle mh = 
MethodHandles.lookup().findStatic(Test_2.class, "testMethod", 
MethodType.methodType(int.class, int.class)); <--- 
Test_2.class gets initialized and verified.


 public static void main(String[] args) throws Throwable {
 Test_1.mh.invoke();
 }
}

public class Test_2 {
 static int testMethod(int value) { return (value + 1); }
}

So there should be more clear explanation what is the correct or 
expected behaviour at this point and why OpenJDK doesn't comply with 
the document to delay the initialization of the method's class to 
mh.invoke().


Best Regards
Cheng Jin


RE: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Cheng Jin
Hi Raffaello,

The code snippet I posted previously was not the original test I verified on 
Java 17 (which was generated via asmtools to trigger verification) 

Here's the pretty much the test I used:

Test_1.java
import java.lang.invoke.*;

public class Test_1 {

 static MethodHandle mh;

 static {
 try {
 mh = MethodHandles.lookup().findStatic(Test_2.class,
"testMethod", MethodType.methodType(int[].class));
 } catch (NoSuchMethodException | IllegalAccessException e) {
 e.printStackTrace();
 }
 }

 public static void main(String[] args) throws Throwable {
 //Test_1.mh.invoke();
 }

}

Test_2.jasm
super public class Test_2
version 52:0
{
  static Method "":"()V"
stack 5 locals 12
  {
bipush  -2;
istore  7;
iload   7;
sipush  997;
if_icmplt   L127;
iconst_0;
istore  8;
L127:   stack_frame_type full;
locals_map int, class "[B", class "[B", class 
"[Ljava/lang/Class;", class "[I", class "[I", class "[I", int, bogus, bogus, 
float, float;
iload   8;
istore  8;
return;
  }
  public Method "":"()V"
stack 1 locals 1
  {
aload_0;
invokespecial   Method java/lang/Object."":"()V";
return;
  }

  static Method testMethod:"()[I"
stack 6 locals 1
  {
getstatic   Field testArray:"[I";
areturn;
  }

} // end Class Test_2

Jdk17/bin/java  -jar asmtools.jar jasm  Test_2.jasm   // create the .class file 
of Test_2
Jdk17/bin/java  Test_1
java.lang.IllegalAccessException: no such method: 
Test_2.testMethod()int[]/invokeStatic
at 
java.base/java.lang.invoke.MemberName.makeAccessException(MemberName.java:972)
at 
java.base/java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1117)
at 
java.base/java.lang.invoke.MethodHandles$Lookup.resolveOrFail(MethodHandles.java:3649)
at 
java.base/java.lang.invoke.MethodHandles$Lookup.findStatic(MethodHandles.java:2588)
at Test_1.(Test_1.java:9)
Caused by: java.lang.VerifyError: Inconsistent stackmap frames at branch target 
15
Exception Details:
  Location:
Test_2.()V @15: iload
  Reason:
Type top (current frame, locals[0]) is not assignable to integer (stack 
map, locals[0])
  Current Frame:
bci: @9
flags: { }
locals: { top, top, top, top, top, top, top, integer }
stack: { integer, integer }
  Stackmap Frame:
bci: @15
flags: { }
locals: { integer, '[B', '[B', '[Ljava/lang/Class;', '[I', '[I', '[I', 
integer, top, top, float, float }
stack: { }
  Bytecode:
000: 10fe 3607 1507 1103 e5a1 0006 0336 0815
010: 0836 08b1
  Stackmap Table:

full_frame(@15,{Integer,Object[#11],Object[#11],Object[#20],Object[#5],Object[#5],Object[#5],Integer,Top,Top,Float,Float},{})

at java.base/java.lang.invoke.MethodHandleNatives.resolve(Native Method)
at 
java.base/java.lang.invoke.MemberName$Factory.resolve(MemberName.java:1085)
at 
java.base/java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1114)
... 3 more

Test_2 was intentionally modified to throw VerifyError if initialized.
As you see above,  mh.inovke() was commented out in Test_1.main() but the 
Test_2 was still verified in the test.
So it is impossible to verify Test_2 if it is not initialized, which only means 
Test_2 is initialized during the lookup.findstatic rather than mh.invoke().

Best Regards
Cheng

- Original Message -
> From: "Cheng Jin" 
> To: "core-libs-dev" 
> Sent: Thursday, March 17, 2022 5:42:57 PM
> Subject: When to initialize the method's class for 
> MethodHandles.Lookup.findStatic()?

> Hi there,
> 
> The document of
> INVALID URI REMOVED
> n_java_javase_17_docs_api_java.base_java_lang_invoke_MethodHandles.Loo
> kup.html-23findStatic-28java.lang.Class-2Cjava.lang.String-2Cjava.lang
> .invoke.MethodType-29=DwIFaQ=jf_iaSHvJObTbx-siA1ZOg=X90f3XIRXAH8
> hbNam6bIUlWfF_qUAezL9ue7M7bFuPQ=Xt-10pHYoPWnY6dByKowR4yeLtEs7kZkKUgt
> bxKUGvM=dPAMGMxphLLXT9N4ZdukiNvWyvRPAGkcGCBLTy_sm0c=
> in the Java API is ambiguous in terms of when to initialize the 
> method's class as follows (the same description as in other OpenJDK 
> versions)
> 
> If the returned method handle is invoked, the method's class will be 
> initialized, if it has not already been initialized.
> 
> 
> It occurs to me that the method's class should be initialized when 
> invoking the method handle but OpenJDK actually chooses to do the 
> initialization in
> lookup.findStatic() rather than in mh.invoke() e.g.
> import java.lang.invoke.*;
> 
> public class Test_1 {
>static MethodHandle mh = MethodHandles.lookup().findStatic(Test_2.class,
>"testMethod", MethodType.methodType(int.class, int.class)); <---
>Test_2.class gets 

Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2

2022-03-17 Thread Mikael Vidstedt
On Thu, 17 Mar 2022 20:56:34 GMT, Mikael Vidstedt  wrote:

> Note: this PR replaces the one I messed up earlier.
> 
> Background, from JBS:
> 
> src/java.base/share/native/libverify/check_code.c: In function 
> 'read_all_code':
> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' may 
> be used uninitialized [-Werror=maybe-uninitialized]
> 942 | check_and_push(context, lengths, VM_MALLOC_BLK);
> | ^~~
> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
> 2 of type 'const void *' to 'check_and_push' declared here
> 4145 | static void check_and_push(context_type *context, const void *ptr, int 
> kind)
> | ^~
> 
> Because the second argument of check_and_push is "const void*" GCC assumes 
> that the malloc:ed data, which has not yet been initialized, will not be/can 
> not be modified later which in turn suggests it may be used without ever 
> being initialized.
> 
> The same general issue was addressed in JDK-8266168, presumably for GCC 11.1.
> 
> Details:
> 
> Instead of sprinkling more calloc calls around or using pragmas/gcc 
> attributes I chose to change the check_and_push function to take a 
> (non-const) void* argument, and provide a new wrapper function 
> check_and_push_const which handles the const argument case. For the 
> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
> const conversion.
> 
> To avoid having multiple ways of solving the same problem I also chose to 
> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
> call.
> 
> Testing:
> 
> tier1 + builds-tier{2,3,4,5}

Added hotspot-runtime to the new version of the PR.

-

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


Re: RFR: 8283277: ISO 4217 Amendment 171 Update

2022-03-17 Thread Joe Wang
On Thu, 17 Mar 2022 18:10:17 GMT, Naoto Sato  wrote:

> This is to incorporate the ISO 4217 amendment 171 for Sierra Leonean LEONE 
> redenomination (removing 3 zeros). Its effective date is 4/1, but I went 
> ahead as JDK19 won't be released by 4/1.

Marked as reviewed by joehw (Reviewer).

-

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


Re: RFR: 8283277: ISO 4217 Amendment 171 Update

2022-03-17 Thread Iris Clark
On Thu, 17 Mar 2022 18:10:17 GMT, Naoto Sato  wrote:

> This is to incorporate the ISO 4217 amendment 171 for Sierra Leonean LEONE 
> redenomination (removing 3 zeros). Its effective date is 4/1, but I went 
> ahead as JDK19 won't be released by 4/1.

Marked as reviewed by iris (Reviewer).

-

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


Integrated: 8283325: US_ASCII decoder relies on String.decodeASCII being exhaustive

2022-03-17 Thread Claes Redestad
On Thu, 17 Mar 2022 16:08:42 GMT, Claes Redestad  wrote:

> Adjust `String.decodeASCII` to deal with the possibility that 
> `StringCoding.countPositives` can return a value less than the exact number 
> of leading positive bytes. This is the likely cause of a number of 
> intermittent but somewhat common errors on aarch64 tests in our CI.

This pull request has now been integrated.

Changeset: 002e3667
Author:Claes Redestad 
URL:   
https://git.openjdk.java.net/jdk/commit/002e3667443d94e2303c875daf72cf1ccbbb0099
Stats: 61 lines in 2 files changed: 61 ins; 0 del; 0 mod

8283325: US_ASCII decoder relies on String.decodeASCII being exhaustive

Reviewed-by: rriggs, dcubed

-

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


Re: RFR: 8283325: US_ASCII decoder relies on String.decodeASCII being exhaustive [v2]

2022-03-17 Thread Claes Redestad
On Thu, 17 Mar 2022 19:47:14 GMT, Claes Redestad  wrote:

>> Adjust `String.decodeASCII` to deal with the possibility that 
>> `StringCoding.countPositives` can return a value less than the exact number 
>> of leading positive bytes. This is the likely cause of a number of 
>> intermittent but somewhat common errors on aarch64 tests in our CI.
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add comment, remove redundant Arrays.fill

Thanks for reviews!

-

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


RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2

2022-03-17 Thread Mikael Vidstedt
Note: this PR replaces the one I messed up earlier.

Background, from JBS:

src/java.base/share/native/libverify/check_code.c: In function 'read_all_code':
src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' may 
be used uninitialized [-Werror=maybe-uninitialized]
942 | check_and_push(context, lengths, VM_MALLOC_BLK);
| ^~~
src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 2 
of type 'const void *' to 'check_and_push' declared here
4145 | static void check_and_push(context_type *context, const void *ptr, int 
kind)
| ^~

Because the second argument of check_and_push is "const void*" GCC assumes that 
the malloc:ed data, which has not yet been initialized, will not be/can not be 
modified later which in turn suggests it may be used without ever being 
initialized.

The same general issue was addressed in JDK-8266168, presumably for GCC 11.1.

Details:

Instead of sprinkling more calloc calls around or using pragmas/gcc attributes 
I chose to change the check_and_push function to take a (non-const) void* 
argument, and provide a new wrapper function check_and_push_const which handles 
the const argument case. For the (non-const) VM_MALLOC_BKP that means the 
pointer never needs to go through a const conversion.

To avoid having multiple ways of solving the same problem I also chose to 
revert the change made in JDK-8266168, reverting the calloc back to a malloc 
call.

Testing:

tier1 + builds-tier{2,3,4,5}

-

Commit messages:
 - 8283059: Uninitialized warning in check_code.c with GCC 11.2

Changes: https://git.openjdk.java.net/jdk/pull/7859/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7859=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8283059
  Stats: 30 lines in 1 file changed: 9 ins; 0 del; 21 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7859.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7859/head:pull/7859

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


Re: RFR: 8279488: ProcessBuilder inherits contextClassLoader when spawning a process reaper thread

2022-03-17 Thread Roger Riggs
On Tue, 18 Jan 2022 15:57:58 GMT, Roger Riggs  wrote:

> The thread factory used to create the process reaper threads unnecessarily 
> inherits the callers thread context classloader.
> The result is retention of the class loader.
> 
> The thread factory used for the pool of process reaper threads is modified to 
> use an InnocuousThread with a given stacksize.
> The test verifies that the process reaper threads have a null context 
> classloader.

Please Review.

-

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


Re: RFR: 8283325: US_ASCII decoder relies on String.decodeASCII being exhaustive [v2]

2022-03-17 Thread Roger Riggs
On Thu, 17 Mar 2022 19:47:14 GMT, Claes Redestad  wrote:

>> Adjust `String.decodeASCII` to deal with the possibility that 
>> `StringCoding.countPositives` can return a value less than the exact number 
>> of leading positive bytes. This is the likely cause of a number of 
>> intermittent but somewhat common errors on aarch64 tests in our CI.
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add comment, remove redundant Arrays.fill

Marked as reviewed by rriggs (Reviewer).

-

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


Withdrawn: 8283059: Uninitialized warning in check_code.c with GCC 11.2

2022-03-17 Thread Mikael Vidstedt
On Fri, 11 Mar 2022 23:38:10 GMT, Mikael Vidstedt  wrote:

> Background, from JBS:
> 
> src/java.base/share/native/libverify/check_code.c: In function 
> 'read_all_code': 
> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' may 
> be used uninitialized [-Werror=maybe-uninitialized] 
>   942 | check_and_push(context, lengths, VM_MALLOC_BLK); 
>   | ^~~ 
> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
> 2 of type 'const void *' to 'check_and_push' declared here 
>  4145 | static void check_and_push(context_type *context, const void *ptr, 
> int kind) 
>   | ^~ 
> 
> 
> Because the second argument of check_and_push is "const void*" GCC assumes 
> that the malloc:ed data, which has not yet been initialized, will not be/can 
> not be modified later which in turn suggests it may be used without ever 
> being initialized. 
> 
> The same general issue was addressed in 
> [JDK-8266168](https://bugs.openjdk.java.net/browse/JDK-8266168), presumably 
> for GCC 11.1.
> 
> 
> Details:
> 
> Instead of sprinkling more calloc calls around or using pragmas/gcc 
> attributes I chose to change the check_and_push function to take a 
> (non-const) void* argument, and provide a new wrapper function 
> `check_and_push_const` which handles the const argument case. For the 
> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
> const conversion.
> 
> To avoid having multiple ways of solving the same problem I also chose to 
> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
> call.
> 
> Testing:
> 
> tier1 + builds-tier{2,3,4,5}

This pull request has been closed without being integrated.

-

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2 [v2]

2022-03-17 Thread Mikael Vidstedt
On Thu, 17 Mar 2022 20:36:31 GMT, Mikael Vidstedt  wrote:

>> Background, from JBS:
>> 
>> src/java.base/share/native/libverify/check_code.c: In function 
>> 'read_all_code': 
>> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' 
>> may be used uninitialized [-Werror=maybe-uninitialized] 
>>   942 | check_and_push(context, lengths, VM_MALLOC_BLK); 
>>   | ^~~ 
>> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
>> 2 of type 'const void *' to 'check_and_push' declared here 
>>  4145 | static void check_and_push(context_type *context, const void *ptr, 
>> int kind) 
>>   | ^~ 
>> 
>> 
>> Because the second argument of check_and_push is "const void*" GCC assumes 
>> that the malloc:ed data, which has not yet been initialized, will not be/can 
>> not be modified later which in turn suggests it may be used without ever 
>> being initialized. 
>> 
>> The same general issue was addressed in 
>> [JDK-8266168](https://bugs.openjdk.java.net/browse/JDK-8266168), presumably 
>> for GCC 11.1.
>> 
>> 
>> Details:
>> 
>> Instead of sprinkling more calloc calls around or using pragmas/gcc 
>> attributes I chose to change the check_and_push function to take a 
>> (non-const) void* argument, and provide a new wrapper function 
>> `check_and_push_const` which handles the const argument case. For the 
>> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
>> const conversion.
>> 
>> To avoid having multiple ways of solving the same problem I also chose to 
>> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
>> call.
>> 
>> Testing:
>> 
>> tier1 + builds-tier{2,3,4,5}
>
> Mikael Vidstedt has updated the pull request incrementally with 50 additional 
> commits since the last revision:
> 
>  - Spell out check_and_push_malloc_block
>  - Fix code
>  - 8282773: Refactor parsing of integer VM options
>
>Reviewed-by: dholmes, kbarrett
>  - 8283274: Improve @jvms usage in java.base
>
>Reviewed-by: iris
>  - 8283188: Build time regression caused by JDK-8278917
>
>Reviewed-by: kbarrett, tschatzl
>  - 8283320: Error message for Windows libraries always points to 
> --with-msvcr-dll no matter the actual file name
>
>Reviewed-by: erikj, ihse
>  - 8283056: show abstract machine code in hs-err for all VM crashes
>
>Reviewed-by: thartmann, dholmes
>  - 8282727: Parallel: Remove PSPromotionManager::_totally_drain
>
>Reviewed-by: tschatzl, kbarrett
>  - 8281146: Replace StringCoding.hasNegatives with countPositives
>
>Co-authored-by: Lutz Schmidt 
>Co-authored-by: Martin Doerr 
>Reviewed-by: kvn, lucy, rriggs
>  - 8282602: Refactor awt classes javadoc to use @throws instead of @exception
>
>Reviewed-by: aivanov, prr
>  - ... and 40 more: 
> https://git.openjdk.java.net/jdk/compare/baf72097...eb04d78a

Um.. I think I managed to mess up this PR when I brought in latest master *and* 
tried it both with the current and new version of gcc. Sorry about that. I'll 
withdraw this one and open a new PR

-

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2 [v2]

2022-03-17 Thread Mikael Vidstedt
> Background, from JBS:
> 
> src/java.base/share/native/libverify/check_code.c: In function 
> 'read_all_code': 
> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' may 
> be used uninitialized [-Werror=maybe-uninitialized] 
>   942 | check_and_push(context, lengths, VM_MALLOC_BLK); 
>   | ^~~ 
> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
> 2 of type 'const void *' to 'check_and_push' declared here 
>  4145 | static void check_and_push(context_type *context, const void *ptr, 
> int kind) 
>   | ^~ 
> 
> 
> Because the second argument of check_and_push is "const void*" GCC assumes 
> that the malloc:ed data, which has not yet been initialized, will not be/can 
> not be modified later which in turn suggests it may be used without ever 
> being initialized. 
> 
> The same general issue was addressed in 
> [JDK-8266168](https://bugs.openjdk.java.net/browse/JDK-8266168), presumably 
> for GCC 11.1.
> 
> 
> Details:
> 
> Instead of sprinkling more calloc calls around or using pragmas/gcc 
> attributes I chose to change the check_and_push function to take a 
> (non-const) void* argument, and provide a new wrapper function 
> `check_and_push_const` which handles the const argument case. For the 
> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
> const conversion.
> 
> To avoid having multiple ways of solving the same problem I also chose to 
> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
> call.
> 
> Testing:
> 
> tier1 + builds-tier{2,3,4,5}

Mikael Vidstedt has updated the pull request incrementally with 50 additional 
commits since the last revision:

 - Spell out check_and_push_malloc_block
 - Fix code
 - 8282773: Refactor parsing of integer VM options
   
   Reviewed-by: dholmes, kbarrett
 - 8283274: Improve @jvms usage in java.base
   
   Reviewed-by: iris
 - 8283188: Build time regression caused by JDK-8278917
   
   Reviewed-by: kbarrett, tschatzl
 - 8283320: Error message for Windows libraries always points to 
--with-msvcr-dll no matter the actual file name
   
   Reviewed-by: erikj, ihse
 - 8283056: show abstract machine code in hs-err for all VM crashes
   
   Reviewed-by: thartmann, dholmes
 - 8282727: Parallel: Remove PSPromotionManager::_totally_drain
   
   Reviewed-by: tschatzl, kbarrett
 - 8281146: Replace StringCoding.hasNegatives with countPositives
   
   Co-authored-by: Lutz Schmidt 
   Co-authored-by: Martin Doerr 
   Reviewed-by: kvn, lucy, rriggs
 - 8282602: Refactor awt classes javadoc to use @throws instead of @exception
   
   Reviewed-by: aivanov, prr
 - ... and 40 more: https://git.openjdk.java.net/jdk/compare/baf72097...eb04d78a

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7794/files
  - new: https://git.openjdk.java.net/jdk/pull/7794/files/baf72097..eb04d78a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7794=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7794=00-01

  Stats: 5146 lines in 353 files changed: 2567 ins; 553 del; 2026 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7794.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7794/head:pull/7794

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2

2022-03-17 Thread Mikael Vidstedt
On Thu, 17 Mar 2022 08:44:57 GMT, Kim Barrett  wrote:

>> src/java.base/share/native/libverify/check_code.c line 472:
>> 
>>> 470: 
>>> 471: static void check_and_push(context_type *context, void *ptr, int kind);
>>> 472: static void check_and_push_const(context_type *context, const void 
>>> *ptr, int kind);
>> 
>> This really needs a comment explaining why two functions are needed.
>
> I think it would be clearer to drop the kind argument and have two functions
> check_and_push_vm_string(context_type*, const char*)
> check_and_push_malloc_block(context_type*, void*)
> with a shared helper that is given the appropriate kind.

I had that at some point and then dropped it. I don't know why and it does seem 
like the better option so I'll try it again.

-

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


Re: RFR: 8283325: US_ASCII decoder relies on String.decodeASCII being exhaustive [v2]

2022-03-17 Thread Daniel D . Daugherty
On Thu, 17 Mar 2022 19:47:14 GMT, Claes Redestad  wrote:

>> Adjust `String.decodeASCII` to deal with the possibility that 
>> `StringCoding.countPositives` can return a value less than the exact number 
>> of leading positive bytes. This is the likely cause of a number of 
>> intermittent but somewhat common errors on aarch64 tests in our CI.
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add comment, remove redundant Arrays.fill

Thumbs up.

-

Marked as reviewed by dcubed (Reviewer).

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


Re: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Raffaello Giulietti

Hi again,

here's code that shows that initialization doesn't happen during lookup 
but only upon invoking the method handle. (I'm on Java 17.)


As long as the 2nd line in main() is commented, you don't see the 
message "Test_2 initialized", which shows that the lookup doesn't 
initialize Test_2.

When you uncomment the line in main(), the message will appear.

So, as advertised, it's the invocation of the method handle that can 
trigger initialization, not the lookup.



HTH
Raffaello



import java.lang.invoke.*;

public class Test_1 {

static MethodHandle mh;

static {
try {
mh = MethodHandles.lookup().findStatic(Test_2.class, 
"testMethod", MethodType.methodType(int.class, int.class));

} catch (NoSuchMethodException | IllegalAccessException e) {
e.printStackTrace();
}
}

public static void main(String[] args) throws Throwable {
System.out.println(mh);
// System.out.println(Test_1.mh.invoke(0));
}

}




public class Test_2 {

static {
System.out.println("Test_2 initialized");
}

static int testMethod(int value) { return (value + 1); }

}




On 2022-03-17 20:38, Raffaello Giulietti wrote:

Hi,

as far as I can see, the code should not even compile, as there's a 
static field in Test_1 which is initialized with an expression that 
throws checked exceptions (findStatic(..)).


In addition, it seems to me that there's nothing in your code that 
reveals whether Test_2 has been initialized during the lookup. How can 
you tell?


Finally, the method handle invocation in Test_1 will throw, as you don't 
pass any argument to a handle that expects one.


Can you perhaps add more details?


Greetings
Raffaello



On 2022-03-17 17:42, Cheng Jin wrote:

Hi there,

The document of  
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#findStatic(java.lang.Class,java.lang.String,java.lang.invoke.MethodType) 
in the Java API is ambiguous in terms of when to initialize the 
method's class as follows (the same description as in other OpenJDK 
versions)


If the returned method handle is invoked, the method's class will be 
initialized, if it has not already been initialized.



It occurs to me that the method's class should be initialized when 
invoking the method handle but OpenJDK actually chooses to do the 
initialization in lookup.findStatic() rather than in mh.invoke()

e.g.
import java.lang.invoke.*;

public class Test_1 {
 static MethodHandle mh = 
MethodHandles.lookup().findStatic(Test_2.class, "testMethod", 
MethodType.methodType(int.class, int.class)); <--- 
Test_2.class gets initialized and verified.


 public static void main(String[] args) throws Throwable {
 Test_1.mh.invoke();
 }
}

public class Test_2 {
 static int testMethod(int value) { return (value + 1); }
}

So there should be more clear explanation what is the correct or 
expected behaviour at this point and why OpenJDK doesn't comply with 
the document to delay the initialization of the method's class to 
mh.invoke().


Best Regards
Cheng Jin


Re: RFR: 8283325: US_ASCII decoder relies on String.decodeASCII being exhaustive [v2]

2022-03-17 Thread Claes Redestad
> Adjust `String.decodeASCII` to deal with the possibility that 
> `StringCoding.countPositives` can return a value less than the exact number 
> of leading positive bytes. This is the likely cause of a number of 
> intermittent but somewhat common errors on aarch64 tests in our CI.

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

  Add comment, remove redundant Arrays.fill

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7855/files
  - new: https://git.openjdk.java.net/jdk/pull/7855/files/aa0470cf..ef9fbaa7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7855=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7855=00-01

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

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


Re: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Raffaello Giulietti

Hi,

as far as I can see, the code should not even compile, as there's a 
static field in Test_1 which is initialized with an expression that 
throws checked exceptions (findStatic(..)).


In addition, it seems to me that there's nothing in your code that 
reveals whether Test_2 has been initialized during the lookup. How can 
you tell?


Finally, the method handle invocation in Test_1 will throw, as you don't 
pass any argument to a handle that expects one.


Can you perhaps add more details?


Greetings
Raffaello



On 2022-03-17 17:42, Cheng Jin wrote:

Hi there,

The document of  
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#findStatic(java.lang.Class,java.lang.String,java.lang.invoke.MethodType)
 in the Java API is ambiguous in terms of when to initialize the method's class 
as follows (the same description as in other OpenJDK versions)

If the returned method handle is invoked, the method's class will be 
initialized, if it has not already been initialized.


It occurs to me that the method's class should be initialized when invoking the 
method handle but OpenJDK actually chooses to do the initialization in 
lookup.findStatic() rather than in mh.invoke()
e.g.
import java.lang.invoke.*;

public class Test_1 {
 static MethodHandle mh = MethodHandles.lookup().findStatic(Test_2.class, 
"testMethod", MethodType.methodType(int.class, int.class)); <--- 
Test_2.class gets initialized and verified.

 public static void main(String[] args) throws Throwable {
 Test_1.mh.invoke();
 }
}

public class Test_2 {
 static int testMethod(int value) { return (value + 1); }
}

So there should be more clear explanation what is the correct or expected 
behaviour at this point and why OpenJDK doesn't comply with the document to 
delay the initialization of the method's class to mh.invoke().

Best Regards
Cheng Jin


RFR: 8283277: ISO 4217 Amendment 171 Update

2022-03-17 Thread Naoto Sato
This is to incorporate the ISO 4217 amendment 171 for Sierra Leonean LEONE 
redenomination (removing 3 zeros). Its effective date is 4/1, but I went ahead 
as JDK19 won't be released by 4/1.

-

Commit messages:
 - 8283277: ISO 4217 Amendment 171 Update

Changes: https://git.openjdk.java.net/jdk/pull/7857/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7857=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8283277
  Stats: 17 lines in 6 files changed: 3 ins; 0 del; 14 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7857.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7857/head:pull/7857

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


Re: RFR: 8283325: US_ASCII decoder relies on String.decodeASCII being exhaustive

2022-03-17 Thread Daniel D . Daugherty
On Thu, 17 Mar 2022 16:08:42 GMT, Claes Redestad  wrote:

> Adjust `String.decodeASCII` to deal with the possibility that 
> `StringCoding.countPositives` can return a value less than the exact number 
> of leading positive bytes. This is the likely cause of a number of 
> intermittent but somewhat common errors on aarch64 tests in our CI.

@cl4es - are you planning to integrate your fix today?

-

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


Re: RFR: 8278794: Infinite loop in DeflaterOutputStream.finish() [v3]

2022-03-17 Thread Lance Andersen
On Thu, 17 Mar 2022 16:56:15 GMT, Ravi Reddy  wrote:

>> Hi All,
>> 
>> This review request contains fix for infinite loop issue in 
>> DeflaterOutputStream.finish() in an exception scenario.
>> 1. The issue is with 'finished' flag not getting set to correct value when 
>> there is an IOException in 
>> DeflaterOutputStream.finish() which will result in Infinite loops for 
>> next write operations on the same deflater.
>> 2. Tighten the condition(to close deflater) in ZipOutputStream using an 
>> already existing 'finish' flag in Deflater class.
>> 3. Added Inflater exception scenarios also to the test case, renaming test 
>> case to CloseInflaterDeflaterTest.java from CloseDeflaterTest.java
>> 
>> Thanks,
>> Ravi
>
> Ravi Reddy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Modified the format of comments across the test case and also done clean up 
> of test case by using less data providers

Hi Ravi,

Thank you for cleaning up the comments.  Much appreciated!

-

Marked as reviewed by lancea (Reviewer).

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


Re: Making enum hashcodes consistent across JVM's

2022-03-17 Thread dfranken . jdk
Hi,

Our use case is that we have different instances of an application
running in a cloud environment and a precomputed hash code is used
downstream to see if object instances generated in different
application instances are the same or not.

An enum was later added to our model which caused the weird behavior
that instances generated by the same application instance had the same
hash, but instances generated by another application instance did not.

So it was weird to me that all of the other fields which either
consisted of Strings, primitives, collections or elements ending in
Strings and primitives generated the same hash code each time between
applications and executions and enums did not.

I understand now that relying on a specific hashcode implementation is
foolish, but because String worked this way I thought it would be nice
if enum worked this way too. But I realize now that the way String does
it, is in fact wrong (but part of unchangeable history) instead of the
other way around.

Kind regards,

Dave

On Thu, 2022-03-17 at 15:59 +0100, Raffaello Giulietti wrote:
> Hi,
> 
> specifying a fixed hashCode() algorithm has the important drawback
> that 
> it prevents evolution.
> 
> For example, as you note, String has a strictly specified algorithm
> [1] 
> that was perhaps a good one in the early days of Java (around 1995)
> but 
> has shown its deficiencies over the years. In particular, it's easy
> to 
> construct collisions, which might impact the performance of popular
> and 
> heavily used hash based data structures, like HashMap and HashSet,
> under 
> some forms of cyberattack.
> 
> Unfortunately, while better String hash algorithms have been
> proposed, 
> they cannot be adopted, as too much code out there relies on the 
> specific details of String::hashCode. (Even bytecode for switches
> over 
> String relies on that particular spec.) It's far too late now to
> change 
> the spec, even for the better.
> 
> In retrospect and IMO, fixing String::hashCode in the spec was a 
> (understandable) mistake.
> 
> 
> More generally, client code that relies on details of any popular 
> library hashCode() implementation has a dangerous dependency on such 
> details and impairs evolution of the library itself (as the example
> with 
> String shows).
> 
> To leave the door open for better implementations proposed in some 
> future, it is best to avoid specifying the details of hashCode(), as 
> done in Record::hashCode [2] ("Implementation Requirements"). After
> all, 
> the general contract reads [3]: "This integer [the hash] need not
> remain 
> consistent from one execution of an application to another execution
> of 
> the same application."
> 
> 
> But what is the use case you have for which you would like a firmly 
> specified Enum::hashCode?
> 
> 
> Greetings
> Raffaello
> 
> 
> 
> [1] 
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/String.html#hashCode()
> [2] 
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Record.html#hashCode()
> [3] 
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Object.html#hashCode()
> 
> 
> 
> On 2022-03-17 13:49, dfranken@gmail.com wrote:
> > Dear all,
> > 
> > Currently enums do not have a well-defined hashCode()
> > implementation so
> > they defer to Object.hashCode() which just uses an internal
> > mechanism
> > to determine the hashcode, likely based on the object's place in
> > the
> > heap.
> > 
> > This may confuse a lot of developers as other classes such as
> > String do
> > have a well-defined hashcode which is consistent across multiple
> > JVM's.
> > 
> > Could it be a good idea to make enums' hashCode() method more
> > reliably
> > and predictable? For instance by using the hashcode of the name of
> > its
> > value?
> > 
> > For example:
> > 
> > class MyEnum { A, B }
> > -> A.hashCode() == A.name().hashCode()
> > 
> > Kind regards,
> > 
> > Dave Franken



Re: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Remi Forax
- Original Message -
> From: "Cheng Jin" 
> To: "core-libs-dev" 
> Sent: Thursday, March 17, 2022 5:42:57 PM
> Subject: When to initialize the method's class for 
> MethodHandles.Lookup.findStatic()?

> Hi there,
> 
> The document of
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#findStatic(java.lang.Class,java.lang.String,java.lang.invoke.MethodType)
> in the Java API is ambiguous in terms of when to initialize the method's class
> as follows (the same description as in other OpenJDK versions)
> 
> If the returned method handle is invoked, the method's class will be
> initialized, if it has not already been initialized.
> 
> 
> It occurs to me that the method's class should be initialized when invoking 
> the
> method handle but OpenJDK actually chooses to do the initialization in
> lookup.findStatic() rather than in mh.invoke()
> e.g.
> import java.lang.invoke.*;
> 
> public class Test_1 {
>static MethodHandle mh = MethodHandles.lookup().findStatic(Test_2.class,
>"testMethod", MethodType.methodType(int.class, int.class)); <---
>Test_2.class gets initialized and verified.
> 
>public static void main(String[] args) throws Throwable {
>Test_1.mh.invoke();
>}
> }
> 
> public class Test_2 {
>static int testMethod(int value) { return (value + 1); }
> }
> 
> So there should be more clear explanation what is the correct or expected
> behaviour at this point and why OpenJDK doesn't comply with the document to
> delay the initialization of the method's class to mh.invoke().

Hi,
if by initialization you mean running the static block, then it's a bug.

As far as i remember, the JVMS spec cleanly separate the Linking exceptions 
from the Runtime exceptions.
https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-6.html#jvms-6.5.invokestatic

The linking exceptions occurs when calling findStatic() while the runtime 
exceptions will appear at runtime when calling invokeExact()/invoke().

> 
> Best Regards
> Cheng Jin

regards,
Rémi


Re: RFR: 8283287: ClassLoader.c cleanups [v2]

2022-03-17 Thread Thomas Stuefe
On Thu, 17 Mar 2022 16:08:11 GMT, Tyler Steele  wrote:

>> As mentioned in the issue, I'd like to perform the following tidying on 
>> ClassLoader.c
>> 
>> - Alphabetize includes.
>> - Replace 'if (ptr == 0)' with 'if (ptr == NULL)'.
>> - Replace 'return 0' with 'return NULL'.
>
> Tyler Steele has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   removes unneeded errno.h

All good.

Cheers, Thomas

-

Marked as reviewed by stuefe (Reviewer).

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


Re: RFR: 8283237: CallSite should be a sealed class [v2]

2022-03-17 Thread liach
On Thu, 17 Mar 2022 08:07:23 GMT, liach  wrote:

>> Change `CallSite` to a sealed class, as `CallSite` is an abstract class 
>> which does not allow direct subclassing by users per its documentation. 
>> Since I don't have a JBS account, I posted the content for the CSR in a 
>> GitHub Gist at https://gist.github.com/150d5aa7f8b13a4deddf95969ad39d73 and 
>> wish someone can submit a CSR for me.
>
> liach has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   Swap mutable and volatile in the permits list

@jddarcy Could you help me file a CSR for this change?

-

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


Re: RFR: 8278794: Infinite loop in DeflaterOutputStream.finish() [v3]

2022-03-17 Thread Ravi Reddy
> Hi All,
> 
> This review request contains fix for infinite loop issue in 
> DeflaterOutputStream.finish() in an exception scenario.
> 1. The issue is with 'finished' flag not getting set to correct value when 
> there is an IOException in 
> DeflaterOutputStream.finish() which will result in Infinite loops for 
> next write operations on the same deflater.
> 2. Tighten the condition(to close deflater) in ZipOutputStream using an 
> already existing 'finish' flag in Deflater class.
> 3. Added Inflater exception scenarios also to the test case, renaming test 
> case to CloseInflaterDeflaterTest.java from CloseDeflaterTest.java
> 
> Thanks,
> Ravi

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

  Modified the format of comments across the test case and also done clean up 
of test case by using less data providers

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7804/files
  - new: https://git.openjdk.java.net/jdk/pull/7804/files/c50aa1bd..ac867083

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7804=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7804=01-02

  Stats: 94 lines in 1 file changed: 32 ins; 20 del; 42 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7804.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7804/head:pull/7804

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


Re: RFR: 8278794: Infinite loop in DeflaterOutputStream.finish() [v2]

2022-03-17 Thread Ravi Reddy
On Wed, 16 Mar 2022 16:34:37 GMT, Ravi Reddy  wrote:

>> Hi All,
>> 
>> This review request contains fix for infinite loop issue in 
>> DeflaterOutputStream.finish() in an exception scenario.
>> 1. The issue is with 'finished' flag not getting set to correct value when 
>> there is an IOException in 
>> DeflaterOutputStream.finish() which will result in Infinite loops for 
>> next write operations on the same deflater.
>> 2. Tighten the condition(to close deflater) in ZipOutputStream using an 
>> already existing 'finish' flag in Deflater class.
>> 3. Added Inflater exception scenarios also to the test case, renaming test 
>> case to CloseInflaterDeflaterTest.java from CloseDeflaterTest.java
>> 
>> Thanks,
>> Ravi
>
> Ravi Reddy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Modified write(byte []b) to write(byte[] b) to maintain same signature 
> across the test case

> 

Thanks Lance for the review , In the latest commit , I have formatted the 
comments in the test case.

-

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


Re: RFR: 8283225: ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 (aix) [v4]

2022-03-17 Thread Roger Riggs
On Thu, 17 Mar 2022 16:23:14 GMT, Tyler Steele  wrote:

>> As described in the linked issue, NullClassBytesTest fails due an 
>> OutOfMemoryError produced on AIX when the test calls defineClass with a byte 
>> array of size of 0. The native implementation of defineClass then calls  
>> malloc with a size of 0. On AIX malloc(0) returns NULL, while on other 
>> platforms it return a valid address. When NULL is produced by malloc for 
>> this reason, ClassLoader.c incorrectly interprets this as a failure due to a 
>> lack of memory.
>> 
>> ~~This PR modifies ClassLoader.c to produce an OutOfMemoryError only when 
>> `errno == ENOMEM` and to produce a ClassFormatError with the message 
>> "ClassLoader internal allocation failure" in all other cases (in which 
>> malloc returns NULL).~~ [edit: The above no longer describes the PR's 
>> proposed fix. See discussion below]
>> 
>> In addition, I performed some minor tidy-up work in ClassLoader.c by 
>> changing instances of `return 0` to `return NULL`, and `if (some_ptr == 0)` 
>> to `if (some_ptr == NULL)`. This was done to improve the clarity of the code 
>> in ClassLoader.c, but didn't feel worthy of opening a separate issue.
>> 
>> ### Alternatives
>> 
>> It would be possible to address this failure by modifying the test to accept 
>> the OutOfMemoryError on AIX. I thought it was a better solution to modify 
>> ClassLoader.c to produce an OutOfMemoryError only when the system is 
>> actually out of memory.
>> 
>> ### Testing
>> 
>> This change has been tested on AIX and Linux/x86.
>
> Tyler Steele has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Improve comment
>   
>   - Reword to avoid double use of malloc(X)
>   - Remove bug id

Marked as reviewed by rriggs (Reviewer).

-

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


Re: RFR: 8283225: ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 (aix) [v3]

2022-03-17 Thread Tyler Steele
On Thu, 17 Mar 2022 14:48:09 GMT, Roger Riggs  wrote:

>> src/java.base/share/native/libjava/ClassLoader.c line 102:
>> 
>>> 100: }
>>> 101: 
>>> 102: // On malloc(0), implementators of malloc(3) have the choice to 
>>> return either
>> 
>> It is confusing to mix `malloc(0)`, where you are passing an argument zero 
>> to malloc, with `malloc(3)` which actually means the definition of malloc as 
>> per the man page in section 3.
>> 
>> Given this is only an issue on AIX the comment can simply say:
>> 
>> `// On AIX malloc(0) returns NULL which looks like an out-of-memory 
>> condition; so adjust it to malloc(1)`.
>
> I would omit the bug number reference, they get stale and do not age well, 
> cluttering up the source.
> Git blame can be used to find the origin of the comment if needed.

I changed to the streamlined comment and removed the bug id. Thanks for your 
suggestions.

-

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


Re: RFR: 8283225: ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 (aix) [v3]

2022-03-17 Thread Tyler Steele
On Thu, 17 Mar 2022 07:46:21 GMT, Thomas Stuefe  wrote:

>> src/java.base/share/native/libjava/ClassLoader.c line 106:
>> 
>>> 104: // we chose the latter. (see 8283225)
>>> 105: #ifdef _AIX
>>> 106: body = (jbyte *)malloc(length == 0 ? 1 : length);
>> 
>> Using AIX_ONLY this can be simplified:
>> 
>> `body = (jbyte *)malloc(length AIX_ONLY( == 0 ? 1 : length));`
>
>> Using AIX_ONLY this can be simplified:
>> 
>> `body = (jbyte *)malloc(length AIX_ONLY( == 0 ? 1 : length));`
> 
> This is jdk, not hotspot. Do we have AIX_ONLY in the JDK?

As Thomas mentioned, this does not seem to work here.

-

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


Re: RFR: 8283325: US_ASCII decoder relies on String.decodeASCII being exhaustive

2022-03-17 Thread Roger Riggs
On Thu, 17 Mar 2022 16:08:42 GMT, Claes Redestad  wrote:

> Adjust `String.decodeASCII` to deal with the possibility that 
> `StringCoding.countPositives` can return a value less than the exact number 
> of leading positive bytes. This is the likely cause of a number of 
> intermittent but somewhat common errors on aarch64 tests in our CI.

In the test, it might be worth a comment that the 100_000 repeats is enough to 
warm up the compiler and use the intrinsic.

-

Marked as reviewed by rriggs (Reviewer).

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


When to initialize the method's class for MethodHandles.Lookup.findStatic()?

2022-03-17 Thread Cheng Jin
Hi there,

The document of  
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#findStatic(java.lang.Class,java.lang.String,java.lang.invoke.MethodType)
 in the Java API is ambiguous in terms of when to initialize the method's class 
as follows (the same description as in other OpenJDK versions)

If the returned method handle is invoked, the method's class will be 
initialized, if it has not already been initialized.


It occurs to me that the method's class should be initialized when invoking the 
method handle but OpenJDK actually chooses to do the initialization in 
lookup.findStatic() rather than in mh.invoke()
e.g.
import java.lang.invoke.*;

public class Test_1 {
static MethodHandle mh = MethodHandles.lookup().findStatic(Test_2.class, 
"testMethod", MethodType.methodType(int.class, int.class)); <--- 
Test_2.class gets initialized and verified.

public static void main(String[] args) throws Throwable {
Test_1.mh.invoke();
}
}

public class Test_2 {
static int testMethod(int value) { return (value + 1); }
}

So there should be more clear explanation what is the correct or expected 
behaviour at this point and why OpenJDK doesn't comply with the document to 
delay the initialization of the method's class to mh.invoke().

Best Regards
Cheng Jin


Integrated: JDK-8283274: Improve @jvms usage in java.base

2022-03-17 Thread Joe Darcy
On Wed, 16 Mar 2022 16:57:12 GMT, Joe Darcy  wrote:

> As was done for JLS references under JDK-8283234, now the analogous update 
> for JVMS references.

This pull request has now been integrated.

Changeset: 5ef1990d
Author:Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/5ef1990d6ce35a85c86689badba465b6c8f9f4a1
Stats: 12 lines in 6 files changed: 0 ins; 0 del; 12 mod

8283274: Improve @jvms usage in java.base

Reviewed-by: iris

-

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


Re: RFR: 8283225: ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 (aix) [v4]

2022-03-17 Thread Tyler Steele
> As described in the linked issue, NullClassBytesTest fails due an 
> OutOfMemoryError produced on AIX when the test calls defineClass with a byte 
> array of size of 0. The native implementation of defineClass then calls  
> malloc with a size of 0. On AIX malloc(0) returns NULL, while on other 
> platforms it return a valid address. When NULL is produced by malloc for this 
> reason, ClassLoader.c incorrectly interprets this as a failure due to a lack 
> of memory.
> 
> ~~This PR modifies ClassLoader.c to produce an OutOfMemoryError only when 
> `errno == ENOMEM` and to produce a ClassFormatError with the message 
> "ClassLoader internal allocation failure" in all other cases (in which malloc 
> returns NULL).~~ [edit: The above no longer describes the PR's proposed fix. 
> See discussion below]
> 
> In addition, I performed some minor tidy-up work in ClassLoader.c by changing 
> instances of `return 0` to `return NULL`, and `if (some_ptr == 0)` to `if 
> (some_ptr == NULL)`. This was done to improve the clarity of the code in 
> ClassLoader.c, but didn't feel worthy of opening a separate issue.
> 
> ### Alternatives
> 
> It would be possible to address this failure by modifying the test to accept 
> the OutOfMemoryError on AIX. I thought it was a better solution to modify 
> ClassLoader.c to produce an OutOfMemoryError only when the system is actually 
> out of memory.
> 
> ### Testing
> 
> This change has been tested on AIX and Linux/x86.

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

  Improve comment
  
  - Reword to avoid double use of malloc(X)
  - Remove bug id

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7829/files
  - new: https://git.openjdk.java.net/jdk/pull/7829/files/f05c8d85..f0e2800e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7829=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7829=02-03

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

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


RFR: 8283325: US_ASCII decoder relies on String.decodeASCII being exhaustive

2022-03-17 Thread Claes Redestad
Adjust `String.decodeASCII` to deal with the possibility that 
`StringCoding.countPositives` can return a value less than the exact number of 
leading positive bytes. This is the likely cause of a number of intermittent 
but somewhat common errors on aarch64 tests in our CI.

-

Commit messages:
 - Exhaustively consume ASCII bytes in String.decodeASCII

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

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


Re: RFR: 8257733: Move module-specific data from make to respective module [v9]

2022-03-17 Thread Naoto Sato
On Thu, 17 Mar 2022 00:12:38 GMT, Magnus Ihse Bursie  wrote:

>> A lot (but not all) of the data in make/data is tied to a specific module. 
>> For instance, the publicsuffixlist is used by java.base, and fontconfig by 
>> java.desktop. (A few directories, like mainmanifest, is *actually* used by 
>> make for the whole build.) 
>> 
>> These data files should move to the module they belong to. The are, after 
>> all, "source code" for that module that is "compiler" into resulting 
>> deliverables, for that module. (But the "source code" language is not Java 
>> or C, but typically a highly domain specific language or data format, and 
>> the "compilation" is, often, a specialized transformation.) 
>> 
>> This misplacement of the data directory is most visible at code review time. 
>> When such data is changed, most of the time build-dev (or the new build 
>> label) is involved, even though this has nothing to do with the build. While 
>> this is annoying, a worse problem is if the actual team that needs to review 
>> the patch (i.e., the team owning the module) is missed in the review.
>> 
>> ### Modules reviewed
>> 
>> - [x] java.base
>> - [x] java.desktop
>> - [x] jdk.compiler
>> - [x] java.se
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fix typos

LGTM. Thanks for the change, Magnus!

-

Marked as reviewed by naoto (Reviewer).

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


Re: RFR: 8283287: ClassLoader.c cleanups [v2]

2022-03-17 Thread Tyler Steele
> As mentioned in the issue, I'd like to perform the following tidying on 
> ClassLoader.c
> 
> - Alphabetize includes.
> - Replace 'if (ptr == 0)' with 'if (ptr == NULL)'.
> - Replace 'return 0' with 'return NULL'.

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

  removes unneeded errno.h

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7846/files
  - new: https://git.openjdk.java.net/jdk/pull/7846/files/57a92415..920e76fb

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7846=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7846=00-01

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

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


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

2022-03-17 Thread jmehrens
On Wed, 16 Mar 2022 14:54:41 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 one additional 
> commit since the last revision:
> 
>   add since in javadoc

Should RandomWrapper implement toString to include toString representation of 
the RandomGenerator?  I would think that implementations of equals and hashCode 
are not needed but toString might be useful.

-

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


Re: RFR: 8283287: ClassLoader.c cleanups

2022-03-17 Thread Tyler Steele
On Thu, 17 Mar 2022 10:46:35 GMT, Thomas Stuefe  wrote:

>> As mentioned in the issue, I'd like to perform the following tidying on 
>> ClassLoader.c
>> 
>> - Alphabetize includes.
>> - Replace 'if (ptr == 0)' with 'if (ptr == NULL)'.
>> - Replace 'return 0' with 'return NULL'.
>
> src/java.base/share/native/libjava/ClassLoader.c line 27:
> 
>> 25: 
>> 26: #include 
>> 27: #include 
> 
> Whats errno.h for?

Thanks for catching that. It was for [the other 
PR](https://github.com/openjdk/jdk/pull/7829).

-

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


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

2022-03-17 Thread liach
On Wed, 16 Mar 2022 14:54:41 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 one additional 
> commit since the last revision:
> 
>   add since in javadoc

FYI in the CSR review, Joe Darcy points out:

> Okay; I've looked over this CSR in more detail and think some additional 
> supporting changes in java.util.Random should be made to address some 
> existing shortcoming in the spec there.
>
> In particular, I think Random.setSeed's spec should be changed to have a 
> general contract that it is an optional operation, i.e. may throw 
> UnsupportedOperationException as ThreadLocalRandom.setSeed already does. The 
> particulars of Random.setSeed should be moved into an implSpec, which would 
> only apply to that non-overridden method.
>
> (More generally, java.util.Random would likely benefit from a pass to use 
> implNote, implSpec, etc., but I think only setSeed needs to be adjusted for 
> the purposes of this CSR.)
>
> Moving back to Provisional.

So need an update to `Random.setSeed`'s API documentation and update the CSR 
correspondingly.

-

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


Re: Making enum hashcodes consistent across JVM's

2022-03-17 Thread Raffaello Giulietti

Hi,

specifying a fixed hashCode() algorithm has the important drawback that 
it prevents evolution.


For example, as you note, String has a strictly specified algorithm [1] 
that was perhaps a good one in the early days of Java (around 1995) but 
has shown its deficiencies over the years. In particular, it's easy to 
construct collisions, which might impact the performance of popular and 
heavily used hash based data structures, like HashMap and HashSet, under 
some forms of cyberattack.


Unfortunately, while better String hash algorithms have been proposed, 
they cannot be adopted, as too much code out there relies on the 
specific details of String::hashCode. (Even bytecode for switches over 
String relies on that particular spec.) It's far too late now to change 
the spec, even for the better.


In retrospect and IMO, fixing String::hashCode in the spec was a 
(understandable) mistake.



More generally, client code that relies on details of any popular 
library hashCode() implementation has a dangerous dependency on such 
details and impairs evolution of the library itself (as the example with 
String shows).


To leave the door open for better implementations proposed in some 
future, it is best to avoid specifying the details of hashCode(), as 
done in Record::hashCode [2] ("Implementation Requirements"). After all, 
the general contract reads [3]: "This integer [the hash] need not remain 
consistent from one execution of an application to another execution of 
the same application."



But what is the use case you have for which you would like a firmly 
specified Enum::hashCode?



Greetings
Raffaello



[1] 
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/String.html#hashCode()
[2] 
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Record.html#hashCode()
[3] 
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Object.html#hashCode()




On 2022-03-17 13:49, dfranken@gmail.com wrote:

Dear all,

Currently enums do not have a well-defined hashCode() implementation so
they defer to Object.hashCode() which just uses an internal mechanism
to determine the hashcode, likely based on the object's place in the
heap.

This may confuse a lot of developers as other classes such as String do
have a well-defined hashcode which is consistent across multiple JVM's.

Could it be a good idea to make enums' hashCode() method more reliably
and predictable? For instance by using the hashcode of the name of its
value?

For example:

class MyEnum { A, B }
-> A.hashCode() == A.name().hashCode()

Kind regards,

Dave Franken


Re: RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 [v3]

2022-03-17 Thread Roger Riggs
On Thu, 17 Mar 2022 07:44:35 GMT, David Holmes  wrote:

>> Tyler Steele has refreshed the contents of this pull request, and previous 
>> commits have been removed. The incremental views will show differences 
>> compared to the previous content of the PR. The pull request contains one 
>> new commit since the last revision:
>> 
>>   Addresses failure in NullClassTest on AIX.
>>   
>>   - Changes malloc(0) call to malloc(1) on AIX.
>
> src/java.base/share/native/libjava/ClassLoader.c line 102:
> 
>> 100: }
>> 101: 
>> 102: // On malloc(0), implementators of malloc(3) have the choice to 
>> return either
> 
> It is confusing to mix `malloc(0)`, where you are passing an argument zero to 
> malloc, with `malloc(3)` which actually means the definition of malloc as per 
> the man page in section 3.
> 
> Given this is only an issue on AIX the comment can simply say:
> 
> `// On AIX malloc(0) returns NULL which looks like an out-of-memory 
> condition; so adjust it to malloc(1)`.

I would omit the bug number reference, they get stale and do not age well, 
cluttering up the source.
Git blame can be used to find the origin of the comment if needed.

-

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


Re: RFR: 8279508: Auto-vectorize Math.round API [v17]

2022-03-17 Thread Andrew Haley
On Sun, 13 Mar 2022 06:36:15 GMT, Jatin Bhateja  wrote:

>> Summary of changes:
>> - Intrinsify Math.round(float) and Math.round(double) APIs.
>> - Extend auto-vectorizer to infer vector operations on encountering scalar 
>> IR nodes for above intrinsics.
>> - Test creation using new IR testing framework.
>> 
>> Following are the performance number of a JMH micro included with the patch 
>> 
>> Test System: Intel(R) Xeon(R) Platinum 8380 CPU @ 2.30GHz (Icelake Server)
>> 
>> 
>> Benchmark | TESTSIZE | Baseline AVX3 (ops/ms) | Withopt AVX3 (ops/ms) | Gain 
>> ratio | Baseline AVX2 (ops/ms) | Withopt AVX2 (ops/ms) | Gain ratio
>> -- | -- | -- | -- | -- | -- | -- | --
>> FpRoundingBenchmark.test_round_double | 1024.00 | 504.15 | 2209.54 | 4.38 | 
>> 510.36 | 548.39 | 1.07
>> FpRoundingBenchmark.test_round_double | 2048.00 | 293.64 | 1271.98 | 4.33 | 
>> 293.48 | 274.01 | 0.93
>> FpRoundingBenchmark.test_round_float | 1024.00 | 825.99 | 4754.66 | 5.76 | 
>> 751.83 | 2274.13 | 3.02
>> FpRoundingBenchmark.test_round_float | 2048.00 | 412.22 | 2490.09 | 6.04 | 
>> 388.52 | 1334.18 | 3.43
>> 
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8279508: Windows build failure fix.

src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4143:

> 4141:   ldmxcsr(new_mxcsr);
> 4142:   // Move raw bits corresponding to double value 0.5 into scratch 
> register.
> 4143:   mov64(scratch, 4602678819172646912L);

Suggestion:

  mov64(scratch, julong_cast(0.5));

-

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


RFR: 8283279: [Testbug] Improve TestGetSwapSpaceSize

2022-03-17 Thread Severin Gehwolf
Please review this container test improvement. The test in question only makes 
sense iff the total swap space size as reported by the container aware 
OperatingSystemMXBean is `0`. If that's not the case, then something else might 
be amiss, e.g. OperatingSystemMXBean reporting the host swap limits. In such a 
case a passing test tells us nothing. Certainly not if the
fix from [JDK-8242480](https://bugs.openjdk.java.net/browse/JDK-8242480) is 
present or not.

Testing:
- [x] Manual with and without the code fix of JDK-8242480. Still passes with 
the fix, and fails without. Tested the test on cgroups v1 and cgroups v2.

-

Commit messages:
 - 8283279: [Testbug] Improve TestGetSwapSpaceSize

Changes: https://git.openjdk.java.net/jdk/pull/7854/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7854=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8283279
  Stats: 29 lines in 2 files changed: 22 ins; 1 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7854.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7854/head:pull/7854

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


Re: RFR: 8257733: Move module-specific data from make to respective module [v9]

2022-03-17 Thread Erik Joelsson
On Thu, 17 Mar 2022 00:12:38 GMT, Magnus Ihse Bursie  wrote:

>> A lot (but not all) of the data in make/data is tied to a specific module. 
>> For instance, the publicsuffixlist is used by java.base, and fontconfig by 
>> java.desktop. (A few directories, like mainmanifest, is *actually* used by 
>> make for the whole build.) 
>> 
>> These data files should move to the module they belong to. The are, after 
>> all, "source code" for that module that is "compiler" into resulting 
>> deliverables, for that module. (But the "source code" language is not Java 
>> or C, but typically a highly domain specific language or data format, and 
>> the "compilation" is, often, a specialized transformation.) 
>> 
>> This misplacement of the data directory is most visible at code review time. 
>> When such data is changed, most of the time build-dev (or the new build 
>> label) is involved, even though this has nothing to do with the build. While 
>> this is annoying, a worse problem is if the actual team that needs to review 
>> the patch (i.e., the team owning the module) is missed in the review.
>> 
>> ### Modules reviewed
>> 
>> - [x] java.base
>> - [x] java.desktop
>> - [x] jdk.compiler
>> - [x] java.se
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fix typos

Marked as reviewed by erikj (Reviewer).

-

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


Re: Making enum hashcodes consistent across JVM's

2022-03-17 Thread Remi Forax
- Original Message -
> From: "dfranken jdk" 
> To: "core-libs-dev" 
> Sent: Thursday, March 17, 2022 1:49:08 PM
> Subject: Making enum hashcodes consistent across JVM's

> Dear all,
> 
> Currently enums do not have a well-defined hashCode() implementation so
> they defer to Object.hashCode() which just uses an internal mechanism
> to determine the hashcode, likely based on the object's place in the
> heap.

No, and no.

Enum.hashCode() is well defined, it is equivalent to super.hashCode() and it 
can not be overridden by a specific enum (see the javadoc).

> 
> This may confuse a lot of developers as other classes such as String do
> have a well-defined hashcode which is consistent across multiple JVM's.

One of the reasons why String.hashCode() is defined is because the compiler 
(javac) compiles a switch on String using String.hashCode() so the hashCode at 
compile time and at runtime has to be the same.

Having a well defined hashCode was a mistake, first it means that we can not 
change the implementation later (the current hashCode of String has too many 
collisions) and it also make DDOS attacks easier because you can pre-compute a 
list of strings with the same hashCode.

> 
> Could it be a good idea to make enums' hashCode() method more reliably
> and predictable? For instance by using the hashcode of the name of its
> value?

Enum already have a kind of perfect hash, ordinal() which is reliable and 
predictable but it's not a great value as hashCode.

> 
> For example:
> 
> class MyEnum { A, B }
> -> A.hashCode() == A.name().hashCode()

This is not a good hashCode,

  enum Foo { Aa, BB }
  Foo.Aa.hashCode() == Foo.BB.hashCode()


> 
> Kind regards,
> 
> Dave Franken

regards,
Rémi


Making enum hashcodes consistent across JVM's

2022-03-17 Thread dfranken . jdk
Dear all,

Currently enums do not have a well-defined hashCode() implementation so
they defer to Object.hashCode() which just uses an internal mechanism
to determine the hashcode, likely based on the object's place in the
heap.

This may confuse a lot of developers as other classes such as String do
have a well-defined hashcode which is consistent across multiple JVM's.

Could it be a good idea to make enums' hashCode() method more reliably
and predictable? For instance by using the hashcode of the name of its
value?

For example:

class MyEnum { A, B }
-> A.hashCode() == A.name().hashCode()

Kind regards,

Dave Franken


Re: RFR: 8283287: Spring Cleaning for ClassLoader.c

2022-03-17 Thread Thomas Stuefe
On Wed, 16 Mar 2022 21:25:37 GMT, Tyler Steele  wrote:

> As mentioned in the issue, I'd like to perform the following tidying on 
> ClassLoader.c
> 
> - Alphabetize includes.
> - Replace 'if (ptr == 0)' with 'if (ptr == NULL)'.
> - Replace 'return 0' with 'return NULL'.

Looks good in general. Question inline.

src/java.base/share/native/libjava/ClassLoader.c line 27:

> 25: 
> 26: #include 
> 27: #include 

Whats errno.h for?

-

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


Integrated: 8281146: Replace StringCoding.hasNegatives with countPositives

2022-03-17 Thread Claes Redestad
On Wed, 26 Jan 2022 12:51:31 GMT, Claes Redestad  wrote:

> I'm requesting comments and, hopefully, some help with this patch to replace 
> `StringCoding.hasNegatives` with `countPositives`. The new method does a very 
> similar pass, but alters the intrinsic to return the number of leading bytes 
> in the `byte[]` range which only has positive bytes. This allows for dealing 
> much more efficiently with those `byte[]`s that has a ASCII prefix, with no 
> measurable cost on ASCII-only or latin1/UTF16-mostly input.
> 
> Microbenchmark results: 
> https://jmh.morethan.io/?gists=428b487e92e3e47ccb7f169501600a88,3c585de7435506d3a3bdb32160fe8904
> 
> - Only implemented on x86 for now, but I want to verify that implementations 
> of `countPositives` can be implemented with similar efficiency on all 
> platforms that today implement a `hasNegatives` intrinsic (aarch64, ppc etc) 
> before moving ahead. This pretty much means holding up this until it's 
> implemented on all platforms, which can either contributed to this PR or as 
> dependent follow-ups.
> 
> - An alternative to holding up until all platforms are on board is to allow 
> the implementation of `StringCoding.hasNegatives` and `countPositives` to be 
> implemented so that the non-intrinsified method calls into the intrinsified. 
> This requires structuring the implementations differently based on which 
> intrinsic - if any - is actually implemented. One way to do this could be to 
> mimic how `java.nio` handles unaligned accesses and expose which intrinsic is 
> available via `Unsafe` into a `static final` field.
> 
> - There are a few minor regressions (~5%) in the x86 implementation on 
> `encode-/decodeLatin1Short`. Those regressions disappear when mixing inputs, 
> for example `encode-/decodeShortMixed` even see a minor improvement, which 
> makes me consider those corner case regressions with little real world 
> implications (if you have latin1 Strings, you're likely to also have 
> ASCII-only strings in your mix).

This pull request has now been integrated.

Changeset: beedae11
Author:Claes Redestad 
URL:   
https://git.openjdk.java.net/jdk/commit/beedae1141b6b650dc4cedf1f038afc1c8b460dd
Stats: 619 lines in 36 files changed: 278 ins; 61 del; 280 mod

8281146: Replace StringCoding.hasNegatives with countPositives

Co-authored-by: Lutz Schmidt 
Co-authored-by: Martin Doerr 
Reviewed-by: kvn, lucy, rriggs

-

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


Re: RFR: 8281146: Replace StringCoding.hasNegatives with countPositives [v16]

2022-03-17 Thread Claes Redestad
> I'm requesting comments and, hopefully, some help with this patch to replace 
> `StringCoding.hasNegatives` with `countPositives`. The new method does a very 
> similar pass, but alters the intrinsic to return the number of leading bytes 
> in the `byte[]` range which only has positive bytes. This allows for dealing 
> much more efficiently with those `byte[]`s that has a ASCII prefix, with no 
> measurable cost on ASCII-only or latin1/UTF16-mostly input.
> 
> Microbenchmark results: 
> https://jmh.morethan.io/?gists=428b487e92e3e47ccb7f169501600a88,3c585de7435506d3a3bdb32160fe8904
> 
> - Only implemented on x86 for now, but I want to verify that implementations 
> of `countPositives` can be implemented with similar efficiency on all 
> platforms that today implement a `hasNegatives` intrinsic (aarch64, ppc etc) 
> before moving ahead. This pretty much means holding up this until it's 
> implemented on all platforms, which can either contributed to this PR or as 
> dependent follow-ups.
> 
> - An alternative to holding up until all platforms are on board is to allow 
> the implementation of `StringCoding.hasNegatives` and `countPositives` to be 
> implemented so that the non-intrinsified method calls into the intrinsified. 
> This requires structuring the implementations differently based on which 
> intrinsic - if any - is actually implemented. One way to do this could be to 
> mimic how `java.nio` handles unaligned accesses and expose which intrinsic is 
> available via `Unsafe` into a `static final` field.
> 
> - There are a few minor regressions (~5%) in the x86 implementation on 
> `encode-/decodeLatin1Short`. Those regressions disappear when mixing inputs, 
> for example `encode-/decodeShortMixed` even see a minor improvement, which 
> makes me consider those corner case regressions with little real world 
> implications (if you have latin1 Strings, you're likely to also have 
> ASCII-only strings in your mix).

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

  Disallow negative values in TestCountPositives test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7231/files
  - new: https://git.openjdk.java.net/jdk/pull/7231/files/bc5a8c80..6f22e1aa

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7231=15
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7231=14-15

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

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2

2022-03-17 Thread Kim Barrett
On Fri, 11 Mar 2022 23:38:10 GMT, Mikael Vidstedt  wrote:

> Background, from JBS:
> 
> src/java.base/share/native/libverify/check_code.c: In function 
> 'read_all_code': 
> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' may 
> be used uninitialized [-Werror=maybe-uninitialized] 
>   942 | check_and_push(context, lengths, VM_MALLOC_BLK); 
>   | ^~~ 
> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
> 2 of type 'const void *' to 'check_and_push' declared here 
>  4145 | static void check_and_push(context_type *context, const void *ptr, 
> int kind) 
>   | ^~ 
> 
> 
> Because the second argument of check_and_push is "const void*" GCC assumes 
> that the malloc:ed data, which has not yet been initialized, will not be/can 
> not be modified later which in turn suggests it may be used without ever 
> being initialized. 
> 
> The same general issue was addressed in 
> [JDK-8266168](https://bugs.openjdk.java.net/browse/JDK-8266168), presumably 
> for GCC 11.1.
> 
> 
> Details:
> 
> Instead of sprinkling more calloc calls around or using pragmas/gcc 
> attributes I chose to change the check_and_push function to take a 
> (non-const) void* argument, and provide a new wrapper function 
> `check_and_push_const` which handles the const argument case. For the 
> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
> const conversion.
> 
> To avoid having multiple ways of solving the same problem I also chose to 
> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
> call.
> 
> Testing:
> 
> tier1 + builds-tier{2,3,4,5}

Changes requested by kbarrett (Reviewer).

-

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2

2022-03-17 Thread Kim Barrett
On Thu, 17 Mar 2022 08:08:07 GMT, Kim Barrett  wrote:

>> Background, from JBS:
>> 
>> src/java.base/share/native/libverify/check_code.c: In function 
>> 'read_all_code': 
>> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' 
>> may be used uninitialized [-Werror=maybe-uninitialized] 
>>   942 | check_and_push(context, lengths, VM_MALLOC_BLK); 
>>   | ^~~ 
>> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
>> 2 of type 'const void *' to 'check_and_push' declared here 
>>  4145 | static void check_and_push(context_type *context, const void *ptr, 
>> int kind) 
>>   | ^~ 
>> 
>> 
>> Because the second argument of check_and_push is "const void*" GCC assumes 
>> that the malloc:ed data, which has not yet been initialized, will not be/can 
>> not be modified later which in turn suggests it may be used without ever 
>> being initialized. 
>> 
>> The same general issue was addressed in 
>> [JDK-8266168](https://bugs.openjdk.java.net/browse/JDK-8266168), presumably 
>> for GCC 11.1.
>> 
>> 
>> Details:
>> 
>> Instead of sprinkling more calloc calls around or using pragmas/gcc 
>> attributes I chose to change the check_and_push function to take a 
>> (non-const) void* argument, and provide a new wrapper function 
>> `check_and_push_const` which handles the const argument case. For the 
>> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
>> const conversion.
>> 
>> To avoid having multiple ways of solving the same problem I also chose to 
>> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
>> call.
>> 
>> Testing:
>> 
>> tier1 + builds-tier{2,3,4,5}
>
> src/java.base/share/native/libverify/check_code.c line 472:
> 
>> 470: 
>> 471: static void check_and_push(context_type *context, void *ptr, int kind);
>> 472: static void check_and_push_const(context_type *context, const void 
>> *ptr, int kind);
> 
> This really needs a comment explaining why two functions are needed.

I think it would be clearer to drop the kind argument and have two functions
check_and_push_vm_string(context_type*, const char*)
check_and_push_malloc_block(context_type*, void*)
with a shared helper that is given the appropriate kind.

-

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2

2022-03-17 Thread Kim Barrett
On Thu, 17 Mar 2022 06:59:01 GMT, David Holmes  wrote:

>> Background, from JBS:
>> 
>> src/java.base/share/native/libverify/check_code.c: In function 
>> 'read_all_code': 
>> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' 
>> may be used uninitialized [-Werror=maybe-uninitialized] 
>>   942 | check_and_push(context, lengths, VM_MALLOC_BLK); 
>>   | ^~~ 
>> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
>> 2 of type 'const void *' to 'check_and_push' declared here 
>>  4145 | static void check_and_push(context_type *context, const void *ptr, 
>> int kind) 
>>   | ^~ 
>> 
>> 
>> Because the second argument of check_and_push is "const void*" GCC assumes 
>> that the malloc:ed data, which has not yet been initialized, will not be/can 
>> not be modified later which in turn suggests it may be used without ever 
>> being initialized. 
>> 
>> The same general issue was addressed in 
>> [JDK-8266168](https://bugs.openjdk.java.net/browse/JDK-8266168), presumably 
>> for GCC 11.1.
>> 
>> 
>> Details:
>> 
>> Instead of sprinkling more calloc calls around or using pragmas/gcc 
>> attributes I chose to change the check_and_push function to take a 
>> (non-const) void* argument, and provide a new wrapper function 
>> `check_and_push_const` which handles the const argument case. For the 
>> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
>> const conversion.
>> 
>> To avoid having multiple ways of solving the same problem I also chose to 
>> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
>> call.
>> 
>> Testing:
>> 
>> tier1 + builds-tier{2,3,4,5}
>
> src/java.base/share/native/libverify/check_code.c line 4168:
> 
>> 4166: }
>> 4167: 
>> 4168: static void check_and_push_const(context_type *context, const void 
>> *ptr, int kind) {
> 
> IIUC the only `const` cases all pass a `const char*` so perhaps we can make 
> that explicit in the signature? I confess I can't figure out how the passed 
> in memory eventually gets used and it seems bizarre that it could be a 
> freshly malloc'd block or an existing string.

David - The check_and_push idiom checks if ptr is null, doing the out of memory 
handling if so.  Otherwise, add ptr to a collection for later release.  The 
kind determines how the release is done, either JVM_ReleaseUTF or free.

-

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2

2022-03-17 Thread Kim Barrett
On Fri, 11 Mar 2022 23:38:10 GMT, Mikael Vidstedt  wrote:

> Background, from JBS:
> 
> src/java.base/share/native/libverify/check_code.c: In function 
> 'read_all_code': 
> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' may 
> be used uninitialized [-Werror=maybe-uninitialized] 
>   942 | check_and_push(context, lengths, VM_MALLOC_BLK); 
>   | ^~~ 
> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
> 2 of type 'const void *' to 'check_and_push' declared here 
>  4145 | static void check_and_push(context_type *context, const void *ptr, 
> int kind) 
>   | ^~ 
> 
> 
> Because the second argument of check_and_push is "const void*" GCC assumes 
> that the malloc:ed data, which has not yet been initialized, will not be/can 
> not be modified later which in turn suggests it may be used without ever 
> being initialized. 
> 
> The same general issue was addressed in 
> [JDK-8266168](https://bugs.openjdk.java.net/browse/JDK-8266168), presumably 
> for GCC 11.1.
> 
> 
> Details:
> 
> Instead of sprinkling more calloc calls around or using pragmas/gcc 
> attributes I chose to change the check_and_push function to take a 
> (non-const) void* argument, and provide a new wrapper function 
> `check_and_push_const` which handles the const argument case. For the 
> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
> const conversion.
> 
> To avoid having multiple ways of solving the same problem I also chose to 
> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
> call.
> 
> Testing:
> 
> tier1 + builds-tier{2,3,4,5}

Changes requested by kbarrett (Reviewer).

src/java.base/share/native/libverify/check_code.c line 472:

> 470: 
> 471: static void check_and_push(context_type *context, void *ptr, int kind);
> 472: static void check_and_push_const(context_type *context, const void *ptr, 
> int kind);

This really needs a comment explaining why two functions are needed.

-

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


Re: RFR: 8283237: CallSite should be a sealed class [v2]

2022-03-17 Thread liach
> Change `CallSite` to a sealed class, as `CallSite` is an abstract class which 
> does not allow direct subclassing by users per its documentation. Since I 
> don't have a JBS account, I posted the content for the CSR in a GitHub Gist 
> at https://gist.github.com/150d5aa7f8b13a4deddf95969ad39d73 and wish someone 
> can submit a CSR for me.

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

  Swap mutable and volatile in the permits list

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7840/files
  - new: https://git.openjdk.java.net/jdk/pull/7840/files/ff8996e6..eef61c31

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7840=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7840=00-01

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

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


Re: RFR: 8283237: CallSite should be a sealed class

2022-03-17 Thread Rémi Forax
On Thu, 17 Mar 2022 07:32:40 GMT, liach  wrote:

>> src/java.base/share/classes/java/lang/invoke/CallSite.java line 88:
>> 
>>> 86:  */
>>> 87: public
>>> 88: abstract sealed class CallSite permits ConstantCallSite, 
>>> VolatileCallSite, MutableCallSite {
>> 
>> Nitpicking with my JSR 292 hat,
>> given that the permits clause is reflected in the javadoc,
>> the order should be ConstantCS, MutableCS and VolatileCS,
>> it's both in the lexical order and in the "memory access" of setTarget() 
>> order , from stronger access to weaker access.
>
> I agree that Constant, Mutable, Volatile order is better, ranked by the 
> respective cost for `setTarget()` and (possibly) invocation, and earlier ones 
> in the list are more preferable if conditions allow.
> 
> However, in the current API documentation, the order is Constant, Mutable, 
> and Volatile. Should I update that or leave it?
> 
> /*
>  * 
>  * If a mutable target is not required, an {@code invokedynamic} 
> instruction
>  * may be permanently bound by means of a {@linkplain ConstantCallSite 
> constant call site}.
>  * If a mutable target is required which has volatile variable semantics,
>  * because updates to the target must be immediately and reliably witnessed 
> by other threads,
>  * a {@linkplain VolatileCallSite volatile call site} may be used.
>  * Otherwise, if a mutable target is required,
>  * a {@linkplain MutableCallSite mutable call site} may be used.
>  * 
>  */

For me, this is unrelated, for this paragraph it's easier to explain the 
semantics of MutableCallSite with an otherwise, i.e. it's mutable but you do 
not want the cost of a volatile acces.

-

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


Re: RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 [v3]

2022-03-17 Thread Thomas Stuefe
On Wed, 16 Mar 2022 21:10:14 GMT, Tyler Steele  wrote:

>> As described in the linked issue, NullClassBytesTest fails due an 
>> OutOfMemoryError produced on AIX when the test calls defineClass with a byte 
>> array of size of 0. The native implementation of defineClass then calls  
>> malloc with a size of 0. On AIX malloc(0) returns NULL, while on other 
>> platforms it return a valid address. When NULL is produced by malloc for 
>> this reason, ClassLoader.c incorrectly interprets this as a failure due to a 
>> lack of memory.
>> 
>> ~~This PR modifies ClassLoader.c to produce an OutOfMemoryError only when 
>> `errno == ENOMEM` and to produce a ClassFormatError with the message 
>> "ClassLoader internal allocation failure" in all other cases (in which 
>> malloc returns NULL).~~ [edit: The above no longer describes the PR's 
>> proposed fix. See discussion below]
>> 
>> In addition, I performed some minor tidy-up work in ClassLoader.c by 
>> changing instances of `return 0` to `return NULL`, and `if (some_ptr == 0)` 
>> to `if (some_ptr == NULL)`. This was done to improve the clarity of the code 
>> in ClassLoader.c, but didn't feel worthy of opening a separate issue.
>> 
>> ### Alternatives
>> 
>> It would be possible to address this failure by modifying the test to accept 
>> the OutOfMemoryError on AIX. I thought it was a better solution to modify 
>> ClassLoader.c to produce an OutOfMemoryError only when the system is 
>> actually out of memory.
>> 
>> ### Testing
>> 
>> This change has been tested on AIX and Linux/x86.
>
> Tyler Steele has refreshed the contents of this pull request, and previous 
> commits have been removed. The incremental views will show differences 
> compared to the previous content of the PR. The pull request contains one new 
> commit since the last revision:
> 
>   Addresses failure in NullClassTest on AIX.
>   
>   - Changes malloc(0) call to malloc(1) on AIX.

p.s. this issue puts the finger on a sore point. We have corrected mallocs in 
the JDK in a number of places, e.g. in libverify:

(https://github.com/openjdk/jdk/blob/3da5204b3c3a3f95bddcdcfe2628c2e0ed8a12d9/src/java.base/share/native/libverify/check_code.c#L91-L102)

(which is also strictly speaking incorrect since the spec says to return a 
*non-unique* pointer).

More generally, we have with AIX the problem that APIs have different behavior 
than the more usual nixes or are missing, and we need a porting layer to 
provide missing or change different APIs. Beside AIX-ish malloc, one example is 
dladdr, which is missing.

In hotspot, we have os/aix, so we are fine. See 
`os/aix/hotspot/os/aix/porting_aix.cpp`. In the JDK I never found a good place 
for a porting layer, since the different JDK binaries don't have a common 
layer. So we have multiple versions of aix malloc and dladdr sprinkled across 
the libraries, which I always found embarrassing. (outside hotspot we implement 
dladdr at least in java.desktop/aix/native/libawt/porting_aix.c and 
java.base/aix/native/libjli/java_md_aix.c).

If you find a way to commonize that code across JDK libraries, that would be 
cool. I even thought about providing a porting library completely independent 
from the OpenJDK itself, more like a system library. We did this for our 
internal iSeries port but the logistics were annoying, so we did not do it for 
AIX. But you at IBM may have a better idea.

Cheers, Thomas

-

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


Re: RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 [v3]

2022-03-17 Thread Thomas Stuefe
On Thu, 17 Mar 2022 07:44:39 GMT, David Holmes  wrote:

> Using AIX_ONLY this can be simplified:
> 
> `body = (jbyte *)malloc(length AIX_ONLY( == 0 ? 1 : length));`

This is jdk, not hotspot. Do we have AIX_ONLY in the JDK?

-

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


Re: RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 [v3]

2022-03-17 Thread David Holmes
On Wed, 16 Mar 2022 21:10:14 GMT, Tyler Steele  wrote:

>> As described in the linked issue, NullClassBytesTest fails due an 
>> OutOfMemoryError produced on AIX when the test calls defineClass with a byte 
>> array of size of 0. The native implementation of defineClass then calls  
>> malloc with a size of 0. On AIX malloc(0) returns NULL, while on other 
>> platforms it return a valid address. When NULL is produced by malloc for 
>> this reason, ClassLoader.c incorrectly interprets this as a failure due to a 
>> lack of memory.
>> 
>> ~~This PR modifies ClassLoader.c to produce an OutOfMemoryError only when 
>> `errno == ENOMEM` and to produce a ClassFormatError with the message 
>> "ClassLoader internal allocation failure" in all other cases (in which 
>> malloc returns NULL).~~ [edit: The above no longer describes the PR's 
>> proposed fix. See discussion below]
>> 
>> In addition, I performed some minor tidy-up work in ClassLoader.c by 
>> changing instances of `return 0` to `return NULL`, and `if (some_ptr == 0)` 
>> to `if (some_ptr == NULL)`. This was done to improve the clarity of the code 
>> in ClassLoader.c, but didn't feel worthy of opening a separate issue.
>> 
>> ### Alternatives
>> 
>> It would be possible to address this failure by modifying the test to accept 
>> the OutOfMemoryError on AIX. I thought it was a better solution to modify 
>> ClassLoader.c to produce an OutOfMemoryError only when the system is 
>> actually out of memory.
>> 
>> ### Testing
>> 
>> This change has been tested on AIX and Linux/x86.
>
> Tyler Steele has refreshed the contents of this pull request, and previous 
> commits have been removed. The incremental views will show differences 
> compared to the previous content of the PR. The pull request contains one new 
> commit since the last revision:
> 
>   Addresses failure in NullClassTest on AIX.
>   
>   - Changes malloc(0) call to malloc(1) on AIX.

src/java.base/share/native/libjava/ClassLoader.c line 102:

> 100: }
> 101: 
> 102: // On malloc(0), implementators of malloc(3) have the choice to 
> return either

It is confusing to mix `malloc(0)`, where you are passing an argument zero to 
malloc, with `malloc(3)` which actually means the definition of malloc as per 
the man page in section 3.

Given this is only an issue on AIX the comment can simply say:

`// On AIX malloc(0) returns NULL which looks like an out-of-memory condition; 
so adjust it to malloc(1)`.

src/java.base/share/native/libjava/ClassLoader.c line 106:

> 104: // we chose the latter. (see 8283225)
> 105: #ifdef _AIX
> 106: body = (jbyte *)malloc(length == 0 ? 1 : length);

Using AIX_ONLY this can be simplified:

`body = (jbyte *)malloc(length AIX_ONLY( == 0 ? 1 : length));`

-

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


Re: RFR: 8283237: CallSite should be a sealed class

2022-03-17 Thread liach
On Thu, 17 Mar 2022 06:47:13 GMT, Rémi Forax  wrote:

>> Change `CallSite` to a sealed class, as `CallSite` is an abstract class 
>> which does not allow direct subclassing by users per its documentation. 
>> Since I don't have a JBS account, I posted the content for the CSR in a 
>> GitHub Gist at https://gist.github.com/150d5aa7f8b13a4deddf95969ad39d73 and 
>> wish someone can submit a CSR for me.
>
> src/java.base/share/classes/java/lang/invoke/CallSite.java line 88:
> 
>> 86:  */
>> 87: public
>> 88: abstract sealed class CallSite permits ConstantCallSite, 
>> VolatileCallSite, MutableCallSite {
> 
> Nitpicking with my JSR 292 hat,
> given that the permits clause is reflected in the javadoc,
> the order should be ConstantCS, MutableCS and VolatileCS,
> it's both in the lexical order and in the "memory access" of setTarget() 
> order , from stronger access to weaker access.

I agree that Constant, Mutable, Volatile order is better, ranked by the 
respective cost for `setTarget()` and (possibly) invocation, and earlier ones 
in the list are more preferable if conditions allow.

However, in the current API documentation, the order is Constant, Mutable, and 
Volatile. Should I update that or leave it?

/*
 * 
 * If a mutable target is not required, an {@code invokedynamic} instruction
 * may be permanently bound by means of a {@linkplain ConstantCallSite constant 
call site}.
 * If a mutable target is required which has volatile variable semantics,
 * because updates to the target must be immediately and reliably witnessed by 
other threads,
 * a {@linkplain VolatileCallSite volatile call site} may be used.
 * Otherwise, if a mutable target is required,
 * a {@linkplain MutableCallSite mutable call site} may be used.
 * 
 */

-

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


Re: RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 [v3]

2022-03-17 Thread Thomas Stuefe
On Wed, 16 Mar 2022 21:10:14 GMT, Tyler Steele  wrote:

>> As described in the linked issue, NullClassBytesTest fails due an 
>> OutOfMemoryError produced on AIX when the test calls defineClass with a byte 
>> array of size of 0. The native implementation of defineClass then calls  
>> malloc with a size of 0. On AIX malloc(0) returns NULL, while on other 
>> platforms it return a valid address. When NULL is produced by malloc for 
>> this reason, ClassLoader.c incorrectly interprets this as a failure due to a 
>> lack of memory.
>> 
>> ~~This PR modifies ClassLoader.c to produce an OutOfMemoryError only when 
>> `errno == ENOMEM` and to produce a ClassFormatError with the message 
>> "ClassLoader internal allocation failure" in all other cases (in which 
>> malloc returns NULL).~~ [edit: The above no longer describes the PR's 
>> proposed fix. See discussion below]
>> 
>> In addition, I performed some minor tidy-up work in ClassLoader.c by 
>> changing instances of `return 0` to `return NULL`, and `if (some_ptr == 0)` 
>> to `if (some_ptr == NULL)`. This was done to improve the clarity of the code 
>> in ClassLoader.c, but didn't feel worthy of opening a separate issue.
>> 
>> ### Alternatives
>> 
>> It would be possible to address this failure by modifying the test to accept 
>> the OutOfMemoryError on AIX. I thought it was a better solution to modify 
>> ClassLoader.c to produce an OutOfMemoryError only when the system is 
>> actually out of memory.
>> 
>> ### Testing
>> 
>> This change has been tested on AIX and Linux/x86.
>
> Tyler Steele has refreshed the contents of this pull request, and previous 
> commits have been removed. The incremental views will show differences 
> compared to the previous content of the PR. The pull request contains one new 
> commit since the last revision:
> 
>   Addresses failure in NullClassTest on AIX.
>   
>   - Changes malloc(0) call to malloc(1) on AIX.

This is much better, thanks.

Cheers, Thomas

-

Marked as reviewed by stuefe (Reviewer).

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


Re: RFR: 8283059: Uninitialized warning in check_code.c with GCC 11.2

2022-03-17 Thread David Holmes
On Fri, 11 Mar 2022 23:38:10 GMT, Mikael Vidstedt  wrote:

> Background, from JBS:
> 
> src/java.base/share/native/libverify/check_code.c: In function 
> 'read_all_code': 
> src/java.base/share/native/libverify/check_code.c:942:5: error: 'lengths' may 
> be used uninitialized [-Werror=maybe-uninitialized] 
>   942 | check_and_push(context, lengths, VM_MALLOC_BLK); 
>   | ^~~ 
> src/java.base/share/native/libverify/check_code.c:4145:13: note: by argument 
> 2 of type 'const void *' to 'check_and_push' declared here 
>  4145 | static void check_and_push(context_type *context, const void *ptr, 
> int kind) 
>   | ^~ 
> 
> 
> Because the second argument of check_and_push is "const void*" GCC assumes 
> that the malloc:ed data, which has not yet been initialized, will not be/can 
> not be modified later which in turn suggests it may be used without ever 
> being initialized. 
> 
> The same general issue was addressed in 
> [JDK-8266168](https://bugs.openjdk.java.net/browse/JDK-8266168), presumably 
> for GCC 11.1.
> 
> 
> Details:
> 
> Instead of sprinkling more calloc calls around or using pragmas/gcc 
> attributes I chose to change the check_and_push function to take a 
> (non-const) void* argument, and provide a new wrapper function 
> `check_and_push_const` which handles the const argument case. For the 
> (non-const) VM_MALLOC_BKP that means the pointer never needs to go through a 
> const conversion.
> 
> To avoid having multiple ways of solving the same problem I also chose to 
> revert the change made in JDK-8266168, reverting the calloc back to a malloc 
> call.
> 
> Testing:
> 
> tier1 + builds-tier{2,3,4,5}

src/java.base/share/native/libverify/check_code.c line 4168:

> 4166: }
> 4167: 
> 4168: static void check_and_push_const(context_type *context, const void 
> *ptr, int kind) {

IIUC the only `const` cases all pass a `const char*` so perhaps we can make 
that explicit in the signature? I confess I can't figure out how the passed in 
memory eventually gets used and it seems bizarre that it could be a freshly 
malloc'd block or an existing string.

-

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


Re: RFR: 8283237: CallSite should be a sealed class

2022-03-17 Thread Rémi Forax
On Wed, 16 Mar 2022 13:09:30 GMT, liach  wrote:

> Change `CallSite` to a sealed class, as `CallSite` is an abstract class which 
> does not allow direct subclassing by users per its documentation. Since I 
> don't have a JBS account, I posted the content for the CSR in a GitHub Gist 
> at https://gist.github.com/150d5aa7f8b13a4deddf95969ad39d73 and wish someone 
> can submit a CSR for me.

src/java.base/share/classes/java/lang/invoke/CallSite.java line 88:

> 86:  */
> 87: public
> 88: abstract sealed class CallSite permits ConstantCallSite, 
> VolatileCallSite, MutableCallSite {

Nitpicking with my JSR 292 hat,
given that the permits clause is reflected in the javadoc,
the order should be ConstantCS, MutableCS and VolatileCS,
it's both in the lexical order and in the "memory access" of setTarget() order 
, from stronger access to weaker access.

-

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