Re: RFR: 8329816: Add SLEEF version 3.6.1 [v2]
On Thu, 23 May 2024 12:49:42 GMT, Hamlin Li wrote: >> Okay, suffix works fine too. But the files currently in the patch is named >> e.g. `sleefinline_advsimd.h`, which does not indicate any platform at all. >> Is it a generic file, and the platform specific ones are still missing from >> this PR? > > I think both `sleefinline_advsimd.h` and `sleefinline_sve.h` are specific for > arm. > In the future, on riscv the corresponding file name will be > `sleefinline_rvvm1.h`. > > Only `misc.h` is a generic file shared among platforms. Oh, you mean that the `sve` suffix signals that it is for ARM. I thought you were talking about having a name like `sleefinline_aarch64.h`. Sure, if the only files generated by sleef are ever .h files, the logic for chosing which to include can be done entirely by `#ifdef`s in the source code. But if there ever needs to be different .c or .cpp files to include in the build, the build system needs to be able to determine automatically if they should be included or included, and that can only be made if the path or the file name includes the CPU moniker. Personally, I think it would show good alignment with the prevailing norms in the JDK to also use this way of naming files for .h files. But I confess that for .h files it is more a matter of style, rather than a necessity from the build system. - PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1611964018
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v2]
On Wed, 22 May 2024 09:39:43 GMT, Hamlin Li wrote: >> make/devkit/createSleef.sh line 32: >> >>> 30: # >>> 31: # 1. cd >>> 32: # 2. bash /make/devkit/createSleef.sh aarch64-gcc.cmake >>> /devkit >> >> So you'd need a different copy of sleef for each platform? The files you >> have put in `linux/native/libvectormath`, what platform are they for? Should >> we not put them in a platform-specific subdirectory? > >> So you'd need a different copy of sleef for each platform? > > I think it's one or more. > >> The files you have put in linux/native/libvectormath, what platform are they >> for? Should we not put them in a platform-specific subdirectory? > > we could, but not necessary, as long as they have different suffixes, and > normally that suffixes indicate what platform it's for. Okay, suffix works fine too. But the files currently in the patch is named e.g. `sleefinline_advsimd.h`, which does not indicate any platform at all. Is it a generic file, and the platform specific ones are still missing from this PR? - PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1611448592
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v2]
On Thu, 27 Jun 2024 21:56:19 GMT, Mikael Vidstedt wrote: >> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to >> optimize vector math operations by leveraging the SLEEF library. For legal >> reasons the actual contribution of the SLEEF files needs to be handled >> separately. This enhancement adds the relevant files, enabling the rest of >> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. > > Mikael Vidstedt 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 three additional > commits since the last revision: > > - Add SLEEF version 3.6.1 > - Merge branch 'master' into 8329816-sleef > - 8329816: Add SLEEF version 3.6 make/devkit/createSleef.sh line 32: > 30: # > 31: # 1. cd > 32: # 2. bash /make/devkit/createSleef.sh aarch64-gcc.cmake > /devkit So you'd need a different copy of sleef for each platform? The files you have put in `linux/native/libvectormath`, what platform are they for? Should we not put them in a platform-specific subdirectory? - PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1609623756
Re: RFR: 8329816: Add SLEEF version 3.6.1
On Fri, 10 May 2024 22:32:29 GMT, Mikael Vidstedt wrote: > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to > optimize vector math operations by leveraging the SLEEF library. For legal > reasons the actual contribution of the SLEEF files needs to be handled > separately. This enhancement adds the relevant files, enabling the rest of > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. I understand that you want to avoid renaming files, if they are imported. That is a good point. Moving them to an arch subdirectory does not seem like much additional hassle (there's apparently still a lot of manual work involved in upgrading the source from the third party upstream), and might help readers that are not deeply familiar with these platforms. But then again, if we only talk about header files, it is not strictly needed, so if you don't want to do it, then skip it. - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2128971931
Integrated: 8329288: Update Visual Studio visibility support for POSIX functions
On Tue, 18 Jun 2024 11:29:20 GMT, Magnus Ihse Bursie wrote: > From the JBS description: > > We use various POSIX functions in the JDK in shared code, and possibly even > in Windows-specific code. The UCRT optionally provides the relevant > functionality under alternate names with leading underscores, and optionally > provides the POSIX names and by default warns about their use. > https://learn.microsoft.com/en-us/cpp/c-runtime-library/compatibility?view=msvc-170 > > We currently define _CRT_NONSTDC_NO_DEPRECATE to request the POSIX names be > defined and to suppress the deprecation warnings. However, that macro seems > to be undocumented (or perhaps is no longer documented). Instead, recent > versions of that compatibility page suggest the use of > _CRT_NONSTDC_NO_WARNINGS and _CRT_DECLARE_NONSTDC_NAMES. We should update our > usage accordingly. This pull request has now been integrated. Changeset: 78682fe7 Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/78682fe78e18268b1857855c3595b4d118808c66 Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod 8329288: Update Visual Studio visibility support for POSIX functions Reviewed-by: kbarrett - PR: https://git.openjdk.org/jdk/pull/19766
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote: >> This patch contains a set of changes to improve static builds. They will >> pave the way for implementing a full static-only java launcher. The changes >> here will: >> >> 1) Make sure non-exported symbols are made local in the static libraries. >> This means that the risk of symbol conflict is the same for static libraries >> as for dynamic libraries (i.e. in practice zero, as long as a consistent >> naming scheme is used for exported functions). >> >> 2) Remove the work-arounds to exclude duplicated symbols. >> >> 3) Fix some code in hotspot and the JDK libraries that did not work properly >> with a static java launcher. >> >> The latter fixes are copied from or inspired by the work done by >> @jianglizhou and her team as part of the Project Leyden [Hermetic >> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add dummy implementation of os::lookup_function for Windows The reason the gtest failed was that we build a static library libgtest.a, which is linked with the gtest libjvm.so. With the changes in this PR, libgtest.a was being built using the `ld -r` + `objcopy --localize-hidden` method. This caused some weird issues with gcc, related to C++ code and the `COMDAT` object info. I failed to track down any proper solution, so instead I added a patch where the libraries that we explicitly declare as `STATIC_LIBRARIES` are linked as before, without the partial linking step. These libraries are only intended for internal consumption (that is, they are linked to and used by another, "external" library), and so the extra protection added by the partial linking is not really needed. It's a bit sad that this did not work, but it is no big deal. It won't affect files released in the image, and it will not be a regression as compared to now. - PR Comment: https://git.openjdk.org/jdk/pull/19478#issuecomment-2178961562
Re: RFR: 8333268: Fixes for static build [v4]
> This patch contains a set of changes to improve static builds. They will pave > the way for implementing a full static-only java launcher. The changes here > will: > > 1) Make sure non-exported symbols are made local in the static libraries. > This means that the risk of symbol conflict is the same for static libraries > as for dynamic libraries (i.e. in practice zero, as long as a consistent > naming scheme is used for exported functions). > > 2) Remove the work-arounds to exclude duplicated symbols. > > 3) Fix some code in hotspot and the JDK libraries that did not work properly > with a static java launcher. > > The latter fixes are copied from or inspired by the work done by @jianglizhou > and her team as part of the Project Leyden [Hermetic > Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Add dummy implementation of os::lookup_function for Windows - Changes: - all: https://git.openjdk.org/jdk/pull/19478/files - new: https://git.openjdk.org/jdk/pull/19478/files/4ab70df3..b88d813e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19478=03 - incr: https://webrevs.openjdk.org/?repo=jdk=19478=02-03 Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19478.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19478/head:pull/19478 PR: https://git.openjdk.org/jdk/pull/19478
Re: RFR: 8333268: Fixes for static build [v3]
> This patch contains a set of changes to improve static builds. They will pave > the way for implementing a full static-only java launcher. The changes here > will: > > 1) Make sure non-exported symbols are made local in the static libraries. > This means that the risk of symbol conflict is the same for static libraries > as for dynamic libraries (i.e. in practice zero, as long as a consistent > naming scheme is used for exported functions). > > 2) Remove the work-arounds to exclude duplicated symbols. > > 3) Fix some code in hotspot and the JDK libraries that did not work properly > with a static java launcher. > > The latter fixes are copied from or inspired by the work done by @jianglizhou > and her team as part of the Project Leyden [Hermetic > Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Do not use partial linking when building static libraries for internal consumption - Changes: - all: https://git.openjdk.org/jdk/pull/19478/files - new: https://git.openjdk.org/jdk/pull/19478/files/e1c46562..4ab70df3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19478=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19478=01-02 Stats: 7 lines in 1 file changed: 7 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19478.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19478/head:pull/19478 PR: https://git.openjdk.org/jdk/pull/19478
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods
On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee wrote: > This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 Build changes are trivially fine. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19774#pullrequestreview-2128470410
Re: RFR: 8331553: Windows JVM leaks Event and Thread handles when multiple threads are used
On Tue, 18 Jun 2024 21:01:15 GMT, Daniel Jeliński wrote: > We use 2 ParkEvent instances per thread. The ParkEvent objects are never > freed, but they are recycled when a thread dies, so the number of live > ParkEvent instances is proportional to the maximum number of threads that > were live at any time. > > On Windows, the ParkEvent object wraps a kernel Event object. Kernel objects > are a limited and costly resource. In this PR, I replace the use of kernel > events with user-space synchronization. > > The new implementation uses WaitOnAddress and WakeByAddressSingle methods to > implement synchronization. The methods are available since Windows 8. We only > support Windows 10 and newer, so OS support should not be a problem. > > WaitOnAddress was observed to return spuriously, so I added the necessary > code to recalculate the timeout and continue waiting. > > Tier1-5 tests passed. Performance tests were... inconclusive. For example, > `ThreadOnSpinWaitProducerConsumer` reported 30% better results, while > `LockUnlock.testContendedLock` results were 50% worse. > > Thoughts? Build change is trivially fine - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19778#pullrequestreview-2128375909
Re: RFR: 8334166: Enable binary check
On Wed, 12 Jun 2024 22:38:37 GMT, Zhao Song wrote: > @kevinrushforth said in > [SKARA-2289](https://bugs.openjdk.org/browse/SKARA-2289), 'In general, our > repositories contain source code and not binary files. There are exceptions > to this for images and other similar resources, but otherwise the policy for > most repos is to avoid binary files'. Skara is able to identify binary files > when executing jcheck, but this check is not enabled. This looks much better. I think we can enable this check for the JDK repo, but let's hear what the others say as well. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19683#pullrequestreview-2126169104 PR Comment: https://git.openjdk.org/jdk/pull/19683#issuecomment-2176671524
Re: RFR: 8333268: Fixes for static build [v2]
On Tue, 18 Jun 2024 16:19:39 GMT, Magnus Ihse Bursie wrote:
>> This patch contains a set of changes to improve static builds. They will
>> pave the way for implementing a full static-only java launcher. The changes
>> here will:
>>
>> 1) Make sure non-exported symbols are made local in the static libraries.
>> This means that the risk of symbol conflict is the same for static libraries
>> as for dynamic libraries (i.e. in practice zero, as long as a consistent
>> naming scheme is used for exported functions).
>>
>> 2) Remove the work-arounds to exclude duplicated symbols.
>>
>> 3) Fix some code in hotspot and the JDK libraries that did not work properly
>> with a static java launcher.
>>
>> The latter fixes are copied from or inspired by the work done by
>> @jianglizhou and her team as part of the Project Leyden [Hermetic
>> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime).
>
> Magnus Ihse Bursie 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 seven additional
> commits since the last revision:
>
> - Merge branch 'master' into static-linking-progress
> - Merge branch 'master' into static-linking-progress
> - Move the exported JVM_IsStaticallyLinked to a better location
> - Use runtime lookup of static vs dynamic instead of #ifdef STATIC_BUILD
> - Copy fix for init_system_properties_values on linux
> - Make sure we do not try to build static libraries on Windows
> - 8333268: Fixes for static build
src/hotspot/os/linux/os_linux.cpp line 605:
> 603:
> 604: // Get rid of /{client|server|hotspot}, if binary is libjvm.so.
> 605: // Or, cut off /.
@jianglizhou This code is based on changes in the Hermetic Java repo, but I do
not fully understand neither the comment nor what the purpose is. If you could
explain this a bit I'd be grateful.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r1644855137
Re: RFR: 8333268: Fixes for static build
On Thu, 30 May 2024 19:35:44 GMT, Magnus Ihse Bursie wrote: > Do os::lookup_function need to be implemented on Windows too, for symmetry, > even if it is only used on Unix platforms? @AlanBateman suggested to add `lookup_function` to os_windows.cpp as well, but just let it contain ShouldNotReachHere. That sounds like a good solution to me. - PR Comment: https://git.openjdk.org/jdk/pull/19478#issuecomment-2176657975
Re: RFR: 8334166: Enable binary check
On Thu, 13 Jun 2024 17:50:21 GMT, Phil Race wrote: >> @kevinrushforth said in >> [SKARA-2289](https://bugs.openjdk.org/browse/SKARA-2289), 'In general, our >> repositories contain source code and not binary files. There are exceptions >> to this for images and other similar resources, but otherwise the policy for >> most repos is to avoid binary files'. Skara is able to identify binary files >> when executing jcheck, but this check is not enabled. > > If skara really is unable to distinguish a png from an exe, then I agree the > warning needs to be toned way down to an "FYI", not even a warning. > And something less scary than the yellow triangle with a "!" as the graphic > too. @prrace We don't add png files that often, and when we do, I think we can live with a warning sign. But I agree that the text needs to be more clear that this is not necessarily forbidden, but requires an additionl check and confirmation that it is okay in this case. - PR Comment: https://git.openjdk.org/jdk/pull/19683#issuecomment-2167790933
Re: RFR: 8333268: Fixes for static build [v2]
> This patch contains a set of changes to improve static builds. They will pave > the way for implementing a full static-only java launcher. The changes here > will: > > 1) Make sure non-exported symbols are made local in the static libraries. > This means that the risk of symbol conflict is the same for static libraries > as for dynamic libraries (i.e. in practice zero, as long as a consistent > naming scheme is used for exported functions). > > 2) Remove the work-arounds to exclude duplicated symbols. > > 3) Fix some code in hotspot and the JDK libraries that did not work properly > with a static java launcher. > > The latter fixes are copied from or inspired by the work done by @jianglizhou > and her team as part of the Project Leyden [Hermetic > Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). Magnus Ihse Bursie 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 seven additional commits since the last revision: - Merge branch 'master' into static-linking-progress - Merge branch 'master' into static-linking-progress - Move the exported JVM_IsStaticallyLinked to a better location - Use runtime lookup of static vs dynamic instead of #ifdef STATIC_BUILD - Copy fix for init_system_properties_values on linux - Make sure we do not try to build static libraries on Windows - 8333268: Fixes for static build - Changes: - all: https://git.openjdk.org/jdk/pull/19478/files - new: https://git.openjdk.org/jdk/pull/19478/files/6b24a789..e1c46562 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19478=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19478=00-01 Stats: 2608 lines in 114 files changed: 1321 ins; 955 del; 332 mod Patch: https://git.openjdk.org/jdk/pull/19478.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19478/head:pull/19478 PR: https://git.openjdk.org/jdk/pull/19478
RFR: 8329288: Update Visual Studio visibility support for POSIX functions
>From the JBS description: We use various POSIX functions in the JDK in shared code, and possibly even in Windows-specific code. The UCRT optionally provides the relevant functionality under alternate names with leading underscores, and optionally provides the POSIX names and by default warns about their use. https://learn.microsoft.com/en-us/cpp/c-runtime-library/compatibility?view=msvc-170 We currently define _CRT_NONSTDC_NO_DEPRECATE to request the POSIX names be defined and to suppress the deprecation warnings. However, that macro seems to be undocumented (or perhaps is no longer documented). Instead, recent versions of that compatibility page suggest the use of _CRT_NONSTDC_NO_WARNINGS and _CRT_DECLARE_NONSTDC_NAMES. We should update our usage accordingly. - Commit messages: - Remove undocumented _CRT_NONSTDC_NO_DEPRECATE - Refactor to extract common defines between JDK and JVM Changes: https://git.openjdk.org/jdk/pull/19766/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19766=00 Issue: https://bugs.openjdk.org/browse/JDK-8329288 Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/19766.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19766/head:pull/19766 PR: https://git.openjdk.org/jdk/pull/19766
Re: RFR: 8332854: Unable to build openjdk with --with-harfbuzz=system
On Sun, 16 Jun 2024 21:31:50 GMT, Phil Race wrote: > Verified on Ubuntu 24.04 Marked as reviewed by ihse (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19739#pullrequestreview-2123005441
Re: RFR: 8333268: Fixes for static build
On Thu, 30 May 2024 13:00:21 GMT, Magnus Ihse Bursie wrote: > This patch contains a set of changes to improve static builds. They will pave > the way for implementing a full static-only java launcher. The changes here > will: > > 1) Make sure non-exported symbols are made local in the static libraries. > This means that the risk of symbol conflict is the same for static libraries > as for dynamic libraries (i.e. in practice zero, as long as a consistent > naming scheme is used for exported functions). > > 2) Remove the work-arounds to exclude duplicated symbols. > > 3) Fix some code in hotspot and the JDK libraries that did not work properly > with a static java launcher. > > The latter fixes are copied from or inspired by the work done by @jianglizhou > and her team as part of the Project Leyden [Hermetic > Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). The GHA tests fails when building gtest on Linux. This will require some investigation. - PR Comment: https://git.openjdk.org/jdk/pull/19478#issuecomment-2168647325
Re: RFR: 8333268: Fixes for static build
On Thu, 30 May 2024 13:00:21 GMT, Magnus Ihse Bursie wrote:
> This patch contains a set of changes to improve static builds. They will pave
> the way for implementing a full static-only java launcher. The changes here
> will:
>
> 1) Make sure non-exported symbols are made local in the static libraries.
> This means that the risk of symbol conflict is the same for static libraries
> as for dynamic libraries (i.e. in practice zero, as long as a consistent
> naming scheme is used for exported functions).
>
> 2) Remove the work-arounds to exclude duplicated symbols.
>
> 3) Fix some code in hotspot and the JDK libraries that did not work properly
> with a static java launcher.
>
> The latter fixes are copied from or inspired by the work done by @jianglizhou
> and her team as part of the Project Leyden [Hermetic
> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime).
Some open questions:
* Do `os::lookup_function` need to be implemented on Windows too, for symmetry,
even if it is only used on Unix platforms?
* Many of the changes in Hotspot boils down to `os::dll_load` doing the wrong
thing when running with a static build. Perhaps we should provide a better
function that knows how to find and load a symbol for both static and dynamic
builds, and use that instead of making a lot of tests for static/dynamic on
each location we need to look up a symbol from some other JDK library.
* I managed to replace most of the #ifdef STATIC_BUILD with runtime checks.
There are some places remaining though. Apart from the #ifdefs needed for
JNI/JVMTI, which will need spec changes to address, there are code in
java_md_macosx.m, jio.c and awt_Mlib.c that I did not manage to turn into
runtime checks. They will need some more thorough work than just changing an
`#ifdef` to an `if () {`.
* And of course, the code in the build system to share all .o files except the
two linktype files is still under development...
I moved this away from Draft state, since I think it needs some visibility,
especially since it touches several different parts of the code base, and such
reviews tend to take time.
I think the code here is good and basically okay to integrate. This patch will
not on it's own solve the entire problem of building a proper static launcher,
but it takes several important steps along the way. I think the changes here
are reasonable to integrate into mainline at this point.
-
PR Comment: https://git.openjdk.org/jdk/pull/19478#issuecomment-2140743300
PR Comment: https://git.openjdk.org/jdk/pull/19478#issuecomment-2168635393
RFR: 8333268: Fixes for static build
This patch contains a set of changes to improve static builds. They will pave the way for implementing a full static-only java launcher. The changes here will: 1) Make sure non-exported symbols are made local in the static libraries. This means that the risk of symbol conflict is the same for static libraries as for dynamic libraries (i.e. in practice zero, as long as a consistent naming scheme is used for exported functions). 2) Remove the work-arounds to exclude duplicated symbols. 3) Fix some code in hotspot and the JDK libraries that did not work properly with a static java launcher. The latter fixes are copied from or inspired by the work done by @jianglizhou and her team as part of the Project Leyden [Hermetic Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). - Commit messages: - Merge branch 'master' into static-linking-progress - Move the exported JVM_IsStaticallyLinked to a better location - Use runtime lookup of static vs dynamic instead of #ifdef STATIC_BUILD - Copy fix for init_system_properties_values on linux - Make sure we do not try to build static libraries on Windows - 8333268: Fixes for static build Changes: https://git.openjdk.org/jdk/pull/19478/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19478=00 Issue: https://bugs.openjdk.org/browse/JDK-8333268 Stats: 440 lines in 28 files changed: 203 ins; 74 del; 163 mod Patch: https://git.openjdk.org/jdk/pull/19478.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19478/head:pull/19478 PR: https://git.openjdk.org/jdk/pull/19478
Re: RFR: 8333477: Delete extra empty spaces in Makefiles [v2]
On Fri, 7 Jun 2024 13:34:45 GMT, Julian Waters wrote: >> I find the extra trailing newlines through below shell command: >> >> for i in `find . -iname "Makefile*" | sed "/./build/d"` ; do tail -n 2 $i | >> grep -c "^$" | grep -q "^1$" ; if [[ 0 -eq $? ]] ; then echo $i ; fi ; done >> >> >> There are only two files has been found: >> >> ./test/jdk/java/rmi/reliability/benchmark/bench/rmi/Makefile >> ./test/jdk/java/rmi/reliability/benchmark/bench/Makefile > > Ah, I had not realized that there was more than 1 newline. GitHub's UI > confused me here, so we're good to go GitHub's UI assumes the final line has an line break. If it is missing, it displays a red at the end of the last line. If there is an empty line showing up in the UI, then it is an additional empty line. - PR Review Comment: https://git.openjdk.org/jdk/pull/19537#discussion_r1639649265
Re: RFR: 8331552: Update to use jtreg 7.4
On Thu, 2 May 2024 09:48:51 GMT, Christian Stein wrote: > Please review the change to update to using `jtreg` **7.4**. > > The primary change is to the `jib-profiles.js` file, which specifies the > version of jtreg to use, for those systems that rely on this file. In > addition, the `requiredVersion` has been updated in the various `TEST.ROOT` > files. > > Testing: _tier1-tier5 pending..._ Looks good to me. I assume that you have run an extensive set of tests to verify that this does not break, even in higher tiers? - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19052#pullrequestreview-2118094672
Re: RFR: 8330586: GHA: Drop additional gcc/glibc packages installation for x86_32
On Thu, 18 Apr 2024 14:28:37 GMT, Aleksey Shipilev wrote: > We have added these long time ago with > [JDK-8308086](https://bugs.openjdk.org/browse/JDK-8308086) and > [JDK-8293165](https://bugs.openjdk.org/browse/JDK-8293165) to stabilize > x86_32 sysroot a bit. We never backported those below JDK 21. Marked as reviewed by ihse (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18842#pullrequestreview-2118091215
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v32]
On Thu, 6 Jun 2024 09:47:30 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Fix default description of keep-packaged-modules Marked as reviewed by ihse (Reviewer). The wording was much better than what I suggested. Thanks. - PR Review: https://git.openjdk.org/jdk/pull/14787#pullrequestreview-2101492920 PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2151870228
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v31]
On Wed, 5 Jun 2024 17:31:44 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request incrementally with six > additional commits since the last revision: > > - Move JImageHelper > - Update wording on multi-hop. > - Remove printStackTrace() > - Fix comment. Stream.toList() > - Use pattern matching for instanceof in JRTArchive::equals > - Rename JLINK_PRODUCE_RUNTIME_LINK_JDK to JLINK_PRODUCE_LINKABLE_RUNTIME As Erik says. You need to add something like: `DEFAULT_DESC: [the inverse of --enable-runtime-link-image]`. - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2151836837
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]
On Mon, 3 Jun 2024 19:10:22 GMT, Alan Bateman wrote: > Does that proposal sound good? What you basically is saying is that the default value of `packaged-modules` should be `! runtime-link-image` (i.e. the inverse)? - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2146996167
Integrated: 8333301: Remove static builds using --enable-static-build
On Thu, 30 May 2024 19:14:43 GMT, Magnus Ihse Bursie wrote: > The original way of building static libraries in the JDK was to use the > configure argument --enable-static-build, which set the value of the make > variable STATIC_BUILD. (Note that this is not the same as the source code > definition STATIC_BUILD, which will be set by other means as well.) > > This method only ever worked on macOS, and has long since stopped working. It > was originally introduced for the Mobile Project, but I've now learn that not > even they use it anymore. > > It just adds clutter to the build system, and should be removed. This pull request has now been integrated. Changeset: f0bffbce Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/f0bffbce35bb06e724857e8651dd429c4f9df284 Stats: 218 lines in 14 files changed: 4 ins; 206 del; 8 mod 801: Remove static builds using --enable-static-build Reviewed-by: sgehwolf, erikj - PR: https://git.openjdk.org/jdk/pull/19487
Re: RFR: 8333128: Linux x86_32 configure fail with --with-hsdis=binutils --with-binutils-src [v2]
On Mon, 3 Jun 2024 09:16:36 GMT, SendaoYan wrote: >> Hi all, >> This PR try to fix linux x86_32 configure fail with `--with-hsdis=binutils >> --with-binutils-src`. The libiberty.a locates in >> `build/linux-x86-server-fastdebug/configure-support/binutils-install/lib32` >> on linux ubuntu20. The change has been verified, the risk is low. >> >> Additional testing: >> >> - [x] linux x86_32 centos7 configure >> - [x] linux x86_64 centos7 configure >> - [x] linux x86_32 ubuntu20 configure >> - [x] linux x86_64 ubuntu20 configure >> >> >> [configure-linux-centos7-x86_32.log](https://github.com/user-attachments/files/15523974/configure-linux-centos7-x86_32.log) >> [configure-linux-centos7-x86_64.log](https://github.com/user-attachments/files/15523976/configure-linux-centos7-x86_64.log) >> [configure-ubuntu20-x86_32.log](https://github.com/user-attachments/files/15523977/configure-ubuntu20-x86_32.log) >> [configure-ubuntu20-x86_64.log](https://github.com/user-attachments/files/15523978/configure-ubuntu20-x86_64.log) > > SendaoYan has updated the pull request incrementally with one additional > commit since the last revision: > > "or $BINUTILS_INSTALL_DIR/lib64" should probably be on the new line in this > comment Marked as reviewed by ihse (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19511#pullrequestreview-2093377467
RFR: 8333301: Remove static builds using --enable-static-build
The original way of building static libraries in the JDK was to use the configure argument --enable-static-build, which set the value of the make variable STATIC_BUILD. (Note that this is not the same as the source code definition STATIC_BUILD, which will be set by other means as well.) This method only ever worked on macOS, and has long since stopped working. It was originally introduced for the Mobile Project, but I've now learn that not even they use it anymore. It just adds clutter to the build system, and should be removed. - Commit messages: - 801: Remove static builds using --enable-static-build Changes: https://git.openjdk.org/jdk/pull/19487/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19487=00 Issue: https://bugs.openjdk.org/browse/JDK-801 Stats: 218 lines in 14 files changed: 4 ins; 206 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/19487.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19487/head:pull/19487 PR: https://git.openjdk.org/jdk/pull/19487
Re: RFR: 8333282: Better warning if newly build JDK fails to run
On Thu, 30 May 2024 15:02:38 GMT, Magnus Ihse Bursie wrote:
> If the newly built JDK fails to run ("DOA"), we will get a strange error
> message about jdk.compiler-gendata errors from make. The reason is not at all
> obvious.
>
> Instead, we should make a simple check that we can actually use the new JDK
> before starting to use it for the first time, and report clearly to the user
> if this is not the case.
I had completely missed the `runnable-buildjdk` target. Yes, that was basically
what I was trying to re-create.
It might be a race condition, but this particular place was always where I have
hit problems first if there is an issue with the newly built JDK. It used to be
`generate-link-opt-data`, but my guess is that this is further down the
dependency chain now, since the symbol changes to use the current JDK.
I agree with your general analysis though. We should add the test to
`runnable-buildjdk` (that's the easy part), and make sure we use it everywhere
that we should (there I might need some help to look if I miss some places).
-
PR Comment: https://git.openjdk.org/jdk/pull/19484#issuecomment-2140213983
RFR: 8333282: Better warning if newly build JDK fails to run
If the newly built JDK fails to run ("DOA"), we will get a strange error
message about jdk.compiler-gendata errors from make. The reason is not at all
obvious.
Instead, we should make a simple check that we can actually use the new JDK
before starting to use it for the first time, and report clearly to the user if
this is not the case.
-
Commit messages:
- 8333282: Better warning if newly build JDK fails to run
Changes: https://git.openjdk.org/jdk/pull/19484/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=19484=00
Issue: https://bugs.openjdk.org/browse/JDK-8333282
Stats: 72 lines in 2 files changed: 66 ins; 2 del; 4 mod
Patch: https://git.openjdk.org/jdk/pull/19484.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19484/head:pull/19484
PR: https://git.openjdk.org/jdk/pull/19484
Re: Hermetic Java project meeting
On 2024-05-21 20:51, Jiangli Zhou wrote: Magnus will send out his changes as PR draft for initial review for deciding on how to move forward with the non-makefile changes. This is now published in https://github.com/openjdk/jdk/pull/19478. /Magnus
Integrated: 8333189: Make sure clang on linux uses lld as linker
On Wed, 29 May 2024 15:01:27 GMT, Magnus Ihse Bursie wrote: > When compiling with clang on linux, clang can decide to pick up the bfd > linker instead of lld, the LLVM linker. This will invalidate assumptions > about command lines that are passed on to the linker. We should use > -fuse-ld=lld to force clang to always pick lld as the linker, so we can be > sure that the command lines will work. This pull request has now been integrated. Changeset: 789ac8b2 Author: Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/789ac8b2768671ec83a7ed4a72c5fe27a1734c5e Stats: 10 lines in 4 files changed: 8 ins; 0 del; 2 mod 8333189: Make sure clang on linux uses lld as linker Reviewed-by: jiangli, erikj - PR: https://git.openjdk.org/jdk/pull/19456
Re: RFR: 8333189: Make sure clang on linux uses lld as linker
On Wed, 29 May 2024 15:10:58 GMT, Jiangli Zhou wrote: >> When compiling with clang on linux, clang can decide to pick up the bfd >> linker instead of lld, the LLVM linker. This will invalidate assumptions >> about command lines that are passed on to the linker. We should use >> -fuse-ld=lld to force clang to always pick lld as the linker, so we can be >> sure that the command lines will work. > > Marked as reviewed by jiangli (Reviewer). @jianglizhou Did you verify that this works on your system? Apparently your environment and mine has differed, since you have been able to build with clang without this patch. If you did not, I'd appreciate if you could take it for a spin, so I know I won't break anything in environments similar to your. - PR Comment: https://git.openjdk.org/jdk/pull/19456#issuecomment-2137761321
RFR: 8333189: Make sure clang on linux uses lld as linker
When compiling with clang on linux, clang can decide to pick up the bfd linker instead of lld, the LLVM linker. This will invalidate assumptions about command lines that are passed on to the linker. We should use -fuse-ld=lld to force clang to always pick lld as the linker, so we can be sure that the command lines will work. - Commit messages: - 8333189: Make sure clang on linux uses lld as linker Changes: https://git.openjdk.org/jdk/pull/19456/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19456=00 Issue: https://bugs.openjdk.org/browse/JDK-8333189 Stats: 10 lines in 4 files changed: 8 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19456.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19456/head:pull/19456 PR: https://git.openjdk.org/jdk/pull/19456
Re: Hermetic Java project meeting
On 2024-05-07 06:04, Jiangli Zhou wrote: I did think I correctly changed every dynamic check that you had added to a compile-time check, so it bewilders me somewhat when you say that jvm.cfg is not needed in your branch. Can you verify and confirm that the static launcher actually works in your branch, if there is no "lib/jvm.cfg" present? In my /leyden/build/linux-x86_64-server-slowdebug/images/jdk directory: $ mv lib/jvm.cfg lib/jvm.cfg.no_used $ find . | grep jvm.cfg ./lib/jvm.cfg.no_used $ bin/javastatic -cp HelloWorld HelloWorld I was very much mislead by this. I was sure I hade made some mistake when I picked out the changes you have made for static builds (and removed all the other changes, e.g. for the hermetic jar files), since you said this worked for you. I have been scrutinizing the difference between your branch and mine, over and over again, without understanding what the difference could be. Finally I did what I should have done at the very beginning, and actually tested building and running your branch. It did not work either. So why did you claim it worked? I kept digging, and I found out the reason. You had indeed implemented a fix for this, but only on Linux. I was testing on macOS. (It is also not implemented for Windows, but since I'm still struggling to find a way to create proper static builds there it is less of a problem for now.) My branch worked just as well as your on Linux. I have now fixed so it works on macOS too. With this hurdle out of the way, I can get back to doing real work on the patch. Unfortunately this detour took far too much time. :-( /Magnus
Integrated: 8331921: Hotspot assembler files should use common logic to setup exported functions
On Wed, 8 May 2024 13:30:59 GMT, Magnus Ihse Bursie wrote: > As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we > are not consistently setting all assembler directives correctly, we can get > errors that are not detected by the linker. > > We should stop duplicating this code and create a unified macro to properly > setup functions, and use it everywhere. This pull request has now been integrated. Changeset: cabe3374 Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/cabe337400a0bd61d73bf3ca66e16266267299c7 Stats: 604 lines in 22 files changed: 124 ins; 347 del; 133 mod 8331921: Hotspot assembler files should use common logic to setup exported functions Reviewed-by: coleenp, erikj, dholmes - PR: https://git.openjdk.org/jdk/pull/19138
Re: RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions [v3]
On Tue, 28 May 2024 02:41:42 GMT, David Holmes wrote: > What testing has been done in our CI? I have run tier-1. And, as I said, I had also made the typical build change test: comparing the build results bit-for-bit. On macOS and Windows, there were no differences. On linux, there were a few technical changes in the symbol table, but that should not really affect the code. - PR Comment: https://git.openjdk.org/jdk/pull/19138#issuecomment-2134999226
Re: RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions [v3]
> As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we > are not consistently setting all assembler directives correctly, we can get > errors that are not detected by the linker. > > We should stop duplicating this code and create a unified macro to properly > setup functions, and use it everywhere. Magnus Ihse Bursie has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits: - Merge branch 'master' into hotspot-assembler-functions - Merge branch 'master' into hotspot-assembler-functions - Fix copyright headers - Fix building on macos/aarch64 - Use % instead of @ due to arm assembler - 8331921: Hotspot assembler files should use common logic to setup exported functions - Changes: https://git.openjdk.org/jdk/pull/19138/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19138=02 Stats: 604 lines in 22 files changed: 124 ins; 347 del; 133 mod Patch: https://git.openjdk.org/jdk/pull/19138.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19138/head:pull/19138 PR: https://git.openjdk.org/jdk/pull/19138
Re: RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions [v2]
On Tue, 28 May 2024 02:39:01 GMT, David Holmes wrote: >> Magnus Ihse Bursie has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains five commits: >> >> - Merge branch 'master' into hotspot-assembler-functions >> - Fix copyright headers >> - Fix building on macos/aarch64 >> - Use % instead of @ due to arm assembler >> - 8331921: Hotspot assembler files should use common logic to setup >> exported functions > > make/hotspot/lib/CompileJvm.gmk line 53: > >> 51: >> -I$(TOPDIR)/src/hotspot/os_cpu/$(HOTSPOT_TARGET_OS)_$(HOTSPOT_TARGET_CPU_ARCH) >> \ >> 52: -I$(TOPDIR)/src/hotspot/os/$(HOTSPOT_TARGET_OS) \ >> 53: -I$(TOPDIR)/src/hotspot/os/$(HOTSPOT_TARGET_OS_TYPE) \ > > What does the second line evaluate to on Windows? Does it duplicate the prior > line? Yes, it does. It is a bit unfortunate, but it is also hard to avoid with the limited expressions that are available in Make syntax. (It can be done but at the cost of decreased readability.) It is completely harmless, though, and we have the same construct in other places in the build system. - PR Review Comment: https://git.openjdk.org/jdk/pull/19138#discussion_r1617044188
Re: RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions [v2]
On Thu, 23 May 2024 16:33:14 GMT, Magnus Ihse Bursie wrote: >> As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we >> are not consistently setting all assembler directives correctly, we can get >> errors that are not detected by the linker. >> >> We should stop duplicating this code and create a unified macro to properly >> setup functions, and use it everywhere. > > Magnus Ihse Bursie has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains five commits: > > - Merge branch 'master' into hotspot-assembler-functions > - Fix copyright headers > - Fix building on macos/aarch64 > - Use % instead of @ due to arm assembler > - 8331921: Hotspot assembler files should use common logic to setup exported > functions Can I have a second Hotspot reviewer on this? Or are the reviews from Coleen and Erik good enough? - PR Comment: https://git.openjdk.org/jdk/pull/19138#issuecomment-2133719877
Re: RFR: 8330542: Template for Creating Strict JAXP Configuration File [v14]
On Fri, 24 May 2024 16:36:32 GMT, Joe Wang wrote: >> Add two sample configuration files: >> >> jaxp-strict.properties: used to set strict configuration, stricter than >> jaxp.properties in previous versions such as JDK 22 >> >>> jaxp-compat.properties: used to regain compatibility from any more >>> restricted configuration than previous versions such as JDK 22 >> >> Updated on 5/16/2024 >> >> Design change: >> The design is changed to include in the JDK two configuration files that are >> the default jaxp.properties and jaxp-strict.properties, instead of three, >> dropping jaxp-compat.properties. >> >> Updated on 5/18/2024 >> >> Withdraw changes to jaxp.properties. The original idea was to match >> jaxp-strict.properties with regard to the properties. However, that change >> impact the configuration process, resulting in tests that verify the process >> to fail. >> >> Updated on 5/23/2024 >> >> Provide a template `jaxp-strict.template` instead of a properties file. This >> template can be used to create custom configuration files. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > rename the template to jaxp-strict.properties.template LGTM - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18831#pullrequestreview-2078188172
Re: RFR: 8332849: Update doc/testing.{md,html} (spelling and stale information)
On Thu, 23 May 2024 23:22:51 GMT, Mikael Vidstedt wrote: > Update the testing doc to remove some stale information (AOT_MODULES, removed > in [JDK-8264805](https://bugs.openjdk.org/browse/JDK-8264805)) and fix some > spelling issues. > > Testing: tier1 Thanks! ❤️ - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19375#pullrequestreview-2076318537
Re: RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions [v2]
On Thu, 23 May 2024 16:33:14 GMT, Magnus Ihse Bursie wrote: >> As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we >> are not consistently setting all assembler directives correctly, we can get >> errors that are not detected by the linker. >> >> We should stop duplicating this code and create a unified macro to properly >> setup functions, and use it everywhere. > > Magnus Ihse Bursie has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains five commits: > > - Merge branch 'master' into hotspot-assembler-functions > - Fix copyright headers > - Fix building on macos/aarch64 > - Use % instead of @ due to arm assembler > - 8331921: Hotspot assembler files should use common logic to setup exported > functions Thanks Coleen! Any second reviewer? - PR Comment: https://git.openjdk.org/jdk/pull/19138#issuecomment-2128984554
Re: RFR: 8330542: Template for Creating Strict JAXP Configuration File [v13]
On Fri, 24 May 2024 05:26:40 GMT, Joe Wang wrote: >> Add two sample configuration files: >> >> jaxp-strict.properties: used to set strict configuration, stricter than >> jaxp.properties in previous versions such as JDK 22 >> >>> jaxp-compat.properties: used to regain compatibility from any more >>> restricted configuration than previous versions such as JDK 22 >> >> Updated on 5/16/2024 >> >> Design change: >> The design is changed to include in the JDK two configuration files that are >> the default jaxp.properties and jaxp-strict.properties, instead of three, >> dropping jaxp-compat.properties. >> >> Updated on 5/18/2024 >> >> Withdraw changes to jaxp.properties. The original idea was to match >> jaxp-strict.properties with regard to the properties. However, that change >> impact the configuration process, resulting in tests that verify the process >> to fail. >> >> Updated on 5/23/2024 >> >> Provide a template `jaxp-strict.template` instead of a properties file. This >> template can be used to create custom configuration files. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > add a template instead of a property file; remove implNote; update test and > make script accordingly. I would recommend naming the new file `jaxp-strict.properties.template` instead. This would follow the pattern we have used in the JDK, and I think it is much better at providing clarify as to what this file actually is -- a template for a `.properties` file. - PR Comment: https://git.openjdk.org/jdk/pull/18831#issuecomment-2128979310
Re: RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions
On Wed, 8 May 2024 13:30:59 GMT, Magnus Ihse Bursie wrote: > As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we > are not consistently setting all assembler directives correctly, we can get > errors that are not detected by the linker. > > We should stop duplicating this code and create a unified macro to properly > setup functions, and use it everywhere. Ping? Any takers? - PR Comment: https://git.openjdk.org/jdk/pull/19138#issuecomment-2127585577
Re: RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions [v2]
> As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we > are not consistently setting all assembler directives correctly, we can get > errors that are not detected by the linker. > > We should stop duplicating this code and create a unified macro to properly > setup functions, and use it everywhere. Magnus Ihse Bursie has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits: - Merge branch 'master' into hotspot-assembler-functions - Fix copyright headers - Fix building on macos/aarch64 - Use % instead of @ due to arm assembler - 8331921: Hotspot assembler files should use common logic to setup exported functions - Changes: https://git.openjdk.org/jdk/pull/19138/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19138=01 Stats: 604 lines in 22 files changed: 124 ins; 347 del; 133 mod Patch: https://git.openjdk.org/jdk/pull/19138.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19138/head:pull/19138 PR: https://git.openjdk.org/jdk/pull/19138
Re: JDK-8170635 -- adding a base library to java
On 2024-05-16 11:34, Suchismith Roy wrote: Hi Magnus Could you provide an existing PR/implementation I can refer from ? There is no such previous all-encompassing library to just "copy". In fact, the core problem here is that we are even missing such a place to put common code. This is not a suitable beginner's bug in the JDK. Solving this is as much about politics and policy as about a technical implementation. /Magnus Thanks Suchismith Roy *From: *build-dev on behalf of Suchismith Roy *Date: *Wednesday, 8 May 2024 at 11:47 PM *To: *Magnus Ihse Bursie , Thomas Stüfe *Cc: *build-dev@openjdk.org *Subject: *[EXTERNAL] RE: JDK-8170635 -- adding a base library to java Thanks for the reply @Magnus Ihse Bursie Is there any example of PRs which create such libraries that I can refer to ? Is OSAL similar to how os. cpp is defined and respective platforms implement them ? From: Magnus Ihse Bursie Thanks for the reply @Magnus Ihse Bursie <mailto:magnus.ihse.bur...@oracle.com> Is there any example of PRs which create such libraries that I can refer to ? Is OSAL similar to how os.cpp is defined and respective platforms implement them ? *From: *Magnus Ihse Bursie *Date: *Tuesday, 7 May 2024 at 6:18 PM *To: *Thomas Stüfe , Suchismith Roy *Cc: *build-dev@openjdk.org *Subject: *[EXTERNAL] Re: JDK-8170635 -- adding a base library to java On 2024-05-06 16:36, Thomas Stüfe wrote: > Not sure if you meant to address this mail to a specific person. I > assume with proposal you mean this: > https://mail.openjdk.org/pipermail/build-dev/2016-September/017746.html ? > > If yes, my proposal was to move dladdr out of the OpenJDK code base > into an independent library that would be maintained by IBM, > hopefully, and would be a prerequisite for building the JDK. > If no, whose proposal did you mean? Oh, this is an old bug you're picking up Suchismith! I read through the discussion from 2016. It seems that the suggestion to make an external 3rd party library was only supported by Thomas, and that the general agreement among the other participants was that we should have a general, base-level "OSAL" (OS abstraction library) in the JDK, that could be used by both Hotspot and libjli, as well as other JDK libraries. Creating such a library would be a much larger effort than just adding a AIX implementation of dladdr to it, if it existed. The current structure of the JDK does not readily lend itself to such a library, neither in terms of source code layout nor build system. With that said, I do think it would be good if we had such a library. There are more cases than the AIX dladdr issue that is duplicated, like jio_snprintf() and friends. This has actually caused some headaches when doing static builds, since duplication of these functions are not allowed when creating a single linked instance. (The current duplication in dynamic libraries is just ugly and bad programming, not a compilation error.) But it is a much larger question than just fixing an AIX issue. /Magnus
Integrated: 8332808: Always set java.io.tmpdir to a suitable value in the build
On Thu, 23 May 2024 10:27:47 GMT, Magnus Ihse Bursie wrote: > We should pass a good value for java.io.tmpdir to all java invocations in the > build, that redirect any temporary files to somewhere under the build > directory. > > This bug was created as a result of the discussion regarding > [JDK-8331348](https://bugs.openjdk.org/browse/JDK-8331348). This pull request has now been integrated. Changeset: 90758f67 Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/90758f6735620776fcb60da9e0e2c91a4f53aaf1 Stats: 9 lines in 3 files changed: 6 ins; 0 del; 3 mod 8332808: Always set java.io.tmpdir to a suitable value in the build Reviewed-by: erikj - PR: https://git.openjdk.org/jdk/pull/19362
Re: RFR: 8332808: Always set java.io.tmpdir to a suitable value in the build [v2]
> We should pass a good value for java.io.tmpdir to all java invocations in the > build, that redirect any temporary files to somewhere under the build > directory. > > This bug was created as a result of the discussion regarding > [JDK-8331348](https://bugs.openjdk.org/browse/JDK-8331348). Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: We should only explicitly append JAVA_FLAGS_TMPDIR for JAVAC and BUILD_JAVAC, not the JAVA invocations. - Changes: - all: https://git.openjdk.org/jdk/pull/19362/files - new: https://git.openjdk.org/jdk/pull/19362/files/0e778b99..a9897ba3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19362=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19362=00-01 Stats: 4 lines in 1 file changed: 0 ins; 1 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19362.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19362/head:pull/19362 PR: https://git.openjdk.org/jdk/pull/19362
Re: RFR: 8332808: Always set java.io.tmpdir to a suitable value in the build
On Thu, 23 May 2024 10:27:47 GMT, Magnus Ihse Bursie wrote: > We should pass a good value for java.io.tmpdir to all java invocations in the > build, that redirect any temporary files to somewhere under the build > directory. > > This bug was created as a result of the discussion regarding > [JDK-8331348](https://bugs.openjdk.org/browse/JDK-8331348). And yes, the way we build up our java/javac command lines is really messy and need an overhaul. I'll come to it, at some point, I hope... - PR Comment: https://git.openjdk.org/jdk/pull/19362#issuecomment-2126765313
RFR: 8332808: Always set java.io.tmpdir to a suitable value in the build
We should pass a good value for java.io.tmpdir to all java invocations in the build, that redirect any temporary files to somewhere under the build directory. This bug was created as a result of the discussion regarding [JDK-8331348](https://bugs.openjdk.org/browse/JDK-8331348). - Commit messages: - 8332808: Always set java.io.tmpdir to a suitable value in the build Changes: https://git.openjdk.org/jdk/pull/19362/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19362=00 Issue: https://bugs.openjdk.org/browse/JDK-8332808 Stats: 9 lines in 3 files changed: 7 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19362.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19362/head:pull/19362 PR: https://git.openjdk.org/jdk/pull/19362
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]
On Fri, 17 May 2024 13:38:25 GMT, Maurizio Cimadamore wrote: >> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting >> the use of JNI in the following ways: >> >> * `System::load` and `System::loadLibrary` are now restricted methods >> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods >> * binding a JNI `native` method declaration to a native implementation is >> now considered a restricted operation >> >> This PR slightly changes the way in which the JDK deals with restricted >> methods, even for FFM API calls. In Java 22, the single >> `--enable-native-access` was used both to specify a set of modules for which >> native access should be allowed *and* to specify whether illegal native >> access (that is, native access occurring from a module not specified by >> `--enable-native-access`) should be treated as an error or a warning. More >> specifically, an error is only issued if the `--enable-native-access flag` >> is used at least once. >> >> Here, a new flag is introduced, namely >> `illegal-native-access=allow/warn/deny`, which is used to specify what >> should happen when access to a restricted method and/or functionality is >> found outside the set of modules specified with `--enable-native-access`. >> The default policy is `warn`, but users can select `allow` to suppress the >> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This >> aligns the treatment of restricted methods with other mechanisms, such as >> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. >> >> Some changes were required in the package-info javadoc for >> `java.lang.foreign`, to reflect the changes in the command line flags >> described above. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments Build changes look good. Thanks for trimming down NATIVE_ACCESS_MODULES. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19213#pullrequestreview-2070573791
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a stricter default configuration [v12]
On Tue, 21 May 2024 20:28:37 GMT, Joe Wang wrote: >> Add two sample configuration files: >> >> jaxp-strict.properties: used to set strict configuration, stricter than >> jaxp.properties in previous versions such as JDK 22 >> >>> jaxp-compat.properties: used to regain compatibility from any more >>> restricted configuration than previous versions such as JDK 22 >> >> Updated on 5/16/2024 >> >> Design change: >> The design is changed to include in the JDK two configuration files that are >> the default jaxp.properties and jaxp-strict.properties, instead of three, >> dropping jaxp-compat.properties. >> >> Updated on 5/18/2024 >> >> Withdraw changes to jaxp.properties. The original idea was to match >> jaxp-strict.properties with regard to the properties. However, that change >> impact the configuration process, resulting in tests that verify the process >> to fail. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > add a note to module-info make/modules/java.xml/Copy.gmk line 35: > 33: $(eval $(call SetupCopyFiles, COPY_XML_MODULE_CONF, \ > 34: DEST := $(CONF_DST_DIR), \ > 35: FILES := $(wildcard > $(TOPDIR)/src/java.xml/share/conf/jaxp*.properties*), \ I don't think you need, nor should have, the asterisk after the extension. You are only copying `.properties` files. Suggestion: FILES := $(wildcard $(TOPDIR)/src/java.xml/share/conf/jaxp*.properties), \ - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1609565653
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v25]
On Wed, 22 May 2024 07:44:08 GMT, Magnus Ihse Bursie wrote: >> ... still missing... > > Actually, this is a bit strange. I thought jcheck would look for missing > newline at EOF, and that properties files were included in the check > nowadays. I'll need to check this out. I did some testing and it turns out that this is indeed not checked. I believe this is a miss in the Skara reimplementation of jcheck. I've opened https://bugs.openjdk.org/browse/SKARA-2265 to track this. Nevertheless, it would be good if you could fix this. :) - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1609491669
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v25]
On Wed, 22 May 2024 07:42:39 GMT, Magnus Ihse Bursie wrote: >> src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties line >> 165: >> >>> 163: >>> 164: runtime.link.info=Linking based on the current run-time image. >>> 165: runtime.link.jprt.path.extra=(run-time image) >> >> Missing newline at last line. > > ... still missing... Actually, this is a bit strange. I thought jcheck would look for missing newline at EOF, and that properties files were included in the check nowadays. I'll need to check this out. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1609459326
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v25]
On Thu, 4 Apr 2024 20:52:59 GMT, Magnus Ihse Bursie wrote: >> Severin Gehwolf has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - Use shorter name for the build tool >> - Review feedback from Erik J. >> - Remove dependency on CDS which isn't needed anymore > > src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties line > 165: > >> 163: >> 164: runtime.link.info=Linking based on the current run-time image. >> 165: runtime.link.jprt.path.extra=(run-time image) > > Missing newline at last line. ... still missing... - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1609457478
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
On Thu, 9 May 2024 07:50:00 GMT, Julian Waters wrote:
>> WIP
>>
>> This changeset contains hsdis for Windows/gcc Port. It supports both the
>> binutils and capstone backends, though the LLVM backend is left out due to
>> compatibility issues encountered during the build. Currently, which gcc
>> distributions are supported is still to be clarified, as several, ranging
>> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason,
>> the build system hack in place at the moment to compile the binutils backend
>> on Windows is still left in place, for now.
>
> Julian Waters has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains two commits:
>
> - Add check for compiler in configure
> - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
Hi Julian, sorry for not getting back to you.
The problem from my PoV is that this is a very large and intrusive patch in the
build of the actual product, for a claimed problem in the tangential hsdis
library. I have not understood a pressing business need to get a Windows/gcc
port for hsdis, which means I can't really prioritize trying to understand this
patch.
As you know, the build system does not currently really separate between "the
OS is Windows" and "the toolchain is Microsoft". I understand that you want to
fix that, and in theory I support it. However, you must also realize that
making a complete fix of this requires a lot of work, for something that there
is no clearly defined need. (After all, cl.exe works fine to create the build,
has no apparent issues, and is as far as an "official" compiler for Windows as
you can get.) That makes it fall squarely in the WIBNIs bucked ("wouldn't it be
nice if...").
If you can fix just the hsdis build without changing anything outside the hsdis
Makefiles, that would be a different story. Such a change would be limited in
scope, easy to say it will not affect the product proper, and be easier to
review.
-
PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2112546029
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v28]
On Wed, 15 May 2024 11:55:39 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 108 commits: > > - Merge branch 'master' into jdk-8311302-jmodless-link > - Simplify JLINK_JDK_EXTRA_OPTS var handling > - Only add runtime track files for linkable runtimes > - Generate the runtime image link diff at jlink time > >But only do that once the plugin-pipeline has run. The generation is >enabled with the hidden option '--generate-linkable-runtime' and >the resource pools available at jlink time are being used to generate >the diff. > - Merge branch 'master' into jdk-8311302-jmodless-link > - Use shorter name for the build tool > - Review feedback from Erik J. > - Remove dependency on CDS which isn't needed anymore > - Fix coment > - Fix comment > - ... and 98 more: https://git.openjdk.org/jdk/compare/957eb611...8cca277e Thanks! Now I am 100% happy with the build changes. :) - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14787#pullrequestreview-2057984229
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v26]
On Tue, 7 May 2024 16:52:12 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 105 commits: > > - Generate the runtime image link diff at jlink time > >But only do that once the plugin-pipeline has run. The generation is >enabled with the hidden option '--generate-linkable-runtime' and >the resource pools available at jlink time are being used to generate >the diff. > - Merge branch 'master' into jdk-8311302-jmodless-link > - Use shorter name for the build tool > - Review feedback from Erik J. > - Remove dependency on CDS which isn't needed anymore > - Fix coment > - Fix comment > - Fix typo > - Revert some now unneded build changes > - Follow build tools naming convention > - ... and 95 more: https://git.openjdk.org/jdk/compare/23a72a1f...67aea4ca make/Images.gmk line 100: > 98: > 99: ifeq ($(JLINK_PRODUCE_RUNTIME_LINK_JDK), true) > 100: JLINK_JDK_EXTRA_OPTS := $(JLINK_JDK_EXTRA_OPTS) > --generate-linkable-runtime I just noticed this slight improvement: Suggestion: JLINK_JDK_EXTRA_OPTS += --generate-linkable-runtime - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1594774220
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v26]
On Tue, 7 May 2024 16:52:12 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 105 commits: > > - Generate the runtime image link diff at jlink time > >But only do that once the plugin-pipeline has run. The generation is >enabled with the hidden option '--generate-linkable-runtime' and >the resource pools available at jlink time are being used to generate >the diff. > - Merge branch 'master' into jdk-8311302-jmodless-link > - Use shorter name for the build tool > - Review feedback from Erik J. > - Remove dependency on CDS which isn't needed anymore > - Fix coment > - Fix comment > - Fix typo > - Revert some now unneded build changes > - Follow build tools naming convention > - ... and 95 more: https://git.openjdk.org/jdk/compare/23a72a1f...67aea4ca The new build changes look extremely trivial. From a pure build PoV, this is a much simpler solution. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14787#pullrequestreview-2045924409
Re: RFR: 8331886: Allow markdown src file overrides [v2]
On Wed, 8 May 2024 14:29:18 GMT, Erik Joelsson wrote: >> For c, c++ and java source files, we have a built in system for letting >> more specific files override if there are multiple files with the same name >> found, by letting the first found file with a given name override any later >> found files with that name. This is typically used for OS specific variants >> or when needing to override a source file with a file from a custom source >> set. We would like to make it possible to do the same for markdown files >> used to generate man pages. >> >> This will not have an immediate use i OpenJDK, but is needed for a custom >> override in proprietary code. >> >> The change in Docs.gmk removes unnecessary extra loops so that >> SetupProcessMarkdown is called only once per module. This is necessary for >> the override mechanism to kick in for each module src set. >> >> The logic in ProcessMarkdown.gmk is more or less copied from >> SetupNativeCompilation. > > Erik Joelsson 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 three additional > commits since the last revision: > > - Remove debug printing > - Merge branch 'master' into JDK-8331886-override-markdown > - JDK-8331886 Marked as reviewed by ihse (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19132#pullrequestreview-2045917930
Re: RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions
On Wed, 8 May 2024 13:30:59 GMT, Magnus Ihse Bursie wrote: > As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we > are not consistently setting all assembler directives correctly, we can get > errors that are not detected by the linker. > > We should stop duplicating this code and create a unified macro to properly > setup functions, and use it everywhere. When making a build comparison, the build result was identical on Windows (of course), and macOS. On linux/x64 and linux/aarch64, there was a slight difference, in that a few functions did not properly set `.type`, so they appeared as `NOTYPE` instead of `FUNC` in the symbol table. I have not done comparison builds for other linux platforms, but I assume that GHA passing is good enough. - PR Comment: https://git.openjdk.org/jdk/pull/19138#issuecomment-2100776636
RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions
As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we are not consistently setting all assembler directives correctly, we can get errors that are not detected by the linker. We should stop duplicating this code and create a unified macro to properly setup functions, and use it everywhere. - Commit messages: - Fix copyright headers - Fix building on macos/aarch64 - Use % instead of @ due to arm assembler - 8331921: Hotspot assembler files should use common logic to setup exported functions Changes: https://git.openjdk.org/jdk/pull/19138/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19138=00 Issue: https://bugs.openjdk.org/browse/JDK-8331921 Stats: 604 lines in 22 files changed: 124 ins; 347 del; 133 mod Patch: https://git.openjdk.org/jdk/pull/19138.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19138/head:pull/19138 PR: https://git.openjdk.org/jdk/pull/19138
Re: RFR: 8331541: [i386] linking with libjvm.so fails after JDK-8283326
On Thu, 2 May 2024 08:23:38 GMT, Vladimir Petko wrote: > Hi, > > The `.type _SafeFetch32_impl,@function` symbol declaration contains a > spurious underscore causing an undefined symbol in libjvm.so. > > I am not sure about change in default flags. I have removed the flag that > allows linking with undefined symbols > because having the flag on might cause bugs like this one or > https://bugs.openjdk.org/browse/JDK-8329983 as the build succeeds even if > some symbols are not defined. > Openjdk builds successfully without it on amd64, i386, armhf, arm64, s390, > ppc64el and riscv64 with this change[1]. riscv64 build was done locally > inside vm: > > Finished building target 'images' in configuration > 'linux-riscv64-server-release' > ubuntu@ubuntu:~/jdk$ > > Unfortunately I do not know why it was introduced, so I might be missing > something. > I am happy to drop it from this one or move it to a separate issue/pr. > > [1] > https://launchpad.net/~vpa1977/+archive/ubuntu/october-21/+sourcepub/16076564/+listing-archive-extra I created https://bugs.openjdk.org/browse/JDK-8331921 to track setting up proper assembly functions consistently. - PR Comment: https://git.openjdk.org/jdk/pull/19048#issuecomment-2100375665
Re: RFR: 8331541: [i386] linking with libjvm.so fails after JDK-8283326
On Thu, 2 May 2024 08:23:38 GMT, Vladimir Petko wrote: > Hi, > > The `.type _SafeFetch32_impl,@function` symbol declaration contains a > spurious underscore causing an undefined symbol in libjvm.so. > > I am not sure about change in default flags. I have removed the flag that > allows linking with undefined symbols > because having the flag on might cause bugs like this one or > https://bugs.openjdk.org/browse/JDK-8329983 as the build succeeds even if > some symbols are not defined. > Openjdk builds successfully without it on amd64, i386, armhf, arm64, s390, > ppc64el and riscv64 with this change[1]. riscv64 build was done locally > inside vm: > > Finished building target 'images' in configuration > 'linux-riscv64-server-release' > ubuntu@ubuntu:~/jdk$ > > Unfortunately I do not know why it was introduced, so I might be missing > something. > I am happy to drop it from this one or move it to a separate issue/pr. > > [1] > https://launchpad.net/~vpa1977/+archive/ubuntu/october-21/+sourcepub/16076564/+listing-archive-extra So, after sleeping on this, here is my analysis/verdict: 1) The fix on renaming the `.type` directive looks good. This is the actual bug fix. 2) We can remove `--allow-shlib-undefined`. That just overrides `--no-allow-shlib- undefined`, which has been the default for years and years, and there is no good reason to have it. It is not strictly related to the bug fix per se, but we can do that in this PR as well to keep things simpler. 3) The reason this could happen was since we used assembler code. A similar problem in C++ code would not have been allowed by the compiler. 4) If we introduce a common macro as I suggest, we can avoid this ever happening again. So while the linker cannot guarantee the consistency when linking libjvm.so (which I really would have liked), by using a stricter scheme when defining symbols in assembly code, we can make sure to avoid it. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19048#pullrequestreview-2045354117
Re: RFR: 8331541: [i386] linking with libjvm.so fails after JDK-8283326
On Thu, 2 May 2024 08:23:38 GMT, Vladimir Petko wrote: > Hi, > > The `.type _SafeFetch32_impl,@function` symbol declaration contains a > spurious underscore causing an undefined symbol in libjvm.so. > > I am not sure about change in default flags. I have removed the flag that > allows linking with undefined symbols > because having the flag on might cause bugs like this one or > https://bugs.openjdk.org/browse/JDK-8329983 as the build succeeds even if > some symbols are not defined. > Openjdk builds successfully without it on amd64, i386, armhf, arm64, s390, > ppc64el and riscv64 with this change[1]. riscv64 build was done locally > inside vm: > > Finished building target 'images' in configuration > 'linux-riscv64-server-release' > ubuntu@ubuntu:~/jdk$ > > Unfortunately I do not know why it was introduced, so I might be missing > something. > I am happy to drop it from this one or move it to a separate issue/pr. > > [1] > https://launchpad.net/~vpa1977/+archive/ubuntu/october-21/+sourcepub/16076564/+listing-archive-extra Ok, I think I'm understanding this better now. The error occurs since the `.type`, `.globl` and `.hidden` directives do not match -- we define the global symbol to be `SafeFetch32_impl` but then we set the type of the non-existing symbol `_SafeFetch32_impl`. Somehow this tricks the linker into accepting this as an undefined symbol. The `.type` directive is not without purpose -- it sets the type of the symbol to be a function. If omitted, the type will be `NOTYPE`. Apparently this does not break the program but there is no reason to remove the `.type` directives. Instead, we should have a common macro, something like this: #define DECLARE_FUNC(func) \ .globl func ; \ .hidden func ; \ .type func,@function ; \ func: in a shared file, and include and use it for all symbols in our hotspot assembly files. I was thinking somewhat along those lines last time I was poking around there (when introducing .hidden for the removal of the hotspot map files), but never really got around to it. This bug really shows why we should do that. - PR Comment: https://git.openjdk.org/jdk/pull/19048#issuecomment-2100322142
Re: RFR: 8331886: Allow markdown src file overrides
On Wed, 8 May 2024 01:02:41 GMT, Erik Joelsson wrote: > For c, c++ and java source files, we have a built in system for letting more > specific files override if there are multiple files with the same name found, > by letting the first found file with a given name override any later found > files with that name. This is typically used for OS specific variants or when > needing to override a source file with a file from a custom source set. We > would like to make it possible to do the same for markdown files used to > generate man pages. > > This will not have an immediate use i OpenJDK, but is needed for a custom > override in proprietary code. > > The change in Docs.gmk removes unnecessary extra loops so that > SetupProcessMarkdown is called only once per module. This is necessary for > the override mechanism to kick in for each module src set. > > The logic in ProcessMarkdown.gmk is more or less copied from > SetupNativeCompilation. Looks good (apart from the PrintVar) This was particularly hard to review, since neither Github nor Webrev could make any sense of this. For other reviewers, I can add that the only substantial change in ProcessMarkdown.gmk is wrapping the entire function body in a `ifeq ($$($1_$2_OUTPUT_FILE_PROCESSED), )` check (and updating this variable upon entry). - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19132#pullrequestreview-2045243623 PR Comment: https://git.openjdk.org/jdk/pull/19132#issuecomment-2100237486
Re: RFR: 8331886: Allow markdown src file overrides
On Wed, 8 May 2024 01:02:41 GMT, Erik Joelsson wrote: > For c, c++ and java source files, we have a built in system for letting more > specific files override if there are multiple files with the same name found, > by letting the first found file with a given name override any later found > files with that name. This is typically used for OS specific variants or when > needing to override a source file with a file from a custom source set. We > would like to make it possible to do the same for markdown files used to > generate man pages. > > This will not have an immediate use i OpenJDK, but is needed for a custom > override in proprietary code. > > The change in Docs.gmk removes unnecessary extra loops so that > SetupProcessMarkdown is called only once per module. This is necessary for > the override mechanism to kick in for each module src set. > > The logic in ProcessMarkdown.gmk is more or less copied from > SetupNativeCompilation. make/common/ProcessMarkdown.gmk line 43: > 41: # Only continue if this target file hasn't been processed already. This > lets > 42: # the first found source file override any other with the same name. > 43: $$(call PrintVar, $1_$2_INPUT_FILE) Left-over debug code. - PR Review Comment: https://git.openjdk.org/jdk/pull/19132#discussion_r1593782745
Re: JDK-8170635 -- adding a base library to java
On 2024-05-06 16:36, Thomas Stüfe wrote: Not sure if you meant to address this mail to a specific person. I assume with proposal you mean this: https://mail.openjdk.org/pipermail/build-dev/2016-September/017746.html ? If yes, my proposal was to move dladdr out of the OpenJDK code base into an independent library that would be maintained by IBM, hopefully, and would be a prerequisite for building the JDK. If no, whose proposal did you mean? Oh, this is an old bug you're picking up Suchismith! I read through the discussion from 2016. It seems that the suggestion to make an external 3rd party library was only supported by Thomas, and that the general agreement among the other participants was that we should have a general, base-level "OSAL" (OS abstraction library) in the JDK, that could be used by both Hotspot and libjli, as well as other JDK libraries. Creating such a library would be a much larger effort than just adding a AIX implementation of dladdr to it, if it existed. The current structure of the JDK does not readily lend itself to such a library, neither in terms of source code layout nor build system. With that said, I do think it would be good if we had such a library. There are more cases than the AIX dladdr issue that is duplicated, like jio_snprintf() and friends. This has actually caused some headaches when doing static builds, since duplication of these functions are not allowed when creating a single linked instance. (The current duplication in dynamic libraries is just ugly and bad programming, not a compilation error.) But it is a much larger question than just fixing an AIX issue. /Magnus
Re: Hermetic Java project meeting
On 2024-05-07 06:04, Jiangli Zhou wrote: On Tue, Apr 30, 2024 at 5:42 AM Magnus Ihse Bursie wrote: I am not sure why clang insisted on picking up ld and not lld. I remeber trying with -fuse-ld=lld, and that it did not work either. Unfortunately, I don't remember exactly what the problems were. I started reinstalling my Linux workstation yesterday, but something went wrong, and it failed so hard that it got semi-bricked by the new installation, so I need to redo everything from scratch. :-( After that is done, I'll re-test. Hopefully this was just my old installation that was too broken. I decided to spend the time to reinstall my machine. Now linking with clang works. Kind of. For some reason, it still picks up binutils ld and not lld, and then -l:libc++.a does not work, but when I replaced it with -l:libstdc++.a it worked just fine. I guess we need to either forcefully add -fuse-ld=lld to our clang compilation lines, or figure out if clang is going to call the binutils or llvm ld, and select the right option. I still find the logic for how clang and gcc locates the default linker to be mostly magic. I guess I need to make a deep dive in understanding this to be able to resolve this properly. The JDK and VM code has pre-existing assumptions about the JDK directories and dynamic linking (e.g. the .so). JLI_IsStaticJDK|JLI_SetStaticJDK|JVM_IsStaticJDK|JVM_SetStaticJDK is needed for static JDK support to handle those cases correctly. CreateExecutionEnvironment that I mentioned earlier is one of the examples. I'm quite certain the issue that you are running into is due to the incorrect static check/handling in CreateExecutionEnvironment. I'll have a look at that, thanks for the pointer. In my branch, I am only using compile-time #ifdef checks for static vs dynamic. In the long run, the runtime checks that you have done are a good thing, but at the moment they are just adding intrusive changes without providing any benefit -- if we can't reuse .o files between dynamic and static compilation, there is no point in introducing a runtime check when we already have a working compile-time check. I haven't seen your branch/code. I'd suggest not going with the #ifdef checks as that's the opposite direction of what we want to achieve. It doesn't seem to be worth your effort to add more #ifdef checks in order to do static linking build work, even those are for temporary testing reasons. Okay... My understanding was that you wanted to push for the quickest possible integration of building a static java launcher into mainline. To do that as fast as possible, we need to use the existing framework for separating statically and dynamically linked libraries, which means doing compile time checks using #ifdefs. Are you saying now that the priorities has changed, and that you want to start by introducing your framework for the runtime lookup if we are static or dynamic? To be honest, I think your prototype is rather hacky in how you implement this, and I reckon that it will require quite a lot of work to be accepted into mainline. I also think you need a CSR for changing the Hotspot/JDK behavior wrt this, which further adds to the process. If you want to go that route instead, then I'll put my work on hold until you have gotten a working solution for the runtime lookup in mainline. I gather this means that there is no real stress for me anymore. /Magnus
Re: RFR: 8331541: [i386] linking with libjvm.so fails after JDK-8283326
On Thu, 2 May 2024 08:23:38 GMT, Vladimir Petko wrote: > Hi, > > The `.type _SafeFetch32_impl,@function` symbol declaration contains a > spurious underscore causing an undefined symbol in libjvm.so. > > I am not sure about change in default flags. I have removed the flag that > allows linking with undefined symbols > because having the flag on might cause bugs like this one or > https://bugs.openjdk.org/browse/JDK-8329983 as the build succeeds even if > some symbols are not defined. > Openjdk builds successfully without it on amd64, i386, armhf, arm64, s390, > ppc64el and riscv64 with this change[1]. riscv64 build was done locally > inside vm: > > Finished building target 'images' in configuration > 'linux-riscv64-server-release' > ubuntu@ubuntu:~/jdk$ > > Unfortunately I do not know why it was introduced, so I might be missing > something. > I am happy to drop it from this one or move it to a separate issue/pr. > > [1] > https://launchpad.net/~vpa1977/+archive/ubuntu/october-21/+sourcepub/16076564/+listing-archive-extra I'm not sure why we have `--allow-shlib-undefined`, but changing this should not affect libjvm building. Do you really see a difference in build success/failure depending on that line? - PR Comment: https://git.openjdk.org/jdk/pull/19048#issuecomment-2098040712
Re: undefined symbols in libjvm.so
Undefined symbols in libjvm.so is no good, and it should not be allowed. However, I am unsure about your analysis. This flag only affect linking of executables, not shared libraries. We do set "-Wl,-z,defs" when linking shared libraries, and this should complain about undefined symbols. /Magnus On 2024-05-06 21:57, Vladimir Petko wrote: Hi, I have recently encountered bugs caused by undefined symbols in libjvm.so[1][2]. The root cause of those issues is the expression in make/autoconf/flags-ldflags.m4: --- if test "x$TOOLCHAIN_TYPE" = xgcc; then EXECUTABLE_LDFLAGS="$EXECUTABLE_LDFLAGS -Wl,--allow-shlib-undefined" ... -- OpenJDK master builds fine after fixing[1] on the following architectures: amd64, i386, armhf, arm64, s390, ppc64el and riscv64. Would it be possible to consider removing -Wl,--allow-shlib-undefined from the build flags? Best Regards, Vladimir. [1] https://bugs.openjdk.org/browse/JDK-8331541 [2] https://bugs.openjdk.org/browse/JDK-8329983
Re: RFR: 8327476: Upgrade JLine to 3.26.1 [v6]
On Fri, 3 May 2024 07:27:09 GMT, Jan Lahoda wrote: >> This is a patch that: >> a) upgrades the JLine inside the JDK to 3.25.1 >> b) since the new version of JLine has a FFM backend, our custom native >> backends are removed, and replaced with the FFM backend >> >> Some changes had to be made to the original JLine in order to fit into the >> JDK. Most of them were already done for the previous version (repackaging, >> avoiding non-ASCII characters, commenting out logging, adding ability to >> modify to wrap the InputStream used by the terminal), and have only been >> transferred to the new one. The main two new changes are: >> - fixes to the FFM backend, so that it works on Linux and JDK 22. These have >> been proposed to JLine itself: https://github.com/jline/jline3/pull/945 >> - disabling the `NativeFileDescriptorCreator`, as I believe we don't need >> it, and cannot make it work easily >> >> There's a full patch between the >> `src/jdk.internal.le/share/classes/jdk/internal/org` and the merged content >> of the corresponding sources of these original JLine sub-projects: >> https://github.com/jline/jline3/tree/jline-parent-3.25.1/reader >> https://github.com/jline/jline3/tree/jline-parent-3.25.1/terminal-ffm >> https://github.com/jline/jline3/tree/jline-parent-3.25.1/terminal >> the patch is here: >> https://cr.openjdk.org/~jlahoda/8327476/jline-3.25.1-upgrade.diff >> >> I've also cleaned the patch a little removing most of the changes for the >> rename. The result is here: >> https://cr.openjdk.org/~jlahoda/8327476/jline-3.25.1-upgrade-significant.diff > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Removing trailing whitespace. Marked as reviewed by ihse (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18142#pullrequestreview-2038354269
Re: RFR: 8330539: Use #include instead of -Dalloca'(size)'=__builtin_alloca'(size)' for AIX
On Thu, 2 May 2024 09:54:14 GMT, Joachim Kern wrote: > We need to find a better way to handle alloca on AIX. > > See the discussion in the PR for https://bugs.openjdk.org/browse/JDK-8329257, > e.g. https://github.com/openjdk/jdk/pull/18536#discussion_r1568650313 in > which three alternatives are suggested. Quoting: > > Let me summarize the choices we have and ask for your vote. > Magnus dislikes the -Dalloca'(size)'=__builtin_alloca'(size)' in > flags-cflags.m4 I introduced to get rid of > > #if defined(_AIX) > #include > #endif > > in globalDefinitions_gcc.hpp. > > We have four possible solutions > > 1. Reintroduce > > #if defined(_AIX) > #include > #endif > > in globalDefinitions_gcc.hpp. > > 2. Unconditionally introduce only #include in > globalDefinitions_gcc.hpp. This should work for all platforms using this > header including the unofficial Windows/gcc Port, although only AIX needs it. > > 3. Add > > #if defined(_AIX) > #include > #endif > > to the sources using alloca(). These are > /hotspot/share/runtime/os.cpp > /hotspot/share/runtime/javaThread.cpp > /hotspot/share/utilities/vmError.cpp > Here we need the AIX condition, because otherwise the classic Windows Build > (NTAMD64) fails. > > 4. Replace -Dalloca'(size)'=__builtin_alloca'(size)' in flags-cflags.m4 by > -U__STRICT_ANSI__ at the same place. Explanation can also found in > https://github.com/openjdk/jdk/pull/18536#discussion_r1583360569 and > following. > > I will implement the solution with the most likes and having no dislike. Marked as reviewed by ihse (Reviewer). Looks good to me too. - PR Review: https://git.openjdk.org/jdk/pull/19053#pullrequestreview-2036941518 PR Comment: https://git.openjdk.org/jdk/pull/19053#issuecomment-2091668231
Re: RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler [v70]
On Wed, 3 Apr 2024 02:38:21 GMT, Julian Waters wrote: >> Bumping > >> @TheShermanTanker I tried to help you get this done. I added fixes to a copy >> of your branch on my personal fork, but then it turned out I could not push >> them to your branch. :-( >> >> It ended up with me creating a new PR, #18584. As a bonus, I think it might >> be easier to review with a fresh start. This PR has grown quite heavy with >> lots of comments and commits. >> >> I hope you don't feel like I'm stealing this away from you. You have done a >> great job, and shown a lot of patience of carrying this all the way here. >> But I also got the impression that you would appreciate my assistance in >> getting the last pieces in place so we can integrate this. > > Not at all, I don't feel like you're stealing this from me. In fact, I should > be the one apologising for giving you extra work! Thanks for taking this up, > I once again apologise for making you do this instead, I've been very busy > since Thursday (working on OpenJDK while in lectures at times), and during my > breaks I've been too drained to continue, so i really appreciate your help :) > > I will keep this open until the other Pull Request has been integrated, in > case this might still be needed @TheShermanTanker You can close this PR. The bug was fixed with https://github.com/openjdk/jdk/pull/18584 instead. - PR Comment: https://git.openjdk.org/jdk/pull/15096#issuecomment-2090226777
Re: RFR: 8331331: :tier1 target explanation in doc/testing.md is incorrect [v2]
On Wed, 1 May 2024 14:46:14 GMT, SendaoYan wrote: >> Hi, >> >> In doc/testing.md file, it says: >> >> As an example, :tier1 will expand to >> jtreg:$(TOPDIR)/test/hotspot/jtreg:tier1 jtreg:$(TOPDIR)/test/jdk:tier1 >> jtreg:$(TOPDIR)/test/langtools:tier1 jtreg:$(TOPDIR)/test/nashorn:tier1 >> jtreg:$(TOPDIR)/test/jaxp:tier1. >> >> The actual situation is :tier1 doesn't contains test/nashorn:tier1, and the >> document missed test/lib-test:tier1 >> >> $ make -n test-tier1 &> test-tier1.log >> $ grep "Running test " test-tier1.log >> Running test 'jtreg:test/hotspot/jtreg:tier1' >> Running test 'jtreg:test/jdk:tier1' >> Running test 'jtreg:test/langtools:tier1' >> Running test 'jtreg:test/jaxp:tier1' >> Running test 'jtreg:test/lib-test:tier1' >> >> Only change the document, no risk. > > SendaoYan has updated the pull request incrementally with one additional > commit since the last revision: > > deliberately making it shorter and ading ... show that this is not am > exhaustive list Thanks! - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19002#pullrequestreview-2035444214
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 30 Apr 2024 14:00:25 GMT, Martin Doerr wrote: >> For the impatient, I suggest adopting mechanism 2, i.e. unconditionally >> include in globalDefinitions_gcc.hpp. >> >> We can't include in shared code, and there is a use in shared code >> (in the relatively recently added JavaThread::pretouch_stack). >> >> When I questioned whether we needed to include at all, I referred >> to a Linux man page I'd found on the internet (the same page mdoerr linked >> to), which says (in part) >> >> "By default, modern compilers automatically translate all uses of alloca() >> into the built-in ..." >> >> Apparently I should have kept digging, because it seems that page is >> old/incorrect. A seemingly more recent Linux man page describes a different >> way of handling it that is closer to what we're seeing, but still not quite >> correct. >> >> glibc's includes if __USE_MISC is defined. >> One of the ways __USE_MISC can become defined is if _GNU_SOURCE is defined, >> and we define that for both gcc and clang toolchains. >> >> We include in globalDefinitions_gcc.hpp. So when building with >> gcc, >> globalDefinitions.hpp implicitly includes . >> >> The glibc definition of alloca is >> >> #ifdef __GNUC__ >> # define alloca(size)__builtin_alloca (size) >> #endif /* GCC. */ >> >> So that explains why we don't need any explicit include of when >> building with gcc. I expect there's something similar going on with Visual >> Studio and Xcode/clang. But apparently not with Open XLC clang. > > Ok for me. Let's hear what @kimbarrett thinks. It might be easier to get input if you create a new PR with the change. This discussion is hidden deep down in a closed PR. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1584947979
Re: RFR: 8331020: Assign LANG to C++ for Windows code that uses C++
On Tue, 30 Apr 2024 13:50:22 GMT, Julian Waters wrote: >> make/modules/java.desktop/lib/AwtLibraries.gmk line 102: >> >>> 100: $(eval $(call SetupJdkLibrary, BUILD_LIBAWT, \ >>> 101: NAME := awt, \ >>> 102: LANG := $(if $(filter $(OPENJDK_TARGET_OS), windows), C++, C), \ >> >> No, this is not okay. You need to do this as for the LIBJSOUND_LINK_TYPE >> above. The same goes for the one below, too. > > Oh, I'm surprised! I thought that you'd prefer the more lambda-like approach. > I guess the other way of LIBAWT_LINK_TYPE works too in that case There are two reasons: 1) To keep up with the style elsewhere, were we use "ifeq (... platform)" to setup platform-specific arguments. Even if this was not an ideal style, it would still make sense to keep to one way of doing it. 2) I actually think that is better. We have gravitated towards that solution over the years. The make syntax is hard to read and easy to get wrong. We try to make the arguments in the Setup calls trivial, and if we can't do that in place, then we create a "local" variable (by prefixing it with the name of the lib) outside the Setup call, were we can use more space to clearly show what is going on. In fact, I really dislike the `$(if...)` syntax, and use it only if I must. It is hopeless to see what is the if-clause and the else-clause, and it is way too easy to get a "false positive" since you do not compare the variable with another value, but checks if it evaluates to non-empty. - PR Review Comment: https://git.openjdk.org/jdk/pull/18927#discussion_r1584903238
Re: RFR: 8331020: Assign LANG to C++ for Windows code that uses C++
On Tue, 30 Apr 2024 13:53:11 GMT, Julian Waters wrote: > I switched it back to LANG since in the original change you switched it to > LINK_TYPE from LANG after one of my objections. I had since retracted that > objection and have been feeling bad about it. Have you since changed your > mind about LANG vs LINK_TYPE in that time? Yes, I have changed my mind. I think your objection back then was valid; I created an argument which implied a wider scope than it really delivered, with some vague hand-waving about future extensions. It is better to be more concrete here and now, and rename the parameter if we ever add more meaning to it. - PR Comment: https://git.openjdk.org/jdk/pull/18927#issuecomment-2085433698
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 30 Apr 2024 12:33:19 GMT, Joachim Kern wrote: >> I don't think leaving out `-std=c++14` for AIX is a good solution. > > I got it. And what about simply disabling the `__STRICT_ANSI__` with > `CFLAGS_OS_DEF_JVM="-DAIX -D_LARGE_FILES -U__STRICT_ANSI__"` in > flags-cflags.m4 for AIX. This worked too. The build is fine. So what you are saing is basically replacing CFLAGS_OS_DEF_JVM="-DAIX -Dalloca'(size)'=__builtin_alloca'(size)' -D_LARGE_FILES" ``` with CFLAGS_OS_DEF_JVM="-DAIX -D_LARGE_FILES -U__STRICT_ANSI__" ``` ? Yeah, that'll work, I guess. "strict ansi" sounds like a problematic thing to have enabled, and that it is added by `-std=c++14` sounds close to a bug in my ears. So a "workaround" where this is disabled seem reasonable. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1584754261
Re: Hermetic Java project meeting
On 2024-04-26 03:15, Jiangli Zhou wrote: On Thu, Apr 25, 2024 at 9:28 AM Magnus Ihse Bursie wrote: Just to be more clear, that's with using `objcopy` to localize non-exported symbols for all JDK static libraries and libjvm.a, not just libjvm.a right? Yes. Can you please include the compiler or linker errors on linux/clang? It is a bit tricky. The problem arises at the partial linking step. The problem seem to arise out of how clang converts a request to link into an actual call to ld. I enabled debug code (printing the command line, and running clang with `-v` to get it to print the actual command line used to run ld) and ran it on GHA, where it worked fine. This is how it looks there: WILL_RUN: /usr/bin/clang -v -m64 -r -o /home/runner/work/jdk/jdk/build/linux-x64/support/native/java.rmi/librmi/static/librmi_relocatable.o /home/runner/work/jdk/jdk/build/linux-x64/support/native/java.rmi/librmi/static/GC.o Ubuntu clang version 14.0.0-1ubuntu1.1 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/10 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/11 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/12 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/13 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/9 Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/13 Candidate multilib: .;@m64 Selected multilib: .;@m64 "/usr/bin/ld" -z relro --hash-style=gnu --build-id --eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 -o /home/runner/work/jdk/jdk/build/linux-x64/support/native/java.rmi/librmi/static/librmi_relocatable.o -L/usr/bin/../lib/gcc/x86_64-linux-gnu/13 -L/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../lib64 -L/lib/x86_64-linux-gnu -L/lib/../lib64 -L/usr/lib/x86_64-linux-gnu -L/usr/lib/../lib64 -L/usr/lib/llvm-14/bin/../lib -L/lib -L/usr/lib -r /home/runner/work/jdk/jdk/build/linux-x64/support/native/java.rmi/librmi/static/GC.o In contrast, on my machine it looks like this: WILL_RUN: /usr/local/clang+llvm-13.0.1-x86_64-linux-gnu-ubuntu-18.04/bin/clang -v -m64 -r -o /localhome/git/jdk-ALT/build/clangherm/support/native/java.rmi/librmi/static/librmi_relocatable.o /localhome/git/jdk-ALT/build/clangherm/support/native/java.rmi/librmi/static/GC.o clang version 13.0.1 Target: x86_64-unknown-linux-gnu Thread model: posix InstalledDir: /usr/local/clang+llvm-13.0.1-x86_64-linux-gnu-ubuntu-18.04/bin Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9 Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9 Candidate multilib: .;@m64 Candidate multilib: 32;@m32 Candidate multilib: x32;@mx32 Selected multilib: .;@m64 "/usr/bin/ld" --hash-style=both --eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 -o /localhome/git/jdk-ALT/build/clangherm/support/native/java.rmi/librmi/static/librmi_relocatable.o /lib/x86_64-linux-gnu/crt1.o /lib/x86_64-linux-gnu/crti.o /usr/lib/gcc/x86_64-linux-gnu/9/crtbegin.o -L/usr/lib/gcc/x86_64-linux-gnu/9 -L/usr/lib/gcc/x86_64-linux-gnu/9/../../../../lib64 -L/lib/x86_64-linux-gnu -L/lib/../lib64 -L/usr/lib/x86_64-linux-gnu -L/usr/lib/../lib64 -L/usr/local/clang+llvm-13.0.1-x86_64-linux-gnu-ubuntu-18.04/bin/../lib -L/lib -L/usr/lib -r /localhome/git/jdk-ALT/build/clangherm/support/native/java.rmi/librmi/static/GC.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/gcc/x86_64-linux-gnu/9/crtend.o /lib/x86_64-linux-gnu/crtn.o /usr/bin/ld: cannot find -lgcc_s /usr/bin/ld: cannot find -lgcc_s clang-13: error: linker command failed with exit code 1 (use -v to see invocation) I don't understand what makes clang think it should include "-lgcc --as-needed -lgcc_s" and the crt*.o files when doing a partial link. In fact, the entire process on how clang (and gcc) builds up the linker command line is bordering on black magic to me. I think it can be affected by variables set at compile time (at least this was the case for gcc, last I checked), or maybe it picks up some kind of script from the environment. That's why I believe my machine could just be messed up. I could get a bit further by passing "-nodefaultlibs" (or whatever it was), but then the generated .o file were messed up wrt to library symbols and it failed dramatically when trying to do the final link of the static java launcher. Looks like you are using /usr/bin/ld and not lld. I haven't run into this type of issue. Have you tried -fuse-ld=lld? I am not sure why clang insisted on picking up ld and not lld. I remeber trying with -fuse-ld=lld, and that it did not work either. Unfortunately, I don't remember exactly what the problems were. I started reinstalling my Linux workstation yesterday, but something went wrong, and it failed so hard that it got s
Re: RFR: 8331020: Assign LANG to C++ for Windows code that uses C++
On Wed, 24 Apr 2024 01:02:36 GMT, Julian Waters wrote: > Currently, on Windows LANG is not assigned to C++ for some code that does use > C++. This just works because link.exe does not bother about what kind of code > it is currently linking. gcc however, does. It doesn't hurt to assign LANG to > C++ as a formality in such cases, which this changeset does. This also > renames LINK_TYPE to LANG, which the original change to remove the TOOLCHAIN > parameter used to do Please revert back to the LINK_TYPE name. As long as it is not used for anything else, this is a better description. If we start to use it to have a broader meaning, we can rename it then. make/hotspot/gensrc/GensrcAdlc.gmk line 45: > 43: endif > 44: else ifeq ($(call isBuildOs, windows), true) > 45: ifeq ($(TOOLCHAIN_TYPE), microsoft) You're kind of sneaking in some of your "support other toolchain than ms on windows" changes here. While it does not matter that much, right now we have the assumption that platform=windows <=> toolchain=microsoft in the entire code base. With that assumption, this change looks strange. So I'd rather not take this in now, but instead do a complete integration of the changes needed to support multiple toolchains on Windows. make/hotspot/lib/CompileGtest.gmk line 98: > 96: -I$(GTEST_FRAMEWORK_SRC)/googlemock/include \ > 97: $(addprefix -I,$(GTEST_TEST_SRC)), \ > 98: CFLAGS_windows := -EHsc, \ Just to clarify: these kind of changes are okay, since for mainline it is equivalent if you do `CFLAGS_windows` or `CFLAGS_microsoft`, so if it helps you I am completely okay with converting one kind of check to another. (It was the double-checking that I objected to.) make/modules/java.desktop/lib/AwtLibraries.gmk line 102: > 100: $(eval $(call SetupJdkLibrary, BUILD_LIBAWT, \ > 101: NAME := awt, \ > 102: LANG := $(if $(filter $(OPENJDK_TARGET_OS), windows), C++, C), \ No, this is not okay. You need to do this as for the LIBJSOUND_LINK_TYPE above. The same goes for the one below, too. - PR Comment: https://git.openjdk.org/jdk/pull/18927#issuecomment-2085199170 PR Review Comment: https://git.openjdk.org/jdk/pull/18927#discussion_r1584720911 PR Review Comment: https://git.openjdk.org/jdk/pull/18927#discussion_r1584722581 PR Review Comment: https://git.openjdk.org/jdk/pull/18927#discussion_r1584723694
Re: RFR: 8331331: :tier1 target explanation in doc/testing.md is incorrect
On Mon, 29 Apr 2024 16:14:45 GMT, SendaoYan wrote: > Hi, > > In doc/testing.md file, it says: > > As an example, :tier1 will expand to jtreg:$(TOPDIR)/test/hotspot/jtreg:tier1 > jtreg:$(TOPDIR)/test/jdk:tier1 jtreg:$(TOPDIR)/test/langtools:tier1 > jtreg:$(TOPDIR)/test/nashorn:tier1 jtreg:$(TOPDIR)/test/jaxp:tier1. > > The actual situation is :tier1 doesn't contains test/nashorn:tier1, and the > document missed test/lib-test:tier1 > > $ make -n test-tier1 &> test-tier1.log > $ grep "Running test " test-tier1.log > Running test 'jtreg:test/hotspot/jtreg:tier1' > Running test 'jtreg:test/jdk:tier1' > Running test 'jtreg:test/langtools:tier1' > Running test 'jtreg:test/jaxp:tier1' > Running test 'jtreg:test/lib-test:tier1' > > Only change the document, no risk. This fix works, but I like David's idea better. It was meant as an example, not necessarily kept up to date, so by deliberately making it shorter and ading `...` we show that this is not am exhaustive list. - PR Comment: https://git.openjdk.org/jdk/pull/19002#issuecomment-2085193115
Re: Usage of iconv()
On 2024-04-25 20:30, Philip Race wrote: On 4/24/24 4:24 AM, Magnus Ihse Bursie wrote: That is a good question. libiconv is used only on macOS and AIX, for a few libraries, as you say. I just tried removing -liconv from the macOS dependencies and recompiled, just to see what would happen. There were three instances for macOS: libsplashscreen, libjdwp and libinstrument. libsplashscreen uses it in splashscreen_sys.m, where it is used to convert the jar file name. This is called from the launcher, in java.base/share/native/libjli/java.c libjdwp uses it in utf_util.c, where it is used to convert file name and command lines, iiuc. It is likely that we have similar (but better) ways to convert charsets elsewhere in our libraries that can be used instead of libiconv. I guess these are not the only two places where a filename must be converted from the platform charset to UTF8. So whatever replacement there might be, it needs to be something that is available very early in the life of the VM, in fact before there is a VM running. Agreed. But it seems to be that this is something that needs to be handled by libjli, to properly deal with paths and command lines. I'm wondering if the places which to *not* use iconv (or similar) is actually incorrect. /Magnus -phil.
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 30 Apr 2024 09:36:52 GMT, Joachim Kern wrote: >> On AIX `stdlib.h` also would define `alloca`, if `__STRICT_ANSI__` wouldn't >> be set. >> >> >> 780 #if !defined(__xlC__) || defined(__ibmxl__) || defined(__cplusplus) >> 781 #if defined(__IBMCPP__) && !defined(__ibmxl__) >> 782extern "builtin" char *__alloca (size_t); >> 783 # define alloca __alloca >> 784 #elif defined(__GNUC__) && !defined(__STRICT_ANSI__) >> 785#undef alloca >> 786#define alloca(size) __builtin_alloca (size) >> 787 #endif >> >> >> A small plain Testprogramm not using all of the flags we used in jdk build, >> does not set `__STRICT_ANSI__` and then `alloca` is defined correct. > > The compiler flag introducing __STRICT_ANSI__ is -std=c++14. If I omit this > explicit compiler flag the default is used, which is also c++14. But the > default does not set __STRICT_ANSI__ but 2 other defines. I will try a build > without -std=c++14 and if this works, we have a solution. Nevertheless i will > interrogate IBM what the hell this behavior should be. I don't think leaving out `-std=c++14` for AIX is a good solution. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1584529538
Re: Hermetic Java project meeting
Just to be more clear, that's with using `objcopy` to localize non-exported symbols for all JDK static libraries and libjvm.a, not just libjvm.a right? Yes. Can you please include the compiler or linker errors on linux/clang? It is a bit tricky. The problem arises at the partial linking step. The problem seem to arise out of how clang converts a request to link into an actual call to ld. I enabled debug code (printing the command line, and running clang with `-v` to get it to print the actual command line used to run ld) and ran it on GHA, where it worked fine. This is how it looks there: WILL_RUN: /usr/bin/clang -v -m64 -r -o /home/runner/work/jdk/jdk/build/linux-x64/support/native/java.rmi/librmi/static/librmi_relocatable.o /home/runner/work/jdk/jdk/build/linux-x64/support/native/java.rmi/librmi/static/GC.o Ubuntu clang version 14.0.0-1ubuntu1.1 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/10 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/11 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/12 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/13 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/9 Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/13 Candidate multilib: .;@m64 Selected multilib: .;@m64 "/usr/bin/ld" -z relro --hash-style=gnu --build-id --eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 -o /home/runner/work/jdk/jdk/build/linux-x64/support/native/java.rmi/librmi/static/librmi_relocatable.o -L/usr/bin/../lib/gcc/x86_64-linux-gnu/13 -L/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../lib64 -L/lib/x86_64-linux-gnu -L/lib/../lib64 -L/usr/lib/x86_64-linux-gnu -L/usr/lib/../lib64 -L/usr/lib/llvm-14/bin/../lib -L/lib -L/usr/lib -r /home/runner/work/jdk/jdk/build/linux-x64/support/native/java.rmi/librmi/static/GC.o In contrast, on my machine it looks like this: WILL_RUN: /usr/local/clang+llvm-13.0.1-x86_64-linux-gnu-ubuntu-18.04/bin/clang -v -m64 -r -o /localhome/git/jdk-ALT/build/clangherm/support/native/java.rmi/librmi/static/librmi_relocatable.o /localhome/git/jdk-ALT/build/clangherm/support/native/java.rmi/librmi/static/GC.o clang version 13.0.1 Target: x86_64-unknown-linux-gnu Thread model: posix InstalledDir: /usr/local/clang+llvm-13.0.1-x86_64-linux-gnu-ubuntu-18.04/bin Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9 Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9 Candidate multilib: .;@m64 Candidate multilib: 32;@m32 Candidate multilib: x32;@mx32 Selected multilib: .;@m64 "/usr/bin/ld" --hash-style=both --eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 -o /localhome/git/jdk-ALT/build/clangherm/support/native/java.rmi/librmi/static/librmi_relocatable.o /lib/x86_64-linux-gnu/crt1.o /lib/x86_64-linux-gnu/crti.o /usr/lib/gcc/x86_64-linux-gnu/9/crtbegin.o -L/usr/lib/gcc/x86_64-linux-gnu/9 -L/usr/lib/gcc/x86_64-linux-gnu/9/../../../../lib64 -L/lib/x86_64-linux-gnu -L/lib/../lib64 -L/usr/lib/x86_64-linux-gnu -L/usr/lib/../lib64 -L/usr/local/clang+llvm-13.0.1-x86_64-linux-gnu-ubuntu-18.04/bin/../lib -L/lib -L/usr/lib -r /localhome/git/jdk-ALT/build/clangherm/support/native/java.rmi/librmi/static/GC.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/gcc/x86_64-linux-gnu/9/crtend.o /lib/x86_64-linux-gnu/crtn.o /usr/bin/ld: cannot find -lgcc_s /usr/bin/ld: cannot find -lgcc_s clang-13: error: linker command failed with exit code 1 (use -v to see invocation) I don't understand what makes clang think it should include "-lgcc --as-needed -lgcc_s" and the crt*.o files when doing a partial link. In fact, the entire process on how clang (and gcc) builds up the linker command line is bordering on black magic to me. I think it can be affected by variables set at compile time (at least this was the case for gcc, last I checked), or maybe it picks up some kind of script from the environment. That's why I believe my machine could just be messed up. I could get a bit further by passing "-nodefaultlibs" (or whatever it was), but then the generated .o file were messed up wrt to library symbols and it failed dramatically when trying to do the final link of the static java launcher. I have also tried to extract all the changes (and only the changes) related to static build from the hermetic-java-runtime branch (ignoring the JavaHome/resource loading changes), to see if I could get something like StaticLink.gmk in mainline. I thought I was doing quite fine, but after a while I realized my testing was botched since the launcher had actually loaded the libraries dynamically instead, even though they were statically linked. :-( I am currently trying to bisect my way thought my repo to
Re: Usage of iconv()
That is a good question. libiconv is used only on macOS and AIX, for a few libraries, as you say. I just tried removing -liconv from the macOS dependencies and recompiled, just to see what would happen. There were three instances for macOS: libsplashscreen, libjdwp and libinstrument. Out of these, libinstrument compiled and linked fine without the -liconv argument. It looks like iconv is referenced in unix/.../EncodingSupport_md.c, but otoh it looks like it is as much (or as little) referenced on macOS as on linux (where we never have linked with -liconv) so it is perhaps just dead code. I did not study it in detail. The code looks very much duplicated from libjdwp. The other two actually failed linking, so they do use libiconv. libsplashscreen uses it in splashscreen_sys.m, where it is used to convert the jar file name. libjdwp uses it in utf_util.c, where it is used to convert file name and command lines, iiuc. It is likely that we have similar (but better) ways to convert charsets elsewhere in our libraries that can be used instead of libiconv. I guess these are not the only two places where a filename must be converted from the platform charset to UTF8. /Magnus On 2024-04-23 14:11, Bernd Eckenfels wrote: Du to a glibc security alert about a charset in iconv() I checked OpenJDK (since I was quite sure encoding is handled in JCL), however there are a few utilities (related to libinstrument and splash Screens) which use iconv. If I see it correctly it’s mostly used for utf8 so it should not expose this particular globe weakness, but I still wonder if that dependency is needed. For some platforms like AIX it even drags on an additional library dependency. (Not to mention different charger tables and especially ugly locale dependencies for containers). Gruß Bernd — https://bernd.eckenfels.net
Re: RFR: 8314488: Compile the JDK as C++17 [v6]
On Fri, 19 Jan 2024 12:08:21 GMT, Julian Waters wrote: >> Julian Waters has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Require clang 13 in toolchain.m4 > > Should I split the compiler upgrades into a different change and integrate > that first? Going off the conversation in this thread it would seem like the > compiler upgrade would benefit us a lot more than just having C++17 (The > noreturn attribute is one big motivating factor for instance) and it might > help if the compiler upgrades were not delayed by the discussion of when to > jump to C++17 @TheShermanTanker I suggest you close this PR. If we are going to switch to C++17, it should start by a discussion in the mailing list, not with a PR (the change itself is trivial). - PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-2074487573
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Tue, 23 Apr 2024 13:56:32 GMT, Julian Waters wrote: > WIP > > This changeset contains hsdis for Windows/gcc Port. It supports both the > binutils and capstone backends, though the LLVM backend is left out due to > compatibility issues encountered during the build. Currently, which gcc > distributions are supported is still to be clarified, as several, ranging > from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, > the build system hack in place at the moment to compile the binutils backend > on Windows is still left in place, for now. Please mark the PR as draft it is not intended for review. - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2074481887
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Tue, 23 Apr 2024 13:56:32 GMT, Julian Waters wrote: > WIP > > This changeset contains hsdis for Windows/gcc Port. It supports both the > binutils and capstone backends, though the LLVM backend is left out due to > compatibility issues encountered during the build. Currently, which gcc > distributions are supported is still to be clarified, as several, ranging > from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, > the build system hack in place at the moment to compile the binutils backend > on Windows is still left in place, for now. There's a huge amount of changes for just hsdis... You might have to separate out the infrastructure changes that seem to amount to most of the changes here. This is going to take me a while to get through. - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2072458273
Re: Hermetic Java project meeting
I will not be able to participate in the meeting today. Let me report a bit on my work this week. I have made a proof of concept branch which can properly compile static java on macos with clang and linux with gcc. (With "properly" I mean hiding non-exported symbols). I still have problems replicating the build with clang on linux. I suspect my linux workstation has a broken installation of clang. That machine has grown more erratic over time, and I need to reinstall it, but I'm procrastinating spending the time doing that... So for now I'm just skipping the clang on linux part. I have also made great progress on windows. Julian's hint about objcopy working fine on COFF, and being present in cygwin, got me to realize that this was in effect a possible way forward. My PoC currently manages to extract a list of exported symbols from the static libs using dumpbin. This is necessary since --localize-hidden does not work on COFF (the concept of "hidden" symbols is an ELF-only concept), and instead we need to use the --keep-global-symbols option, which (despite the name) converts all symbols in the given list to global and all other to local. I am currently working actively with getting the next steps done in this PoC. I have initiated talks with SAP, and they are interested in helping out getting static linking working on AIX (given that it is possible to do with not too much effort.) I have also tried to extract all the changes (and only the changes) related to static build from the hermetic-java-runtime branch (ignoring the JavaHome/resource loading changes), to see if I could get something like StaticLink.gmk in mainline. I thought I was doing quite fine, but after a while I realized my testing was botched since the launcher had actually loaded the libraries dynamically instead, even though they were statically linked. :-( I am currently trying to bisect my way thought my repo to understand where things went wrong. This problem was exaggerated by the fact that we cannot build *only* static libs, so the dynamic ones where there alongside to be (improperly) picked up. I might want to spend some time on fixing this first. That will help both with speeding up the build/test cycle for static builds, and help avoid that kind of issue repeating. That will require some more refactoring in the core build/link code though. Experimenting with the static launcher on linux/gcc and macos made me realize that we will need to know the set of external libraries needed by each individual JDK library. When building a dynamic library, this knowledge (e.g. -liconv -framework Application) is encoded into the lib*.so file by the linker. But not so for a static library. Instead, we need to store this information for each JDK library, and then in the end, when we want to pick up all static libraries and link them together to form the "javastatic" executable, we need to pass this set of external libraries to the linker. This was done haphazardly in StaticLink.gmk in the hermetic-java-runtime branch, where an arbitrary subset of external libraries were hard-coded. Before integration in mainline can be possible, this information needs to be collected correctly and automatically for all included JDK libraries. Fortunately, it is not likely to be too hard. I basically just need to store the information from the LIBS provided to the NativeCompilation, and pick that up for all static libraries we include in the static launcher. (A complication is that we need to de-duplicate the list, and that some libraries are specified using two words, like "-framework Application" on macos, so it will take some care getting it right.) I have also been thinking about possible ways that we can share compiled obj files between static and dynamic libraries, even if we cannot do it fully. Most files do not need the STATIC_BUILD define and will thus be identical for both static and dynamic builds. It might be possible to just hard-code the exact files that needs to be different. It's ugly, and I still would like to make sure we press forward with the spec changes to JNI/JVMTI, but it would work as a stop-gap measure. /Magnus
Integrated: 8330820: Remove remnants of operator_new.cpp in build system
On Mon, 22 Apr 2024 13:11:02 GMT, Magnus Ihse Bursie wrote: > In [JDK-8282469](https://bugs.openjdk.org/browse/JDK-8282469), the file > operator_new.cpp was removed from Hotspot. Unfortunately, some ugly hacks > related to this file remained in the build system. They should be removed. This pull request has now been integrated. Changeset: 3e65d90b Author: Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/3e65d90b4ddb52878ebdc2150790c0333b9c0920 Stats: 10 lines in 2 files changed: 0 ins; 9 del; 1 mod 8330820: Remove remnants of operator_new.cpp in build system Reviewed-by: tbell - PR: https://git.openjdk.org/jdk/pull/18886
RFR: 8330820: Remove remnants of operator_new.cpp in build system
In [JDK-8282469](https://bugs.openjdk.org/browse/JDK-8282469), the file operator_new.cpp was removed from Hotspot. Unfortunately, some ugly hacks related to this file remained in the build system. They should be removed. - Commit messages: - 8330820: Remove remnants of operator_new.cpp in build system Changes: https://git.openjdk.org/jdk/pull/18886/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18886=00 Issue: https://bugs.openjdk.org/browse/JDK-8330820 Stats: 10 lines in 2 files changed: 0 ins; 9 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18886.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18886/head:pull/18886 PR: https://git.openjdk.org/jdk/pull/18886
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 17 Apr 2024 15:09:50 GMT, Kim Barrett wrote:
>> https://man7.org/linux/man-pages/man3/alloca.3.html sounds like solution 2
>> is the cleanest one ("standards conformance"). It is also the version with
>> minimal code and which will even work with future alloca usages :-)
>> If solution 2 has any disadvantage, I'd prefer solution 3.
>
> I'm aware of this discussion and looking into the issues, but a personal
> matter has intervened and it will take
> me a while to respond properly. Maybe next week.
I opened https://bugs.openjdk.org/browse/JDK-8330539 so we don't lose track of
this, but we can keep the discussion/voting here.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1569528717
Integrated: 8330107: Separate out "awt" libraries from Awt2dLibraries.gmk
On Thu, 11 Apr 2024 13:53:23 GMT, Magnus Ihse Bursie wrote: > The file to build most of the java.desktop native libraries, > Awt2dLibraries.gmk, is large and unstructured, making it hard to navigate. > > I want to split it into two parts, one for the AWT libraries, and one for the > 2D libraries. I also used this opportunity to change the order to be more > logical (e.g. grouping "image" libraries and "font" libraries in 2D). This pull request has now been integrated. Changeset: 5841cb3b Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/5841cb3b51e45e7c3aaa086e179815fa8184f22d Stats: 1724 lines in 4 files changed: 880 ins; 843 del; 1 mod 8330107: Separate out "awt" libraries from Awt2dLibraries.gmk Reviewed-by: erikj - PR: https://git.openjdk.org/jdk/pull/18743
Re: RFR: 8330107: Separate out "awt" libraries from Awt2dLibraries.gmk [v3]
On Tue, 16 Apr 2024 10:03:27 GMT, Magnus Ihse Bursie wrote: >> The file to build most of the java.desktop native libraries, >> Awt2dLibraries.gmk, is large and unstructured, making it hard to navigate. >> >> I want to split it into two parts, one for the AWT libraries, and one for >> the 2D libraries. I also used this opportunity to change the order to be >> more logical (e.g. grouping "image" libraries and "font" libraries in 2D). > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Make the split based on the name "awt" instead, and document the reason > - Rename 2dLibraries.gmk to ClientLibraries.gmk Yes, I really want to do this. I am sorry that we could not reach an agreement, but I need to be able to work with these files in a better way than is currently possible. Thanks Erik and Phil. - PR Comment: https://git.openjdk.org/jdk/pull/18743#issuecomment-2061165564
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 17 Apr 2024 11:52:42 GMT, Julian Waters wrote: >> @magicus @TheShermanTanker @TheRealMDoerr @kimbarrett >> Let me summarize the choices we have and ask for your vote. >> Julian dislikes the `-Dalloca'(size)'=__builtin_alloca'(size)'` in >> `flags-cflags.m4` I introduced to get rid of >> >> #if defined(_AIX) >> #include >> #endif >> >> in `globalDefinitions_gcc.hpp`. >> >> We have three possible solutions >> >> 1. Reintroduce >> >> #if defined(_AIX) >> #include >> #endif >> >> in `globalDefinitions_gcc.hpp`. >> >> 2. Unconditionally introduce only `#include ` in >> `globalDefinitions_gcc.hpp`. This should work for all platforms using this >> header including the unofficial Windows/gcc Port, although only AIX needs >> it. >> >> 3. Add >> >> #if defined(_AIX) >> #include >> #endif >> >> to the sources using alloca(). These are >> /hotspot/share/runtime/os.cpp >> /hotspot/share/runtime/javaThread.cpp >> /hotspot/share/utilities/vmError.cpp >> Here we need the AIX condition, because otherwise the classic Windows Build >> (NTAMD64) fails. >> >> I will implement the solution with the most likes and having no dislike. > > I don't mind all 3, though I certainly prefer 1 and 3 over 2 (The way I see > it, the AIX macro check is more of a message to the programmer than it is > important to the compiler, so I prefer the options that have it. However, I > also don't mind if we were to go the way of option 2, this is more of a > preference thing). The fact that only 3 files need it is also surprising to > me, and makes option 3 seem like a good fit (Again, personal preference) > > Magnus and Kim, what do you guys think? If there are just 3 files using alloca, I strongly prefer solution 3. I think solution 1 has already been rejected by Kim. (Also, for the record, it was me, not Julian, who expressed dislike about the `-Dalloca'(size)'=__builtin_alloca'(size)'` change) - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1568754458
Re: RFR: 8330107: Separate out "awt" libraries from Awt2dLibraries.gmk [v3]
On Tue, 16 Apr 2024 10:03:27 GMT, Magnus Ihse Bursie wrote: >> The file to build most of the java.desktop native libraries, >> Awt2dLibraries.gmk, is large and unstructured, making it hard to navigate. >> >> I want to split it into two parts, one for the AWT libraries, and one for >> the 2D libraries. I also used this opportunity to change the order to be >> more logical (e.g. grouping "image" libraries and "font" libraries in 2D). > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Make the split based on the name "awt" instead, and document the reason > - Rename 2dLibraries.gmk to ClientLibraries.gmk And just to put some perspective here: The .gmk files for all other modules are at an average of 2 kB. The Awt2dLibraries.gmk file is 32 kB. Even with the split the two remaining parts will be 16 kB, which is still larger than the next biggest file (java.base/gensrc/GensrcBuffer.gmk) at 14 kB. So this is really an outlier. - PR Comment: https://git.openjdk.org/jdk/pull/18743#issuecomment-2059720604
Re: RFR: 8330107: Separate out "awt" libraries from Awt2dLibraries.gmk [v3]
On Tue, 16 Apr 2024 10:03:27 GMT, Magnus Ihse Bursie wrote: >> The file to build most of the java.desktop native libraries, >> Awt2dLibraries.gmk, is large and unstructured, making it hard to navigate. >> >> I want to split it into two parts, one for the AWT libraries, and one for >> the 2D libraries. I also used this opportunity to change the order to be >> more logical (e.g. grouping "image" libraries and "font" libraries in 2D). > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Make the split based on the name "awt" instead, and document the reason > - Rename 2dLibraries.gmk to ClientLibraries.gmk **I** am the one working with this file all the time, not you. There is no good IDE support for Makefiles, especially not with the level of customization that we have done to make. I am positively struggling with navigating this file, time and time again. Can that not be enough reason for you to accept that I want to split this file into more manageable chunks? Nothing here will affect how the actual client libraries are built. It is *purely* about making this part of the JDK build less of a complete PITA for me. Why are you opposing this? I tried two different approaches, and suggested a third (alphabetic) and you just say "no". I don't find this very constructive or helpful. This is not the first time we split a makefile into "arbitrary" chunks. Since make is not a proper programming language, there is no way to express abstraction, objects or other clear delimiters. The most recently file that was split this way was JdkNativeCompilation.gmk. One of the first was configure.ac. All those split were "arbitrary" and "artificial", but they are still good enough to provide help to the poor sods that need to work with those files. - PR Comment: https://git.openjdk.org/jdk/pull/18743#issuecomment-2059708051
