Re: RFR: 8327389: Remove use of HOTSPOT_BUILD_USER
On Wed, 6 Mar 2024 11:57:27 GMT, Andrew John Hughes wrote: > The HotSpot code has inserted the value of `HOTSPOT_BUILD_USER` into the > internal version string since before the JDK was open-sourced, so the > reasoning for this is not in the public history. See > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/abstract_vm_version.cpp#L284 > > ~~~ > $ /usr/lib/jvm/java-21-openjdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (21.0.2+13) for linux-amd64 JRE (21.0.2+13), built > on 2024-01-19T16:39:52Z by "mockbuild" with gcc 8.5.0 20210514 (Red Hat > 8.5.0-20) > ~~~ > > It is not clear what value this provides. By default, the value is set to the > system username, but it can be set to a static value using > `--with-build-user`. > > In trying to compare builds, this is a source of difference if the builds are > produced under different usernames and `--with-build-user` is not set to the > same value. > > It may also be a source of information leakage from the host system. It is > not clear what value it provides to the end user of the build. > > This patch proposes removing it altogether, but at least knowing why it needs > to be there would be progress from where we are now. We could also consider a > middle ground of only setting it for "adhoc" builds, those without set > version information, as is the case with the overall version output. > > With this patch: > > ~~~ > $ ~/builder/23/images/jdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) for > linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on 2024-03-06T00:23:37Z > with gcc 13.2.1 20230826 > > $ ~/builder/23/images/jdk/bin/java -XX:ErrorHandlerTest=1 > > $ grep '^vm_info' > /localhome/andrew/projects/openjdk/upstream/jdk/hs_err_pid1119824.log > vm_info: OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) > for linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on > 2024-03-06T00:23:37Z with gcc 13.2.1 20230826 > ~~~ > > The above are from a fastdebug build but I also built a product build with no > issues. The internal version is reported in crash reports (hs_err file) as well as via `-Xinternalversion`. My recollection is that the username was needed/used to identify whether a hs_err report came from an official build, back when official builds were created via a very specific build process and a specific user name. These days it seems redundant given the way the rest of the version string is constructed both for "official" builds and personal builds. I can't see any tests that look at the output from `-Xinternalversion`. - PR Comment: https://git.openjdk.org/jdk/pull/18136#issuecomment-1982679336
Integrated: 8327173: HotSpot Style Guide needs update regarding nullptr vs NULL
On Mon, 4 Mar 2024 08:38:09 GMT, Kim Barrett wrote: > Please review this change to update the HotSpot Style Guide's discussion of > nullptr and its use. > > I suggest this is an editorial rather than substantive change to the style > guide. As such, the normal HotSpot PR process can be used for this change. This pull request has now been integrated. Changeset: 3d37b286 Author:Kim Barrett URL: https://git.openjdk.org/jdk/commit/3d37b28642fd2715ab5c365426409c61939a1436 Stats: 14 lines in 2 files changed: 5 ins; 0 del; 9 mod 8327173: HotSpot Style Guide needs update regarding nullptr vs NULL Reviewed-by: shade, dholmes, kvn - PR: https://git.openjdk.org/jdk/pull/18101
Re: RFR: 8327173: HotSpot Style Guide needs update regarding nullptr vs NULL [v3]
On Tue, 5 Mar 2024 18:16:51 GMT, Vladimir Kozlov wrote: >> Kim Barrett 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: >> >> - Merge branch 'master' into nullptr-style >> - respond to shipilev comments >> - update nullptr usage > > Approved. Thanks for reviews @vnkozlov , @kimbarrett , and @dholmes-ora . - PR Comment: https://git.openjdk.org/jdk/pull/18101#issuecomment-1981979030
Re: RFR: 8327173: HotSpot Style Guide needs update regarding nullptr vs NULL [v3]
> Please review this change to update the HotSpot Style Guide's discussion of > nullptr and its use. > > I suggest this is an editorial rather than substantive change to the style > guide. As such, the normal HotSpot PR process can be used for this change. Kim Barrett 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: - Merge branch 'master' into nullptr-style - respond to shipilev comments - update nullptr usage - Changes: - all: https://git.openjdk.org/jdk/pull/18101/files - new: https://git.openjdk.org/jdk/pull/18101/files/13dc01ac..461bf280 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18101&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18101&range=01-02 Stats: 72989 lines in 714 files changed: 2663 ins; 68821 del; 1505 mod Patch: https://git.openjdk.org/jdk/pull/18101.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18101/head:pull/18101 PR: https://git.openjdk.org/jdk/pull/18101
Re: RFR: JDK-8298405: Implement JEP 467: Markdown Documentation Comments [v46]
> Please review a patch to add support for Markdown syntax in documentation > comments, as described in the associated JEP. > > Notable features: > > * support for `///` documentation comments in `JavaTokenizer` > * new module `jdk.internal.md` -- a private copy of the `commonmark-java` > library > * updates to `DocCommentParser` to treat `///` comments as Markdown > * updates to the standard doclet to render Markdown comments in HTML Jonathan Gibbons has updated the pull request incrementally with two additional commits since the last revision: - fix `textOf` method to accommodate Markdown content. - avoid relying on unspecified behavior `List.toString` - Changes: - all: https://git.openjdk.org/jdk/pull/16388/files - new: https://git.openjdk.org/jdk/pull/16388/files/292ff0f1..fc76e8c7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16388&range=45 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16388&range=44-45 Stats: 43 lines in 3 files changed: 34 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/16388.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16388/head:pull/16388 PR: https://git.openjdk.org/jdk/pull/16388
RFR: 8327476: Upgrade JLine to 3.25.1
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 - Depends on: https://git.openjdk.org/jdk/pull/18106 Commit messages: - Merge branch 'native-access-modules1' into jline-upgrade-3.25.1 - Merge branch 'native-access-modules1' into jline-upgrade-3.25.1 - Re-enabling the exec provider. - Cleanup. - Fixing test. - Fixing Crl-C/VINTR handling on Linux. - Cleanup. - Enabling native access for jdk.internal.le. - Merge branch 'native-access-modules1' into jline-upgrade-3.25.1 - Ctrl-C should not kill JShell. - ... and 8 more: https://git.openjdk.org/jdk/compare/e30e4529...d6c2072f Changes: https://git.openjdk.org/jdk/pull/18142/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18142&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8327476 Stats: 12893 lines in 142 files changed: 5623 ins; 5298 del; 1972 mod Patch: https://git.openjdk.org/jdk/pull/18142.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18142/head:pull/18142 PR: https://git.openjdk.org/jdk/pull/18142
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v2]
On Tue, 5 Mar 2024 18:54:55 GMT, Alan Bateman wrote:
>> Jan Lahoda has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Apply suggestions from code review
>>
>> Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
>> Co-authored-by: Maurizio Cimadamore
>> <54672762+mcimadam...@users.noreply.github.com>
>
> src/java.base/share/classes/java/lang/ModuleLayer.java line 896:
>
>> 894: return nameToModule.get(name);
>> 895: }
>> 896:
>
> What would you think about replacing this with addEnableNativeAccess(String
> name) so it can be called by JLA. addEnableNativeAccess. The reason is that
> the JLA methods are usually just calls to some non-public method but the
> changes mean mean there is "core" in the JLA method that is not easy to find.
I've tried to that here:
https://github.com/openjdk/jdk/pull/18106/commits/e17cd3722724cbc6aa298f7b789c6574554af6ea
> src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 812:
>
>> 810: }
>> 811:
>> 812: private static void addEnableNativeAccess(ModuleLayer layer,
>> Set moduleNames, boolean shouldWarn) {
>
> The private methods in this class have a short comment to summarise what they
> do.
I've tried to add a comment here:
https://github.com/openjdk/jdk/pull/18106/commits/e17cd3722724cbc6aa298f7b789c6574554af6ea
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1515171359
PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1515171099
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v3]
> Currently, JDK modules load by the bootstrap and platform ClassLoaders are > automatically granted the native access. I am working on an upgrade of JLine > inside the `jdk.internal.le` module, and I would like to replace the current > native bindings with FFM-based bindings (which are now somewhat provided by > JLine). But, for that, native access is needed for the `jdk.internal.le` > module. We could possibly move the module to the platform ClassLoader, but it > seems it might be better to have more control over which modules have the > native access. > > This patch introduces an explicit list of modules that will automatically be > granted the native access. Note this patch is not yet intended to change the > end behavior - the list of modules granted native access is supposed to be > the same as modules in the boot and platform ClassLoaders. Jan Lahoda has updated the pull request incrementally with three additional commits since the last revision: - Merge remote-tracking branch 'origin/native-access-modules1' into native-access-modules1 - Reflecting review feedback. - Updating copyright headers. - Changes: - all: https://git.openjdk.org/jdk/pull/18106/files - new: https://git.openjdk.org/jdk/pull/18106/files/6127044d..e30e4529 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18106&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18106&range=01-02 Stats: 36 lines in 9 files changed: 11 ins; 9 del; 16 mod Patch: https://git.openjdk.org/jdk/pull/18106.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18106/head:pull/18106 PR: https://git.openjdk.org/jdk/pull/18106
Re: RFR: 8325878: Require minimum Clang version 13
On Thu, 15 Feb 2024 05:19:45 GMT, Kim Barrett wrote: > Please review this change that updates the minimum supported version of Clang > to be used for building OpenJDK from 3.5 to 13. > > This permits enabling C++17 (JDK-8314488), though Clang 5 might suffice for > that. A minimum of Clang 13 also obtains a critical bug fix for the > [[noreturn]] > attribute (see JDK-8303805). > > Testing: mach5 tier1, which includes building with a recent version of > Xcode/clang. Update to minimum XLC has been integrated, so going ahead with this. - PR Comment: https://git.openjdk.org/jdk/pull/17862#issuecomment-1981706282
Integrated: 8325878: Require minimum Clang version 13
On Thu, 15 Feb 2024 05:19:45 GMT, Kim Barrett wrote: > Please review this change that updates the minimum supported version of Clang > to be used for building OpenJDK from 3.5 to 13. > > This permits enabling C++17 (JDK-8314488), though Clang 5 might suffice for > that. A minimum of Clang 13 also obtains a critical bug fix for the > [[noreturn]] > attribute (see JDK-8303805). > > Testing: mach5 tier1, which includes building with a recent version of > Xcode/clang. This pull request has now been integrated. Changeset: 989fc3e6 Author:Kim Barrett URL: https://git.openjdk.org/jdk/commit/989fc3e6ea65e530055296ac4bc181cb5c6a41ea Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod 8325878: Require minimum Clang version 13 Reviewed-by: jwaters, ihse - PR: https://git.openjdk.org/jdk/pull/17862
Re: RFR: 8325878: Require minimum Clang version 13 [v2]
> Please review this change that updates the minimum supported version of Clang > to be used for building OpenJDK from 3.5 to 13. > > This permits enabling C++17 (JDK-8314488), though Clang 5 might suffice for > that. A minimum of Clang 13 also obtains a critical bug fix for the > [[noreturn]] > attribute (see JDK-8303805). > > Testing: mach5 tier1, which includes building with a recent version of > Xcode/clang. Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits: - Merge branch 'master' into update-clang - Merge branch 'master' into update-clang - Merge branch 'master' into update-clang - update minimum clang version - Changes: https://git.openjdk.org/jdk/pull/17862/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17862&range=01 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17862.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17862/head:pull/17862 PR: https://git.openjdk.org/jdk/pull/17862
Re: RFR: 8325880: Require minimum Open XL C/C++ version 17.1.1 [v2]
> Please review this change that updates the minimum supported version of IBM > Open XL C/C++. SAP dropped support for older versions in JDK 22, only > supporting the version specified in this change. > > I need someone from the aix-ppc porters to test and review the change. Kim Barrett 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: - Merge branch 'master' into update-xlc-version - Merge branch 'master' into update-xlc-version - update XLC version - Changes: - all: https://git.openjdk.org/jdk/pull/17857/files - new: https://git.openjdk.org/jdk/pull/17857/files/cce39269..bfbff283 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17857&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17857&range=00-01 Stats: 99294 lines in 2187 files changed: 16647 ins; 74969 del; 7678 mod Patch: https://git.openjdk.org/jdk/pull/17857.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17857/head:pull/17857 PR: https://git.openjdk.org/jdk/pull/17857
Integrated: 8325880: Require minimum Open XL C/C++ version 17.1.1
On Wed, 14 Feb 2024 22:43:37 GMT, Kim Barrett wrote: > Please review this change that updates the minimum supported version of IBM > Open XL C/C++. SAP dropped support for older versions in JDK 22, only > supporting the version specified in this change. > > I need someone from the aix-ppc porters to test and review the change. This pull request has now been integrated. Changeset: 13c74535 Author:Kim Barrett URL: https://git.openjdk.org/jdk/commit/13c74535dea99133fe71942d1ac4f6df0f742d90 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod 8325880: Require minimum Open XL C/C++ version 17.1.1 Reviewed-by: mdoerr, jwaters, ihse - PR: https://git.openjdk.org/jdk/pull/17857
Re: RFR: 8325880: Require minimum Open XL C/C++ version 17.1.1
On Wed, 14 Feb 2024 22:43:37 GMT, Kim Barrett wrote: > Please review this change that updates the minimum supported version of IBM > Open XL C/C++. SAP dropped support for older versions in JDK 22, only > supporting the version specified in this change. > > I need someone from the aix-ppc porters to test and review the change. No new objections, so going ahead with this. - PR Comment: https://git.openjdk.org/jdk/pull/17857#issuecomment-1981683203
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v18]
On Wed, 6 Mar 2024 17:28:01 GMT, Magnus Ihse Bursie wrote: >> Severin Gehwolf has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Only show runtime image suffix for JDK modules > > make/ToolsJdk.gmk line 88: > >> 86: --add-modules=jdk.jlink >> --add-exports=java.base/jdk.internal.module=ALL-UNNAMED \ >> 87: --add-exports=java.base/jdk.internal.jimage=ALL-UNNAMED \ >> 88: build.tools.runtimelink.JimageDiffGenerator > > While it might be a bit redundant, we try to keep the same name in the make > name, the package and the main class, e.g. something like: > > Suggestion: > > TOOL_JIMAGE_DIFF_GENERATOR = $(BUILD_JAVA_SMALL) -cp > $(BUILDTOOLS_OUTPUTDIR)/jdk_tools_classes \ > --add-modules=jdk.jlink > --add-exports=java.base/jdk.internal.module=ALL-UNNAMED \ > --add-exports=java.base/jdk.internal.jimage=ALL-UNNAMED \ > build.tools.jimagediffgenerator.JimageDiffGenerator > > > This is of course not consistently followed, but for new tooling I think it > would be a good idea to try and follow. Based on some private feedback I've got it seems this will change a bit (move to the `jdk.jlink` module instead). But if still relevant, I'll keep that in mind. Thanks! - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1515040874
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v2]
On Wed, 6 Mar 2024 19:00:19 GMT, Alan Bateman wrote: >> Native access is needed for modules which calls restricted methods [1]. >> AFAICT, java.base, java.desktop and jdk.incubator.vector use >> java.lang.foreign but I don't know if they call restricted methods or not. >> >> https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/foreign/package-summary.html#restricted > >> Native access is needed for modules which calls restricted methods [1]. > > In time, we'll need it for modules using JNI too. We can do this trimming in > another PR to avoid Jan getting pulled into deeply. Doing it in another PR is fine with me. - PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1515009659
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v2]
On Wed, 6 Mar 2024 18:00:25 GMT, Mandy Chung wrote: > Native access is needed for modules which calls restricted methods [1]. In time, we'll need it for modules using JNI too. We can do this trimming in another PR to avoid Jan getting pulled into deeply. - PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1514999868
Re: RFR: 8327389: Remove use of HOTSPOT_BUILD_USER
On Wed, 6 Mar 2024 17:17:28 GMT, Magnus Ihse Bursie wrote: > I agree that it is good to get rid of this. However, the reason it has stuck > around for so long is, eh, that it has been around for so long, so it is a > bit unclear what or who could be relying on this. The example I know is to get a signal that test infrastructures actually picked up my ad-hoc binary build, not some other build from somewhere else. Maybe we should revisit the `git describe` hash idea we kicked along for a while now: https://bugs.openjdk.org/browse/JDK-8274980?focusedId=14452017 -- which would cover that case too. - PR Comment: https://git.openjdk.org/jdk/pull/18136#issuecomment-1981482991
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v2]
On Wed, 6 Mar 2024 15:02:07 GMT, Jan Lahoda wrote: >>> Many of these modules do not need native access in the current >>> implementation. >> >> In addition this will eventually need jlink support. I view the change to >> ModuleBootstrap initialiser to use ModuleLoaderMap.nativeAccessModules() as >> very temporary. It may include many standard/JDK modules that aren't in the >> image. In addition we'll need some way to grant native access at link-time. >> The workaround for the latter right now is to configure default options. > >> Many of these modules do not need native access in the current >> implementation. Should this list be trimmed to list the ones that need >> native access in the current implementation? > > Not sure if I know enough to do the pruning, so I was hoping that could be > done separately (I'd file a bug as Alan suggests). But I can try to prune the > list, if you prefer. Native access is needed for modules which calls restricted methods [1]. AFAICT, java.base, java.desktop and jdk.incubator.vector use java.lang.foreign but I don't know if they call restricted methods or not. https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/foreign/package-summary.html#restricted - PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1514932944
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v18]
On Tue, 27 Feb 2024 15:23:09 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: > > Only show runtime image suffix for JDK modules make/ToolsJdk.gmk line 88: > 86: --add-modules=jdk.jlink > --add-exports=java.base/jdk.internal.module=ALL-UNNAMED \ > 87: --add-exports=java.base/jdk.internal.jimage=ALL-UNNAMED \ > 88: build.tools.runtimelink.JimageDiffGenerator While it might be a bit redundant, we try to keep the same name in the make name, the package and the main class, e.g. something like: Suggestion: TOOL_JIMAGE_DIFF_GENERATOR = $(BUILD_JAVA_SMALL) -cp $(BUILDTOOLS_OUTPUTDIR)/jdk_tools_classes \ --add-modules=jdk.jlink --add-exports=java.base/jdk.internal.module=ALL-UNNAMED \ --add-exports=java.base/jdk.internal.jimage=ALL-UNNAMED \ build.tools.jimagediffgenerator.JimageDiffGenerator This is of course not consistently followed, but for new tooling I think it would be a good idea to try and follow. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1514894494
Re: RFR: 8325878: Require minimum Clang version 13
On Tue, 5 Mar 2024 08:24:20 GMT, Kim Barrett wrote: > I don't want to integrate this until the minimum aix-ppc toolchain has been > updated @kimbarrett Is anyone working on that problem? - PR Comment: https://git.openjdk.org/jdk/pull/17862#issuecomment-1981399543
Re: RFR: 8327389: Remove use of HOTSPOT_BUILD_USER
On Wed, 6 Mar 2024 15:24:59 GMT, Andrew John Hughes wrote: >> I'm fine with this change. I wasn't around when this was introduced, but my >> guess is that it was relevant back when Hotspot and the rest of the JDK were >> often built separately. We have the username in the default $OPT string for >> personal builds, so the HOTSPOT_BUILD_USER is just redundant information. >> There is no reason to have the build user recorded in non personal builds. > >> I'm fine with this change. I wasn't around when this was introduced, but my >> guess is that it was relevant back when Hotspot and the rest of the JDK were >> often built separately. We have the username in the default $OPT string for >> personal builds, so the HOTSPOT_BUILD_USER is just redundant information. >> There is no reason to have the build user recorded in non personal builds. > > Thanks Erik. I'll leave this open a little longer for others to comment, but > I also can't see a reason why this would be needed, especially in > non-personal builds. @gnu-andrew You might also want to draw specific attention to this change on hotspot-dev, to reach all developers who might know or care about the user name. It is worth making sure to point out that the big change here is that the hotspot version string format changes; that worries me more than the removal of the user name. (In fact, a safer change might be to hardcode the user name to a dummy value like `openjdk`). - PR Comment: https://git.openjdk.org/jdk/pull/18136#issuecomment-1981393909
Re: RFR: 8327389: Remove use of HOTSPOT_BUILD_USER
On Wed, 6 Mar 2024 11:57:27 GMT, Andrew John Hughes wrote: > The HotSpot code has inserted the value of `HOTSPOT_BUILD_USER` into the > internal version string since before the JDK was open-sourced, so the > reasoning for this is not in the public history. See > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/abstract_vm_version.cpp#L284 > > ~~~ > $ /usr/lib/jvm/java-21-openjdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (21.0.2+13) for linux-amd64 JRE (21.0.2+13), built > on 2024-01-19T16:39:52Z by "mockbuild" with gcc 8.5.0 20210514 (Red Hat > 8.5.0-20) > ~~~ > > It is not clear what value this provides. By default, the value is set to the > system username, but it can be set to a static value using > `--with-build-user`. > > In trying to compare builds, this is a source of difference if the builds are > produced under different usernames and `--with-build-user` is not set to the > same value. > > It may also be a source of information leakage from the host system. It is > not clear what value it provides to the end user of the build. > > This patch proposes removing it altogether, but at least knowing why it needs > to be there would be progress from where we are now. We could also consider a > middle ground of only setting it for "adhoc" builds, those without set > version information, as is the case with the overall version output. > > With this patch: > > ~~~ > $ ~/builder/23/images/jdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) for > linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on 2024-03-06T00:23:37Z > with gcc 13.2.1 20230826 > > $ ~/builder/23/images/jdk/bin/java -XX:ErrorHandlerTest=1 > > $ grep '^vm_info' > /localhome/andrew/projects/openjdk/upstream/jdk/hs_err_pid1119824.log > vm_info: OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) > for linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on > 2024-03-06T00:23:37Z with gcc 13.2.1 20230826 > ~~~ > > The above are from a fastdebug build but I also built a product build with no > issues. What testing have you run? I imagine there are tests that check/read the Hotspot version string. In fact, the more I think about it, the more I believe a better approach is to replace the variable user name with a fixed dummy replacement as a first step. That would only need for verification that nobody really cares about the user name. Then we can, as a follow up, change the version string (if we really care) -- this has a much higher chance of breakage, and it is not by far as important to solve, so it can easily be dropped if it causes problems. - PR Comment: https://git.openjdk.org/jdk/pull/18136#issuecomment-1981394987
Re: RFR: 8327389: Remove use of HOTSPOT_BUILD_USER
On Wed, 6 Mar 2024 11:57:27 GMT, Andrew John Hughes wrote: > The HotSpot code has inserted the value of `HOTSPOT_BUILD_USER` into the > internal version string since before the JDK was open-sourced, so the > reasoning for this is not in the public history. See > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/abstract_vm_version.cpp#L284 > > ~~~ > $ /usr/lib/jvm/java-21-openjdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (21.0.2+13) for linux-amd64 JRE (21.0.2+13), built > on 2024-01-19T16:39:52Z by "mockbuild" with gcc 8.5.0 20210514 (Red Hat > 8.5.0-20) > ~~~ > > It is not clear what value this provides. By default, the value is set to the > system username, but it can be set to a static value using > `--with-build-user`. > > In trying to compare builds, this is a source of difference if the builds are > produced under different usernames and `--with-build-user` is not set to the > same value. > > It may also be a source of information leakage from the host system. It is > not clear what value it provides to the end user of the build. > > This patch proposes removing it altogether, but at least knowing why it needs > to be there would be progress from where we are now. We could also consider a > middle ground of only setting it for "adhoc" builds, those without set > version information, as is the case with the overall version output. > > With this patch: > > ~~~ > $ ~/builder/23/images/jdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) for > linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on 2024-03-06T00:23:37Z > with gcc 13.2.1 20230826 > > $ ~/builder/23/images/jdk/bin/java -XX:ErrorHandlerTest=1 > > $ grep '^vm_info' > /localhome/andrew/projects/openjdk/upstream/jdk/hs_err_pid1119824.log > vm_info: OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) > for linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on > 2024-03-06T00:23:37Z with gcc 13.2.1 20230826 > ~~~ > > The above are from a fastdebug build but I also built a product build with no > issues. I agree that it is good to get rid of this. However, the reason it has stuck around for so long is, eh, that it has been around for so long, so it is a bit unclear what or who could be relying on this. Let's not be too eager to push this, and collect a bit more information about this. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18136#pullrequestreview-1920369274
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v2]
On Wed, 6 Mar 2024 00:56:16 GMT, Jaikiran Pai wrote: >> I believe we could technically reuse the list for boot/platform modules. >> But, the intent here is to provide more control over the modules with native >> access, not only being able to add to the list, but also remove from the >> list. So, to me, it seemed better to have an explicit list, from/to which we >> can remove/add modules easily. > > Hello Jan, my suggestion was based on a misunderstanding that > NATIVE_ACCESS_MODULES is always a superset of boot and platform modules. What > you currently have looks fine to me. And also, for the record, the .conf files are supposed to be in a syntactical subset compatible with both bash and make syntax, so this would not have been possible anyway. :) - PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1514865999
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v2]
On Tue, 5 Mar 2024 12:44:10 GMT, Jan Lahoda wrote: >> Currently, JDK modules load by the bootstrap and platform ClassLoaders are >> automatically granted the native access. I am working on an upgrade of JLine >> inside the `jdk.internal.le` module, and I would like to replace the current >> native bindings with FFM-based bindings (which are now somewhat provided by >> JLine). But, for that, native access is needed for the `jdk.internal.le` >> module. We could possibly move the module to the platform ClassLoader, but >> it seems it might be better to have more control over which modules have the >> native access. >> >> This patch introduces an explicit list of modules that will automatically be >> granted the native access. Note this patch is not yet intended to change the >> end behavior - the list of modules granted native access is supposed to be >> the same as modules in the boot and platform ClassLoaders. > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Apply suggestions from code review > > Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> > Co-authored-by: Maurizio Cimadamore > <54672762+mcimadam...@users.noreply.github.com> Build changes are ok. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18106#pullrequestreview-1920357192
Re: RFR: 8327389: Remove use of HOTSPOT_BUILD_USER
On Wed, 6 Mar 2024 13:57:37 GMT, Erik Joelsson wrote: > I'm fine with this change. I wasn't around when this was introduced, but my > guess is that it was relevant back when Hotspot and the rest of the JDK were > often built separately. We have the username in the default $OPT string for > personal builds, so the HOTSPOT_BUILD_USER is just redundant information. > There is no reason to have the build user recorded in non personal builds. Thanks Erik. I'll leave this open a little longer for others to comment, but I also can't see a reason why this would be needed, especially in non-personal builds. - PR Comment: https://git.openjdk.org/jdk/pull/18136#issuecomment-1981126634
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v2]
On Wed, 6 Mar 2024 09:19:44 GMT, Alan Bateman wrote: >> Many of these modules do not need native access in the current >> implementation. Should this list be trimmed to list the ones that need >> native access in the current implementation? > >> Many of these modules do not need native access in the current >> implementation. > > In addition this will eventually need jlink support. I view the change to > ModuleBootstrap initialiser to use ModuleLoaderMap.nativeAccessModules() as > very temporary. It may include many standard/JDK modules that aren't in the > image. In addition we'll need some way to grant native access at link-time. > The workaround for the latter right now is to configure default options. > Many of these modules do not need native access in the current > implementation. Should this list be trimmed to list the ones that need native > access in the current implementation? Not sure if I know enough to do the pruning, so I was hoping that could be done separately (I'd file a bug as Alan suggests). But I can try to prune the list, if you prefer. - PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1514640785
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v2]
On Tue, 5 Mar 2024 13:58:50 GMT, Jaikiran Pai wrote:
>> Jan Lahoda has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Apply suggestions from code review
>>
>> Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
>> Co-authored-by: Maurizio Cimadamore
>> <54672762+mcimadam...@users.noreply.github.com>
>
> src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 817:
>
>> 815: JLA.addEnableNativeAccessToAllUnnamed();
>> 816: } else if (!JLA.addEnableNativeAccess(layer, name) &&
>> shouldWarn) {
>> 817: warnUnknownModule(ENABLE_NATIVE_ACCESS, name);
>
> Should we instead warn irrespective of whether or not it's user configured
> native access module name or a module name configured in JDK's build
> configuration? Or are the build misconfiguration in the
> `NATIVE_ACCESS_MODULES` expected to be caught much earlier in some other
> place?
For normal "full" JDK build, all the modules with pre-set native access should
be present, and the warning might make sense there. But, the user may jlink a
smaller version of the platform, or use `--limit-modules`, and that may cause
some of the modules are not present. So I don't think it is realistic to
produce a warning here for the modules with pre-set native access.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1514633918
Re: RFR: 8327389: Remove use of HOTSPOT_BUILD_USER
On Wed, 6 Mar 2024 11:57:27 GMT, Andrew John Hughes wrote: > The HotSpot code has inserted the value of `HOTSPOT_BUILD_USER` into the > internal version string since before the JDK was open-sourced, so the > reasoning for this is not in the public history. See > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/abstract_vm_version.cpp#L284 > > ~~~ > $ /usr/lib/jvm/java-21-openjdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (21.0.2+13) for linux-amd64 JRE (21.0.2+13), built > on 2024-01-19T16:39:52Z by "mockbuild" with gcc 8.5.0 20210514 (Red Hat > 8.5.0-20) > ~~~ > > It is not clear what value this provides. By default, the value is set to the > system username, but it can be set to a static value using > `--with-build-user`. > > In trying to compare builds, this is a source of difference if the builds are > produced under different usernames and `--with-build-user` is not set to the > same value. > > It may also be a source of information leakage from the host system. It is > not clear what value it provides to the end user of the build. > > This patch proposes removing it altogether, but at least knowing why it needs > to be there would be progress from where we are now. We could also consider a > middle ground of only setting it for "adhoc" builds, those without set > version information, as is the case with the overall version output. > > With this patch: > > ~~~ > $ ~/builder/23/images/jdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) for > linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on 2024-03-06T00:23:37Z > with gcc 13.2.1 20230826 > > $ ~/builder/23/images/jdk/bin/java -XX:ErrorHandlerTest=1 > > $ grep '^vm_info' > /localhome/andrew/projects/openjdk/upstream/jdk/hs_err_pid1119824.log > vm_info: OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) > for linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on > 2024-03-06T00:23:37Z with gcc 13.2.1 20230826 > ~~~ > > The above are from a fastdebug build but I also built a product build with no > issues. I'm fine with this change. I wasn't around when this was introduced, but my guess is that it was relevant back when Hotspot and the rest of the JDK were often built separately. We have the username in the default $OPT string for personal builds, so the HOTSPOT_BUILD_USER is just redundant information. There is no reason to have the build user recorded in non personal builds. - Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18136#pullrequestreview-1919834395
Re: RFR: 8327460: Compile tests with the same visibility rules as product code [v2]
On Wed, 6 Mar 2024 13:43:00 GMT, Magnus Ihse Bursie wrote: >> Currently, our symbol visibility handling for tests are sloppy; we only >> handle it properly on Windows. We need to bring it up to the same levels as >> product code. This is a prerequisite for >> [JDK-8327045](https://bugs.openjdk.org/browse/JDK-8327045), which in turn is >> a building block for Hermetic Java. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Update line number for dereference_null in TestDwarf I have run the tests in tier1, tier2 and tier3 on the Oracle internal CI, which includes linux/x64, linux/aarch64, windows/x64, macosx/x64 and macosx/aarch64. I am currently running tier 4-10; this will take a while (and I won't integrate until this has finished running). When running multiple high-tier tests like this, the likelihood that I encounter intermittent problems increase. I will ignore test failures that seem likely to be intermittent and not caused by this patch. I might be overdoing the testing, but since this change affects **all** native tests, I want to be absolutely sure I don't break any tests that only execute as part of a high tier. - PR Comment: https://git.openjdk.org/jdk/pull/18135#issuecomment-1980924971
Re: RFR: 8327460: Compile tests with the same visibility rules as product code [v2]
On Wed, 6 Mar 2024 13:43:00 GMT, Magnus Ihse Bursie wrote: >> Currently, our symbol visibility handling for tests are sloppy; we only >> handle it properly on Windows. We need to bring it up to the same levels as >> product code. This is a prerequisite for >> [JDK-8327045](https://bugs.openjdk.org/browse/JDK-8327045), which in turn is >> a building block for Hermetic Java. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Update line number for dereference_null in TestDwarf Build system changes look good to me. The specific test changes probably needs other reviewers. I assume you have run all the affected tests on a wide variety of platforms? - Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18135#pullrequestreview-1919814110
Re: RFR: 8327460: Compile tests with the same visibility rules as product code [v2]
> Currently, our symbol visibility handling for tests are sloppy; we only > handle it properly on Windows. We need to bring it up to the same levels as > product code. This is a prerequisite for > [JDK-8327045](https://bugs.openjdk.org/browse/JDK-8327045), which in turn is > a building block for Hermetic Java. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Update line number for dereference_null in TestDwarf - Changes: - all: https://git.openjdk.org/jdk/pull/18135/files - new: https://git.openjdk.org/jdk/pull/18135/files/1dc79758..b1bf01ad Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18135&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18135&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18135.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18135/head:pull/18135 PR: https://git.openjdk.org/jdk/pull/18135
Re: RFR: 8327389: Remove use of HOTSPOT_BUILD_USER
On Wed, 6 Mar 2024 11:57:27 GMT, Andrew John Hughes wrote: > The HotSpot code has inserted the value of `HOTSPOT_BUILD_USER` into the > internal version string since before the JDK was open-sourced, so the > reasoning for this is not in the public history. See > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/abstract_vm_version.cpp#L284 > > ~~~ > $ /usr/lib/jvm/java-21-openjdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (21.0.2+13) for linux-amd64 JRE (21.0.2+13), built > on 2024-01-19T16:39:52Z by "mockbuild" with gcc 8.5.0 20210514 (Red Hat > 8.5.0-20) > ~~~ > > It is not clear what value this provides. By default, the value is set to the > system username, but it can be set to a static value using > `--with-build-user`. > > In trying to compare builds, this is a source of difference if the builds are > produced under different usernames and `--with-build-user` is not set to the > same value. > > It may also be a source of information leakage from the host system. It is > not clear what value it provides to the end user of the build. > > This patch proposes removing it altogether, but at least knowing why it needs > to be there would be progress from where we are now. We could also consider a > middle ground of only setting it for "adhoc" builds, those without set > version information, as is the case with the overall version output. > > With this patch: > > ~~~ > $ ~/builder/23/images/jdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) for > linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on 2024-03-06T00:23:37Z > with gcc 13.2.1 20230826 > > $ ~/builder/23/images/jdk/bin/java -XX:ErrorHandlerTest=1 > > $ grep '^vm_info' > /localhome/andrew/projects/openjdk/upstream/jdk/hs_err_pid1119824.log > vm_info: OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) > for linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on > 2024-03-06T00:23:37Z with gcc 13.2.1 20230826 > ~~~ > > The above are from a fastdebug build but I also built a product build with no > issues. cross-compile failure is an unrelated risc download issue: ~~~ W: Failure while installing base packages. This will be re-attempted up to five times. W: See /home/runner/work/jdk/jdk/sysroot/debootstrap/debootstrap.log for details (possibly the package /var/cache/apt/archives/libssl3t64_3.1.5-1.1_riscv64.deb is at fault) ~~~ - PR Comment: https://git.openjdk.org/jdk/pull/18136#issuecomment-198085
Re: RFR: 8327389: Remove use of HOTSPOT_BUILD_USER
On Wed, 6 Mar 2024 11:57:27 GMT, Andrew John Hughes wrote: > The HotSpot code has inserted the value of `HOTSPOT_BUILD_USER` into the > internal version string since before the JDK was open-sourced, so the > reasoning for this is not in the public history. See > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/abstract_vm_version.cpp#L284 > > ~~~ > $ /usr/lib/jvm/java-21-openjdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (21.0.2+13) for linux-amd64 JRE (21.0.2+13), built > on 2024-01-19T16:39:52Z by "mockbuild" with gcc 8.5.0 20210514 (Red Hat > 8.5.0-20) > ~~~ > > It is not clear what value this provides. By default, the value is set to the > system username, but it can be set to a static value using > `--with-build-user`. > > In trying to compare builds, this is a source of difference if the builds are > produced under different usernames and `--with-build-user` is not set to the > same value. > > It may also be a source of information leakage from the host system. It is > not clear what value it provides to the end user of the build. > > This patch proposes removing it altogether, but at least knowing why it needs > to be there would be progress from where we are now. We could also consider a > middle ground of only setting it for "adhoc" builds, those without set > version information, as is the case with the overall version output. > > With this patch: > > ~~~ > $ ~/builder/23/images/jdk/bin/java -Xinternalversion > OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) for > linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on 2024-03-06T00:23:37Z > with gcc 13.2.1 20230826 > > $ ~/builder/23/images/jdk/bin/java -XX:ErrorHandlerTest=1 > > $ grep '^vm_info' > /localhome/andrew/projects/openjdk/upstream/jdk/hs_err_pid1119824.log > vm_info: OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) > for linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on > 2024-03-06T00:23:37Z with gcc 13.2.1 20230826 > ~~~ > > The above are from a fastdebug build but I also built a product build with no > issues. Leaving a comment here as a reminder for myself to come back to this, this intrigues me - PR Comment: https://git.openjdk.org/jdk/pull/18136#issuecomment-1980739523
Re: RFR: 8327460: Compile tests with the same visibility rules as product code
On Wed, 6 Mar 2024 11:33:30 GMT, Magnus Ihse Bursie wrote:
> Currently, our symbol visibility handling for tests are sloppy; we only
> handle it properly on Windows. We need to bring it up to the same levels as
> product code. This is a prerequisite for
> [JDK-8327045](https://bugs.openjdk.org/browse/JDK-8327045), which in turn is
> a building block for Hermetic Java.
Most of these changes is just putting the `EXPORT` define in `export.h` instead
(where support for `__attribute__((visibility("default")))` is also added).
However, there are a few other changes worth mentioning:
* `test/jdk/java/lang/Thread/jni/AttachCurrentThread/libImplicitAttach.c` was
missing an export. This had not been discovered before since that file was not
compiled on Windows.
* On macOS, the `main` function of a Java launcher needs to be exported, since
we re-launch the main function when starting a new thread. (This is utterly
weird behavior driven by how on macOS the main thread is reserved for Cocoa
events, and I'm not sure this is properly documented by the JNI specification).
* The `dereference_null` function from `libTestDwarfHelper.h` is looked for in
the Hotspot hs_err stack trace in
`test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java`. When compiling with
`-fvisibility=hidden`, it became inline and the test failed. I solved this by
exporting the function, thus restoring the same situation as was before. I'm
not sure this is the correct or best solution though, since it depends on what
you expect `TestDwarf.java` to really achieve. (You can't expect of it to
perform miracles and show functions that has been inlined in stack traces,
though...)
-
PR Comment: https://git.openjdk.org/jdk/pull/18135#issuecomment-1980739771
RFR: 8327460: Compile tests with the same visibility rules as product code
Currently, our symbol visibility handling for tests are sloppy; we only handle it properly on Windows. We need to bring it up to the same levels as product code. This is a prerequisite for [JDK-8327045](https://bugs.openjdk.org/browse/JDK-8327045), which in turn is a building block for Hermetic Java. - Commit messages: - 8327460: Compile tests with the same visibility rules as product code Changes: https://git.openjdk.org/jdk/pull/18135/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18135&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8327460 Stats: 303 lines in 38 files changed: 69 ins; 159 del; 75 mod Patch: https://git.openjdk.org/jdk/pull/18135.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18135/head:pull/18135 PR: https://git.openjdk.org/jdk/pull/18135
RFR: 8327389: Remove use of HOTSPOT_BUILD_USER
The HotSpot code has inserted the value of `HOTSPOT_BUILD_USER` into the internal version string since before the JDK was open-sourced, so the reasoning for this is not in the public history. See https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/abstract_vm_version.cpp#L284 ~~~ $ /usr/lib/jvm/java-21-openjdk/bin/java -Xinternalversion OpenJDK 64-Bit Server VM (21.0.2+13) for linux-amd64 JRE (21.0.2+13), built on 2024-01-19T16:39:52Z by "mockbuild" with gcc 8.5.0 20210514 (Red Hat 8.5.0-20) ~~~ It is not clear what value this provides. By default, the value is set to the system username, but it can be set to a static value using `--with-build-user`. In trying to compare builds, this is a source of difference if the builds are produced under different usernames and `--with-build-user` is not set to the same value. It may also be a source of information leakage from the host system. It is not clear what value it provides to the end user of the build. This patch proposes removing it altogether, but at least knowing why it needs to be there would be progress from where we are now. We could also consider a middle ground of only setting it for "adhoc" builds, those without set version information, as is the case with the overall version output. With this patch: ~~~ $ ~/builder/23/images/jdk/bin/java -Xinternalversion OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) for linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on 2024-03-06T00:23:37Z with gcc 13.2.1 20230826 $ ~/builder/23/images/jdk/bin/java -XX:ErrorHandlerTest=1 $ grep '^vm_info' /localhome/andrew/projects/openjdk/upstream/jdk/hs_err_pid1119824.log vm_info: OpenJDK 64-Bit Server VM (fastdebug 23-internal-adhoc.andrew.jdk) for linux-amd64 JRE (23-internal-adhoc.andrew.jdk), built on 2024-03-06T00:23:37Z with gcc 13.2.1 20230826 ~~~ The above are from a fastdebug build but I also built a product build with no issues. - Commit messages: - 8327389: Remove use of HOTSPOT_BUILD_USER Changes: https://git.openjdk.org/jdk/pull/18136/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18136&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8327389 Stats: 10 lines in 5 files changed: 0 ins; 8 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18136.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18136/head:pull/18136 PR: https://git.openjdk.org/jdk/pull/18136
Re: RFR: 8327218: Add an ability to specify modules which should have native access enabled [v2]
On Tue, 5 Mar 2024 22:44:20 GMT, Mandy Chung wrote: > Many of these modules do not need native access in the current implementation. In addition this will eventually need jlink support. I view the change to ModuleBootstrap initialiser to use ModuleLoaderMap.nativeAccessModules() as very temporary. It may include many standard/JDK modules that aren't in the image. In addition we'll need some way to grant native access at link-time. The workaround for the latter right now is to configure default options. - PR Review Comment: https://git.openjdk.org/jdk/pull/18106#discussion_r1514113280
