Re: Hermetic Java project meeting
On Tue, May 7, 2024 at 5:26 AM Magnus Ihse Bursie < magnus.ihse.bur...@oracle.com> wrote: > 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. > https://lld.llvm.org/#using-lld has some information on using lld instead of the default linker. > 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. > That's correct. Please see more details below. > 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. > Using #ifdefs is not the most efficient path for us to get static Java launcher support in mainline. That's because most of the runtime changes for static Java support in hermetic-java-runtime branch are already done using `JLI_IsStaticJDK|JVM_IsStaticJDK` checks. We should not convert those to use #ifdefs then later convert the #ifdef back to runtime checks again during the integration work. As suggested and discussed earlier we can aim to get the static Java related changes into mainline incrementally. Following is a path that I think would work effectively and "fast" by limiting potentially wasted efforts: Step 1 - Get the makefile changes for linking `javastatic` without any of the runtime changes; Don't enable any build and testing for `javastatic` in this step yet Step 2 - Incrementally get the runtime changes reviewed and integrated into mainline; Enable building for `javastatic` as a test in github workflow when we can run HelloWorld using static launcher in mainline; Enable testing tier 1 for `javastatic` in workflow when we can run jtreg tests with the static launcher - could be done in a later step; Step 3 - Remove all STATIC_BUILD macros in JDK runtime code; Also cleanup the macros in tests (can be done later) CSR and JNI specification work to support JNI_OnLoad_ and friends for JNI dynamic library and builtin library Step 4 - Build (makefile) changes to support linking .a and .so libraries using the same set of .o objects, to avoid compiling the .c/.c++ source twice Those lay the foundation for the hermetic Java work in mainline. > 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? > By "runtime lookup", I think you were referring to the JNI native library lookup. We can handle them as part of
Re: RFR: 8330182: Start of release updates for JDK 24 [v5]
On Mon, 20 May 2024 22:42:20 GMT, Joe Darcy wrote: >> Get JDK 24 underway. > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 11 commits: > > - Fix-up test. > - Merge branch 'master' into JDK-8330188 > - Adjust test for deprecated options. > - Merge branch 'master' into JDK-8330188 > - Update deprecated options test. > - Merge branch 'master' into JDK-8330188 > - Merge branch 'master' into JDK-8330188 > - Merge branch 'master' into JDK-8330188 > - Correct release date as observed in review feedback. > - Improve javadoc of class file update. > - ... and 1 more: https://git.openjdk.org/jdk/compare/d6b7f9b1...128f53a3 Still looks good. - Marked as reviewed by iris (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18787#pullrequestreview-2067401131
Re: RFR: 8332545: Fix handling of HTML5 entities in Markdown comments
On Mon, 20 May 2024 23:21:31 GMT, Erik Joelsson wrote: > Build change looks good. Thank you - PR Comment: https://git.openjdk.org/jdk/pull/19317#issuecomment-2121488205
Integrated: 8332545: Fix handling of HTML5 entities in Markdown comments
On Mon, 20 May 2024 20:17:10 GMT, Jonathan Gibbons wrote: > Please review a small fix to address a crash when handling HTML5 entities in > a Markdown doc comment. > > The path for the `entities.txt` (originally `entities.properties`) was not > correctly imported when importing the latest version of `commonmark-java`. > And, the makefiles need to be updated to include the `.txt` file in the > image. (The interim module for the interim javadoc already had this fix.) > > A simple new test is provided, containing entities that previously caused > `javadoc` to crash. This pull request has now been integrated. Changeset: 6e805127 Author:Jonathan Gibbons URL: https://git.openjdk.org/jdk/commit/6e805127f8091d46205165746d7c59a40703958d Stats: 79 lines in 3 files changed: 77 ins; 0 del; 2 mod 8332545: Fix handling of HTML5 entities in Markdown comments Reviewed-by: prappo, erikj - PR: https://git.openjdk.org/jdk/pull/19317
Re: RFR: 8332545: Fix handling of HTML5 entities in Markdown comments
On Mon, 20 May 2024 20:17:10 GMT, Jonathan Gibbons wrote: > Please review a small fix to address a crash when handling HTML5 entities in > a Markdown doc comment. > > The path for the `entities.txt` (originally `entities.properties`) was not > correctly imported when importing the latest version of `commonmark-java`. > And, the makefiles need to be updated to include the `.txt` file in the > image. (The interim module for the interim javadoc already had this fix.) > > A simple new test is provided, containing entities that previously caused > `javadoc` to crash. Build change looks good. - Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19317#pullrequestreview-2067198212
Re: RFR: 8330182: Start of release updates for JDK 24 [v5]
> Get JDK 24 underway. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits: - Fix-up test. - Merge branch 'master' into JDK-8330188 - Adjust test for deprecated options. - Merge branch 'master' into JDK-8330188 - Update deprecated options test. - Merge branch 'master' into JDK-8330188 - Merge branch 'master' into JDK-8330188 - Merge branch 'master' into JDK-8330188 - Correct release date as observed in review feedback. - Improve javadoc of class file update. - ... and 1 more: https://git.openjdk.org/jdk/compare/d6b7f9b1...128f53a3 - Changes: https://git.openjdk.org/jdk/pull/18787/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18787=04 Stats: 107 lines in 37 files changed: 46 ins; 7 del; 54 mod Patch: https://git.openjdk.org/jdk/pull/18787.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18787/head:pull/18787 PR: https://git.openjdk.org/jdk/pull/18787
Re: RFR: 8332545: Fix handling of HTML5 entities in Markdown comments
On Mon, 20 May 2024 21:20:29 GMT, Pavel Rappo wrote: > Assuming the test fails without the fix, but passes with it, looks good. Confirmed the test fails without the fix (crash, as reported) and passes with the fix. - PR Comment: https://git.openjdk.org/jdk/pull/19317#issuecomment-2121306491
Re: RFR: 8332545: Fix handling of HTML5 entities in Markdown comments
On Mon, 20 May 2024 20:17:10 GMT, Jonathan Gibbons wrote: > Please review a small fix to address a crash when handling HTML5 entities in > a Markdown doc comment. > > The path for the `entities.txt` (originally `entities.properties`) was not > correctly imported when importing the latest version of `commonmark-java`. > And, the makefiles need to be updated to include the `.txt` file in the > image. (The interim module for the interim javadoc already had this fix.) > > A simple new test is provided, containing entities that previously caused > `javadoc` to crash. Assuming the test fails without the fix, but passes with it, looks good. - Marked as reviewed by prappo (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19317#pullrequestreview-2067063183
RFR: 8332545: Fix handling of HTML5 entities in Markdown comments
Please review a small fix to address a crash when handling HTML5 entities in a Markdown doc comment. The path for the `entities.txt` (originally `entities.properties`) was not correctly imported when importing the latest version of `commonmark-java`. And, the makefiles need to be updated to include the `.txt` file in the image. (The interim module for the interim javadoc already had this fix.) A simple new test is provided, containing entities that previously caused `javadoc` to crash. - Commit messages: - JDK-8332545: Fix handling of HTML5 entities in Markdown comments Changes: https://git.openjdk.org/jdk/pull/19317/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19317=00 Issue: https://bugs.openjdk.org/browse/JDK-8332545 Stats: 79 lines in 3 files changed: 77 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19317.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19317/head:pull/19317 PR: https://git.openjdk.org/jdk/pull/19317
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]
On Mon, 20 May 2024 18:39:31 GMT, Phil Race wrote: >> make/conf/module-loader-map.conf line 105: >> >>> 103: java.smartcardio \ >>> 104: jdk.accessibility \ >>> 105: jdk.attach \ >> >> The list of allowed modules has been rewritten from scratch, by looking at >> the set of modules containing at least one `native` method declaration. > > Should I understand this list to be the set of modules exempt from needing to > specific that native access is allowed ? > ie they always have native access without any warnings, and further that any > attempt to enable warnings, or to disable native access for these modules is > ignored ? Yes, this was added via JDK-8327218. The changes in this PR are just trimming down the list to only the modules that have native code. - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1607147983
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 Have you looked into / thought about how this will work for jpackaged apps ? I suspect that both the existing FFM usage and this will be options the application packager will need to supply when building the jpackaged app - the end user cannot pass in command line VM options. Seems there should be some testing of this as some kind of native access could be a common case for jpackaged apps. - PR Review: https://git.openjdk.org/jdk/pull/19213#pullrequestreview-2066794950
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]
On Mon, 13 May 2024 10:49:30 GMT, Maurizio Cimadamore
wrote:
>> Maurizio Cimadamore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address review comments
>
> make/conf/module-loader-map.conf line 105:
>
>> 103: java.smartcardio \
>> 104: jdk.accessibility \
>> 105: jdk.attach \
>
> The list of allowed modules has been rewritten from scratch, by looking at
> the set of modules containing at least one `native` method declaration.
Should I understand this list to be the set of modules exempt from needing to
specific that native access is allowed ?
ie they always have native access without any warnings, and further that any
attempt to enable warnings, or to disable native access for these modules is
ignored ?
> src/java.desktop/macosx/classes/com/apple/eio/FileManager.java line 61:
>
>> 59: }
>> 60:
>> 61: @SuppressWarnings({"removal", "restricted"})
>
> There are several of these changes. One option might have been to just
> disable restricted warnings when building. But on a deeper look, I realized
> that in all these places we already disabled deprecation warnings for the use
> of security manager, so I also added a new suppression instead.
Sounds reasonable.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1607136237
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1607136808
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Mon, 20 May 2024 13:03:36 GMT, Sean Mullan wrote: >> In XML parlance, a "processor" is an aggregation of parsers, serializers, >> and other things that contribute to the processing. So I think it could be >> either here, but you have a point and if it stays as "processor" then it >> should link #FacPro where the term is defined. > > It's the wording that doesn't sound right, before this you have "making" > which doesn't sound right with "process". Maybe it needs two sentences. Thanks! Same as Alan, I thought you were talking about "processor" as well on the first glance, then realized you're referring to the parallel statements :-) - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606995925
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v11]
> 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: updated jaxp-strict; fixed typo in module-info. - Changes: - all: https://git.openjdk.org/jdk/pull/18831/files - new: https://git.openjdk.org/jdk/pull/18831/files/dfc965c6..55a86db3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18831=10 - incr: https://webrevs.openjdk.org/?repo=jdk=18831=09-10 Stats: 5 lines in 2 files changed: 0 ins; 2 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18831.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831 PR: https://git.openjdk.org/jdk/pull/18831
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Mon, 20 May 2024 07:13:02 GMT, Alan Bateman wrote: >> Joe Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> withdraw changes to jaxp.properties. The configuration process has not >> changed, changing the default configuration would result in many failures >> that test the process. > > src/java.xml/share/conf/jaxp-strict.properties line 17: > >> 15: # java -Djava.xml.config.file=$JAVA_HOME/conf/jaxp-strict.properties >> 16: # >> 17: # The pathname to the configuration file must be valid. If it is not >> absolute, > > I think it would be better to drop this paragraph or else just replace it > with a sentence to say that the java.xml module description specifies the > system property. Thanks Alan. Replaced the paragraph with a reference to the module description. - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606993156
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Mon, 20 May 2024 12:55:24 GMT, Alan Bateman wrote: >> src/java.xml/share/classes/module-info.java line 444: >> >>> 442: * >>> 443: * Deploying with this configuration prevents processors from >>> unknowingly making >>> 444: * outbound network connections to fetch DTDs, or process XML that >>> makes use of >> >> s/process/processing/ > > In XML parlance, a "processor" is an aggregation of parsers, serializers, and > other things that contribute to the processing. So I think it could be either > here, but you have a point and if it stays as "processor" then it should link > #FacPro where the term is defined. It's the wording that doesn't sound right, before this you have "making" which doesn't sound right with "process". Maybe it needs two sentences. - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606754542
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Mon, 20 May 2024 12:48:01 GMT, Sean Mullan wrote: >> Joe Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> withdraw changes to jaxp.properties. The configuration process has not >> changed, changing the default configuration would result in many failures >> that test the process. > > src/java.xml/share/classes/module-info.java line 444: > >> 442: * >> 443: * Deploying with this configuration prevents processors from >> unknowingly making >> 444: * outbound network connections to fetch DTDs, or process XML that >> makes use of > > s/process/processing/ In XML parlance, a "processor" is an aggregation of parsers, serializers, and other things that contribute to the processing. So I think it could be either here, but you have a point and if it stays as "processor" then it should link #FacPro where the term is defined. - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606745477
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Sun, 19 May 2024 05:01: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. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > withdraw changes to jaxp.properties. The configuration process has not > changed, changing the default configuration would result in many failures > that test the process. src/java.xml/share/classes/module-info.java line 444: > 442: * > 443: * Deploying with this configuration prevents processors from > unknowingly making > 444: * outbound network connections to fetch DTDs, or process XML that makes > use of s/process/processing/ - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606737006
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Sun, 19 May 2024 05:01: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. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > withdraw changes to jaxp.properties. The configuration process has not > changed, changing the default configuration would result in many failures > that test the process. Looks good, just one comment on on the jaxp-strict.properties file text. src/java.xml/share/conf/jaxp-strict.properties line 17: > 15: # java -Djava.xml.config.file=$JAVA_HOME/conf/jaxp-strict.properties > 16: # > 17: # The pathname to the configuration file must be valid. If it is not > absolute, I think it would be better to drop this paragraph or else just replace it with a sentence to say that the java.xml module description specifies the system property. - Marked as reviewed by alanb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18831#pullrequestreview-2065520089 PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606350445
