Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Mon, 6 Jun 2022 15:56:47 GMT, Tim Prinzing wrote: > The idea was to reduce duplicate code. Changing to use objects to encapsulate > the up calls got rid of a lot of repeated code and made things simpler and > clearer. Objects are created with the class, method, and signature strings > and then calls can be made and the the error reporting has a context. It was > simply easier to do in c++ and there was no reason not to use it. Fair enough. At least we will now have a good example of what is needed to build a C++ test like this. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make
On Sat, 4 Jun 2022 01:51:20 GMT, Leo Korinth wrote: > One can select a testcase by ID when running a jtreg test case directly from > jtreg (using the testcase.java#testID syntax). However, this has not been > possible to do when launching jtreg indirectly from make. > > This fix attempts to address this issue. I have not tested this thoroughly > yet, I wanted to show the code to get feedback first. The idea is to *not* > emulated destructive imperative assignments through the use of eval. I > instead try to handle it in a functional style without reassigning variables. > I have tried to make the change as small as possible. > > I am not used to change the build system, so I would appreciate thorough > review. > > `IfAppend` and `IfPrepend` are general purpose functions. If similar > functions exists elsewhere inside the code base or in make itself I should > probably use those instead. If they do not exist, they might be useful > elsewhere and maybe should be placed in a common make file instead of the > RunTests.gmk file. Let me ask the obvious "dumb" question ... why does this have to be so complicated? Why isn't the name of the test simply passed through to jtreg as typed? - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing wrote: > Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest Hi Tim, Sorry but I have to ask why this test was created as a C++ program instead of keeping it as a C program likes it predecessors? No need for C++ libs or special exception handling flags in that case. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v4]
On Wed, 1 Jun 2022 03:18:44 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 38 additional commits since > the last revision: > > - Adjust deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Update deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Respond to review feedback. > - Update symbol information for JDK 19 b24. > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - ... and 28 more: > https://git.openjdk.java.net/jdk/compare/c1abb195...54e872b5 Updates look good. Thanks - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8236
Re: Integrated: 8286562: GCC 12 reports some compiler warnings
The new assertion in src/hotspot/share/utilities/globalDefinitions.hpp
inline const char* type2name(BasicType t) {
assert((uint)t < T_CONFLICT + 1, "invalid type");
return type2name_tab[t];
}
is failing with test
compiler/jvmci/errors/TestInvalidDebugInfo.java
I have filed:
https://bugs.openjdk.java.net/browse/JDK-8287491
The test will probably need ProblemListing as it is causing major noise
in our CI.
David
On 28/05/2022 12:12 pm, Yasumasa Suenaga wrote:
On Wed, 11 May 2022 05:58:31 GMT, Yasumasa Suenaga wrote:
I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 on
Fedora 36.
As you can see, the warnings spreads several areas. Let me know if I should
separate them by area.
* -Wstringop-overflow
* src/hotspot/share/oops/array.hpp
*
src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
In member function 'void Array::at_put(int, const T&) [with T = unsigned
char]',
inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at
/home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
inlined from 'void ConstantPool::method_at_put(int, int, int)' at
/home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15,
inlined from 'ConstantPool*
BytecodeConstantPool::create_constant_pool(JavaThread*) const' at
/home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26:
This pull request has now been integrated.
Changeset: 410a25d5
Author:Yasumasa Suenaga
URL:
https://git.openjdk.java.net/jdk/commit/410a25d59a11b6a627bbb0a2c405c2c2be19f464
Stats: 41 lines in 5 files changed: 26 ins; 1 del; 14 mod
8286562: GCC 12 reports some compiler warnings
Reviewed-by: ihse, kbarrett, prr
-
PR: https://git.openjdk.java.net/jdk/pull/8646
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v2]
On Thu, 26 May 2022 23:05:32 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Marked as reviewed by dholmes (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: JDK-8284858: Start of release updates for JDK 20
On Thu, 14 Apr 2022 05:09:14 GMT, Joe Darcy wrote: > Time to start getting ready for JDK 20... One comment below. I ignored the sym files. Everything else appears okay. Thanks. src/java.base/share/classes/jdk/internal/org/objectweb/asm/Opcodes.java line 312: > 310: int V18 = 0 << 16 | 62; > 311: int V19 = 0 << 16 | 63; > 312: int V20 = 0 << 17 | 64; 17 ?? Though why do we even bother with this when the minor version is zero? Simple assignment works. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8287174: Remove deprecated configure arguments
On Mon, 23 May 2022 17:25:30 GMT, Magnus Ihse Bursie wrote: > We have a bunch of configure arguments that has been deprecated for multiple > releases. These should be removed. In effect, this will raise an error > instead of a warning if these argument is included on the command line for > configure. The deprecated arguments stopped having any effect when they were > deprecated. Looks good. Thanks - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8852
Re: RFR: 8286262: Windows: Cleanup deprecation warning suppression
On Sun, 15 May 2022 20:50:25 GMT, Kim Barrett wrote: > Please review this cleanup of deprecation warning suppression when building > for Windows. > > This change consists of several parts. > > (1) Remove the global deprecation warning suppression when building HotSpot > for Windows. > > (2) Add macro definitions requesting suppression of selected sets of > deprecation warnings when building HotSpot for Windows. > > (3) Remove unnecessary forwarding macros for various POSIX functions in > globalDefinitions_visCPP.hpp. These were provided to avoid deprecation > warnings (that were previously also being suppressed by the global request). > They are now covered by the new macros provided by change (2) above. > > An alternative to item (3) is to not define _CRT_NONSTDC_NO_DEPRECATE (in item > (2)) and either retain the forwarding macros or define os:: wrapper functions > for all of the affected functions. We might eventually do the latter because > of other reasons for avoiding some of these functions, but the approach being > taken here is simpler. > > For documentation of _CRT_NONSTDC_NO_DEPRECATE, see: > https://docs.microsoft.com/en-us/cpp/c-runtime-library/compatibility > https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996 > > Similarly for _CRT_SECURE_NO_WARNINGS. > > Perhaps similarly for _WINSOCK_DEPRECATED_NO_WARNINGS (though I didn't find > any documentation for the latter). But it might be better to not supress the > warnings and instead use the alternatives (JDK-8286781). > > Testing: > mach5 tier1 All seems quite reasonable. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8718
Re: pre-submit tests for github PRs
On 23/05/2022 8:22 am, Philip Race wrote: Why is it that the vast majority of PRs are recording spurious looking failures of github pre-submit tests ? https://github.com/openjdk/jdk/pulls?q=type%3Apr+is%3Aopen+label%3Arfr There seems to be so much noise in these that I pay no attention to them any more (except for jcheck) but they seem to cause concern in some contributors who can't tell what they might have broken Even weirder is that we test Linux x86 there ?? When it isn't a mainstream platform. Why ? Especially since it seem to be the most common failure. GHA tests a range of OpenJDK ports, not just the "mainstream platforms". The existing linux-86 failures (and others) are due to the Loom integration which only fully supports a couple of platforms and which broke all the other ports upon initial integration. Some folks in the community have been working through things to fix that. David -phil.
Re: RFR: 8286262: Windows: Cleanup deprecation warning suppression
On Sun, 15 May 2022 20:50:25 GMT, Kim Barrett wrote: > Please review this cleanup of deprecation warning suppression when building > for Windows. > > This change consists of several parts. > > (1) Remove the global deprecation warning suppression when building HotSpot > for Windows. > > (2) Add macro definitions requesting suppression of selected sets of > deprecation warnings when building HotSpot for Windows. > > (3) Remove unnecessary forwarding macros for various POSIX functions in > globalDefinitions_visCPP.hpp. These were provided to avoid deprecation > warnings (that were previously also being suppressed by the global request). > They are now covered by the new macros provided by change (2) above. > > An alternative to item (3) is to not define _CRT_NONSTDC_NO_DEPRECATE (in item > (2)) and either retain the forwarding macros or define os:: wrapper functions > for all of the affected functions. We might eventually do the latter because > of other reasons for avoiding some of these functions, but the approach being > taken here is simpler. > > For documentation of _CRT_NONSTDC_NO_DEPRECATE, see: > https://docs.microsoft.com/en-us/cpp/c-runtime-library/compatibility > https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996 > > Similarly for _CRT_SECURE_NO_WARNINGS. > > Perhaps similarly for _WINSOCK_DEPRECATED_NO_WARNINGS (though I didn't find > any documentation for the latter). But it might be better to not supress the > warnings and instead use the alternatives (JDK-8286781). > > Testing: > mach5 tier1 Sorry Kim I'm having trouble seeing what change corresponds to (1) ?? Also the PR talks only about hotspot, but you're changing the JDK flags as well. ?? - PR: https://git.openjdk.java.net/jdk/pull/8718
Re: RFR: 8280844: Epoch shift synchronization point for Compiler threads is inadequate
On Mon, 16 May 2022 10:17:42 GMT, Markus Grönlund wrote:
> Greetings,
>
> [JDK-8233111](https://bugs.openjdk.java.net/browse/JDK-8233111) attempted to
> address artefact tagging for Compiler threads, letting threads run
> _thread_in_native to avoid the transition. Unfortunately, that attempt proved
> inadequate.
>
> The epoch race is avoided only by performing the transition to _thread_in_vm.
>
> Testing: jdk_jfr
>
> Thanks
> Markus
src/hotspot/share/compiler/compilerEvent.cpp line 126:
> 124: static inline void commit(EventType& event) {
> 125: JavaThread* thread = JavaThread::current();
> 126: assert(thread->thread_state() == _thread_in_native, "invariant");
You don't need this assert as `ThreadInVMfromNative` already has it.
-
PR: https://git.openjdk.java.net/jdk/pull/8724
Re: RFR: 8286744: failure_handler: dmesg command on macos fails to collect data due to permission issues [v2]
On Mon, 16 May 2022 07:54:09 GMT, Jaikiran Pai wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> copyright year > > Thank you Daniel and Lance for the reviews. > > Would anyone from the build team like to provide any additional reviews? @jaikiran this is more a SQE issue that an build issue. @lmesnik may have an opinion. - PR: https://git.openjdk.java.net/jdk/pull/8703
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 11 May 2022 16:00:32 GMT, Maxim Kartashev wrote: >> Matthias Baesken has updated the pull request incrementally with one >> additional commit since the last revision: >> >> adjust API level to Windows 8 for security.cpp and do some cleanup > > This change seem to have made this group of declarations obsolete: > `src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h:157` (follow > the [link]( > https://github.com/openjdk/jdk/blob/89de756ffbefac452c7df559e2a4eb50bf71368b/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h#L157)). > Does anyone have plans to drop those? Have any bugs been filed for that? If > not, I'll attend to that myself. @mkartashev all references to WINVER in the AWT code seem obsolete. Possibly most of the IS_WINxxx usages could also be replaced. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8286429: jpackageapplauncher build fails intermittently in Tier[45]
On Mon, 9 May 2022 23:18:47 GMT, Erik Joelsson wrote: > The way LauncherCommon.gmk is currently written, it's only meant to be > included from "make/module//Launcher.gmk", or at least only from one > single place for each module. This is because the man page generation that > happens in LauncherCommon.gmk is module global. For the jdk.jpackage module, > LauncherCommon.gmk is now called from two separate sub makefiles, both > make/module/jdk.jpackage/Lib.gmk and make/module/jdk.jpackage/Launcher.gmk. > These files are called from the top level targets jdk.jpackage-libs and > jdk.jpackage-launchers respectively. These top level targets are assumed to > be able to run independently of each other, which is normally fine, but now > they define the same rules for the same files. This creates a race where one > may be deleting files that the other one is creating, causing directories to > disappear while files are being written to them. This can fail the build, but > also risks silently corrupting the build. > > This patch fixes this by adding a conditional around the man page generation, > which helps guarantee that man pages are only processed when called from > make/module//Launcher.gmk. It's a bit of a hack, but it's building on > top of the existing design of piggybacking man page generation on the > launchers build. > > Also fixing broken whitespace further down in the file (tabs->space). I'm still unclear how `--disable-manpages` is supposed to affect this logic, as they seemed to be the failing configs. - PR: https://git.openjdk.java.net/jdk/pull/8618
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup This seems okay to me in this form. I agree that explicitly setting this is better than unknowingly using API's that might not be available on supported platforms. Thanks. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup I'm confused. `src\jdk.crypto.mscapi\windows\native\libsunmscapi\security.cpp` doesn't set _WIN32_WINNT so how is that later API being enabled? Does this mean that not setting _WIN32_WINNT means :any API is allowed" ? - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v3]
On Tue, 3 May 2022 07:10:58 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust Copyright year in wepoll.c I agree with Erik - the source locations need to be modified to not define it. If we want to keep a record perhaps add an assertion that the value is what was expected? I still feel we have a disconnect between this and an actual check for what the build and runtime platform version is ... and it isn't at all clear how someone using an API only in a later Windows version, and so setting _WIN32_WINNT to a higher value, will know that this is defined down in the build files ? - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). That only seems to be half of the issue though. If we are defining _WIN32_WINNT=0x0601 because the minimum required OS API support level is Windows 7, then don't we need a check that the build platform is also at least Windows 7? - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). There is obviously a lot of history here and different parts of the codebase had hit different minimum OS version requirements at different times. While at one time we would have had to cater for building on systems with and without a given API those days are long gone for the code being touched here (AFAICS). As Erik states we should be able to set a minimum _WIN32_WINNT_ value needed to build all the codebase and simply have that checked and set at configure time. The code itself would not need to define _WIN32_WINNT. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8284661: Reproducible assembly builds without relative linking [v3]
On Mon, 11 Apr 2022 21:03:22 GMT, Magnus Ihse Bursie wrote: >> Andrew Leonard has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8284661: Reproducible assembly builds without relative linking >> >> Signed-off-by: Andrew Leonard > > Looks like there is way too few .S files in this patch. > > > $ find . -name "*.S" > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_atan2_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_atan_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_tanh_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_tan_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_pow_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_cbrt_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_log_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_log10_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_cosh_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_log1p_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_exp_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_hypot_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_expm1_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_sinh_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_cos_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_acos_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_asin_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_sin_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_exp_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_expm1_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_cos_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_asin_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_hypot_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_sinh_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_sin_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_acos_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_log1p_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_log10_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_tanh_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_atan_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_tan_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_log_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_s_pow_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_cosh_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_cbrt_linux_x86.S > ./jdk.incubator.vector/linux/native/libjsvml/jsvml_d_atan2_linux_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_tanh_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_tanh_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_log10_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_exp_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_log_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_log10_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_cos_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_log1p_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_sin_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_tan_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_pow_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_cosh_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_cosh_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_log1p_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_atan_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_atan_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_tan_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_pow_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_hypot_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_sin_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_hypot_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_asin_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_asin_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_expm1_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_s_cos_windows_x86.S > ./jdk.incubator.vector/windows/native/libjsvml/jsvml_d_sinh_windows_x86.S >
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v24]
On Mon, 11 Apr 2022 12:04:41 GMT, Maurizio Cimadamore wrote: >> src/hotspot/share/prims/scopedMemoryAccess.cpp line 141: >> >>> 139: >>> 140: /* >>> 141: * This function performs a thread-local handshake against all threads >>> running at the time >> >> Nit: thread-local?? > > I was assuming the term "thread-local handshake" was a thing in the VM, as > per: > https://openjdk.java.net/jeps/312 Ha! Mea culpa. I've been too buried in the other kind of thread-local lately. I completely forgot we actually described handshakes that way. :) - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v24]
On Mon, 4 Apr 2022 14:57:30 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-424 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/424 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Fix TestLinkToNativeRBP VM changes look good. Thanks, David src/hotspot/share/prims/scopedMemoryAccess.cpp line 141: > 139: > 140: /* > 141: * This function performs a thread-local handshake against all threads > running at the time Nit: thread-local?? - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7888
Re: Is --with-zlib=bundled broken on MacOS aarch64 12.2.1?
On 29/03/2022 7:20 pm, Magnus Ihse Bursie wrote: On 2022-03-29 03:42, Jaikiran Pai wrote: Hello Magnus, On 28/03/22 5:21 pm, Magnus Ihse Bursie wrote: On 2022-03-28 09:03, David Holmes wrote: On 28/03/2022 4:56 pm, Alan Bateman wrote: On 28/03/2022 07:46, David Holmes wrote: Hi Jai, It isn't obvious to me that the bundled sources are actually intended to build on macOS. There's no include of unistd.h to get the lseek definition. I think the context here is that Jai is chasing an issue that may be bug in the libz on macOS. Building the bundled version and comparing results would lead to useful information. AFAIK, the system zlib has always been used since 7u4 when the macOS was added. I think the Apple JDK did the same. So there may be a small bit of "porting" to do, adds include files, etc. Okay, I suggest adding the include of unistd.h as a starter then. But Jai should note that this is not a build issue per-se - making those sources buildable on all desired platforms is the job of the owners of that src in the OpenJDK. I agree fully, but I'd also like to add that it is not clear that this is not "supposed" to work. If bundled zlib is not buildable on macOS (that was news to me), then the correct build system behavior should have been to block it in configure. So, Jaikiran, if you intend to get this to work on macOS, great! If you're not taking that route, please let me know so I can open a bug on prohibiting bundled zlib on maccOS from configure. I used David's suggestion to add that include header: diff --git a/src/java.base/share/native/libzip/zlib/gzlib.c b/src/java.base/share/native/libzip/zlib/gzlib.c index a814dd8c7b6..9df629d4205 100644 --- a/src/java.base/share/native/libzip/zlib/gzlib.c +++ b/src/java.base/share/native/libzip/zlib/gzlib.c @@ -35,6 +35,7 @@ #if defined(_LARGEFILE64_SOURCE) && _LFS64_LARGEFILE-0 # define LSEEK lseek64 #else +# include # define LSEEK lseek #endif #endif That did help me get past the lseek error, but there are some more errors (as noted in that log) which need to be addressed too. I don't have the necessary knowledge of C ecosystem to decide what set of includes and whether those includes need to be conditional for macOS, are required to get this building. The other thing to consider with this code is that it is bundled code and the real code lies in a separate upstream zlib project[1]. So I think whatever changes we do here might have to be co-ordinated back to that project. Maybe. But when we bundle code we also kind of rip it out of context. It might very well be that the upstream code has an automake system which generates a config.h that has proper includes and defines to get this to build. I mean, obviously there is an existing system zlib on macOS, so *someone* has gotten it to build. Our sources are for a very old version: 1.2.11 (15 Jan 2017) which doesn't appear to support building on macOS. The webpage for zlib states [1]: "zlib for macOS (Mac OS X): zlib is already included as part of macOS" so perhaps it was never intended to be built directly on macOS. David - [1] https://www.zlib.net/ Please go ahead with your plan to create a JBS issue to prohibit this combination in the meantime. Ok, done that: https://bugs.openjdk.java.net/browse/JDK-8283844 What surprises me a bit is that looking at the commit history within this source area, there haven't been any commits in this code for around 4 years now. So I suspect this hasn't been working at least that long or maybe the compiler rules got more stricter during that period. This has most likely never been working, yes. :) We've had a quite strict policy of bundling-or-system-zlib depending on OS, and I don't think anyone has ever tried to override that before. (With the exception of linux, where I'm sure both options will work fine.) /Magnus [1] https://github.com/madler/zlib/tree/v1.2.11 -Jaikiran
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v5]
On Mon, 28 Mar 2022 12:58:20 GMT, Thomas Stuefe wrote:
>> Christian Hagedorn has updated the pull request with a new target base due
>> to a merge or a rebase. The pull request now contains 54 commits:
>>
>> - Updating some comments
>> - Cleanup loading dwarf file and add summary
>> - Review comments of first pass by Thomas except dwarf file loading
>> - Merge branch 'master' into JDK-8242181
>> - Make dwarf tag NOT_PRODUCT
>> - Change log_* to log_develop_* and log_warning to log_develop_info
>> - Update test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java
>>
>>Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com>
>> - Update test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java
>>
>>Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com>
>> - Better formatting of trace output
>> - some code move and more cleanups
>> - ... and 44 more:
>> https://git.openjdk.java.net/jdk/compare/efd3967b...5bea4841
>
> src/hotspot/share/utilities/elfFile.cpp line 450:
>
>> 448: if (buf == nullptr) {
>> 449: return false;
>> 450: }
>
> I'd move this close to and local to where it is used.
>
> Also, you seem to repeat the same pattern a lot "NEW_RESOURCE_ARRAY(n), if
> error return something". I'd factor this out to an utility function or
> utility macro, maybe one where you pass the error return value as macro
> parameter.
Thomas's comment caught my attention in the email. NEW_RESOURCE_ARRAY aborts
the VM on OOM. Use NEW_RESOURCE_ARRAY_RETURN_NULL if you want to continue.
-
PR: https://git.openjdk.java.net/jdk/pull/7126
Re: Is --with-zlib=bundled broken on MacOS aarch64 12.2.1?
On 28/03/2022 4:56 pm, Alan Bateman wrote: On 28/03/2022 07:46, David Holmes wrote: Hi Jai, It isn't obvious to me that the bundled sources are actually intended to build on macOS. There's no include of unistd.h to get the lseek definition. I think the context here is that Jai is chasing an issue that may be bug in the libz on macOS. Building the bundled version and comparing results would lead to useful information. AFAIK, the system zlib has always been used since 7u4 when the macOS was added. I think the Apple JDK did the same. So there may be a small bit of "porting" to do, adds include files, etc. Okay, I suggest adding the include of unistd.h as a starter then. But Jai should note that this is not a build issue per-se - making those sources buildable on all desired platforms is the job of the owners of that src in the OpenJDK. Cheers, David -Alan
Re: Is --with-zlib=bundled broken on MacOS aarch64 12.2.1?
Hi Jai, It isn't obvious to me that the bundled sources are actually intended to build on macOS. There's no include of unistd.h to get the lseek definition. Cheers, David On 28/03/2022 2:50 pm, Jaikiran Pai wrote: I'm using the following set of commands to build the JDK on my Mac M1 12.2.1 version. Specifically, I use the --with-zlib=bundled option during configure: bash configure --with-boot-jdk= --with-zlib=bundled make clean make images This runs into build errors during make images. The configure summary is as follows and the error logs (snippet) is pasted at the end of this mail. Is this some issue with the versions of compiler/tools on my setup or is this genuinely broken? configure log summary: The existing configuration has been successfully updated in /home/me/jdk/build/macosx-aarch64-server-release using configure arguments '--with-boot-jdk=/home/me/java/openjdk/jdk-17.0.2.jdk/Contents/Home --with-zlib=bundled'. Configuration summary: * Name: macosx-aarch64-server-release * Debug level: release * HS debug level: product * JVM variants: server * JVM features: server: 'cds compiler1 compiler2 dtrace epsilongc g1gc jfr jni-check jvmci jvmti management parallelgc serialgc services shenandoahgc vm-structs zgc' * OpenJDK target: OS: macosx, CPU architecture: aarch64, address length: 64 * Version string: 19-internal-adhoc.jaikiran.open (19-internal) * Source date: Determined at build time Tools summary: * Boot JDK: openjdk version "17.0.2" 2022-01-18 OpenJDK Runtime Environment (build 17.0.2+8-86) OpenJDK 64-Bit Server VM (build 17.0.2+8-86, mixed mode, sharing) (at /home/me/java/openjdk/jdk-17.0.2.jdk/Contents/Home) * Toolchain: clang (clang/LLVM from Xcode 13.2.1) * C Compiler: Version 13.0.0 (at /usr/bin/clang) * C++ Compiler: Version 13.0.0 (at /usr/bin/clang++) Build performance summary: * Build jobs: 8 * Memory limit: 16384 MB build errors: /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:276:9: error: implicit declaration of function 'lseek' is invalid in C99 [-Werror,-Wimplicit-function-declaration] LSEEK(state->fd, 0, SEEK_END); /* so gzoffset() is correct */ ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: expanded from macro 'LSEEK' # define LSEEK lseek ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:276:9: note: did you mean 'fseek'? /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: expanded from macro 'LSEEK' # define LSEEK lseek ^ /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/include/stdio.h:162:6: note: 'fseek' declared here int fseek(FILE *, long, int); ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:282:24: error: implicit declaration of function 'lseek' is invalid in C99 [-Werror,-Wimplicit-function-declaration] state->start = LSEEK(state->fd, 0, SEEK_CUR); ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: expanded from macro 'LSEEK' # define LSEEK lseek ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:383:9: error: implicit declaration of function 'lseek' is invalid in C99 [-Werror,-Wimplicit-function-declaration] if (LSEEK(state->fd, state->start, SEEK_SET) == -1) ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: expanded from macro 'LSEEK' # define LSEEK lseek ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:424:15: error: implicit declaration of function 'lseek' is invalid in C99 [-Werror,-Wimplicit-function-declaration] ret = LSEEK(state->fd, offset - state->x.have, SEEK_CUR); ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: expanded from macro 'LSEEK' # define LSEEK lseek ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:520:14: error: implicit declaration of function 'lseek' is invalid in C99 [-Werror,-Wimplicit-function-declaration] offset = LSEEK(state->fd, 0, SEEK_CUR); ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: expanded from macro 'LSEEK' # define LSEEK lseek ^ 5 errors generated. /home/me/jdk/src/java.base/share/native/libzip/zlib/gzread.c:59:15: error: implicit declaration of function 'read' is invalid in C99 [-Werror,-Wimplicit-function-declaration] ret = read(state->fd, buf + *have, get); ^ /home/me/jdk/src/java.base/share/native/libzip/zlib/gzread.c:59:15: note: did you mean 'fread'? /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/include/stdio.h:158:9: note: 'fread' declared here size_t fread(void * __restrict __ptr, size_t __size, size_t __nitems, FILE *
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]
On Tue, 22 Mar 2022 12:08:01 GMT, Fei Yang wrote: >> make/autoconf/libraries.m4 line 152: >> >>> 150: fi >>> 151: >>> 152: # Programs which use C11 or C++11 atomics, like #include , >> >> Use of C++ atomics is not allowed in hotspot code base. See the style guide: >> https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md >> >> That said, I don't see any actual use of C++ atomics. ?? > > I think the old code comment here is a bit too general. It does not mean we > introduce any use of C++ atomics here. > The fact is that RISC-V only has word-sized atomics, it requries libatomic > where other common architectures do not [1]. > So atomic support would require explicit linking against -latomic on RISC-V. > Otherwise we got build errors like: New comment looks good - thanks for clarifying. - PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v3]
On Tue, 22 Mar 2022 11:50:13 GMT, Fei Yang wrote:
>> This PR implements JEP 422: Linux/RISC-V Port [1].
>> The PR starts as a squashed merge of the
>> https://openjdk.java.net/projects/riscv-port branch.
>>
>> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive
>> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are
>> also carried out regularly. So it should be good enough to run most Java
>> programs.
>>
>> [1] https://openjdk.java.net/jeps/422
>
> Fei Yang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Address review comments
Marked as reviewed by dholmes (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]
On Tue, 22 Mar 2022 03:31:16 GMT, Fei Yang wrote:
>> This PR implements JEP 422: Linux/RISC-V Port [1].
>> The PR starts as a squashed merge of the
>> https://openjdk.java.net/projects/riscv-port branch.
>>
>> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive
>> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are
>> also carried out regularly. So it should be good enough to run most Java
>> programs.
>>
>> [1] https://openjdk.java.net/jeps/422
>
> Fei Yang 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 two additional commits since
> the last revision:
>
> - Merge remote-tracking branch 'upstream/master' into JDK-8276799
> - 8276799: Implementation of JEP 422: Linux/RISC-V Port
Hi,
I've looked at everything that is not a RISC-V specific file, except for the C1
changes as the compiler folk will need to approve those.
Some copyrights will need updating to 2022 on the Oracle copyright line please.
I have flagged one issue in regard to C++ atomics - see below.
Thanks,
David
make/autoconf/libraries.m4 line 152:
> 150: fi
> 151:
> 152: # Programs which use C11 or C++11 atomics, like #include ,
Use of C++ atomics is not allowed in hotspot code base. See the style guide:
https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md
That said, I don't see any actual use of C++ atomics. ??
-
Marked as reviewed by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8253495: CDS generates non-deterministic output
On 11/03/2022 4:50 pm, Thomas Stuefe wrote: On Fri, 11 Mar 2022 05:59:00 GMT, David Holmes wrote: Thanks for pointing this out. I ran more tests and found that on certain platforms, there are other structures that have problems with uninitialized gaps. I ended up changing `os::malloc()` to zero the buffer when running with -Xshare:dump. Hopefully one extra check of `if (DumpSharedSpaces)` doesn't matter too much for regular VM executions because `os::malloc()` already has a high overhead. This is raising red flags for me sorry. Every user of the JDK is now paying a penalty because of something only needed when dumping the shared archive. It might not be much but it is the old "death by a thousand cuts". Well, he does it for `DumpSharedSpaces` only. Are you really worried about that one load+conditional jump? As I said (and I'm not the only one who says this :) ) "death by a thousand cuts". @iklam: I dislike the fact that CDS terminology is now in os::malloc. I would give this another flag, "ZapMalloc" or similar, and maybe merge it with the `DEBUG_ONLY(memset(..uninitBlockPad))` above. Is there any way to tell the OS to pre-zero all memory provided to the current process, such that we could set that when dumping and not have to check on each allocation? No. Malloced memory is not provided by the OS but by the glibc, and it may be polluted with whatever the allocator did with it (eg pointers to chain free blocks), or by prior user payload. Glibc has a specific tunable, `glibc.malloc.perturb`, that initializes malloc memory to a given value, but you cannot directly set the value. It is very handy to check for uninitialized memory. Always wanted to add some tests that used it, but never got around. But obviously its nothing you could do in production. And I have to wonder how easy it would be to re-introduce non-deterministic values in these data structures that are being dumped. Does malloc itself even guarantee to return the same set of addresses for the same sequence of requests in different executions of a program? No, of course not. Non-Java threads may still run concurrently, no? System libraries do malloc too. But are pointer *values* even the problem? I assumed that pointer values could be the problem, even if not presently, but apparently all the pointers get rewritten to use offsets from the known base of the shared archive - so not an issue. Cheers, David - PR: https://git.openjdk.java.net/jdk/pull/7748
Re: RFR: 8253495: CDS generates non-deterministic output
On 11/03/2022 4:40 pm, Ioi Lam wrote:
On Fri, 11 Mar 2022 05:59:00 GMT, David Holmes wrote:
I ended up changing `os::malloc()` to zero the buffer when running with
-Xshare:dump. Hopefully one extra check of `if (DumpSharedSpaces)` doesn't
matter too much for regular VM executions because `os::malloc()` already has a
high overhead.
This is raising red flags for me sorry. Every user of the JDK is now paying a penalty
because of something only needed when dumping the shared archive. It might not be much
but it is the old "death by a thousand cuts". Is there any way to tell the OS
to pre-zero all memory provided to the current process, such that we could set that when
dumping and not have to check on each allocation?
I don't know how to tell the OS (or C library) to zero out the buffer returned
by malloc. However, in the current code path, we already have a test for an
uncommon condition when `os::malloc()` calls `MemTracker::record_malloc()`
which calls `MallocTracker::record_malloc()`
void* MallocTracker::record_malloc(void* malloc_base, size_t size, MEMFLAGS
flags,
const NativeCallStack& stack)
{
if (MemTracker::tracking_level() == NMT_detail) {
MallocSiteTable::allocation_at(stack, size, _marker, flags);
}
I can combine the tests for `MemTracker::tracking_level()` and
`DumpSharedSpaces` into a single test and do more work only when the uncommon
path is taken. This would require some refactoring of the
MemTracker/MallocTracker code. I'd rather do that in a separate RFE.
In fact, `MemTracker::_tracking_level` is tested twice in the current
implementation. We can change it to do a single test in the most common case
(NMT_summary) if we really want to cut down the number of tests. But honestly I
don't think this makes any difference.
And I have to wonder how easy it would be to re-introduce non-deterministic
values in these data structures that are being dumped. Does malloc itself even
guarantee to return the same set of addresses for the same sequence of requests
in different executions of a program?
The malloc'ed objects are copied into the CDS archive at deterministic
addresses. Any pointers inside such objects will be relocated.
Okay. I won't object further but I really don't like it - c'est la vie!
I'll let others review the actual code changes in detail.
Cheers,
David
-
PR: https://git.openjdk.java.net/jdk/pull/7748
Re: RFR: 8253495: CDS generates non-deterministic output [v2]
I can't find this comment in the PR so replying via email ...
On 11/03/2022 9:24 am, Ioi Lam wrote:
On Wed, 9 Mar 2022 07:47:19 GMT, Thomas Stuefe wrote:
Ioi Lam has updated the pull request incrementally with one additional commit
since the last revision:
Fixed zero build
src/hotspot/share/utilities/hashtable.hpp line 42:
40:
41: LP64_ONLY(unsigned int _gap;)
42:
For 64-bit, you now lose packing potential in the theoretical case the
following payload does not have to be aligned to 64 bit. E.g. for T=char, where
the whole entry would fit into 8 bytes. Probably does not matter as long as
entries are allocated individually from C-heap which is a lot more wasteful
anyway.
For 32-bit, I think you may have the same problem if the payload starts with a
uint64_t. Would that not be aligned to a 64-bit boundary too? Whether or not
you build on 64-bit?
I think setting the memory, or at least the first 8..16 bytes, of the entry to zero
in BasicHashtable::new_entry could be more robust:
(16 bytes in case the payload starts with a long double but that may be
overthinking it :)
template BasicHashtableEntry*
BasicHashtable::new_entry(unsigned int hashValue) {
char* p = :new (NEW_C_HEAP_ARRAY(char, this->entry_size(), F);
::memset(p, 0, MIN2(this->entry_size(), 16)); // needs reproducable
BasicHashtableEntry* entry = ::new (p) BasicHashtableEntry(hashValue);
return entry;
}
If you are worried about performance, this may also be controlled by a template
parameter, and then you do it just for the system dictionary.
Thanks for pointing this out. I ran more tests and found that on certain
platforms, there are other structures that have problems with uninitialized
gaps. I ended up changing `os::malloc()` to zero the buffer when running with
-Xshare:dump. Hopefully one extra check of `if (DumpSharedSpaces)` doesn't
matter too much for regular VM executions because `os::malloc()` already has a
high overhead.
This is raising red flags for me sorry. Every user of the JDK is now
paying a penalty because of something only needed when dumping the
shared archive. It might not be much but it is the old "death by a
thousand cuts". Is there any way to tell the OS to pre-zero all memory
provided to the current process, such that we could set that when
dumping and not have to check on each allocation?
And I have to wonder how easy it would be to re-introduce
non-deterministic values in these data structures that are being dumped.
Does malloc itself even guarantee to return the same set of addresses
for the same sequence of requests in different executions of a program?
Cheers,
David
-
-
PR: https://git.openjdk.java.net/jdk/pull/7748
Re: RFR: 8253495: CDS generates non-deterministic output [v2]
On Thu, 10 Mar 2022 19:41:03 GMT, Ioi Lam wrote: >> I think he already did. I'm quoting: >> >>> However, the CDS archive also contains a heap dump, which includes Java >>> HashMaps. If I allow those 3 Java threads to start, some HashMaps in the >>> module graph will have unstable ordering. I think the reason is concurrent >>> thread execution causes unstable assignment of the identity_hash for >>> objects in the heap dump. > >> @magicus the issue is not the list of classes dumped, or their format in the >> dump. As Ioi indicated that list is fixed. The issue is with the heap dump >> part of the archive. Running these other threads affects the heap so by not >> running them with end up with a different heap. So the question is whether >> there is anything about having a different heap dumped that we need to be >> concerned about. We dump the heap into the archive for a reason and this >> changes what we dump, > > To be clear, if multiple threads are running, classes could be loaded in a > different order and the symbols will have different orders. This would cause > the vtables in Klass objects to be laid out differently. Trying to fix this > is very difficult. I have a different patch that makes it work but it's just > too complicated. > > So, disabling the threads is also necessary for (easily) sorting the > metaspace objects. @iklam thanks for clarifying. - PR: https://git.openjdk.java.net/jdk/pull/7748
Re: RFR: 8253495: CDS generates non-deterministic output [v2]
On Thu, 10 Mar 2022 12:50:58 GMT, Magnus Ihse Bursie wrote: >> Ioi Lam has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed zero build > > Well, previously we'd get different dumps on different runs. If that was an > issue, surely it would have manifested itself by now? With this change, we'll > just get the same dump each run. I fail to see how that could be a risk. @magicus I think we need @iklam to weigh in here and explain exactly what the "heap dump" consists of and how not running those threads affects its contents. Presently the heap dump is potentially different on each run, IIUC, only due to the order of its contents, not the contents themselves. - PR: https://git.openjdk.java.net/jdk/pull/7748
Re: RFR: 8253495: CDS generates non-deterministic output [v2]
On Thu, 10 Mar 2022 12:11:06 GMT, Magnus Ihse Bursie wrote: >> The "heap dump" aspect of this is not something I'm familiar with, but if >> the threads don't affect the list of classes dumped, they surely must affect >> what is in the heap dump otherwise their execution would not be an issue. So >> you must be sacrificing something by not having these threads start. > > @dholmes-ora That something is "sacrificed" does not follow from that > something is "different". The list of classes to dump is specified in the > lib/classlist file, which is generated during the build. > > The process of creating this involves running a suitable "exercise most > important parts" java program, and logging the classes loaded. This class > file is then post-processed (sorted) to make sure it is reproducible for the > same JDK code base. > > As Ioi say, in this case threads are started freely, and may run in any > non-deterministic order. > > At the next stage, we take this file (which is just done implicitly by > -Xshare:dump), and generate the actual CDS archive, classes.jsa. Now it turns > out this generation is non-deterministic. And Ioi's analysis is that this is > due to thread non-determinism. So if we just disable threads during the dump > process (where we are not really running the JVM "actually" -- it's a special > mode, where we don't even have a Java program to run!), there's no harm in > that. @magicus the issue is not the list of classes dumped, or their format in the dump. As Ioi indicated that list is fixed. The issue is with the heap dump part of the archive. Running these other threads affects the heap so by not running them with end up with a different heap. So the question is whether there is anything about having a different heap dumped that we need to be concerned about. We dump the heap into the archive for a reason and this changes what we dump, - PR: https://git.openjdk.java.net/jdk/pull/7748
Re: RFR: 8253495: CDS generates non-deterministic output [v2]
On Wed, 9 Mar 2022 05:10:44 GMT, Ioi Lam wrote: >> This patch makes the result of "java -Xshare:dump" deterministic: >> - Disabled new Java threads from launching. This is harmless. See comments >> in jvm.cpp >> - Fixed a problem in hashtable ordering in heapShared.cpp >> - BasicHashtableEntry has a gap on 64-bit platforms that may contain random >> bits. Added code to zero it. >> - Enabled checking of $JAVA_HOME/lib/server/classes.jsa in >> make/scripts/compare.sh >> >> Note: $JAVA_HOME/lib/server/classes_ncoops.jsa is still non-deterministic. >> This will be fixed in >> [JDK-8282828](https://bugs.openjdk.java.net/browse/JDK-8282828). >> >> Testing under way: >> - tier1~tier5 >> - Run all *-cmp-baseline jobs 20 times each (linux-aarch64-cmp-baseline, >> windows-x86-cmp-baseline, etc). > > Ioi Lam has updated the pull request incrementally with one additional commit > since the last revision: > > Fixed zero build The "heap dump" aspect of this is not something I'm familiar with, but if the threads don't affect the list of classes dumped, they surely must affect what is in the heap dump otherwise their execution would not be an issue. So you must be sacrificing something by not having these threads start. - PR: https://git.openjdk.java.net/jdk/pull/7748
Re: RFR: 8253495: CDS generates non-deterministic output [v2]
On Wed, 9 Mar 2022 05:10:44 GMT, Ioi Lam wrote: >> This patch makes the result of "java -Xshare:dump" deterministic: >> - Disabled new Java threads from launching. This is harmless. See comments >> in jvm.cpp >> - Fixed a problem in hashtable ordering in heapShared.cpp >> - BasicHashtableEntry has a gap on 64-bit platforms that may contain random >> bits. Added code to zero it. >> - Enabled checking of $JAVA_HOME/lib/server/classes.jsa in >> make/scripts/compare.sh >> >> Note: $JAVA_HOME/lib/server/classes_ncoops.jsa is still non-deterministic. >> This will be fixed in >> [JDK-8282828](https://bugs.openjdk.java.net/browse/JDK-8282828). >> >> Testing under way: >> - tier1~tier5 >> - Run all *-cmp-baseline jobs 20 times each (linux-aarch64-cmp-baseline, >> windows-x86-cmp-baseline, etc). > > Ioi Lam has updated the pull request incrementally with one additional commit > since the last revision: > > Fixed zero build I have reservations about contorting things this way just to get "deterministic output". The VM needs to fully initialize and then become quiescent before the dump occurs, and as I say below if you don't start other threads then you potentially remove part of the archive because classes won't be loaded by those threads. I think if you care about the order of dumping classes then you should control that order, you don't try to force the order of loading. Can't you sort things before dumping? ie rehash/rebuild the hashtables etc so it has a canonical ordering? I see this was mentioned in the bug report and is considered a largish/complex fix, but it would be the proper fix IMO. Thanks, David src/hotspot/share/prims/jvm.cpp line 2873: > 2871: // execute in parallel, symbols and classes may be loaded in > 2872: // random orders which will make the resulting CDS archive > 2873: // non-deterministic. Yes but by not starting these threads you are potentially excluding a range of classes from the shared archive! - PR: https://git.openjdk.java.net/jdk/pull/7748
Re: RFR: JDK-8282700: Properly handle several --without options during configure [v2]
On Mon, 7 Mar 2022 08:07:30 GMT, Julian Waters wrote: >> Some of the --without options are not properly handled and will crash when >> processed (For example, --without-version-string), in other cases the >> --without-* option will actually silently produce incorrect results instead >> of actually doing what --without-* implies (For example, >> --without-build-user and all the --with-vendor-* options). The most elegant >> way to solve this would simply be to handle such cases and display warnings >> when they're encountered (or if the option is critical to the build process, >> throwing an error) >> >> Even if it doesn't make sense to pass said option however, it should display >> a warning instead of letting configure exit with a confusing error when it's >> run > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Handle remaining options and change critical options to error when > --without is passed I've never felt that configure's `--without-x` options really make any sense for the bulk of our `--with-x` options that define "properties". We have a bunch of properties that have to be set and the default is in a configuration file. You can then use `--with-x=foo` to override that. If you pass "" as the value then you get what you deserve if the build fails. Passing --without-x makes no sense at all IMHO. - PR: https://git.openjdk.java.net/jdk/pull/7713
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo I eyeballed the diff file and all seems okay. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7268
Re: Windows confusing object file names for other file types
On 3/03/2022 11:29 pm, Julian Waters wrote: Windows seems to commonly confuse object file names that are created during the build process I don't understand what you mean by that, please elaborate. Thanks, David , despite .obj being the normal file format for object files on it (Weirdly enough), which is pretty likely because of pre-defined registry entries and applications that come together with it with fresh installs. This does sort of make it a pretty troublesome issue to solve, and I can't really think of any good all encompassing fix other than to change the object file suffix to something else (Which would be pretty simple to do but I don't know if that would have any unwanted side effects elsewhere), I'm not sure if this is something the proposed solution would be ok for or if there are better ways to do this? best regards, Julian P.S. Windows stop being so bloated please ;-;
Re: RFR: JDK-8282532: Allow explicitly setting build platform alongside --openjdk-target [v7]
On Thu, 3 Mar 2022 16:48:40 GMT, Julian Waters wrote: >> Currently the only other option for manually configuring the build platform >> while cross compiling are devkits, which don't work on certain systems and >> are also more focused on differentiating the build and target compilers >> instead. This patch adds the ability to explicitly set the build platform >> through a new option, which can be especially helpful for when autodetection >> fails and devkits cannot be relied on, and also for simpler cross >> compilation cases (Like the one described in building.md) >> >> WIP: Translation from markdown to html > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Revert unwanted changes I suspect you used a different version of pandoc - hence the incidental changes. It also appears that the previous changes to building.md were not used to regenerate building.html. - PR: https://git.openjdk.java.net/jdk/pull/7656
Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v25]
On Thu, 24 Feb 2022 10:52:00 GMT, Alan Hayward wrote: >> Alan Hayward has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 34 commits: >> >> - Merge master >> - Merge master >> - Merge master >> - Error on -XX:-PreserveFramePointer -XX:UseBranchProtection=pac-ret >> - Add comments to enter calls >> - Set PreserveFramePointer if use_rop_protection is set >> - Merge enter_subframe into enter >> - Review fixups >> - Documentation updates >> - Update copyrights to 2022 >> - ... and 24 more: >> https://git.openjdk.java.net/jdk/compare/022d8070...c4e0ee31 > > Any more comments? Otherwise I'll integrate later @a74nh this seems to have broken the Zero build: src/hotspot/share/gc/shared/barrierSetNMethod.cpp:58:33: error: 'pauth_strip_pointer' was not declared in this scope 58 | AARCH64_ONLY(return_address = pauth_strip_pointer(return_address)); I'm guessing a missing include file. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8281525: Enable Zc:strictStrings flag in Visual Studio build
On Mon, 21 Feb 2022 19:55:14 GMT, Daniel Jeliński wrote: > Please review this PR that enables > [Zc:strictStrings](https://docs.microsoft.com/en-us/cpp/build/reference/zc-strictstrings-disable-string-literal-type-conversion?view=msvc-170) > compiler flag, which makes assigning a string literal to a non-const pointer > a compile-time error. > > This type of assignment is [disallowed by C++ standard since > C++11](https://en.cppreference.com/w/cpp/language/string_literal). Writing to > a string literal through a non-const pointer [produces a run-time > error](https://docs.microsoft.com/en-us/cpp/cpp/string-and-character-literals-cpp?view=msvc-170#microsoft-specific-1). > > The included code changes are trivial; I added `const` keyword to variable > and parameter declarations where needed, and added explicit casts to > non-const pointers where adding `const` was not feasible. > > I verified that the build passes both with and without `--enable-debug`, both > with VS2017 and VS2019. Hi Daniel, Hotspot changes look fine. Some of the casts in other code look odd to me, but I'll leave that for the security-libs folk to comment on. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7565
Re: macro expansion producing 'defined' has undefined behavior
Hi Simmias, On 14/02/2022 1:24 pm, 殷玉婷 wrote: Hi, When I ran make images in my local computer to compile the openjdk9u by guideline <>, I encountered the following problems: 9u is not expected to be built by recent Xcode, it was a much older release (not sure if anyone is actually updating it at all): https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms You either need to downgrade your build tools, or else upgrade the OpenJDK version you want to build. Cheers, David Compiling 61 files for BUILD_INTERIM_jdk.jdeps Compiling 457 files for BUILD_INTERIM_jdk.javadoc In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/test/native/gc/g1/test_freeRegionList.cpp:25: In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/g1CollectedHeap.inline.hpp:28: In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/g1CollectedHeap.hpp:33: In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp:31: /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/heapRegionSet.hpp:126:5: error: macro expansion producing 'defined' has undefined behavior [-Werror,-Wexpansion-to-defined] #if HEAP_REGION_SET_FORCE_VERIFY ^ /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/heapRegionSet.hpp:53:38: note: expanded from macro 'HEAP_REGION_SET_FORCE_VERIFY' #define HEAP_REGION_SET_FORCE_VERIFY defined(ASSERT) ^ In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/test/native/gc/g1/test_freeRegionList.cpp:25: In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/g1CollectedHeap.inline.hpp:28: In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/g1CollectedHeap.hpp:37: /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/g1HeapVerifier.hpp:67:5: error: macro expansion producing 'defined' has undefined behavior [-Werror,-Wexpansion-to-defined] #if HEAP_REGION_SET_FORCE_VERIFY ^ /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/heapRegionSet.hpp:53:38: note: expanded from macro 'HEAP_REGION_SET_FORCE_VERIFY' #define HEAP_REGION_SET_FORCE_VERIFY defined(ASSERT) ^ 2 errors generated. make[3]: *** [/Users/simmias/java/github/openjdk/jdk9u-master/build/macosx-x86_64-normal-server-release/hotspot/variant-server/libjvm/gtest/objs/test_freeRegionList.o] Error 1 make[3]: *** Waiting for unfinished jobs make[2]: *** [hotspot-server-libs] Error 2 make[2]: *** Waiting for unfinished jobs ?: API? ?: ??, ??? -Xlint:deprecation ? ERROR: Build failed for target 'images' in configuration 'macosx-x86_64-normal-server-release' (exit code 2) === Output from failing command(s) repeated here === * For target hotspot_variant-server_libjvm_gtest_objs_test_freeRegionList.o: In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/test/native/gc/g1/test_freeRegionList.cpp:25: In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/g1CollectedHeap.inline.hpp:28: In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/g1CollectedHeap.hpp:33: In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp:31: /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/heapRegionSet.hpp:126:5: error: macro expansion producing 'defined' has undefined behavior [-Werror,-Wexpansion-to-defined] #if HEAP_REGION_SET_FORCE_VERIFY ^ /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/heapRegionSet.hpp:53:38: note: expanded from macro 'HEAP_REGION_SET_FORCE_VERIFY' #define HEAP_REGION_SET_FORCE_VERIFY defined(ASSERT) ^ In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/test/native/gc/g1/test_freeRegionList.cpp:25: In file included from /Users/simmias/java/github/openjdk/jdk9u-master/hotspot/src/share/vm/gc/g1/g1CollectedHeap.inline.hpp:28: ... (rest of output omitted) * All command lines available in /Users/simmias/java/github/openjdk/jdk9u-master/build/macosx-x86_64-normal-server-release/make-support/failure-logs. === End of repeated output === No indication of failed target found. Hint: Try searching the build log for '] Error'. Hint: See common/doc/building.html#troubleshooting for assistance. make[1]: *** [main] Error 2 make: *** [images] Error 2 I tried the ways on the internet which added the env(CGO_CPPFLAGS), but it did not work. I am really looking forward to your reply. Thanks Local env: os: mac 11.5.2 xcode version: 13.2.1 xcode-select version: 2384 planning
Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v14]
On Wed, 2 Feb 2022 10:18:38 GMT, Andrew Haley wrote: >> And this change will keep ROP protection enabled if we fall into the "this >> VM was built without ROP-protection support.". In that case we'll be >> protecting generated code, but the VM itself won't be protected. This will >> run without crashing. > >> And this change will keep ROP protection enabled if we fall into the "this >> VM was built without ROP-protection support.". In that case we'll be >> protecting generated code, but the VM itself won't be protected. This will >> run without crashing. > > That's perfect. Okay, that makes sense. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: Segfault when building openjdk13 with openjdk12
On 2/02/2022 3:53 pm, Abigail G wrote: On Wed, 2022-02-02 at 15:48 +1000, David Holmes wrote: Hi Abigail, On 2/02/2022 3:01 pm, Abigail G wrote: Hello, I'm working on packaging several openjdk versions for Void Linux. When I build jdk13 with jdk12 as the boot jdk, I get a segfault very soon into the build. I am not a java developer, so I'm not quite sure what to make of the log, trace, and core dump. Anyone have any ideas? Could just be a bug in 12, fixed later. Interestingly, when I repeatedly restart the build without removing any of the partially-built state, it eventually succeeds (usually after 1-3 retries), and I have been able to use the result to build jdk14 without issue. I've attached a .zip with the relevant build logs, hotspot error log, and core dump (if that doesn't work: https://0x0.st/oHxe.zip). The mailing list strips attachments and the link to the zip file gave a file with a single empty directory in it - no logs. Cheers, David The scripts I'm using to build jdk12 and 13 are available here: https://github.com/classabbyamp/void-packages/blob/openjdk17/srcpkgs/openjdk12-bootstrap/template https://github.com/classabbyamp/void-packages/blob/openjdk17/srcpkgs/openjdk13-bootstrap/template TIA, Abigail G Whoops, looks like I made the zip wrong, this one should work: https://0x0.st/oHxy.zip Unfortunately no symbolic info in the hs-err file so nothing to go on. All I can suggest is that you build the latest 12u to use to bootstrap the latest 13u etc. Cheers, David Abigail G
Re: Segfault when building openjdk13 with openjdk12
Hi Abigail, On 2/02/2022 3:01 pm, Abigail G wrote: Hello, I'm working on packaging several openjdk versions for Void Linux. When I build jdk13 with jdk12 as the boot jdk, I get a segfault very soon into the build. I am not a java developer, so I'm not quite sure what to make of the log, trace, and core dump. Anyone have any ideas? Could just be a bug in 12, fixed later. Interestingly, when I repeatedly restart the build without removing any of the partially-built state, it eventually succeeds (usually after 1-3 retries), and I have been able to use the result to build jdk14 without issue. I've attached a .zip with the relevant build logs, hotspot error log, and core dump (if that doesn't work: https://0x0.st/oHxe.zip). The mailing list strips attachments and the link to the zip file gave a file with a single empty directory in it - no logs. Cheers, David The scripts I'm using to build jdk12 and 13 are available here: https://github.com/classabbyamp/void-packages/blob/openjdk17/srcpkgs/openjdk12-bootstrap/template https://github.com/classabbyamp/void-packages/blob/openjdk17/srcpkgs/openjdk13-bootstrap/template TIA, Abigail G
Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v14]
On Tue, 1 Feb 2022 11:05:46 GMT, Alan Hayward wrote:
>> src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 429:
>>
>>> 427: #else
>>> 428: warning("UseROPProtection specified, but not supported in the
>>> VM.");
>>> 429: #endif
>>
>> If we issue these warnings should `_rop_protection` still be set true?
>
> As per this conversation:
> https://github.com/openjdk/jdk/pull/6334#discussion_r791722292
>
> The idea was, the user is explicitly asking for asking for pac-ret so we
> should honour that. Whereas standard would only enable what is supported for
> that system.
But we can't honour that because it is not supported. Further, the suggestion
in the referenced discussion seemed to be based on the assumption that doing so
would be harmless because it is NOP based, but you have indicated that may not
be the case and so it may actually lead to a crash!
-
PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v14]
On Mon, 24 Jan 2022 15:56:06 GMT, Alan Hayward wrote:
>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>>
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>>
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>>
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>>
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>>
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>>
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix popframe failures
A few stylistic nits below. And one query.
Thanks,
David
src/hotspot/cpu/aarch64/globals_aarch64.hpp line 123:
> 121: range(1, 99) \
> 122: product(ccstr, UseBranchProtection, "none", \
> 123: "Branch Protection to use: none,standard,pac-ret")\
Nit: spaces after commas please.
src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5286:
> 5284:
> 5285: void MacroAssembler::enter()
> 5286: {
Style nit: opening braces go on the same line as the function declaration.
src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 417:
> 415: // Enable PAC if this code has been built with branch-protection and
> the CPU/OS supports it.
> 416: #ifdef __ARM_FEATURE_PAC_DEFAULT
> 417: if (_features & CPU_PACA) {
Style nit: no implicit booleans - expand as "if ( A & B != 0)"
src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 429:
> 427: #else
> 428: warning("UseROPProtection specified, but not supported in the VM.");
> 429: #endif
If we issue these warnings should `_rop_protection` still be set true?
src/hotspot/share/gc/shared/barrierSetNMethod.cpp line 58:
> 56:
> 57: address return_address = *return_address_ptr;
> 58: AARCH64_ONLY(return_address=pauth_strip_pointer(return_address));
Style nit: spaces around binary operators please. There are a couple of
occurrences.
src/hotspot/share/runtime/frame.cpp line 1115:
> 1113: AARCH64_ONLY(if (!pauth_ptr_is_raw(x)) {
> 1114: return false;
> 1115: })
Style nit: Use ifdef for multi-line code blocks.
-
PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files
Hi Christian, Sorry for the delay in coming back to this, I wanted to see what other feedback arose. On 25/01/2022 7:43 pm, Christian Hagedorn wrote: Hi David This will be really useful - thank you. :) I'm glad to hear that! :-) Thanks for your overall comments! All build file changes need to be seen by the build team. Right, thanks for adding it again. That said I have two general concerns both related to executing non-async-signal-safe code in the signal handler via the error reporting logic. Now I know we already do a ton of stuff in error reporting not guaranteed (in any way) to be safe to do in a signal handler, but whenever we add something new I have to ask the question: how likely is this additional code to lead to secondary failures (hangs or crashes)? That's a valid concern. I've also asked myself this question when I had initially started using some assertions. We should not crash again during error reporting. I've therefore tried to be as conservative as possible and added bailouts instead, also in loops when reading data. But of course, this is just a best effort and by no means a guarantee to be safe (especially in terms of crashes). What could be alternatives to make this better? If the parsing code turns out to be very problematic in a signal handling context, then we could disable it in that context. So we really want to try and do a lot of testing by throwing random signals at the VM and see what breaks. Secondly, on the same issue the use of unified logging within this code seems even more likely to be problematic - I'm not aware of us currently using UL during error reporting. It may work in basic usecases but if it triggers logfile rotation or other more complex actions what then? I haven't thought about this before. To be honest, I think UL printing of the `dwarf` tag is only useful during development when adding something new to the parser or when debugging. I don't see much value of these messages otherwise - even less for a Java user. As a first step, I could change the logs from `log_X()` to `log_develop_X()` but that just shifts the problem to non-product builds. Another option (or additional thing) could be to guard the log messages with a new develop flag that's disabled by default. By setting it for development, we accept that it might be unsafe which should be fine. I think changing the logging to develop only is a reasonable step. I don't see logging of crash handling / error reporting as generally useful for the end user. Thanks, David Thanks, Christian - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8280916: Simplify HotSpot Style Guide editorial changes
On Sun, 30 Jan 2022 00:39:20 GMT, Kim Barrett wrote: > Please review this change to the HotSpot Style Guide change process. > > The current process involves gathering consensus among the HotSpot Group > Members. That's fine for changes of substance. But it seems overly weighty > for editorial changes that don't affect the substance of the guide, but only > it's clarity or accuracy. > > The proposed change would permit the normal PR process to be used for such > changes, but require the requisite reviewers to additionally be HotSpot Group > Members. > > Note that there have already been a couple of changes that effectively > followed the proposed new process. > https://bugs.openjdk.java.net/browse/JDK-8274169 > https://bugs.openjdk.java.net/browse/JDK-8280182 > > This is a modification of the Style Guide, so rough consensus among the > HotSpot Group members is required to make this change. Only Group members > should vote for approval (via the github PR), though reasoned objections or > comments from anyone will be considered. A decision on this proposal will not > be made before Monday 14-Feb-2022 at 12h00 UTC. > > Since we're piggybacking on github PRs here, please use the PR review process > to approve (click on Review Changes > Approve), rather than sending a "vote: > yes" email reply that would be normal for a CFV. Marked as reviewed by dholmes (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7281
Re: RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder) [v15]
On 27/01/2022 9:01 pm, Goetz Lindenmaier wrote: On Mon, 24 Jan 2022 22:23:33 GMT, Tyler Steele wrote: Just in time for the holidays I have completed an implementation of the JFR functionality for AIX. As a side note, this is my first submission to OpenJDK ### Implementation notes and alternatives considered After modifying the build system to allow the --enable-jvm-feature-jfr to work on AIX, my task was to implement the interfaces from os_perf.hpp. The os_perf_aix.cpp implementation that existed was, I believe, a copy of the Linux implementation. A review of the code in that file showed that NetworkInterface, CPUPerformanceInterface, and SystemProcessInterface would require modification to work on AIX. Using the Linux implementation as a guide, I initially expected to use files from the aix procfs like /proc//psinfo, and /proc//status in a manner similar to the Linux implementation. However, I ended up using libperfstat for all functionality required by the interfaces. ### Testing Testing for JFR seems to be quite light in the repo both before and after this change. In addition to performing manual testing, I have added some basic sanity checks that ensure events can be created and committed (using jtreg), and performs some basic checks on the results of the interface member functions (using gtest). ### More notes I've sent an email to the JFR group about a TOC overflow warning I encountered while building (for the release server target). I believe the fix is to pass -qpic=large when using the xlc toolchain, but my modifications to flags-cflags.m4 have not been successful in removing this warning. Tyler Steele has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - Merge branch 'master' into JDK-8203290 - Implements JFR on AIX - Implements interfaces from os_perf.hpp in os_perf_aix.cpp - Updates libperfstat_aix to contain functionality needed by os_perf_aix - Implements missing functionality in loadlib_aix (Fixes failure noted by TestNativeLibraies.java) - Removes platform checks for --enable-feature-jfr (Now enable-able on all platforms) - Enables TestNetworkUtilizationEvent.java which now passes on AIX - Updates AIX JavaThread::pd_get_top_frame_for_profiling with changes from Linux Hi, IANALE :) basic rule I epouse is to never touch anyone else's copyright line On first sight, this makes sense. But it is not the reality. All external contributors are asked to update the copyright year in the Oracle Copyright line if they contribute a fix. Yes, I should expand that statement with "unless requested to by a representative of that copyright holder". :) Also, when we did the port, we added the Oracle line to many new files because they were derived from files with Oracle copyright. That makes sense too. Cheers, David - But the files in question here are special, they are some of the few that were completely developed by us, i.e., by Thomas. I think Tyler should not update the SAP Copyright, because we do not have any rights on the code he implements. I agree with Tyler that as he signed the OCA Oracle has the Copyright on the code once he opens a PR with the patch. So adding an Oracle line can not be wrong. Alternatively, he also could add his own Copyright line. Isn't there any documentation about this? All I found is this: http://cr.openjdk.java.net/~alundblad/styleguide/index-v6.html I'll ping Iris and Jesper, maybe they can shed light on this. - PR: https://git.openjdk.java.net/jdk/pull/6885
Re: RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder) [v15]
On 27/01/2022 10:19 am, Tyler Steele wrote: On Tue, 25 Jan 2022 06:10:19 GMT, Thomas Stuefe wrote: Tyler Steele has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - Merge branch 'master' into JDK-8203290 - Implements JFR on AIX - Implements interfaces from os_perf.hpp in os_perf_aix.cpp - Updates libperfstat_aix to contain functionality needed by os_perf_aix - Implements missing functionality in loadlib_aix (Fixes failure noted by TestNativeLibraies.java) - Removes platform checks for --enable-feature-jfr (Now enable-able on all platforms) - Enables TestNetworkUtilizationEvent.java which now passes on AIX - Updates AIX JavaThread::pd_get_top_frame_for_profiling with changes from Linux src/hotspot/os/aix/libperfstat_aix.cpp line 2: 1: /* 2: * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved. Is there a reason for this copyright addition? Yes there is, but it might not be a correct one as David's and your comments seem to indicate The thought was that as a member of a company that has signed the OCA, work I do falls under Oracle's copyright. So any files I modify ought to have their copyright years updated. When there wasn't an Oracle copyright line, I added one (this may have been the problematic choice). Any guidance on how to do this properly is appreciated. IANAL but a basic rule I epouse is to never touch anyone else's copyright line. If a file with a non-Oracle copyright is updated significantly by anyone they can add their own copyright line appropriate to their situation. But AFAIK only Oracle employees should add Oracle copyright lines. Cheers, David -
Re: RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder) [v15]
On 25/01/2022 7:14 pm, Thomas Stuefe wrote: src/hotspot/os/aix/libperfstat_aix.cpp line 2: 1: /* 2: * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved. Is there a reason for this copyright addition? Just FYI that is an invalid copyright line for Oracle. There are only two years when they are different. This will cause a build failure in the validate-headers target. But I suspect this copyright line should not have been added. David -
Re: RFR: 8278753: Runtime crashes with access violation during JNI_CreateJavaVM call
On Tue, 25 Jan 2022 00:20:19 GMT, Yumin Qi wrote:
> Please review,
> When jlink with --compress=2, zip is used to compress the files while doing
> copy. The user case failed to load zip.dll, since zip.dll is not set in PATH.
> This failure is after we get NULL from GetModuleHandle("zip.dll"), then do
> LoadLibrary("zip.dll") will have same result.
> The fix is calling load_zip_library of ClassLoader first --- if zip library
> already loaded just return the cached handle for following usage, if not,
> load zip library and cached the handle.
>
> Tests: tier1,4,7 in test
>Manually tested user case, and checked output of jimage list for
> jlinked files using --compress=2.
>
> Thanks
> Yumin
Seems okay as long as there are no concerns from jimage folk about usage
scenarios we may not be aware of.
Thanks,
David
-
Marked as reviewed by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7206
Re: RFR: 8278753: Runtime crashes with access violation during JNI_CreateJavaVM call
On Tue, 25 Jan 2022 00:20:19 GMT, Yumin Qi wrote:
> Please review,
> When jlink with --compress=2, zip is used to compress the files while doing
> copy. The user case failed to load zip.dll, since zip.dll is not set in PATH.
> This failure is after we get NULL from GetModuleHandle("zip.dll"), then do
> LoadLibrary("zip.dll") will have same result.
> The fix is calling load_zip_library of ClassLoader first --- if zip library
> already loaded just return the cached handle for following usage, if not,
> load zip library and cached the handle.
>
> Tests: tier1,4,7 in test
>Manually tested user case, and checked output of jimage list for
> jlinked files using --compress=2.
>
> Thanks
> Yumin
Hi Yumin,
So let me see if I have this clear.
- The jimage code was using the OS code (dlopen/loadlibrary etc) to try and
load the zip library when needed.
- The VM, which is always loaded first, always used to load the zip library
unconditionally, hence the OS would simply return the JVM's zip handle to the
jimage code.
- When we changed the VM to only load the zip library if needed (not realizing
jimage may also need it) then the jimage code would now only succeed if the zip
library was in the appropriate lookup paths for the OS.
- The fix is to change the jimage code so that it asks the JVM for the zip
library, as the JVM is always setup correctly to find it.
Does that sum it up?
Thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/7206
Re: RFR: 8278753: Runtime crashes with access violation during JNI_CreateJavaVM call
On Tue, 25 Jan 2022 00:20:19 GMT, Yumin Qi wrote:
> Please review,
> When jlink with --compress=2, zip is used to compress the files while doing
> copy. The user case failed to load zip.dll, since zip.dll is not set in PATH.
> This failure is after we get NULL from GetModuleHandle("zip.dll"), then do
> LoadLibrary("zip.dll") will have same result.
> The fix is calling load_zip_library of ClassLoader first --- if zip library
> already loaded just return the cached handle for following usage, if not,
> load zip library and cached the handle.
>
> Tests: tier1,4,7 in test
>Manually tested user case, and checked output of jimage list for
> jlinked files using --compress=2.
>
> Thanks
> Yumin
This needs reviewing by the jimage folk too.
-
PR: https://git.openjdk.java.net/jdk/pull/7206
Re: RFR: 8280182: HotSpot Style Guide has stale link to chromium style guide
On Tue, 18 Jan 2022 23:09:12 GMT, Liam Miller-Cushon wrote: > Update links to the chromium style guide in the HotSpot Style Guide. Looks good. Thanks for noticing the problem and fixing it. David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7138
Re: Configuration Error in JDK Build Java 8 (JDK8u) for VS
Adding back the mailing list. Hi Ken, On 18/01/2022 12:51 am, Ken Taylor wrote: Hi David, The instructions I am following are at Building the JDK <https://openjdk.java.net/groups/build/doc/building.html> Those instructions are for mainline JDK development and not directly applicable to 8u. Unfortunately the 8u README-builds.html file has not been updated either: https://github.com/openjdk/jdk8u/blob/master/README-builds.html but 8u supports building with up to VS 2017 at the moment. Cheers, David - See the section: Native Compiler (Toolchain) Requirements Microsoft Visual Studio I cloned the Git Repo for jdk8u, master branch. Ken On Sunday, January 16, 2022, 10:35:53 PM CST, David Holmes wrote: Hi Ken, On 13/01/2022 9:52 am, Ken Taylor wrote: > I am trying to build JDK 8 from the jdk8u repo, master branch.Project was forked and cloned into a Windows 10 VM.Building in Cygwin. > It appears that the change to support VS 2019 has not been checked in. > > === > > JDK Build Instructions: > Microsoft Visual Studio > > The minimum accepted version of Visual Studio is 2017. Older versions will not be accepted by configure and will not work. > The maximum accepted version of Visual Studio is 2019. What repo exactly and what file contains these build instructions? Thanks, David > If you have multiple versions of Visual Studio installed, configure will by default pick the latest. > > You can request a specific version to be used by setting > --with-toolchain-version, e.g. --with-toolchain-version=2017. > > === > > Our Command: > > ./configure > --with-boot-jdk=/cygdrive/c/Programs/Java/jdk1.8.0_301 > --with-tools-dir='/cygdrive/c/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Auxiliary' > --with-toolchain-version=2019 > > === > > Error: > > configure: Using default toolchain microsoft (Microsoft Visual Studio) > checking for link... /usr/bin/link > checking if the first found link.exe is actually the Cygwin link tool... yes > configure: Visual Studio version 2019 is not valid. > configure: Valid Visual Studio versions: 2010 2012 2013 2015 2017. > > === > > Was this something that got missed and is there a work-around, or should I install VS 2017? BTW, VS 2022 also does not work. > > Thanks for any help you can provide. > Ken Taylor > >
Re: RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder) [v8]
On 15/01/2022 2:13 am, Tyler Steele wrote: To follow up on an item mentioned above so that it is documented here: I reviewed the functionality from `src/hotspot/os/aix/libperfstat_aix` and found it didn't fit well with the needs of this pr. If it were extended in the future to include the ability to call at least `libperfstat_process` and `libperfstat_cpu` (in addition to `libperfstat_cpu_total`), then using it here would be a reasonable choice. There may be some other functionality required, but those stand out to me as the big ones. Wouldn't adding the needed functionality to the existing libperfstat_aix be preferable here? David - PR: https://git.openjdk.java.net/jdk/pull/6885
Re: Configuration Error in JDK Build Java 8 (JDK8u) for VS
Hi Ken, On 13/01/2022 9:52 am, Ken Taylor wrote: I am trying to build JDK 8 from the jdk8u repo, master branch.Project was forked and cloned into a Windows 10 VM.Building in Cygwin. It appears that the change to support VS 2019 has not been checked in. === JDK Build Instructions: Microsoft Visual Studio The minimum accepted version of Visual Studio is 2017. Older versions will not be accepted by configure and will not work. The maximum accepted version of Visual Studio is 2019. What repo exactly and what file contains these build instructions? Thanks, David If you have multiple versions of Visual Studio installed, configure will by default pick the latest. You can request a specific version to be used by setting --with-toolchain-version, e.g. --with-toolchain-version=2017. === Our Command: ./configure --with-boot-jdk=/cygdrive/c/Programs/Java/jdk1.8.0_301 --with-tools-dir='/cygdrive/c/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Auxiliary' --with-toolchain-version=2019 === Error: configure: Using default toolchain microsoft (Microsoft Visual Studio) checking for link... /usr/bin/link checking if the first found link.exe is actually the Cygwin link tool... yes configure: Visual Studio version 2019 is not valid. configure: Valid Visual Studio versions: 2010 2012 2013 2015 2017. === Was this something that got missed and is there a work-around, or should I install VS 2017? BTW, VS 2022 also does not work. Thanks for any help you can provide. Ken Taylor
Re: Need OpenJDK to be used on PowerPC for our products.
On 7/01/2022 5:21 am, tim.b...@oracle.com wrote: Forwarded Message Subject: RE: Need OpenJDK to be used on PowerPC for our products. Date: Thu, 6 Jan 2022 19:16:10 + From: Dipendu Ghosh To: mailman-owner Hi, I am an Engineer from Keysight Technologies. In Keysight, we have a few products where we use Java. At present we are using Java distribution from Oracle. Since there is some licensing issue raised by Oracle we need to move to OpenJDK distributions. We have a requirement where we need to deploy Java 8 on the PowerPC processor and Java is not readily available on this particular platform. Will it be possible if you can provide us with Open JDK which can be used on the PowerPC processor? The AdoptOpenJDK project provides such builds: https://adoptium.net/releases.html?variant=openjdk8 Cheers, David - I am trying to follow the steps to compile OpenJDK for PPC following the link https://openjdk.java.net/groups/build/doc/building.html but I am stuck at cross compilation. Any help in this regard will be highly appreciated. Will be eagerly waiting to hear back from you on a positive note. Thanks & Regards, Dipendu Ghosh Senior Software Engineer II, Keysight Technologies dipendu.gh...@keysight.com
Re: RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder)
On Wed, 22 Dec 2021 02:59:43 GMT, Erik Gahlin wrote: >> Just in time for the holidays I have completed an implementation of the JFR >> functionality for AIX. As a side note, this is my first submission to >> OpenJDK >> >> ### Implementation notes and alternatives considered >> >> After modifying the build system to allow the --enable-jvm-feature-jfr to >> work on AIX, my task was to implement the interfaces from os_perf.hpp. The >> os_perf_aix.cpp implementation that existed was, I believe, a copy of the >> Linux implementation. A review of the code in that file showed that >> NetworkInterface, CPUPerformanceInterface, and SystemProcessInterface would >> require modification to work on AIX. Using the Linux implementation as a >> guide, I initially expected to use files from the aix procfs like >> /proc//psinfo, and /proc//status in a manner similar to the Linux >> implementation. However, I ended up using libperfstat for all functionality >> required by the interfaces. >> >> ### Testing >> >> Testing for JFR seems to be quite light in the repo both before and after >> this change. In addition to performing manual testing, I have added some >> basic sanity checks that ensure events can be created and committed (using >> jtreg), and performs some basic checks on the results of the interface >> member functions (using gtest). >> >> ### More notes >> >> I've sent an email to the JFR group about a TOC overflow warning I >> encountered while building (for the release server target). I believe the >> fix is to pass -qpic=large when using the xlc toolchain, but my >> modifications to flags-cflags.m4 have not been successful in removing this >> warning. > > I don't have access to AIX, but there are 500+ tests in test/jdk/jdk/jfr you > might want to try out. @egahlin beat me to it - yes the JFR testing is primarily under jdk not hotspot, so we don't really need the new sanity tests. Also are you aware that we have `src/hotspot/os/aix/libperfstat_aix.*` ? - PR: https://git.openjdk.java.net/jdk/pull/6885
Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled
On Sun, 19 Dec 2021 07:37:19 GMT, Alan Bateman wrote: >> Enable the security manager in rmiregistry's launcher arguments. > > As things stand, `rmiregsitry -J-Djava.security.manager` and `rmiregistry > -J-Djava.security.manager=allow` are equivalent because rmiregistry sets the > default SM. Some difference may be observed if someone is running rmiregistry > with a custom system class loader set, or custom file system provider, or > running it with a JVM TI agent that is looking at events between VMStart and > VMInit but these seem somewhat obscure scenarios for a command line program > If rmiregstry worked with ToolProvider then I think there would be more to > discuss. If the final patch is to have the launcher set the default security > manager then we may want to change RegistryImpl.createRegistry to fail if not > set. > > The warning that is emitted for both cases is expected. JEP 411 is very clear > that it there is no mechanism to suppress it. We may need to add a release > note to document that rmiregistry will emit a warning a startup, that will at > least be something to point at in the event that anyone asks or reports an > issue. In the same kind of PR (https://github.com/openjdk/jdk18/pull/53) for `jstatd` @AlanBateman writes: > This looks okay in that it is now worked exactly as it did in JDK 17. And that PR uses `allow` as I have suggested here (to preserve existing behaviour). All the affected launchers should be fixed in the same consistent way. - PR: https://git.openjdk.java.net/jdk18/pull/45
Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled
On Fri, 17 Dec 2021 20:01:27 GMT, Stuart Marks wrote: > Enable the security manager in rmiregistry's launcher arguments. My concern is that having the SM installed during part of VM initialization could lead to different behaviour compared to installing the SM after that. This may be a small risk but it is a risk, and not something we are likely to observe in our own testing. If there is no way to hide those warning messages (@SuppressWarnings in the code?) then we have a serious flaw in our warning system. As you say these warnings are for the developer not the end user! - PR: https://git.openjdk.java.net/jdk18/pull/45
Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled
On Fri, 17 Dec 2021 20:01:27 GMT, Stuart Marks wrote: > Enable the security manager in rmiregistry's launcher arguments. Hi Stuart, I think specifying "allow" would be the behaviour preserving change here. That avoids any risk that enabling the SM earlier changes some behaviour during VM initialization. Cheers, David - Changes requested by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk18/pull/45
Re: RFR: 8278275: Initial nroff manpage generation for JDK 19
On Mon, 13 Dec 2021 05:51:37 GMT, David Holmes wrote: > Trivial update to change the version to 19-ea, and update the single > reference to the "current release". > > Content changes for 19 will follow. > > Thanks, > David Thanks for the reviews Erik, Jon and Iris! - PR: https://git.openjdk.java.net/jdk/pull/6811
Integrated: 8278275: Initial nroff manpage generation for JDK 19
On Mon, 13 Dec 2021 05:51:37 GMT, David Holmes wrote: > Trivial update to change the version to 19-ea, and update the single > reference to the "current release". > > Content changes for 19 will follow. > > Thanks, > David This pull request has now been integrated. Changeset: 624f3094 Author:David Holmes URL: https://git.openjdk.java.net/jdk/commit/624f3094b89976a0be0a1d1d4ce304f4be38fb9e Stats: 29 lines in 28 files changed: 0 ins; 0 del; 29 mod 8278275: Initial nroff manpage generation for JDK 19 Reviewed-by: erikj, jjg, iris - PR: https://git.openjdk.java.net/jdk/pull/6811
Re: RFR: 8278275: Initial nroff manpage generation for JDK 19
On Mon, 13 Dec 2021 05:51:37 GMT, David Holmes wrote: > Trivial update to change the version to 19-ea, and update the single > reference to the "current release". > > Content changes for 19 will follow. > > Thanks, > David This "mechanical" update is just being sent to the build mailing list for review. Paging @jonathan-gibbons too :) - PR: https://git.openjdk.java.net/jdk/pull/6811
RFR: 8278275: Initial nroff manpage generation for JDK 19
Trivial update to change the version to 19-ea, and update the single reference to the "current release". Content changes for 19 will follow. Thanks, David - Commit messages: - 8278275:Initial nroff manpage generation for JDK 19 Changes: https://git.openjdk.java.net/jdk/pull/6811/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6811=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278275 Stats: 29 lines in 28 files changed: 0 ins; 0 del; 29 mod Patch: https://git.openjdk.java.net/jdk/pull/6811.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6811/head:pull/6811 PR: https://git.openjdk.java.net/jdk/pull/6811
Re: RFR: 8251400: Fix incorrect addition of library to test in JDK-8237858
On Thu, 2 Dec 2021 22:45:37 GMT, Magnus Ihse Bursie wrote: > In JDK-8237858, -pthread was added to all native tests, instead of the one > single test that needed it. In the meantime, two new tests with pthread > dependencies has crept in unnoticed due to this. Looks good. I don't think `-pthread` would actually hurt anything though. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6682
Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v7]
On Thu, 2 Dec 2021 09:16:59 GMT, Alan Hayward wrote: > @dholmes-ora > > Fixed flags based on comments on the CSR: Flag updates look good - thanks. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: JDK-8273146: Start of release updates for JDK 19 [v3]
On Tue, 23 Nov 2021 19:23:40 GMT, Joe Darcy wrote: >> The time to get JDK 19 underway draws nigh, please review this usual set of >> start-of-release updates, including CSRs for the javac and javax.lang.model >> updates: >> >> JDK-8277512: Add SourceVersion.RELEASE_19 >> https://bugs.openjdk.java.net/browse/JDK-8277512 >> >> JDK-8277514: Add source 19 and target 19 to javac >> https://bugs.openjdk.java.net/browse/JDK-8277514 >> >> Clean local tier 1 test runs for langtools, jdk, and hotspot. > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains five additional commits since > the last revision: > > - Remove SharedSpaces options from VMDeprecatedOptions.java. > - Merge branch 'master' into JDK-8273146 > - Respond to review feedback. > - Add --release 18 information. Good tier1 test results. > - JDK-8273146: Start of release updates for JDK 19 Update is good. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6493
Re: RFR: JDK-8273146: Start of release updates for JDK 19 [v2]
On Mon, 22 Nov 2021 04:30:38 GMT, Joe Darcy wrote: >> The time to get JDK 19 underway draws nigh, please review this usual set of >> start-of-release updates, including CSRs for the javac and javax.lang.model >> updates: >> >> JDK-8277512: Add SourceVersion.RELEASE_19 >> https://bugs.openjdk.java.net/browse/JDK-8277512 >> >> JDK-8277514: Add source 19 and target 19 to javac >> https://bugs.openjdk.java.net/browse/JDK-8277514 >> >> Clean local tier 1 test runs for langtools, jdk, and hotspot. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Update looks good. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6493
Re: RFR: JDK-8273146: Start of release updates for JDK 19
On Mon, 22 Nov 2021 03:15:51 GMT, Joe Darcy wrote: > The time to get JDK 19 underway draws nigh, please review this usual set of > start-of-release updates, including CSRs for the javac and javax.lang.model > updates: > > JDK-8277512: Add SourceVersion.RELEASE_19 > https://bugs.openjdk.java.net/browse/JDK-8277512 > > JDK-8277514: Add source 19 and target 19 to javac > https://bugs.openjdk.java.net/browse/JDK-8277514 > > Clean local tier 1 test runs for langtools, jdk, and hotspot. Hi Joe, I looked at all the non-sym file changes. Only one potential issue spotted. Thanks, David test/langtools/tools/javac/versions/Versions.java line 91: > 89: SEVENTEEN(false, "61.0", "17", Versions::checksrc17), > 90: EIGHTEEN(false, "62.0", "18", Versions::checksrc18), > 91: NINETEEN(false, "63.0", "19", Versions::checksrc18); checkSrc19? - PR: https://git.openjdk.java.net/jdk/pull/6493
Re: RFR: JDK-8276422 Add command-line option to disable finalization
On Thu, 18 Nov 2021 07:16:56 GMT, Kim Barrett wrote:
> There is nothing here to make the various GCs take advantage of finalization
> being disabled. Is the plan to leave that to followup changes?
@kimbarrett I provided the basic VM parts here. I'm not aware of what
specifically a GC might optimise if it knows there can be no finalizers, but
that seems like something the GC folk should look to providing as a follow up.
Thanks.
> src/hotspot/share/oops/instanceKlass.hpp line 338:
>
>> 336:
>> 337: // Queries finalization state
>> 338: static bool finalization_enabled() { return _finalization_enabled; }
>
> Predicate functions like this are often named "is_xxx"; that idiom is common
> in this class.
This was intended as an accessor function, similar to `count()` or `offset()`
not a query as-in `is_shared_boot_class()`. As it is a boolean field you could
convert it to a query instead.
-
PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization
On Thu, 18 Nov 2021 07:44:05 GMT, Aleksey Shipilev wrote: >> @shipilev not sure what you mean by "a flag on the Java side". The Java >> code just queries the VM for the finalization enabled/disabled state and >> uses that to control things. > > Yeah, "flag" is `Holder.ENABLED` here. I mean, are Java methods > `registerFinalizer` and `runFinalization` called only by VM? If so, can VM > check the whole thing on VM side, without going to Java and asking back from > there? `registerFinalizer` does not expect to be called and only uses the "flag" as a form of assertion. `runFinalization` is called from Java code. - PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization
On Thu, 18 Nov 2021 07:27:30 GMT, Aleksey Shipilev wrote: >> @stuart-marks not sure I see how anything is different here compared to the >> existing logic. The `Finalizer` class is explicitly initialized quite early >> in the init process, but if a preceding class's initialization created an >> object with a finalizer then that same upcall would be involved. > > Do we even have to have a flag on Java side? It looks like these calls are > only done as the upcalls from VM, so we might just keep the flag on VM side? @shipilev not sure what you mean by "a flag on the Java side". The Java code just queries the VM for the finalization enabled/disabled state and uses that to control things. - PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization
On Thu, 18 Nov 2021 05:20:02 GMT, Stuart Marks wrote:
>> src/java.base/share/classes/java/lang/ref/Finalizer.java line 195:
>>
>>> 193:
>>> 194: static {
>>> 195: if (Holder.ENABLED) {
>>
>> Hello Stuart,
>> My understanding of the the lazy `Holder` is that it's there to delay the
>> static initialization of the code that's part of the `Holder`. In this case
>> here, the `Holder` is being used right within the `static` block of the
>> `Finalizer` class, that too as the first thing. In this case, is that
>> `Holder` class necessary?
>
> Huh, good catch! This was mostly left over from an earlier version of the
> flag that used system properties, which aren't initialized until after the
> Finalizer class is initialized.
>
> It might be the case that the Holder can be removed at this point, since the
> finalization-enabled bit is no longer in a system property and is in a native
> class member that should be available before the VM is started.
>
> I say "might" though because this occurs early in system startup, and weird
> things potentially happen. For example, suppose the first object with a
> finalizer is created before the Finalizer class is initialized. The VM will
> perform an upcall to Finalizer::register. An ordinary call to a static method
> will ensure the class is initialized before proceeding with the call, but
> this VM upcall is a special case I'll have to investigate this some more.
@stuart-marks not sure I see how anything is different here compared to the
existing logic. The `Finalizer` class is explicitly initialized quite early in
the init process, but if a preceding class's initialization created an object
with a finalizer then that same upcall would be involved.
-
PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: JDK-8276422 Add command-line option to disable finalization
On Thu, 18 Nov 2021 01:34:36 GMT, Stuart Marks wrote: > Pretty much what it says. The new option controls a static member in > InstanceKlass that's consulted to determine whether the finalization > machinery is activated for instances when a class is loaded. A new native > method is added so that this state can be queried from Java. This is used to > control whether a finalizer thread is created and to disable the `System` and > `Runtime::runFinalization` methods. Includes tests for the above. Hi Stuart, This all looks fine to me. The hotspot part needs a second reviewer (especially as I contributed a chunk of that code :) ). Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6442
Re: RFR: 8277012: Use blessed modifier order in src/utils
On Thu, 11 Nov 2021 14:32:18 GMT, Magnus Ihse Bursie wrote: > I ran bin/blessed-modifier-order.sh on source code in src/utils. This scripts > verifies that modifiers are in the "blessed" order, and fixes it otherwise. I > have manually checked the changes made by the script to make sure they are > sound. > > There are no clear ownership of this code, but I believe it's kind of > hotspot-related. Looks fine. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6354
Re: RFR: 8275872: Sync J2DBench run and analyze Makefile targets with build.xml [v2]
On Tue, 26 Oct 2021 11:13:19 GMT, Jiří Vaněk wrote: >> Jiří Vaněk 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. > > Attmept done. Looks like the summary is there already once. Thanx again. @judovana please do not use "force push" in a PR as it makes the review process very difficult and can leave orphaned comments. You can simply push additional changes after the initial commit. The skara tooling with flatten all commits into one single, properly formulated, commit that will actually be pushed to the OpenJDK repo. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/6068
Re: RFR: 8275874: [JVMCI] use volatile accessors for all unaligned reads in c2v_readFieldValue
On Mon, 25 Oct 2021 14:33:27 GMT, Doug Simon wrote: > [JDK-8275645](https://bugs.openjdk.java.net/browse/JDK-8275645) resulted in > loosing single-copy atomicity for reads in `c2v_readFieldValue`. This PR > fixes that by using `_field_acquire` accessors for all aligned reads > and only using `_field` accessors for unaligned reads. Isn't the title of this issue expressed incorrectly? - PR: https://git.openjdk.java.net/jdk/pull/6109
Re: RFR: 8248584: Enable CHECK_UNHANDLED_OOPS for Windows fastdebug builds
On Tue, 19 Oct 2021 19:21:24 GMT, Harold Seigel wrote: > Please review this small change to enable CHECK_UNHANDLED_OOPs for Windows > fastdebug builds. The change was tested by running Mach5 tiers 1-6 on > Windows-x64-debug. > > Thanks, Harold Seems fine to me. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6023
Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v6]
On Sat, 16 Oct 2021 11:11:59 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-419 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/419 > > Maurizio Cimadamore has updated the pull request incrementally with two > additional commits since the last revision: > > - * use `invokeWithArguments` to simplify new test > - Add test for liveness check with high-aririty downcalls >(make sure that if an exception occurs in a downcall because of liveness, >ref count of other resources are left intact). Hotspot changes seem fine. Thanks, David - PR: https://git.openjdk.java.net/jdk/pull/5907
Re: Unable to build Client/Server variant for OpenJDK-11 on ARMv7-A
Hi John, On 14/10/2021 3:20 am, John Cummings wrote: Thank you both for the responses! I am now using the https://github.com/openjdk/jdk11u-dev/ <https://github.com/openjdk/jdk11u-dev/> tree that you linked and I have trimmed down my cxxflags and cflags to just -O2 and -pipe, and added I would not mess with the optimisation level. The crash looks like the wrong instruction is being used, possibly due to the forced optimisation level. David - --with-debug-level=fastdebug. I've tried with zero build, which still works, but server, client still crash immediately, but now they also give me an assert internal error: Server: # # To suppress the following error report, specify this argument # after -XX: or in .hotspotrc: SuppressErrorAt=/assembler_arm.hpp:240 # # A fatal error has been detected by the Java Runtime Environment: # # Internal Error (/home/johnc/Documents/openjdk/jdk11u-dev/src/hotspot/cpu/arm/assembler_arm.hpp:240), pid=1467, tid=1468 # assert(_disp == 0 && (_shift_imm >> 5) == 0) failed: encoding constraint # # JRE version: (11.0.14) (fastdebug build ) # Java VM: OpenJDK Server VM (fastdebug 11.0.14-internal+0-adhoc.johnc.jdk11u-dev, mixed mode, serial gc, linux-) # Core dump will be written. Default location: /usr/lib/jvm/jdk11/server/jdk/bin/core # Client: # # To suppress the following error report, specify this argument # after -XX: or in .hotspotrc: SuppressErrorAt=/assembler_arm.hpp:240 # # A fatal error has been detected by the Java Runtime Environment: # # Internal Error (/home/johnc/Documents/openjdk/jdk11u-dev/src/hotspot/cpu/arm/assembler_arm.hpp:240), pid=1296, tid=1297 # assert(_disp == 0 && (_shift_imm >> 5) == 0) failed: encoding constraint # # JRE version: (11.0.14) (fastdebug build ) # Java VM: OpenJDK Client VM (fastdebug 11.0.14-internal+0-adhoc.johnc.jdk11u-dev, mixed mode, serial gc, linux-) # Core dump will be written. Default location: /usr/lib/jvm/jdk11/client/jre/bin/core # And if it helps, here's the output of -Xlog:all on the client fastdebug build # ./java -Xlog:all [0.012s][info][os] Use of CLOCK_MONOTONIC is supported [0.012s][info][os] Use of pthread_condattr_setclock is supported [0.012s][info][os] Relative timed-wait using pthread_cond_timedwait is associated with CLOCK_MONOTONIC [0.012s][info][os] HotSpot is running with glibc 2.18, NPTL 2.18 [0.013s][info][os] SafePoint Polling address: 0xb6f03000 [0.016s][info][biasedlocking] Aligned thread 0xb5f17160 to 0xb5f17400 [0.016s][info][os,thread ] Thread attached (tid: 1484, pthread id: 3054040160). [0.018s][info][os ] attempting shared library load of /usr/lib/jvm/jdk11/client/jdk/lib/libzip.so [0.019s][info][os ] shared library load of /usr/lib/jvm/jdk11/client/jdk/lib/libzip.so was successful [0.020s][info][class,path ] bootstrap loader class path=/usr/lib/jvm/jdk11/client/jdk/lib/modules [0.020s][info][class,path ] opened: /usr/lib/jvm/jdk11/client/jdk/lib/modules [0.020s][info][class,load ] opened: /usr/lib/jvm/jdk11/client/jdk/lib/modules [0.020s][info][pagesize ] CodeCache: min=160K max=32M base=0xb3d5f000 page_size=4K size=32M [0.022s][info][os,cpu ] CPU:total 2 (initial active 2) (ARMv7), vfp [0.022s][info][os,cpu ] /proc/cpuinfo: [0.022s][info][os,cpu ] Processor : ARMv7 Processor rev 4 (v7l) processor : 0 BogoMIPS : 1810.43 processor : 1 BogoMIPS : 1823.53 Features : swp half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpv4 idiva idivt CPU implementer : 0x41 CPU architecture: 7 CPU variant : 0x0 CPU part : 0xc07 CPU revision : 4 Hardware : sun7i Revision : Serial : 07c1120f5552484880758685165166cb [0.022s][info][os,cpu ] Online cpus: [0.022s][info][os,cpu ] 0-1 [0.022s][info][os,cpu ] Offline cpus: [0.023s][info][os,cpu ] [0.024s][info][startuptime ] StubRoutines generation 1, 0.0007363 secs [0.025s][info][pagesize ] Heap: min=6M max=200M base=0xa740 page_size=2M size=200M [0.025s][info][pagesize ] Card Table: min=409601B max=409601B base=0xb3c7a000 page_size=4K size=404K [0.064s][info][gc ] Using Serial [0.065s][info][cds ] UseSharedSpaces: Specified shared archive not found. [0.068s][info][startuptime ] Genesis, 0.0438007 secs [0.068s][info][startuptime ] TemplateTable initialization, 0.0001120 secs I am also still trying to build the mainline jdk as suggested, but I am currently facing some difficulties with that as the toolchain I was using before did not meet the minimum requirements for gcc (toolchain is 4.8.5) and I am trying to get one working with a newer gcc. I was also unable to use the builds you have linked, as the system's glibc I'm using (2.18) is also too outdated to run those. -------- *From:* David Holmes *Sent:* Sunday, Octo
Re: RFR: 8275008: gtest build failure due to stringop-overflow warning with gcc11
On Sun, 10 Oct 2021 13:05:40 GMT, Jie Fu wrote: > Hi all, > > gtest build fails due to stringop-overflow warning with gcc11. > > This is because gcc11 seems to be smart enough to detect the following > stringop-overflow at test/hotspot/gtest/memory/test_guardedMemory.cpp:125:11. > > * For target hotspot_variant-server_libjvm_gtest_objs_test_guardedMemory.o: > In file included from /usr/include/string.h:495, > from > /home/jdk/src/hotspot/share/utilities/globalDefinitions_gcc.hpp:35, > from > /home/jdk/src/hotspot/share/utilities/globalDefinitions.hpp:35, > from /home/jdk/src/hotspot/share/memory/allocation.hpp:29, > from > /home/jdk/test/hotspot/gtest/memory/test_guardedMemory.cpp:25: > In function 'void* memset(void*, int, size_t)', > inlined from 'virtual void > GuardedMemory_buffer_overrun_tail_Test::TestBody()' at > /home/jdk/test/hotspot/gtest/memory/test_guardedMemory.cpp:125:11: Okay. Seems a bit heavy handed to disable across all gtests but okay. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5881
Re: Unable to build Client/Server variant for OpenJDK-11 on ARMv7-A
On 9/10/2021 3:58 pm, Aleksey Shipilev wrote: Hi, This does not look a build issue per se, but rather a Hotspot bug. The SIGILL in ThreadGroup.add looks to me like an issue with atomic operations as that is the first synchronized method that gets executed during VM initialization. David - On 10/8/21 7:46 PM, John Cummings wrote: I am using the source from https://hg.openjdk.java.net/jdk/jdk11. This is an extremely old JDK 11 tree. The most actual one is here: https://github.com/openjdk/jdk11u-dev/ FWIW, ARM32 builds from that repo work for me on RPi 4: https://builds.shipilev.net/openjdk-jdk11-dev/ I am at the point where I am unsure where to proceed or what to change, and haven't been able to find a solution anywhere thus far to build a client version of OpenJDK-11. If there's any additional information I can/should provide, please let me know. Additional things to try: *) Build fastdebug, trying to see if any VM assert is triggered; pass --with-debug-level=fastdebug to configure *) Build/test the JDK mainline (https://github.com/openjdk/jdk/), trying to see if this is a fixed issue that is missing a 11u backport *) Trim down --with-extra-cflags and --with-extra-cxxflags, looking for some special flag that triggers the bug There were plenty of ARM32 bugs fixed (backported) in 11u, but I also see some that are only fixed in later JDKs, for example: https://bugs.openjdk.java.net/browse/JDK-8222825
Re: RFR: 8275008: gtest build failure due to stringop-overflow warning with gcc11
On Sun, 10 Oct 2021 13:05:40 GMT, Jie Fu wrote: > Hi all, > > gtest build fails due to stringop-overflow warning with gcc11. > > This is because gcc11 seems to be smart enough to detect the following > stringop-overflow at test/hotspot/gtest/memory/test_guardedMemory.cpp:125:11. > > * For target hotspot_variant-server_libjvm_gtest_objs_test_guardedMemory.o: > In file included from /usr/include/string.h:495, > from > /home/jdk/src/hotspot/share/utilities/globalDefinitions_gcc.hpp:35, > from > /home/jdk/src/hotspot/share/utilities/globalDefinitions.hpp:35, > from /home/jdk/src/hotspot/share/memory/allocation.hpp:29, > from > /home/jdk/test/hotspot/gtest/memory/test_guardedMemory.cpp:25: > In function 'void* memset(void*, int, size_t)', > inlined from 'virtual void > GuardedMemory_buffer_overrun_tail_Test::TestBody()' at > /home/jdk/test/hotspot/gtest/memory/test_guardedMemory.cpp:125:11: Hi Jie, Can we not just disable the warning in this specific test? Thanks, David - PR: https://git.openjdk.java.net/jdk/pull/5881
Re: RFR: 8274083: Update testing docs to mention tiered testing [v5]
On Thu, 23 Sep 2021 12:53:23 GMT, Aleksey Shipilev wrote:
>> Now that OpenJDK has more or less complete `tier{1,2,3,4}` definitions,
>> let's mention them in `testing.md`.
>>
>> Current patch is my braindump, I am open for suggestions :)
>
> Aleksey Shipilev has updated the pull request incrementally with one
> additional commit since the last revision:
>
> More fixes
Marked as reviewed by dholmes (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/5615
Re: RFR: 8274083: Update testing docs to mention tiered testing [v3]
On 23/09/2021 8:33 pm, Thomas Stuefe wrote:
On Thu, 23 Sep 2021 09:19:42 GMT, Aleksey Shipilev wrote:
Now that OpenJDK has more or less complete `tier{1,2,3,4}` definitions, let's
mention them in `testing.md`.
Current patch is my braindump, I am open for suggestions :)
Aleksey Shipilev has updated the pull request incrementally with one additional
commit since the last revision:
More fixes
I like this. Thanks for doing this (and for the actual definition of the
tiers). This hopefully makes communicating about test coverage much easier.
As I understood, tier1-3 are the same as Oracle tier1-3?
Not exactly no. Our tier definitions are tuples of (tests, platforms,
flags). The sets of tests will be close but not exactly the same.
David
-
Marked as reviewed by stuefe (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5615
Re: RFR: 8274169: HotSpot Style Guide has stale link to chromium style guide
On 23/09/2021 8:53 pm, Magnus Ihse Bursie wrote: On Thu, 23 Sep 2021 02:56:05 GMT, David Holmes wrote: Update links to the chromium style guide in the HotSpot Style Guide. Update looks good and trivial. Aside: no idea why changes to these files mapped to build-dev ?? Thanks, David @dholmes-ora The build team has traditionally been shepherding the "misc" parts of the source tree, like the "docs" directory, that no other group claims responsibility for. (Also, most of the docs are related to building and testing anyway.) Okay but I think we need to change that for the Hotspot Style Guide. :) Cheers, David - PR: https://git.openjdk.java.net/jdk/pull/5641
Re: RFR: 8274083: Update testing docs to mention tiered testing [v2]
On Wed, 22 Sep 2021 12:28:18 GMT, Aleksey Shipilev wrote:
>> Now that OpenJDK has more or less complete `tier{1,2,3,4}` definitions,
>> let's mention them in `testing.md`.
>>
>> Current patch is my braindump, I am open for suggestions :)
>
> Aleksey Shipilev has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Revert accidental edit
> - More discussion
Hi Aleksey,
I don't have any significant misgivings about this version - thanks.
One correction needed below.
Thanks,
David
doc/testing.html line 73:
> 71: Common Test Groups
> 72: In the ideal world, contributors would be advised to run all the tests
> for every change. But in real world, one could only be expected to run as
> many tests as practical, while being mindful of the scope for the change, the
> testing resources available, etc.
> 73: The source tree currently defines a few common test groups in the
> relevant TEST.groups files. The test groups are either covering
> a specific component, for example hotspot_gc. It is a good idea
> to look into TEST.groups files to get a sense what tests are
> relevant to a particular JDK component.
You state "The test groups are either ..." but there is no "or ..." in that
sentence.
-
Marked as reviewed by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5615
Re: RFR: 8274169: HotSpot Style Guide has stale link to chromium style guide
On Wed, 22 Sep 2021 20:48:11 GMT, Liam Miller-Cushon wrote: > Update links to the chromium style guide in the HotSpot Style Guide. Update looks good and trivial. Aside: no idea why changes to these files mapped to build-dev ?? Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5641
Re: RFR: 8274083: Update testing docs to mention tiered testing
On Tue, 21 Sep 2021 16:06:28 GMT, Aleksey Shipilev wrote: >> doc/testing.html line 77: >> >>> 75: tier1: This test group is the first line of defense >>> against bugs. Multiple developers run these tests every day. Normally, at >>> least this tier is ran before integration. Because of the widespread, the >>> tests in tier1 are carefully selected and optimized to run >>> fast, and to run in the most stable manner. The test failures in >>> tier1 are usually followed up on quickly, either with fixes, >>> or adding relevant tests to problem list. >> href="../.github/workflows/">GitHub Actions workflows, if enabled, run >>> tier1 tests. >>> 76: tier2: This test group covers even more ground. >>> These contain, among other things, tests that either run for too long to be >>> at tier1, test unstable and/or experimental features, test >>> less essential JDK components. >>> 77: tier3: This test group covers more stressful tests, >>> the tests for corner cases not covered by previous tiers, plus the tests >>> that require GUIs. As such, this suite should either be run with low >>> concurrency (TEST_JOBS=1), or without headful tests >>> (JTREG_KEYWORDS=\!headful), or both. >> >> It's not clear to me that these descriptions are useful. Instead of "First >> line of defense against bugs" then it might be better to say something about >> the hotspot and core library tests that are typically run in tier1. Folks >> contributing to some areas of the core, I/O, and networking libraries will >> need to learn about tier2 as that is where many of the tests for these APIs >> and implementations are. So I wouldn't use words like "unstable", >> "experimental", "less essential" but maybe say that it includes tests that >> may require some configuration, like changing firewall rules to allow >> multicast or other traffic. > > I'll take another stab at it tomorrow. > > The whole tiered testing thing encapsulates that some tests are more > important to run (at least, first or regularly) than others. So we need to > somehow capture that. Discussing what tests are actually running is not > relevant, because `TEST.groups` is the source of truth there. > > For a contributor, these descriptions should tell why, when and how often > would you use a particular tier. In other words, good docs capture some > thinking about how contributors are supposed to use the existing tests. It > seems to be a broad consensus that we run `tier1` before non-trivial > integrations. It seems to be a rule of thumb that we care a lot about `tier1` > stability and speed, and we push out less stable, slower or > specially-configured tests to higher tiers. As I inspect the test groups, I > think `tier2` is for regular but non-`tier1`-grade tests, `tier3` is mostly > for stress tests, and `tier4` is (somewhat by construction) is "everything > else". > > From here, this what I'd like to tell contributors: run `tier1` early and > often, the bugs there are likely to be real product bugs and should be looked > at as soon as possible, run `tier1` + `tier2` to execute all regular tests > but suspect test bugs more, run `tier3` to add even more stress testing and > suspect environmental problems when they fail, run `tier4` to run everything > that previous tiers missed and hope for the best. Maybe I should just put > that in the docs :) Ideal world: run every test on every platform before every integration. Real world: run as many tests as is practical having consideration for time of testing and the cost of testing resources, and the scope of the change. It is assumed/expected you will have run all tests pertaining to the area of code change, that can reasonably be expected to be run. The tiered testing is intended to give broader coverage to ensure no unintended consequences of a change. At a minimum run all tier1 tests on all non-trivial integrations. If you have time and resources, and think a change may have broader impact than what has been explicitly tested, then run further tiers. - PR: https://git.openjdk.java.net/jdk/pull/5615
Re: RFR: 8273801: Handle VMTYPE for "core" VM variant
On 20/09/2021 9:43 pm, Magnus Ihse Bursie wrote: On Thu, 16 Sep 2021 12:02:38 GMT, David Holmes wrote: Building with --with-jvm-variants=core currently produces a binary that replies an odd version: $ build/linux-x86_64-core-fastdebug/images/jdk/bin/java -version openjdk version "18-internal" 2022-03-15 OpenJDK Runtime Environment (fastdebug build 18-internal+0-adhoc.shade.jdk) OpenJDK 64-Bit VM (fastdebug build 18-internal+0-adhoc.shade.jdk, interpreted mode) It should actually say "OpenJDK 64-Bit Core VM", I think. Build just misses a simple definition of `VMTYPE` for core variant. After the patch: $ build/linux-x86_64-core-fastdebug/images/jdk/bin/java -client -version openjdk version "18-internal" 2022-03-15 OpenJDK Runtime Environment (fastdebug build 18-internal+0-adhoc.shade.jdk) OpenJDK 64-Bit Core VM (fastdebug build 18-internal+0-adhoc.shade.jdk, interpreted mode) Begs the question ... if there is no C1 and no C2 then what is the name of the directory where libjvm.so lives? @dholmes-ora If you are building `server`, then it is called `server` regardless of what JVM features you enable or disable. Doh! Of course. The variant determines the name. Thanks, David The way the build system works is that a "JVM variant" is a set of enabled JVM features with a given name. You can roll your own JVM variant by starting with `custom`, which has an empty initial set of features enabled. (But currently, you cannot change the name from the configure command line.) If core is built only when porting to new platforms, we could emulate that behavior by allowing the user to change the JVM variant name as well, like `configure --with-jvm-variant=server --with-jvm-features=-compiler1,-compiler2 --with-jvm-name=core` or something like that. (And just generate VMTYPE dynamically using Sentence Case). I think this can be seriously considered. But I suspect I should try to dug out from my mailbox archives whom it was last told me they needed/wanted core. - PR: https://git.openjdk.java.net/jdk/pull/5525
Re: RFR: 8273801: Handle VMTYPE for "core" VM variant
On Wed, 15 Sep 2021 09:55:22 GMT, Aleksey Shipilev wrote: > Building with --with-jvm-variants=core currently produces a binary that > replies an odd version: > > > $ build/linux-x86_64-core-fastdebug/images/jdk/bin/java -version > openjdk version "18-internal" 2022-03-15 > OpenJDK Runtime Environment (fastdebug build 18-internal+0-adhoc.shade.jdk) > OpenJDK 64-Bit VM (fastdebug build 18-internal+0-adhoc.shade.jdk, > interpreted mode) > > > It should actually say "OpenJDK 64-Bit Core VM", I think. Build just misses a > simple definition of `VMTYPE` for core variant. > > After the patch: > > > $ build/linux-x86_64-core-fastdebug/images/jdk/bin/java -client -version > openjdk version "18-internal" 2022-03-15 > OpenJDK Runtime Environment (fastdebug build 18-internal+0-adhoc.shade.jdk) > OpenJDK 64-Bit Core VM (fastdebug build 18-internal+0-adhoc.shade.jdk, > interpreted mode) Begs the question ... if there is no C1 and no C2 then what is the name of the directory where libjvm.so lives? - PR: https://git.openjdk.java.net/jdk/pull/5525
Re: RFR: 8273801: Handle VMTYPE for "core" VM variant
On 15/09/2021 11:34 pm, Aleksey Shipilev wrote: On Wed, 15 Sep 2021 12:45:51 GMT, David Holmes wrote: But perhaps we should be looking to remove "core" going forward? Yes, we should consider it. @magicus [told me](https://github.com/openjdk/jdk/pull/5440#discussion_r705464312) that somebody wanted for "core" to remain, the last time the removal was suggested. I guess is it somewhat useful in porting work: producing the builds where only template interpreter is implemented. Good point - I hadn't considered that. But I wonder whether it is actually still functional? If so and we want to keep it we should probably build it regularly. David - PR: https://git.openjdk.java.net/jdk/pull/5525
Re: RFR: 8273797: Stop impersonating "server" VM in all VM variants
On Wed, 15 Sep 2021 10:02:19 GMT, Aleksey Shipilev wrote: > As the follow-up for Zero-specific JDK-8273494, we might want to clean up > build system logic for all VM variants: stop impersonating "server" VMs for > all of them. This basically leaves "core" and "custom" variants to be handled > with this patch. > > Additional testing: > - [x] Linux x86_64 `core` build passes, `libjvm.so` now in `core`, `jvm.cfg` > mentions `-core KNOWN` > - [x] Linux x86_64 `custom` (+serialgc,+compiler2) build passes, `libjvm.so` > now in `custom`, `jvm.cfg` mentions `-custom KNOWN` >From a purely "mechanical" perspective this is a good simplification and >cleanup. Did you test building all variants into one image? Cheers, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5526
