RFR: JDK-8299470: sun/jvm/hotspot/SALauncher.java handling of negative rmiport args
The test serviceability/sa/sadebugd/SADebugDTest.java can pass under some circumstances a negative rmiport (--rmiport -1) to SALauncher.java. This leads to a somewhat misleading message `[debugd] Argument is expected for 'rmiport' ` (we set an argument [-1] but probably this is not what is really expected) and additionally the real exception is not shown. Probably also a warning in case of negative rmiport values should be printed because they seem to lead to errors. - Commit messages: - JDK-8299470 Changes: https://git.openjdk.org/jdk/pull/11811/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11811&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8299470 Stats: 6 lines in 1 file changed: 5 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11811.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11811/head:pull/11811 PR: https://git.openjdk.org/jdk/pull/11811
Re: RFR: JDK-8299470: sun/jvm/hotspot/SALauncher.java handling of negative rmiport args
On Mon, 2 Jan 2023 14:31:14 GMT, Matthias Baesken wrote: > The test serviceability/sa/sadebugd/SADebugDTest.java can pass under some > circumstances a negative rmiport (--rmiport -1) to SALauncher.java. > This leads to a somewhat misleading message > `[debugd] Argument is expected for 'rmiport' ` > (we set an argument [-1] but probably this is not what is really expected) > and additionally the real exception is not shown. > Probably also a warning in case of negative rmiport values should be printed > because they seem to lead to errors. I adjusted the exception message a bit to show what is wrong (-1 as an argument is not expected/supported). Removed one warning output because it seems we do not get there. Any reviews ? - PR: https://git.openjdk.org/jdk/pull/11811
Re: RFR: JDK-8299470: sun/jvm/hotspot/SALauncher.java handling of negative rmiport args [v2]
> The test serviceability/sa/sadebugd/SADebugDTest.java can pass under some > circumstances a negative rmiport (--rmiport -1) to SALauncher.java. > This leads to a somewhat misleading message > `[debugd] Argument is expected for 'rmiport' ` > (we set an argument [-1] but probably this is not what is really expected) > and additionally the real exception is not shown. > Probably also a warning in case of negative rmiport values should be printed > because they seem to lead to errors. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Improve exception message for SAGetoptException - Changes: - all: https://git.openjdk.org/jdk/pull/11811/files - new: https://git.openjdk.org/jdk/pull/11811/files/0844170b..9c5d46d8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11811&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11811&range=00-01 Stats: 4 lines in 2 files changed: 0 ins; 3 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11811.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11811/head:pull/11811 PR: https://git.openjdk.org/jdk/pull/11811
Re: RFR: JDK-8299470: sun/jvm/hotspot/SALauncher.java handling of negative rmiport args [v3]
> The test serviceability/sa/sadebugd/SADebugDTest.java can pass under some > circumstances a negative rmiport (--rmiport -1) to SALauncher.java. > This leads to a somewhat misleading message > `[debugd] Argument is expected for 'rmiport' ` > (we set an argument [-1] but probably this is not what is really expected) > and additionally the real exception is not shown. > Probably also a warning in case of negative rmiport values should be printed > because they seem to lead to errors. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust Copyright years, omit one output line - Changes: - all: https://git.openjdk.org/jdk/pull/11811/files - new: https://git.openjdk.org/jdk/pull/11811/files/9c5d46d8..d42cc159 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11811&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11811&range=01-02 Stats: 4 lines in 2 files changed: 0 ins; 1 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/11811.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11811/head:pull/11811 PR: https://git.openjdk.org/jdk/pull/11811
Re: RFR: JDK-8299470: sun/jvm/hotspot/SALauncher.java handling of negative rmiport args [v3]
On Wed, 4 Jan 2023 14:19:18 GMT, Kevin Walls wrote:
> In the "if (useRmiPort) {" block, we should be failing the test if rmiPort is
> -1, saying something like "cannot find an rmiPort, findUnreservedFreePort
> returns -1"
>
> A similar abort if setting registryPort gets -1 might also be good?
Hi Kevin, I think you are correct, adjusting the test
test/hotspot/jtreg/serviceability/sa/sadebugd/SADebugDTest.java in those two
cases (rmiPort / registryPort -1) probably makes sense. Should we do it in
another JBS issue or here ?
Additionally , should we fail the test in these cases , or pass with some
warning - message ?
-
PR: https://git.openjdk.org/jdk/pull/11811
Re: RFR: JDK-8299470: sun/jvm/hotspot/SALauncher.java handling of negative rmiport args [v4]
> The test serviceability/sa/sadebugd/SADebugDTest.java can pass under some > circumstances a negative rmiport (--rmiport -1) to SALauncher.java. > This leads to a somewhat misleading message > `[debugd] Argument is expected for 'rmiport' ` > (we set an argument [-1] but probably this is not what is really expected) > and additionally the real exception is not shown. > Probably also a warning in case of negative rmiport values should be printed > because they seem to lead to errors. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust SADebugDTest in case we get port -1 - Changes: - all: https://git.openjdk.org/jdk/pull/11811/files - new: https://git.openjdk.org/jdk/pull/11811/files/d42cc159..d81273c0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11811&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11811&range=02-03 Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11811.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11811/head:pull/11811 PR: https://git.openjdk.org/jdk/pull/11811
Re: RFR: JDK-8299470: sun/jvm/hotspot/SALauncher.java handling of negative rmiport args [v3]
On Wed, 4 Jan 2023 14:39:14 GMT, Kevin Walls wrote: > SADebugDTest is only one test, so seems OK to have it fail as soon as we > realise we need a port, and it has a value of -1. > > I would do it in this change as they are so connected, but really whichever > works best for you. (I don't see other uses of findUnreservedFreePort() in > the same test hierarchy, so this task should not keep on growing... 8-) ) Hi Kevin, I adjusted the mentioned test, please have a look ! - PR: https://git.openjdk.org/jdk/pull/11811
Re: RFR: JDK-8299470: sun/jvm/hotspot/SALauncher.java handling of negative rmiport args [v5]
> The test serviceability/sa/sadebugd/SADebugDTest.java can pass under some > circumstances a negative rmiport (--rmiport -1) to SALauncher.java. > This leads to a somewhat misleading message > `[debugd] Argument is expected for 'rmiport' ` > (we set an argument [-1] but probably this is not what is really expected) > and additionally the real exception is not shown. > Probably also a warning in case of negative rmiport values should be printed > because they seem to lead to errors. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Small output adjustments - Changes: - all: https://git.openjdk.org/jdk/pull/11811/files - new: https://git.openjdk.org/jdk/pull/11811/files/d81273c0..1020fe27 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11811&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11811&range=03-04 Stats: 4 lines in 3 files changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/11811.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11811/head:pull/11811 PR: https://git.openjdk.org/jdk/pull/11811
Integrated: JDK-8299470: sun/jvm/hotspot/SALauncher.java handling of negative rmiport args
On Mon, 2 Jan 2023 14:31:14 GMT, Matthias Baesken wrote: > The test serviceability/sa/sadebugd/SADebugDTest.java can pass under some > circumstances a negative rmiport (--rmiport -1) to SALauncher.java. > This leads to a somewhat misleading message > `[debugd] Argument is expected for 'rmiport' ` > (we set an argument [-1] but probably this is not what is really expected) > and additionally the real exception is not shown. > Probably also a warning in case of negative rmiport values should be printed > because they seem to lead to errors. This pull request has now been integrated. Changeset: 2ccdefc8 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/2ccdefc81c0ea2ea5c4380bb045aa82ad1eb8205 Stats: 13 lines in 3 files changed: 8 ins; 0 del; 5 mod 8299470: sun/jvm/hotspot/SALauncher.java handling of negative rmiport args Reviewed-by: clanger, sspitsyn, kevinw - PR: https://git.openjdk.org/jdk/pull/11811
RFR: JDK-8299657: sun/tools/jhsdb/SAGetoptTest.java fails after 8299470
Some exception/error message changed with 8299470 so we have to adjust the test. Current error is Unexpected error 'Successor argument without leading - is expected for 'd' but we got '-c'' java.lang.RuntimeException: Bad option test 4 failed at SAGetoptTest.badOptionsTest(SAGetoptTest.java:124) at SAGetoptTest.main(SAGetoptTest.java:149) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) at java.base/java.lang.reflect.Method.invoke(Method.java:578) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:125) at java.base/java.lang.Thread.run(Thread.java:1623) - Commit messages: - JDK-8299657 Changes: https://git.openjdk.org/jdk/pull/11860/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11860&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8299657 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/11860.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11860/head:pull/11860 PR: https://git.openjdk.org/jdk/pull/11860
Re: RFR: JDK-8299657: sun/tools/jhsdb/SAGetoptTest.java fails after 8299470
On Thu, 5 Jan 2023 08:55:32 GMT, Matthias Baesken wrote: > Some exception/error message changed with 8299470 so we have to adjust the > test. > Current error is > > Unexpected error 'Successor argument without leading - is expected for 'd' > but we got '-c'' > java.lang.RuntimeException: Bad option test 4 failed > at SAGetoptTest.badOptionsTest(SAGetoptTest.java:124) > at SAGetoptTest.main(SAGetoptTest.java:149) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) > at java.base/java.lang.reflect.Method.invoke(Method.java:578) > at > com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:125) > at java.base/java.lang.Thread.run(Thread.java:1623) Hi Christoph, thanks for the review ! Yes I tested it , works now on linux x86_64 . - PR: https://git.openjdk.org/jdk/pull/11860
Integrated: JDK-8299657: sun/tools/jhsdb/SAGetoptTest.java fails after 8299470
On Thu, 5 Jan 2023 08:55:32 GMT, Matthias Baesken wrote: > Some exception/error message changed with 8299470 so we have to adjust the > test. > Current error is > > Unexpected error 'Successor argument without leading - is expected for 'd' > but we got '-c'' > java.lang.RuntimeException: Bad option test 4 failed > at SAGetoptTest.badOptionsTest(SAGetoptTest.java:124) > at SAGetoptTest.main(SAGetoptTest.java:149) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) > at java.base/java.lang.reflect.Method.invoke(Method.java:578) > at > com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:125) > at java.base/java.lang.Thread.run(Thread.java:1623) This pull request has now been integrated. Changeset: 1ca31d34 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/1ca31d34fcba5e9861104402466b5dd4cccdbafd Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod 8299657: sun/tools/jhsdb/SAGetoptTest.java fails after 8299470 Reviewed-by: clanger - PR: https://git.openjdk.org/jdk/pull/11860
RFR: JDK-8299957: Enhance error logging in instrument coding with additional jplis_assert_msg
There are a few places in the instrument coding where errors occur in our jtreg test, but the already existing error logging method jplis_assert_msg / jplis_assert is unfortunately missing so not much details are shown. This could be enhanced. - Commit messages: - JDK-8299957 Changes: https://git.openjdk.org/jdk/pull/11960/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11960&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8299957 Stats: 8 lines in 2 files changed: 4 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/11960.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11960/head:pull/11960 PR: https://git.openjdk.org/jdk/pull/11960
Integrated: JDK-8299957: Enhance error logging in instrument coding with additional jplis_assert_msg
On Thu, 12 Jan 2023 08:10:29 GMT, Matthias Baesken wrote: > There are a few places in the instrument coding where errors occur in our > jtreg test, but the already existing error logging method jplis_assert_msg / > jplis_assert is unfortunately missing so not much details are shown. This > could be enhanced. This pull request has now been integrated. Changeset: be8e6d05 Author: Matthias Baesken URL: https://git.openjdk.org/jdk/commit/be8e6d05db2f623626506e64a2fb7caf755d5d06 Stats: 8 lines in 2 files changed: 4 ins; 0 del; 4 mod 8299957: Enhance error logging in instrument coding with additional jplis_assert_msg Reviewed-by: sspitsyn - PR: https://git.openjdk.org/jdk/pull/11960
Re: RFR: 8300659: Refactor TestMemoryAwareness to use WhiteBox api for host values [v2]
On Thu, 19 Jan 2023 17:24:57 GMT, Severin Gehwolf wrote: >> Please review this refactoring of a container test. It now uses WhiteBox to >> retrieve the host values it asserts for. In terms of functionality this is >> basically a no-op except for the now more precise assertion on systems with >> swap accounting disabled at the kernel level. >> >> *Testing* >> - [x] Container tests on Linux x86_64 cgv1 and cgv2 >> - [x] Container tests on Linux x86_64 cgv1 and cgv2 on systems with >> swapaccount=0 >> - [x] GHA in progress >> >> Thoughts? > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains one additional > commit since the last revision: > > 8300659: Refactor TestMemoryAwareness to use WhiteBox api for host values Hi Severin, what do you think about renaming the methods to WB_HostPhysicalMemory / WB_HostPhysicalSwap to make it even more clear that the host values are meant ? On Linux we have values `_physical_memory = (julong)sysconf(_SC_PHYS_PAGES) * (julong)sysconf(_SC_PAGESIZE);` and `julong os::physical_memory()` from os_linux.cpp (including OSContainer::memory_limit_in_bytes()) so these could be 2 different values . And please adjust the COPYRIGHT years to 2023. - PR: https://git.openjdk.org/jdk/pull/12097
Re: RFR: 8300659: Refactor TestMemoryAwareness to use WhiteBox api for host values [v4]
On Tue, 24 Jan 2023 14:34:35 GMT, Severin Gehwolf wrote: >> Please review this refactoring of a container test. It now uses WhiteBox to >> retrieve the host values it asserts for. In terms of functionality this is >> basically a no-op except for the now more precise assertion on systems with >> swap accounting disabled at the kernel level. >> >> *Testing* >> - [x] Container tests on Linux x86_64 cgv1 and cgv2 >> - [x] Container tests on Linux x86_64 cgv1 and cgv2 on systems with >> swapaccount=0 >> - [x] GHA in progress >> >> Thoughts? > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Update copyright year to 2023 LGTM - Marked as reviewed by mbaesken (Reviewer). PR: https://git.openjdk.org/jdk/pull/12097
Re: RFR: 8304725: AsyncGetCallTrace can cause SIGBUS on M1 [v5]
On Mon, 3 Apr 2023 08:29:55 GMT, Johannes Bechberger wrote: >> Fixes the issue by disabling PCDesc cache modifications when in ASGCT. >> >> Tested on my M1 mac. > > Johannes Bechberger has updated the pull request incrementally with two > additional commits since the last revision: > > - Fix fix > - Fix minor issues LGTM. I would like a short comment line in nmethod.cpp and/or forte.cpp shortly describing what you do and why but up to you. - Marked as reviewed by mbaesken (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13144#pullrequestreview-1374643965
Re: RFR: 8306278: jvmtiAgentList.cpp:253 assert(offset >= 0) failed: invariant occurs on AIX after JDK-8257967
On Tue, 18 Apr 2023 16:59:29 GMT, Markus Grönlund wrote: > Greetings, > > For most platforms, os::dll_address_to_library_name() only sets offset = -1 > in case of errors. If there is an error, the function returns false. This is > fine. > > On AIX, the offset, being optional, is invariantly set to -1, even in the > case of non-errors. > > Easiest to remove the assertion for a positive offset. > > Thanks > Markus Thanks, looks good to me. One of my colleague has an idea to improve os::dll_address_to_library_name on AIX to support the offsets but this is something for the future so still good to have your fix. - Marked as reviewed by mbaesken (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13513#pullrequestreview-1391391403
Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code
On Thu, 25 May 2023 09:14:14 GMT, JoKern65 wrote: > When using the new xlc17 compiler (based on a recent clang) to build OpenJDk > on AIX , we run into various "warnings as errors". > Some of those are in shared codebase and could be addressed by small > adjustments. > A lot of those changes are in hotspot, some might be somewhere else in the > OpenJDK C/C++ code. In the macro define checks, you both check macros _AIX and AIX, sometimes this, sometimes that. Might be possible to just use one of the two for consistency, but I am fine with using both as well, leave this up to you. - PR Comment: https://git.openjdk.org/jdk/pull/14146#issuecomment-1562859320
Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code
On Thu, 25 May 2023 16:13:49 GMT, JoKern65 wrote: >> test/jdk/java/io/File/libGetXSpace.c line 128: >> >>> 126: #else >>> 127: struct statfs buf; >>> 128: int result = statfs((char*)chars, &buf); >> >> Is this working around a bug in IBM's declaration? >> >> Also, pre-existing, the cast seems really suspicious. The type of `chars` >> is `jchar*`, which is a >> sequence of 16bit characters. Does this actually work? If so, how? > > This is IBMs declaration of statfs > `extern int statfs(char *, struct statfs *);` > So the compiler will not accept a `const char*` > Indeed I do not know if this ever worked, but my change makes it not worse. Here is the documentation of statfs on AIX https://www.ibm.com/docs/en/aix/7.2?topic=s-statfs-fstatfs-statfs64-fstatfs64-ustat-subroutine (showing the IBM declaration Joachim told us) . - PR Review Comment: https://git.openjdk.org/jdk/pull/14146#discussion_r1206326207
Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code
On Fri, 26 May 2023 07:12:07 GMT, Matthias Baesken wrote: >> This is IBMs declaration of statfs >> `extern int statfs(char *, struct statfs *);` >> So the compiler will not accept a `const char*` >> Indeed I do not know if this ever worked, but my change makes it not worse. > > Here is the documentation of statfs on AIX > https://www.ibm.com/docs/en/aix/7.2?topic=s-statfs-fstatfs-statfs64-fstatfs64-ustat-subroutine > (showing the IBM declaration Joachim told us) . > Also, pre-existing, the cast seems really suspicious. The type of `chars` is > `jchar*`, which is a sequence of 16bit characters. Does this actually work? > If so, how? Probably a jchar to char conversion would be needed for the array elements, maybe like this here (or is there a better utility function in the codebase) ? See jCharArrayToCKCharArray https://github.com/openjdk/jdk/blob/199b1bf5009120efd1fd37a1ddabc0c6fb84f62c/src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_util.c#L644 I am not aware of an AIX statfs for wchars but maybe I miss something. But it is a separate issue of Java_GetXSpace_getSpace0 anyway and should be handled in another bug. - PR Review Comment: https://git.openjdk.org/jdk/pull/14146#discussion_r1206370114
Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code [v2]
On Fri, 26 May 2023 10:05:56 GMT, JoKern65 wrote: > src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c:1251:7: error: > '_ALLBSD_SOURCE' is not defined, evaluates to 0 [-Werror,-Wundef] > #elif _ALLBSD_SOURCE > Should probably better be `#elif defined(_ALLBSD_SOURCE)` - PR Comment: https://git.openjdk.org/jdk/pull/14146#issuecomment-1564168769
Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code [v2]
On Fri, 26 May 2023 10:18:37 GMT, JoKern65 wrote: > src/java.base/share/native/libjli/java.c:2311:22: error: format string is not > a string literal [-Werror,-Wformat-nonliteral] vfprintf(stderr, fmt, vl); ^~~ We disable this warning too for clang on all platforms in BUILD_LIBJLI ( DISABLED_WARNINGS_clang := **format-nonlitera**l deprecated-non-prototype, \ ), so I think we should do it the same way for BUILD_LIBJLI_STATIC on AIX. - PR Comment: https://git.openjdk.org/jdk/pull/14146#issuecomment-1564193886
Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code [v2]
On Fri, 26 May 2023 10:18:37 GMT, JoKern65 wrote:
> Here are the reasons for the disabled warnings in
> make/modules/java.base/lib/CoreLibraries.gmk
> DISABLED_WARNINGS_clang_aix_ProcessHandleImpl_unix.c := sign-compare,
> DISABLED_WARNINGS_clang_aix := gnu-pointer-arith, DISABLED_WARNINGS_clang :=
> gnu-pointer-arith format-nonliteral deprecated-non-prototype, \
>
> src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:638 comparison of
> integers of different signs: 'int' and 'unsigned long' if (ret <
> sizeof(psinfo_t)) { ^
>
`fread` returns a `size_t ` on Linux and AIX, could you please check Mac/BSD
too ?
https://github.com/openjdk/jdk/blob/d3b9b364da8c11c9b4dd14a6451a7b24f41202e7/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c#L636
We should probably change to size_t ret (from type int), then the warning
would not occur, correct ?
-
PR Comment: https://git.openjdk.org/jdk/pull/14146#issuecomment-1564207011
Re: RFR: JDK-8309462: vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/TestDescription.java crashing due to empty while loop
On Tue, 6 Jun 2023 09:51:09 GMT, JoKern65 wrote:
> The sys_thread_3() function contains an empty while loop, which by the
> standard can be optimized away. Please refer to discussion in
> https://github.com/llvm/llvm-project/issues/60622
> The xlc17 compiler is doing so, and IBM claims that they are following the
> standard and will not fix this on compiler side.
> So we have (at least) 3 ways to circumvent this behavior.
>
> 1. we can introduce the call of a nop library function, which will hinder the
> optimizer to throw away the loop (This is our proposed solution, but instead
> of a heavy looping thread, the result is a more or less idle thread):
> `#include `
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
> `{`
> `while (1) {`
> ` sleep(1);`
> `}`
> `}`
>
> 2. We can make use of a volatile variable in the loop body which also hinders
> the optimizer to throw away the loop:
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p)`
> `{`
> `volatile int i = 1;`
> `while (i) {`
> ` i += 2;`
> `}`
> `}`
>
> 3. we can use the __attribute__ ((optnone)) modifier in the function
> declaration to suppress the optimization at all:
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) __attribute__
> ((optnone))`
> `{`
> `while (1) {`
> `}`
> `}`
>
> To make the third approach platform independent, we can implement it in the
> following way:
> In globalDefinitions.hpp
> `#ifndef OPTNONE`
> `#define OPTNONE`
> `#endif`
>
> In globalDefinitions_xlc.hpp
> `// optnone support`
> `//`
> `// To use if a function should not be optimized`
> `// Usage:`
> `// void* func(size_t size) OPTNONE {...}`
> `#define OPTNONE __attribute__(( optnone))``
>
> With this we can change libagentthr001.cpp in a platform independent way to
> `static void`
> `sys_thread_3(jvmtiEnv* jvmti, JNIEnv* jni, void *p) OPTNONE`
> `{`
> ` while (1) {`
> ` }`
> `}`
looks okay to me
-
Marked as reviewed by mbaesken (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14330#pullrequestreview-1464841555
RFR: JDK-8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX
On AIX , new jtreg test com/sun/tools/attach/warnings/DynamicLoadWarningTest.java always failed with the output : --System.err:(294/28579)-- STARTED DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' SUCCESSFUL DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' STARTED DynamicLoadWarningTest::testLoadOneJvmtiAgent '[1] DynamicLoadWarningTest$$Lambda/0x00040020bd88@600d90bb' org.opentest4j.AssertionFailedError: expected: <1> but was: <2> at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197) at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150) at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145) at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:528) at DynamicLoadWarningTest$AppRunner.stderrShouldContain(DynamicLoadWarningTest.java:298) at DynamicLoadWarningTest.testLoadOneJvmtiAgent(DynamicLoadWarningTest.java:125) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) Reason seems to be the different behavior of dlopen on AIX compared to e.g. Linux This is what I find in the manpage of AIX 7.2 or 7.3 : https://www.ibm.com/docs/en/aix/7.2?topic=d-dlopen-subroutine https://www.ibm.com/docs/en/aix/7.3?topic=d-dlopen-subroutine 'If the module is already loaded, it is not loaded again, but a new, unique value will be returned by the dlopen subroutine.' Sounds different to what Linux documents in the manpage: https://man7.org/linux/man-pages/man3/dlopen.3.html 'If the same shared object is opened again with dlopen(), the same object handle is returned.' We skip this special subtest on AIX . - Commit messages: - JDK-8309549 Changes: https://git.openjdk.org/jdk/pull/14393/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14393&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8309549 Stats: 7 lines in 1 file changed: 3 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/14393.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14393/head:pull/14393 PR: https://git.openjdk.org/jdk/pull/14393
Integrated: JDK-8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX
On Fri, 9 Jun 2023 13:39:26 GMT, Matthias Baesken wrote: > On AIX , new jtreg test > com/sun/tools/attach/warnings/DynamicLoadWarningTest.java always failed with > the output : > > --System.err:(294/28579)-- > STARTED DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' > SUCCESSFUL DynamicLoadWarningTest::testLoadJavaAgent 'testLoadJavaAgent()' > STARTED DynamicLoadWarningTest::testLoadOneJvmtiAgent '[1] > DynamicLoadWarningTest$$Lambda/0x00040020bd88@600d90bb' > org.opentest4j.AssertionFailedError: expected: <1> but was: <2> > at > org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) > at > org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) > at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197) > at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150) > at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145) > at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:528) > at > DynamicLoadWarningTest$AppRunner.stderrShouldContain(DynamicLoadWarningTest.java:298) > at > DynamicLoadWarningTest.testLoadOneJvmtiAgent(DynamicLoadWarningTest.java:125) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) > at java.base/java.lang.reflect.Method.invoke(Method.java:580) > > Reason seems to be the different behavior of dlopen on AIX compared to e.g. > Linux > This is what I find in the manpage of AIX 7.2 or 7.3 : > https://www.ibm.com/docs/en/aix/7.2?topic=d-dlopen-subroutine > https://www.ibm.com/docs/en/aix/7.3?topic=d-dlopen-subroutine > 'If the module is already loaded, it is not loaded again, but a new, unique > value will be returned by the dlopen subroutine.' > > Sounds different to what Linux documents in the manpage: > https://man7.org/linux/man-pages/man3/dlopen.3.html > 'If the same shared object is opened again with dlopen(), the same object > handle is returned.' > > We skip this special subtest on AIX . This pull request has now been integrated. Changeset: 4d66d977 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/4d66d977450e083214da3dba6ad4ed851c6c1cb4 Stats: 7 lines in 1 file changed: 3 ins; 0 del; 4 mod 8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX Reviewed-by: alanb, cjplummer, sspitsyn - PR: https://git.openjdk.org/jdk/pull/14393
Re: RFR: 8309462: [AIX] vmTestbase/nsk/jvmti/RunAgentThread/agentthr001/TestDescription.java crashing due to empty while loop
On Mon, 12 Jun 2023 14:36:56 GMT, Martin Doerr wrote: > Test fix for test failing on AIX because of undefined behavior in current > implementation. Marked as reviewed by mbaesken (Reviewer). - PR Review: https://git.openjdk.org/jdk21/pull/9#pullrequestreview-1475121715
RFR: JDK-8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX
After push of [JDK-8307478](https://bugs.openjdk.org/browse/JDK-8307478) , the following test started to fail on AIX : com/sun/tools/attach/warnings/DynamicLoadWarningTest.java ; failure output : java.lang.RuntimeException: 'WARNING: A JVM TI agent has been loaded dynamically' found in stderr at jdk.test.lib.process.OutputAnalyzer.stderrShouldNotContain(OutputAnalyzer.java:320) at DynamicLoadWarningTest$AppRunner.stderrShouldNotContain(DynamicLoadWarningTest.java:308) at DynamicLoadWarningTest.testLoadOneJvmtiAgent(DynamicLoadWarningTest.java:138) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) Should be handled in a similar way to [JDK-8309549](https://bugs.openjdk.org/browse/JDK-8309549) . - Commit messages: - JDK-8310191 Changes: https://git.openjdk.org/jdk/pull/14515/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14515&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8310191 Stats: 5 lines in 1 file changed: 2 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/14515.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14515/head:pull/14515 PR: https://git.openjdk.org/jdk/pull/14515
Re: RFR: JDK-8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX [v2]
> After push of [JDK-8307478](https://bugs.openjdk.org/browse/JDK-8307478) , > the following test started to fail on AIX : > com/sun/tools/attach/warnings/DynamicLoadWarningTest.java ; failure output : > > java.lang.RuntimeException: 'WARNING: A JVM TI agent has been loaded > dynamically' found in stderr > at > jdk.test.lib.process.OutputAnalyzer.stderrShouldNotContain(OutputAnalyzer.java:320) > at > DynamicLoadWarningTest$AppRunner.stderrShouldNotContain(DynamicLoadWarningTest.java:308) > at > DynamicLoadWarningTest.testLoadOneJvmtiAgent(DynamicLoadWarningTest.java:138) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) > at java.base/java.lang.reflect.Method.invoke(Method.java:580) > > Should be handled in a similar way to > [JDK-8309549](https://bugs.openjdk.org/browse/JDK-8309549) . Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Rearrange and simplify test coding - Changes: - all: https://git.openjdk.org/jdk/pull/14515/files - new: https://git.openjdk.org/jdk/pull/14515/files/8694eb48..af005ebc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14515&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14515&range=00-01 Stats: 15 lines in 1 file changed: 6 ins; 8 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/14515.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14515/head:pull/14515 PR: https://git.openjdk.org/jdk/pull/14515
Re: RFR: JDK-8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX
On Fri, 16 Jun 2023 10:44:47 GMT, Matthias Baesken wrote: > After push of [JDK-8307478](https://bugs.openjdk.org/browse/JDK-8307478) , > the following test started to fail on AIX : > com/sun/tools/attach/warnings/DynamicLoadWarningTest.java ; failure output : > > java.lang.RuntimeException: 'WARNING: A JVM TI agent has been loaded > dynamically' found in stderr > at > jdk.test.lib.process.OutputAnalyzer.stderrShouldNotContain(OutputAnalyzer.java:320) > at > DynamicLoadWarningTest$AppRunner.stderrShouldNotContain(DynamicLoadWarningTest.java:308) > at > DynamicLoadWarningTest.testLoadOneJvmtiAgent(DynamicLoadWarningTest.java:138) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) > at java.base/java.lang.reflect.Method.invoke(Method.java:580) > > Should be handled in a similar way to > [JDK-8309549](https://bugs.openjdk.org/browse/JDK-8309549) . Hi Alan, I adjusted / simplified the coding. - PR Comment: https://git.openjdk.org/jdk/pull/14515#issuecomment-1594561066
Integrated: JDK-8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX
On Fri, 16 Jun 2023 10:44:47 GMT, Matthias Baesken wrote: > After push of [JDK-8307478](https://bugs.openjdk.org/browse/JDK-8307478) , > the following test started to fail on AIX : > com/sun/tools/attach/warnings/DynamicLoadWarningTest.java ; failure output : > > java.lang.RuntimeException: 'WARNING: A JVM TI agent has been loaded > dynamically' found in stderr > at > jdk.test.lib.process.OutputAnalyzer.stderrShouldNotContain(OutputAnalyzer.java:320) > at > DynamicLoadWarningTest$AppRunner.stderrShouldNotContain(DynamicLoadWarningTest.java:308) > at > DynamicLoadWarningTest.testLoadOneJvmtiAgent(DynamicLoadWarningTest.java:138) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) > at java.base/java.lang.reflect.Method.invoke(Method.java:580) > > Should be handled in a similar way to > [JDK-8309549](https://bugs.openjdk.org/browse/JDK-8309549) . This pull request has now been integrated. Changeset: 6a63badd Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/6a63badd8ea3e79cd9fc3cb33aff499fc9a6d3f1 Stats: 20 lines in 1 file changed: 8 ins; 8 del; 4 mod 8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX Reviewed-by: alanb, cjplummer - PR: https://git.openjdk.org/jdk/pull/14515
RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work
Currently, a number of tests fail on macOS because they miss the core file
(e.g. serviceability/sa/TestJmapCore.java).
The reason is that configure detects on some setups that codesign does not work
("checking if debug mode codesign is possible... no) .
So adding the needed entitlement to generate cores is not done in the build.
This is currently not checked later in the tests.
But without the entitlement, a core is not generated.
-
Commit messages:
- JDK-8310380
Changes: https://git.openjdk.org/jdk/pull/14562/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8310380
Stats: 36 lines in 3 files changed: 35 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/14562.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14562/head:pull/14562
PR: https://git.openjdk.org/jdk/pull/14562
[jdk21] RFR: 8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX
8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX - Commit messages: - Backport 4d66d977450e083214da3dba6ad4ed851c6c1cb4 Changes: https://git.openjdk.org/jdk21/pull/44/files Webrev: https://webrevs.openjdk.org/?repo=jdk21&pr=44&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8309549 Stats: 7 lines in 1 file changed: 3 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk21/pull/44.diff Fetch: git fetch https://git.openjdk.org/jdk21.git pull/44/head:pull/44 PR: https://git.openjdk.org/jdk21/pull/44
Re: [jdk21] RFR: 8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX
On Wed, 21 Jun 2023 07:29:03 GMT, Matthias Baesken wrote: > 8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on > AIX Alan and Lucy , thanks ! - PR Comment: https://git.openjdk.org/jdk21/pull/44#issuecomment-1600349549
[jdk21] Integrated: 8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX
On Wed, 21 Jun 2023 07:29:03 GMT, Matthias Baesken wrote: > 8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on > AIX This pull request has now been integrated. Changeset: 60cae33c Author:Matthias Baesken URL: https://git.openjdk.org/jdk21/commit/60cae33c46d74de462e4b5abfc3b7379d98f1311 Stats: 7 lines in 1 file changed: 3 ins; 0 del; 4 mod 8309549: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java fails on AIX Reviewed-by: lucy, alanb Backport-of: 4d66d977450e083214da3dba6ad4ed851c6c1cb4 - PR: https://git.openjdk.org/jdk21/pull/44
[jdk21] RFR: 8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX
8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX - Commit messages: - Backport 6a63badd8ea3e79cd9fc3cb33aff499fc9a6d3f1 Changes: https://git.openjdk.org/jdk21/pull/45/files Webrev: https://webrevs.openjdk.org/?repo=jdk21&pr=45&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8310191 Stats: 20 lines in 1 file changed: 8 ins; 8 del; 4 mod Patch: https://git.openjdk.org/jdk21/pull/45.diff Fetch: git fetch https://git.openjdk.org/jdk21.git pull/45/head:pull/45 PR: https://git.openjdk.org/jdk21/pull/45
[jdk21] Integrated: 8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX
On Wed, 21 Jun 2023 08:27:38 GMT, Matthias Baesken wrote: > 8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second > failure on AIX This pull request has now been integrated. Changeset: 1fc60429 Author:Matthias Baesken URL: https://git.openjdk.org/jdk21/commit/1fc60429a1565ca5357de08c53df53af5d24278f Stats: 20 lines in 1 file changed: 8 ins; 8 del; 4 mod 8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX Reviewed-by: alanb Backport-of: 6a63badd8ea3e79cd9fc3cb33aff499fc9a6d3f1 - PR: https://git.openjdk.org/jdk21/pull/45
RFR: JDK-8310550: Adjust references to rt.jar
There are a few references to rt.jar in comments and in the codebase itself. Some of them might be removed or adjusted. - Commit messages: - JDK-8310550 Changes: https://git.openjdk.org/jdk/pull/14593/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14593&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8310550 Stats: 14 lines in 12 files changed: 0 ins; 7 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/14593.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14593/head:pull/14593 PR: https://git.openjdk.org/jdk/pull/14593
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work
On Wed, 21 Jun 2023 15:25:15 GMT, Christoph Langer wrote:
>> Currently, a number of tests fail on macOS because they miss the core file
>> (e.g. serviceability/sa/TestJmapCore.java).
>> The reason is that configure detects on some setups that codesign does not
>> work ("checking if debug mode codesign is possible... no) .
>> So adding the needed entitlement to generate cores is not done in the build.
>> This is currently not checked later in the tests.
>> But without the entitlement, a core is not generated.
>
> test/lib/jdk/test/lib/Platform.java line 280:
>
>> 278: String line;
>> 279: while ((line = is.readLine()) != null) {
>> 280: System.out.println("STDOUT: " + line);
>
> This output is for every line seems too much. Maybe just print the lines
> where you find "Info.plist=not bound" or "Info.plist entries="?
I found the output useful when analyzing the issue (and it is just a few lines).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1238163572
Re: RFR: JDK-8310550: Adjust references to rt.jar
On Wed, 21 Jun 2023 21:46:03 GMT, David Holmes wrote: >> There are a few references to rt.jar in comments and in the codebase itself. >> Some of them might be removed or adjusted. > > src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java > line 196: > >> 194: >> 195: /** >> 196: * Set whether or not to use ct.sym as an alternate > > As an alternate to what? This needs something else. should "to the image modules files" be used instead ? > test/langtools/tools/javap/4798312/JavapShouldLoadClassesFromRTJarTest.java > line 1: > >> 1: /* > > The name of this test includes RTJar. It needs to be changed too I think. > Does this test actually still test something? It seems to start a javap. So I think it tests something, how important this is and what other tests might cover similar stuff, I do not know unfortunately . - PR Review Comment: https://git.openjdk.org/jdk/pull/14593#discussion_r1238252196 PR Review Comment: https://git.openjdk.org/jdk/pull/14593#discussion_r1238254149
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v2]
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
Matthias Baesken has updated the pull request incrementally with one additional
commit since the last revision:
Some adjustments
-
Changes:
- all: https://git.openjdk.org/jdk/pull/14562/files
- new: https://git.openjdk.org/jdk/pull/14562/files/0d8c68c2..1b604db9
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=00-01
Stats: 37 lines in 5 files changed: 17 ins; 10 del; 10 mod
Patch: https://git.openjdk.org/jdk/pull/14562.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14562/head:pull/14562
PR: https://git.openjdk.org/jdk/pull/14562
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work
On Tue, 20 Jun 2023 13:23:16 GMT, Matthias Baesken wrote:
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
I adjusted COPYRIGHT info, adjusted
TestMutuallyExclusivePlatformPredicates.java .
Also simplified the coding in Platform.java a bit.
-
PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1602340023
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v2]
On Wed, 21 Jun 2023 19:32:23 GMT, Chris Plummer wrote:
>> Matthias Baesken has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Some adjustments
>
> test/lib/jdk/test/lib/util/CoreUtils.java line 131:
>
>> 129: return coreFileLocation; // success!
>> 130: } else {
>> 131: System.out.println("Core file not found, try to find a
>> reason for this");
>
> Suggestion:
>
> System.out.println("Core file not found. Trying to find a reason
> why...");
I changed the message.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1238282996
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v3]
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
Matthias Baesken has updated the pull request incrementally with one additional
commit since the last revision:
move javaPath into caller
-
Changes:
- all: https://git.openjdk.org/jdk/pull/14562/files
- new: https://git.openjdk.org/jdk/pull/14562/files/1b604db9..619b3578
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=01-02
Stats: 32 lines in 2 files changed: 6 ins; 23 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/14562.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14562/head:pull/14562
PR: https://git.openjdk.org/jdk/pull/14562
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v2]
On Thu, 22 Jun 2023 09:53:29 GMT, Matthias Baesken wrote:
>> Currently, a number of tests fail on macOS because they miss the core file
>> (e.g. serviceability/sa/TestJmapCore.java).
>> The reason is that configure detects on some setups that codesign does not
>> work ("checking if debug mode codesign is possible... no) .
>> So adding the needed entitlement to generate cores is not done in the build.
>> This is currently not checked later in the tests.
>> But without the entitlement, a core is not generated.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Some adjustments
Yes it must be 'private static Process' . And I moved the javaPath method into
the caller ,
-
PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1602496320
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v2]
On Thu, 22 Jun 2023 10:54:12 GMT, Christoph Langer wrote: > Looks a bit better. But I think instead of adding a Utils.javaPath() method, > you can do all this path handling in Platform.launchCodesignOnJavaBinary(). > Then even more code would be shared. Okay, I had the idea to later use the method javaPath at some more places, but let's do this separately. - PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1602521579
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v3]
On Thu, 22 Jun 2023 11:51:15 GMT, Matthias Baesken wrote:
>> Currently, a number of tests fail on macOS because they miss the core file
>> (e.g. serviceability/sa/TestJmapCore.java).
>> The reason is that configure detects on some setups that codesign does not
>> work ("checking if debug mode codesign is possible... no) .
>> So adding the needed entitlement to generate cores is not done in the build.
>> This is currently not checked later in the tests.
>> But without the entitlement, a core is not generated.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> move javaPath into caller
Hi Christoph, thanks for the review !
Chris are you okay with the latest rev. of this change?
-
PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1603933840
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v4]
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
Matthias Baesken has updated the pull request incrementally with one additional
commit since the last revision:
small adjustments
-
Changes:
- all: https://git.openjdk.org/jdk/pull/14562/files
- new: https://git.openjdk.org/jdk/pull/14562/files/619b3578..09eb93c6
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=03
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=02-03
Stats: 4 lines in 3 files changed: 0 ins; 0 del; 4 mod
Patch: https://git.openjdk.org/jdk/pull/14562.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14562/head:pull/14562
PR: https://git.openjdk.org/jdk/pull/14562
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v3]
On Thu, 22 Jun 2023 11:51:15 GMT, Matthias Baesken wrote:
>> Currently, a number of tests fail on macOS because they miss the core file
>> (e.g. serviceability/sa/TestJmapCore.java).
>> The reason is that configure detects on some setups that codesign does not
>> work ("checking if debug mode codesign is possible... no) .
>> So adding the needed entitlement to generate cores is not done in the build.
>> This is currently not checked later in the tests.
>> But without the entitlement, a core is not generated.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> move javaPath into caller
Hi Chris, I renamed the method and adjusted the comment.
-
PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1609529567
Integrated: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work
On Tue, 20 Jun 2023 13:23:16 GMT, Matthias Baesken wrote:
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
This pull request has now been integrated.
Changeset: 39c104df
Author:Matthias Baesken
URL:
https://git.openjdk.org/jdk/commit/39c104df44f17c1d65e35becd4272f73e2c6610c
Stats: 56 lines in 4 files changed: 37 ins; 12 del; 7 mod
8310380: Handle problems in core-related tests on macOS when codesign tool does
not work
Reviewed-by: lucy, clanger, cjplummer
-
PR: https://git.openjdk.org/jdk/pull/14562
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v4]
On Tue, 27 Jun 2023 13:45:27 GMT, Matthias Baesken wrote:
>> Currently, a number of tests fail on macOS because they miss the core file
>> (e.g. serviceability/sa/TestJmapCore.java).
>> The reason is that configure detects on some setups that codesign does not
>> work ("checking if debug mode codesign is possible... no) .
>> So adding the needed entitlement to generate cores is not done in the build.
>> This is currently not checked later in the tests.
>> But without the entitlement, a core is not generated.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> small adjustments
Thanks for the review !
-
PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1610866191
Re: RFR: JDK-8310550: Adjust references to rt.jar
On Thu, 22 Jun 2023 14:20:30 GMT, Alan Bateman wrote:
>> There are a few references to rt.jar in comments and in the codebase itself.
>> Some of them might be removed or adjusted.
>
> src/java.sql/share/classes/java/sql/DriverManager.java line 658:
>
>> 656: * (which is invoking this class indirectly)
>> 657: * classloader, so that the JDBC driver class outside the image
>> 658: * can be loaded from here.
>
> This code should probably be changed to use VM.isSystemDomainLoader(callerCL).
>
> I think the comment should be replaced because it doesn't match what it
> actually does and it's nothing to do with the whether the JDBC driver is in
> the run-time image or not. How about:
>
> "If the caller is defined to the bootstrap or platform class loader then use
> the Thread CCL as the initiating class loader so that a JDBC on the class
> path, or bundled with an application, is found."
Hi Alan, regarding usage of class VM I get
'package jdk.internal.misc is declared in module java.base, which does not
export it to module java.sql'
Is there any concern to export it as well to module java.sql ?
And btw did you mean to use it like this, in the if ?
`if (callerCL == null || VM.isSystemDomainLoader(callerCL)) {
callerCL = Thread.currentThread().getContextClassLoader();
}`
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14593#discussion_r1245164604
Re: RFR: JDK-8310550: Adjust references to rt.jar
On Wed, 28 Jun 2023 13:16:30 GMT, Alan Bateman wrote:
>> Hi Alan, regarding usage of class VM I get
>> 'package jdk.internal.misc is declared in module java.base, which does not
>> export it to module java.sql'
>> Is there any concern to export it as well to module java.sql ?
>> And btw did you mean to use it like this, in the if ?
>>
>> `
>> if (callerCL == null || VM.isSystemDomainLoader(callerCL)) {
>> callerCL = Thread.currentThread().getContextClassLoader();
>> }
>> `
>
>> Hi Alan, regarding usage of class VM I get 'package jdk.internal.misc is
>> declared in module java.base, which does not export it to module java.sql'
>> Is there any concern to export it as well to module java.sql ? And btw did
>> you mean to use it like this, in the if ?
>>
>> `if (callerCL == null || VM.isSystemDomainLoader(callerCL)) { callerCL =
>> Thread.currentThread().getContextClassLoader(); }`
>
> It was just a passing comment, I didn't meant to suggest changing it as part
> of this PR. We have always think twice before adding qualified exports from
> java.base and this is case where java.sql is very "non-core", we don't want
> to give it any access to java.base internals.
Hi Alan, thanks for clarifying. So I should only adjust the comment, correct ?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14593#discussion_r1245204791
RFR: JDK-8311026: Some G1 specific tests do not set -XX:+UseG1GC
Most G1 tests set -XX:+UseG1GC, but a few (e.g. gc/g1/TestVerificationInConcurrentCycle.java) miss that. This is usually just fine and no problem because G1 is the default anyway. However in some cases, where a custom JVM changes the default GC, those tests start to fail which is not really necessary. - Commit messages: - JDK-8311026 Changes: https://git.openjdk.org/jdk/pull/14722/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14722&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8311026 Stats: 4 lines in 3 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/14722.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14722/head:pull/14722 PR: https://git.openjdk.org/jdk/pull/14722
Re: RFR: JDK-8310550: Adjust references to rt.jar [v2]
On Wed, 28 Jun 2023 13:22:20 GMT, Matthias Baesken wrote:
>>> Hi Alan, regarding usage of class VM I get 'package jdk.internal.misc is
>>> declared in module java.base, which does not export it to module java.sql'
>>> Is there any concern to export it as well to module java.sql ? And btw did
>>> you mean to use it like this, in the if ?
>>>
>>> `if (callerCL == null || VM.isSystemDomainLoader(callerCL)) { callerCL =
>>> Thread.currentThread().getContextClassLoader(); }`
>>
>> It was just a passing comment, I didn't meant to suggest changing it as part
>> of this PR. We have always think twice before adding qualified exports from
>> java.base and this is case where java.sql is very "non-core", we don't want
>> to give it any access to java.base internals.
>
> Hi Alan, thanks for clarifying. So I should only adjust the comment, correct
> ?
Hi Alan, I adjusted the comment in DriverManager.java .
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14593#discussion_r1247686721
Re: RFR: JDK-8310550: Adjust references to rt.jar [v2]
> There are a few references to rt.jar in comments and in the codebase itself. > Some of them might be removed or adjusted. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust comment in src/java.sql/share/classes/java/sql/DriverManager.java - Changes: - all: https://git.openjdk.org/jdk/pull/14593/files - new: https://git.openjdk.org/jdk/pull/14593/files/5d52b4cb..6665f60b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14593&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14593&range=00-01 Stats: 6 lines in 1 file changed: 1 ins; 2 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/14593.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14593/head:pull/14593 PR: https://git.openjdk.org/jdk/pull/14593
Re: RFR: JDK-8310550: Adjust references to rt.jar [v3]
> There are a few references to rt.jar in comments and in the codebase itself. > Some of them might be removed or adjusted. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: remove import - Changes: - all: https://git.openjdk.org/jdk/pull/14593/files - new: https://git.openjdk.org/jdk/pull/14593/files/6665f60b..9b2232a7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14593&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14593&range=01-02 Stats: 2 lines in 1 file changed: 1 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14593.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14593/head:pull/14593 PR: https://git.openjdk.org/jdk/pull/14593
Re: RFR: JDK-8311026: Some G1 specific tests do not set -XX:+UseG1GC
On Fri, 30 Jun 2023 08:11:47 GMT, Matthias Baesken wrote: > Most G1 tests set -XX:+UseG1GC, but a few (e.g. > gc/g1/TestVerificationInConcurrentCycle.java) miss that. > This is usually just fine and no problem because G1 is the default anyway. > However in some cases, where a custom JVM changes the default GC, those tests > start to fail which is not really necessary. Hi Thomas and Serguei, thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/14722#issuecomment-1614525324
Integrated: JDK-8311026: Some G1 specific tests do not set -XX:+UseG1GC
On Fri, 30 Jun 2023 08:11:47 GMT, Matthias Baesken wrote: > Most G1 tests set -XX:+UseG1GC, but a few (e.g. > gc/g1/TestVerificationInConcurrentCycle.java) miss that. > This is usually just fine and no problem because G1 is the default anyway. > However in some cases, where a custom JVM changes the default GC, those tests > start to fail which is not really necessary. This pull request has now been integrated. Changeset: a7d168b5 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/a7d168b522bb05345a40ae1fb18942ba663d3182 Stats: 4 lines in 3 files changed: 0 ins; 0 del; 4 mod 8311026: Some G1 specific tests do not set -XX:+UseG1GC Reviewed-by: sspitsyn, tschatzl - PR: https://git.openjdk.org/jdk/pull/14722
[jdk21] RFR: 8310380: Handle problems in core-related tests on macOS when codesign tool does not work
8310380: Handle problems in core-related tests on macOS when codesign tool does not work - Commit messages: - Backport 39c104df44f17c1d65e35becd4272f73e2c6610c Changes: https://git.openjdk.org/jdk21/pull/87/files Webrev: https://webrevs.openjdk.org/?repo=jdk21&pr=87&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8310380 Stats: 56 lines in 4 files changed: 37 ins; 12 del; 7 mod Patch: https://git.openjdk.org/jdk21/pull/87.diff Fetch: git fetch https://git.openjdk.org/jdk21.git pull/87/head:pull/87 PR: https://git.openjdk.org/jdk21/pull/87
[jdk21] Integrated: 8310380: Handle problems in core-related tests on macOS when codesign tool does not work
On Fri, 30 Jun 2023 12:35:27 GMT, Matthias Baesken wrote: > 8310380: Handle problems in core-related tests on macOS when codesign tool > does not work This pull request has now been integrated. Changeset: 6f3f4aa2 Author:Matthias Baesken URL: https://git.openjdk.org/jdk21/commit/6f3f4aa2936f5cbe120da1e29e924bc99cfd27d1 Stats: 56 lines in 4 files changed: 37 ins; 12 del; 7 mod 8310380: Handle problems in core-related tests on macOS when codesign tool does not work Reviewed-by: cjplummer Backport-of: 39c104df44f17c1d65e35becd4272f73e2c6610c - PR: https://git.openjdk.org/jdk21/pull/87
Re: RFR: JDK-8310550: Adjust references to rt.jar [v4]
> There are a few references to rt.jar in comments and in the codebase itself. > Some of them might be removed or adjusted. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust comments - Changes: - all: https://git.openjdk.org/jdk/pull/14593/files - new: https://git.openjdk.org/jdk/pull/14593/files/9b2232a7..3a7b057a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14593&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14593&range=02-03 Stats: 4 lines in 4 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/14593.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14593/head:pull/14593 PR: https://git.openjdk.org/jdk/pull/14593
Re: RFR: JDK-8310550: Adjust references to rt.jar [v3]
On Fri, 30 Jun 2023 11:37:10 GMT, Matthias Baesken wrote: >> There are a few references to rt.jar in comments and in the codebase itself. >> Some of them might be removed or adjusted. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove import Hi Christoph, thanks for the suggestions, I added some changes. - PR Comment: https://git.openjdk.org/jdk/pull/14593#issuecomment-1621939153
Re: RFR: JDK-8310550: Adjust references to rt.jar [v5]
> There are a few references to rt.jar in comments and in the codebase itself. > Some of them might be removed or adjusted. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust comment - Changes: - all: https://git.openjdk.org/jdk/pull/14593/files - new: https://git.openjdk.org/jdk/pull/14593/files/3a7b057a..f29c4019 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14593&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14593&range=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/14593.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14593/head:pull/14593 PR: https://git.openjdk.org/jdk/pull/14593
Re: RFR: JDK-8310550: Adjust references to rt.jar [v4]
On Wed, 5 Jul 2023 15:07:15 GMT, Matthias Baesken wrote: >> There are a few references to rt.jar in comments and in the codebase itself. >> Some of them might be removed or adjusted. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust comments Hi Christoph, thanks for the review ! I added the '.' as suggested. Any objections to the latest revision? - PR Comment: https://git.openjdk.org/jdk/pull/14593#issuecomment-1623132227
RFR: JDK-8292595: jdwp utf_util getWideString might leak memory
There seems to be a case where utf_util.c getWideString might leak memory in an early return. - Commit messages: - JDK-8292595 Changes: https://git.openjdk.org/jdk/pull/9918/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9918&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8292595 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/9918.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9918/head:pull/9918 PR: https://git.openjdk.org/jdk/pull/9918
Re: RFR: JDK-8292595: jdwp utf_util getWideString might leak memory
On Thu, 18 Aug 2022 12:30:57 GMT, Alan Bateman wrote:
>> There seems to be a case where utf_util.c getWideString might leak memory in
>> an early return.
>
> src/jdk.jdwp.agent/share/native/libjdwp/utf_util.c line 346:
>
>> 344: if (MultiByteToWideChar(codePage, 0, str, len, wstr, wlen) == 0) {
>> 345: UTF_ERROR(("Can't get WIDE string"));
>> 346: free(wstr);
>
> Is this really an issue? I thought that UTF_ERROR prints the error and aborts
> the program.
Hi Alan, yeah I saw that it prints an error but you are correct it does an
abort() call too. So probably we can keep this as it is.
Another MultiByteToWideChar / malloc case I just saw
https://github.com/openjdk/jdk/blob/master/src/java.instrument/windows/native/libinstrument/EncodingSupport_md.c#L79
shouldn't there be the free call after line 80 ?
-
PR: https://git.openjdk.org/jdk/pull/9918
Re: RFR: JDK-8292595: jdwp utf_util getWideString might leak memory
On Thu, 18 Aug 2022 11:51:52 GMT, Matthias Baesken wrote:
> There seems to be a case where utf_util.c getWideString might leak memory in
> an early return.
In case of failing getWideString, we have already a fallback in place that is
just copying bytes :
wstr = getWideString(CP_UTF8, (char*)utf8, len, &wlen);
if ( wstr == NULL ) {
// Can't allocate WIDE string
goto just_copy_bytes;
}
So in these cases it is probably not needed to abort the VM, because fallback
code exists anyway.
So should we add the free, and replace UTF_ERROR by something like UTF_WARNING
(thats just prints the problem - message without abort) ?
-
PR: https://git.openjdk.org/jdk/pull/9918
RFR: JDK-8292778: EncodingSupport_md.c convertUtf8ToPlatformString wrong placing of free
There seems to be a case where EncodingSupport_md.c convertUtf8ToPlatformString might leak memory because of a wrong placing of free.. - Commit messages: - JDK-8292778 Changes: https://git.openjdk.org/jdk/pull/9981/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9981&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8292778 Stats: 3 lines in 1 file changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/9981.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9981/head:pull/9981 PR: https://git.openjdk.org/jdk/pull/9981
Re: RFR: JDK-8292778: EncodingSupport_md.c convertUtf8ToPlatformString wrong placing of free
On Tue, 23 Aug 2022 13:59:53 GMT, Matthias Baesken wrote: > There seems to be a case where EncodingSupport_md.c > convertUtf8ToPlatformString might leak memory because of a wrong placing of > free.. Thanks for the reviews ! - PR: https://git.openjdk.org/jdk/pull/9981
Integrated: JDK-8292778: EncodingSupport_md.c convertUtf8ToPlatformString wrong placing of free
On Tue, 23 Aug 2022 13:59:53 GMT, Matthias Baesken wrote: > There seems to be a case where EncodingSupport_md.c > convertUtf8ToPlatformString might leak memory because of a wrong placing of > free.. This pull request has now been integrated. Changeset: ad2e0c4d Author: Matthias Baesken URL: https://git.openjdk.org/jdk/commit/ad2e0c4df045261c04b00bfa1faf5c21392edc58 Stats: 3 lines in 1 file changed: 1 ins; 1 del; 1 mod 8292778: EncodingSupport_md.c convertUtf8ToPlatformString wrong placing of free Reviewed-by: rriggs, kevinw, amenkov - PR: https://git.openjdk.org/jdk/pull/9981
Re: RFR: JDK-8292595: jdwp utf_util getWideString might leak memory [v2]
> There seems to be a case where utf_util.c getWideString might leak memory in > an early return. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Introduce UTF_WARNING and use the fallback - Changes: - all: https://git.openjdk.org/jdk/pull/9918/files - new: https://git.openjdk.org/jdk/pull/9918/files/ab7fb549..9331f320 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9918&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9918&range=00-01 Stats: 9 lines in 1 file changed: 3 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/9918.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9918/head:pull/9918 PR: https://git.openjdk.org/jdk/pull/9918
Re: RFR: JDK-8292595: jdwp utf_util getWideString might leak memory [v2]
On Fri, 26 Aug 2022 12:04:57 GMT, Matthias Baesken wrote: >> There seems to be a case where utf_util.c getWideString might leak memory in >> an early return. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Introduce UTF_WARNING and use the fallback Hi, if it is still considered better to always abort in the MultiByteToWideChar failure cases and not use the fallback code, that's fine with me too. - PR: https://git.openjdk.org/jdk/pull/9918
Withdrawn: JDK-8292595: jdwp utf_util getWideString might leak memory
On Thu, 18 Aug 2022 11:51:52 GMT, Matthias Baesken wrote: > There seems to be a case where utf_util.c getWideString might leak memory in > an early return. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/9918
Integrated: JDK-8310550: Adjust references to rt.jar
On Wed, 21 Jun 2023 15:18:19 GMT, Matthias Baesken wrote: > There are a few references to rt.jar in comments and in the codebase itself. > Some of them might be removed or adjusted. This pull request has now been integrated. Changeset: 25cbe85d Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/25cbe85d6f46bed82c7f1266ce52c86943e29d60 Stats: 17 lines in 12 files changed: 0 ins; 8 del; 9 mod 8310550: Adjust references to rt.jar Reviewed-by: erikj, clanger - PR: https://git.openjdk.org/jdk/pull/14593
RFR: JDK-8313670: Simplify shared lib name handling code in some tests
There is coding e.g. in https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 that deals with shared lib naming on different OS. This code should be simplified. - Commit messages: - JDK-8313670 Changes: https://git.openjdk.org/jdk/pull/15151/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15151&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8313670 Stats: 77 lines in 6 files changed: 13 ins; 59 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/15151.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15151/head:pull/15151 PR: https://git.openjdk.org/jdk/pull/15151
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v2]
> There is coding e.g. in > https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 > that deals with shared lib naming on different OS. > This code should be simplified. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: use Platform.sharedLibraryPrefix in more tests - Changes: - all: https://git.openjdk.org/jdk/pull/15151/files - new: https://git.openjdk.org/jdk/pull/15151/files/c99d984d..e708bb68 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15151&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15151&range=00-01 Stats: 9 lines in 3 files changed: 0 ins; 4 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/15151.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15151/head:pull/15151 PR: https://git.openjdk.org/jdk/pull/15151
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v2]
On Wed, 9 Aug 2023 08:42:35 GMT, Matthias Baesken wrote: >> There is coding e.g. in >> https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 >> that deals with shared lib naming on different OS. >> This code should be simplified. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > use Platform.sharedLibraryPrefix in more tests Hello, as suggested I introduced a Platform.buildSharedLibraryName; this can be used to simplify a lot of code in the mentioned tests. - PR Comment: https://git.openjdk.org/jdk/pull/15151#issuecomment-1671099342
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v3]
> There is coding e.g. in > https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 > that deals with shared lib naming on different OS. > This code should be simplified. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Introduce buildSharedLibraryName - Changes: - all: https://git.openjdk.org/jdk/pull/15151/files - new: https://git.openjdk.org/jdk/pull/15151/files/e708bb68..9f9c5b25 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15151&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15151&range=01-02 Stats: 50 lines in 9 files changed: 10 ins; 25 del; 15 mod Patch: https://git.openjdk.org/jdk/pull/15151.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15151/head:pull/15151 PR: https://git.openjdk.org/jdk/pull/15151
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v4]
> There is coding e.g. in > https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 > that deals with shared lib naming on different OS. > This code should be simplified. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust COPYRIGHT headers - Changes: - all: https://git.openjdk.org/jdk/pull/15151/files - new: https://git.openjdk.org/jdk/pull/15151/files/9f9c5b25..5e8dfa79 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15151&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15151&range=02-03 Stats: 6 lines in 6 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/15151.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15151/head:pull/15151 PR: https://git.openjdk.org/jdk/pull/15151
Re: RFR: JDK-8313670: Simplify shared lib name handling code in some tests [v3]
On Wed, 9 Aug 2023 11:06:04 GMT, Matthias Baesken wrote: >> There is coding e.g. in >> https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 >> that deals with shared lib naming on different OS. >> This code should be simplified. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Introduce buildSharedLibraryName Hi Chris and Serguei, thanks for the reviews ! I adjusted the COPYRIGHT years. - PR Comment: https://git.openjdk.org/jdk/pull/15151#issuecomment-1672693986
Integrated: JDK-8313670: Simplify shared lib name handling code in some tests
On Fri, 4 Aug 2023 09:59:41 GMT, Matthias Baesken wrote: > There is coding e.g. in > https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/jfr/event/runtime/TestNativeLibrariesEvent.java#L72 > that deals with shared lib naming on different OS. > This code should be simplified. This pull request has now been integrated. Changeset: 6dba2026 Author: Matthias Baesken URL: https://git.openjdk.org/jdk/commit/6dba2026d72de6a67aa0209749ded8174b088904 Stats: 130 lines in 9 files changed: 22 ins; 87 del; 21 mod 8313670: Simplify shared lib name handling code in some tests Reviewed-by: cjplummer, sspitsyn - PR: https://git.openjdk.org/jdk/pull/15151
RFR: JDK-8314197: AttachListener::pd_find_operation always returning nullptr
AttachListener::pd_find_operation always returns nullptr and seems to be obsolete, so we could probably remove it and clean up the coding a bit. - Commit messages: - JDK-8314197 Changes: https://git.openjdk.org/jdk/pull/15265/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15265&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8314197 Stats: 24 lines in 6 files changed: 0 ins; 24 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/15265.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15265/head:pull/15265 PR: https://git.openjdk.org/jdk/pull/15265
Re: RFR: JDK-8314197: AttachListener::pd_find_operation always returning nullptr
On Mon, 14 Aug 2023 10:13:21 GMT, Matthias Baesken wrote: > AttachListener::pd_find_operation always returns nullptr and seems to be > obsolete, so we could probably remove it and clean up the coding a bit. Hi David, thanks for the review ! If it is 'trivial' , afaik one review is sufficient, is that correct ? - PR Comment: https://git.openjdk.org/jdk/pull/15265#issuecomment-1677224370
Re: RFR: JDK-8314197: AttachListener::pd_find_operation always returning nullptr
On Mon, 14 Aug 2023 10:13:21 GMT, Matthias Baesken wrote: > AttachListener::pd_find_operation always returns nullptr and seems to be > obsolete, so we could probably remove it and clean up the coding a bit. Hi Chris and Serguei, thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/15265#issuecomment-1678545645
Integrated: JDK-8314197: AttachListener::pd_find_operation always returning nullptr
On Mon, 14 Aug 2023 10:13:21 GMT, Matthias Baesken wrote: > AttachListener::pd_find_operation always returns nullptr and seems to be > obsolete, so we could probably remove it and clean up the coding a bit. This pull request has now been integrated. Changeset: 63389272 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/6338927221ee82a556b55ccf79239acb2ac9729a Stats: 24 lines in 6 files changed: 0 ins; 24 del; 0 mod 8314197: AttachListener::pd_find_operation always returning nullptr Reviewed-by: dholmes, cjplummer, sspitsyn - PR: https://git.openjdk.org/jdk/pull/15265
RFR: JDK-8314389: AttachListener::pd_set_flag obsolete
AttachListener::pd_set_flag is the same across platforms (always returning JNI_ERR ). So it can be centralized or removed. - Commit messages: - JDK-8314389 Changes: https://git.openjdk.org/jdk/pull/15304/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15304&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8314389 Stats: 27 lines in 6 files changed: 0 ins; 26 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15304.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15304/head:pull/15304 PR: https://git.openjdk.org/jdk/pull/15304
Re: RFR: JDK-8314389: AttachListener::pd_set_flag obsolete
On Wed, 16 Aug 2023 08:09:42 GMT, Matthias Baesken wrote: > AttachListener::pd_set_flag is the same across platforms (always returning > JNI_ERR ). So it can be centralized or removed. Hi Chris, Martin, Serguei, thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/15304#issuecomment-1683430494
Integrated: JDK-8314389: AttachListener::pd_set_flag obsolete
On Wed, 16 Aug 2023 08:09:42 GMT, Matthias Baesken wrote: > AttachListener::pd_set_flag is the same across platforms (always returning > JNI_ERR ). So it can be centralized or removed. This pull request has now been integrated. Changeset: 5058854b Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/5058854b867323dd6537d7387bf20a9d5f258084 Stats: 27 lines in 6 files changed: 0 ins; 26 del; 1 mod 8314389: AttachListener::pd_set_flag obsolete Reviewed-by: cjplummer, mdoerr, sspitsyn - PR: https://git.openjdk.org/jdk/pull/15304
Re: RFR: JDK-8315214: Do not run sun/tools/jhsdb tests concurrently
On Tue, 29 Aug 2023 13:46:08 GMT, Matthias Baesken wrote: > The sun/tools/jhsdb tests have issues when we run them concurrently . > So add a related config to TEST.root. Hi Alan and Chris , thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/15469#issuecomment-1698584757
Integrated: JDK-8315214: Do not run sun/tools/jhsdb tests concurrently
On Tue, 29 Aug 2023 13:46:08 GMT, Matthias Baesken wrote: > The sun/tools/jhsdb tests have issues when we run them concurrently . > So add a related config to TEST.root. This pull request has now been integrated. Changeset: 1e7e2bcf Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/1e7e2bcf3560e1ad39516fb604e4d8bf85bb54e0 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8315214: Do not run sun/tools/jhsdb tests concurrently Reviewed-by: alanb, cjplummer - PR: https://git.openjdk.org/jdk/pull/15469
RFR: JDK-8315897: some PrivilegedActions missing in JDK code for getting properties
There are some remaining places in 'general' JDK code (= code not related to e.g. a specific tool) getting properties like : osName = System.getProperty(os.name) https://github.com/openjdk/jdk/blob/master/src/java.management/share/classes/sun/management/VMManagementImpl.java#L225 https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/sun/awt/FontConfiguration.java#L134 Those should be a PrivilegedAction . - Commit messages: - JDK-8315897 Changes: https://git.openjdk.org/jdk/pull/15629/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15629&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8315897 Stats: 57 lines in 2 files changed: 36 ins; 4 del; 17 mod Patch: https://git.openjdk.org/jdk/pull/15629.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15629/head:pull/15629 PR: https://git.openjdk.org/jdk/pull/15629
Re: RFR: JDK-8315897: some PrivilegedActions missing in JDK code for getting properties
On Fri, 8 Sep 2023 08:26:16 GMT, Matthias Baesken wrote:
> There are some remaining places in 'general' JDK code (= code not related to
> e.g. a specific tool) getting properties like :
>
> osName = System.getProperty(os.name)
>
> https://github.com/openjdk/jdk/blob/master/src/java.management/share/classes/sun/management/VMManagementImpl.java#L225
>
> https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/sun/awt/FontConfiguration.java#L134
>
> Those should be a PrivilegedAction .
Hi Alan, seems the calling into the VMManagement methods does not work any more
in recent JDK (in 8 it was still possible in a kind of hack-like way but I
tried it with 21 and it does not work any more (or maybe with setting lots of
flags?) ).
import sun.management.VMManagement;
import java.lang.management.*;
import java.lang.reflect.*;
import java.security.*;
public class VMExample {
static class MySm extends SecurityManager {
public void checkPermission(Permission p) {
// okay
}
@Override
public void checkPropertyAccess(String key) {
System.out.println("accessing " + key);
// throw new SecurityException("accessing " + key);
}
}
private static void printInfo() {
try {
// Get the current process id using a reflection hack
RuntimeMXBean runtime = ManagementFactory.getRuntimeMXBean();
Field jvm = runtime.getClass().getDeclaredField("jvm");
jvm.setAccessible(true);
System.out.println("Getting VMManagement object");
VMManagement mgmt = (sun.management.VMManagement) jvm.get(runtime);
System.out.println("OS name:" + mgmt.getOsName());
} catch(Exception ex) {
}
}
public static void main(String[] args) {
//System.setSecurityManager(new SecurityManager());
System.setSecurityManager(new MySm());
System.out.println("Before printInfo ...");
printInfo();
}
}
So probably we could avoid changing VMManagementImpl.java because it is not
needed any more at least in jdk-head .
-
PR Comment: https://git.openjdk.org/jdk/pull/15629#issuecomment-1711480332
Re: RFR: JDK-8315897: some PrivilegedActions missing in JDK code for getting properties
On Fri, 8 Sep 2023 14:01:24 GMT, Alexey Ivanov wrote:
> > So what about FontConfiguration? Is that something using the class directly
> > too?
>
> I think this is not needed either. As far as I can see, the instance of
> `FontConfiguration` is created from `doPrivileged`, therefore these two
> system properties are read inside `doPrivileged` already.
Hi there is a public constructor
` public FontConfiguration(SunFontManager fm) {`
calling setOsNameAndVersion(), so is it really always called from
`doPrivileged` ? (however it is currently only exported in a qualified way)
-
PR Comment: https://git.openjdk.org/jdk/pull/15629#issuecomment-1715235616
Re: RFR: JDK-8315897: some PrivilegedActions missing in JDK code for getting properties
On Fri, 8 Sep 2023 13:02:07 GMT, Alan Bateman wrote: > > So probably we could avoid changing VMManagementImpl.java because it is not > > needed any more at least in jdk-head . > > It's a JDK internal class so should never have been used directly. I can't > imagine what might be using a JDK internal class while running with a SM. > okay thanks for the explanation. So could we remove the existing PrivilegedAction in getCompilerName ? - PR Comment: https://git.openjdk.org/jdk/pull/15629#issuecomment-1715244233
Re: RFR: JDK-8315706: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java real fix for failure on AIX [v5]
On Fri, 15 Sep 2023 07:22:32 GMT, Joachim Kern wrote: >> After push of [JDK-8307478](https://bugs.openjdk.org/browse/JDK-8307478) , >> the following test started to fail on AIX : >> com/sun/tools/attach/warnings/DynamicLoadWarningTest.java; >> The problem was described in >> [JDK-8309549](https://bugs.openjdk.org/browse/JDK-8309549) with a first try >> of a fix. >> A second fix via [JDK-8310191](https://bugs.openjdk.org/browse/JDK-8310191) >> was necessary. >> Both fixes just disable the specific subtest on AIX, without correction of >> the root cause. >> The root cause is, that dlopen() on AIX returns different handles every >> time, even if you load a library twice. There is no official AIX API >> available to get this information on a different way. >> My proposal is, to use the stat64x API with the fields st_device and >> st_inode. After a dlopen() the stat64x() API is called additionally to get >> this information which is then stored parallel to the library handle in the >> jvmtiAgent. For AIX we then can compare these values instead of the library >> handle and get the same functionality as on linux. > > Joachim Kern has updated the pull request incrementally with one additional > commit since the last revision: > > adopt types Marked as reviewed by mbaesken (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15583#pullrequestreview-1630726225
Withdrawn: JDK-8315897: some PrivilegedActions missing in JDK code for getting properties
On Fri, 8 Sep 2023 08:26:16 GMT, Matthias Baesken wrote: > There are some remaining places in 'general' JDK code (= code not related to > e.g. a specific tool) getting properties like : > > osName = System.getProperty(os.name) > > https://github.com/openjdk/jdk/blob/master/src/java.management/share/classes/sun/management/VMManagementImpl.java#L225 > > https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/sun/awt/FontConfiguration.java#L134 > > Those should be a PrivilegedAction . This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/15629
RFR: JDK-8318957: enhance agentlib:jdwp help output by info about allow option
The allow option of agentlib:jdwp has been introduced a long time ago (see JDK-8061228) and is documented here : https://docs.oracle.com/en/java/javase/17/docs/specs/jpda/conninv.html#oracle-vm-invocation-options However it is still missing in the agentlib:jdwp help output and should be added there too. - Commit messages: - JDK-8318957 Changes: https://git.openjdk.org/jdk/pull/16393/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16393&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8318957 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16393.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16393/head:pull/16393 PR: https://git.openjdk.org/jdk/pull/16393
Re: RFR: JDK-8318957: enhance agentlib:jdwp help output by info about allow option [v2]
> The allow option of agentlib:jdwp has been introduced a long time ago (see > JDK-8061228) and is documented here : > https://docs.oracle.com/en/java/javase/17/docs/specs/jpda/conninv.html#oracle-vm-invocation-options > However it is still missing in the agentlib:jdwp help output and should be > added there too. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: More info about address lists - Changes: - all: https://git.openjdk.org/jdk/pull/16393/files - new: https://git.openjdk.org/jdk/pull/16393/files/095bc15a..0b868a03 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16393&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16393&range=00-01 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16393.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16393/head:pull/16393 PR: https://git.openjdk.org/jdk/pull/16393
