On Thu, 22 Feb 2024 07:13:03 GMT, Julian Waters <jwat...@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. > > make/hotspot/lib/CompileJvm.gmk line 149: > >> 147: >> 148: ifeq ($(call isTargetOs, windows), true) >> 149: WIN_EXPORT_FILE := $(JVM_OUTPUTDIR)/win-exports.txt > > Why not .def? I try to stick to established file suffixes if possible. I have never heard of .def before, but maybe that is a standard name for these kinds of files on Windows? If so, I will change it. > src/hotspot/share/oops/accessBackend.cpp line 36: > >> 34: >> 35: #if defined(TARGET_COMPILER_gcc) >> 36: #define HIDDEN __attribute__ ((visibility ("hidden"))) > > Couldn't this use [[gnu::visibility("hidden")]] instead, since HotSpot now > allows C++14 Attributes? But this doesn't look like a very nice solution > regardless We're currently using __attribute__ ((visibility ("X"))) elsewhere, so I think I should stick with that. And, as I said in my long explanation, this is definitely a hack. The reason it is here is that I want to make sure the symbol table is 100% identical before and after this patch. Why this is needed, I don't know. I believe it is a bug in GCC, since we compile with -fvisibility=hidden. But I don't really know. Maybe there is something in the C++ standard that makes these functions behave differently. Perhaps they are even slightly incorrectly defined? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17955#discussion_r1498935395 PR Review Comment: https://git.openjdk.org/jdk/pull/17955#discussion_r1498933843
