On Wed, 21 Feb 2024 23:32:15 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
> **Summary:** Finally get rid of the mapfiles in Hotspot, and replace it with > compiler options and `JNIEXPORT` on all platforms. > > The bug that this PR solves, > [JDK-8017234](https://bugs.openjdk.org/browse/JDK-8017234), was created in > 2013. Even back then the use of mapfiles in Hotspot was dated, so this is > really good riddance with old rubbish. > > This code touches on central but not well understood parts of the Hotspot > dynamic library, which has contributed to why this bug has stayed unresolved > for so long. I will need to explain this fix in more detail than usually > necessary. (Please bare with me if this gets long.) I also anticipate that > not all solutions that I've picked will be accepted, and we'll have to > discuss how to proceed. I think it is better to have actual concrete code to > discuss around, rather than starting by an abstract discussion. To keep this > description short, I will post the discussion as a comment to the PR. > > I have run this PR through tier 1-3 in our CI system. I have also carefully > checked how the resulting dynamic library differs with this patch (not much; > see discussion below). For build system changes, this is often the most > relevant metric. Thanks for doing this! This is a good improvement IMO. If the `SUNWprivate_1.1` part of the symbols is still needed, it can be added using `--default-symver` ld parameter. You might want to run tier1-5 tests; compilation-related changes are often detected by higher-level builds and tests only. src/hotspot/os_cpu/linux_arm/linux_arm_32.S line 35: > 33: .type SpinPause, %function > 34: .globl _Copy_conjoint_bytes > 35: .hidden _Copy_conjoint_bytes This triggered a GHA failure... apparently the method `_Copy_conjoint_bytes` was removed a long time ago, and the `.globl` / `.type` declarations are the only things left. They should also be removed. src/hotspot/share/jvmci/jvmciCompilerToVM.cpp line 74: > 72: > 73: #if defined(TARGET_COMPILER_gcc) > 74: #undef JNIEXPORT Based on the suggestion in [this JBS comment](https://bugs.openjdk.org/browse/JDK-8017234?focusedId=14601421&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14601421), I think we can remove JNIEXPORT from all the functions in this file. We need to keep the JNICALL though; the deprecated Win32 port needs it, not sure if it's used anywhere else. src/hotspot/share/utilities/debug.cpp line 71: > 69: > 70: #if defined(TARGET_COMPILER_gcc) > 71: #undef JNIEXPORT This partially reverts [JDK-8264593](https://bugs.openjdk.org/browse/JDK-8264593); @kevinjwalls will this work for you? ------------- PR Review: https://git.openjdk.org/jdk/pull/17955#pullrequestreview-1895071246 PR Review Comment: https://git.openjdk.org/jdk/pull/17955#discussion_r1498753308 PR Review Comment: https://git.openjdk.org/jdk/pull/17955#discussion_r1498794516 PR Review Comment: https://git.openjdk.org/jdk/pull/17955#discussion_r1498786497
