On Mon, 23 Nov 2020 20:36:11 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> JDK-8254231 breaks the Linux and Windows x86 (32-bit) builds. This contains >> the needed changes to get it working again. >> >> Perhaps the most interesting change is adding the `JNI_ENTRY_CPP_NOENV` >> macro. Using just JNI_ENTRY was causing a linkage failure, due to the >> declaration of the function in the class not having the same linkage >> specifiers. It looks like we can't just specify C linkage for class member >> functions. >> >> However, in this case C linkage is not required, so I've added the new macro >> which doesn't have `extern "C"`. I've also dropped the `JNIEnv*` parameter, >> since it was not being used, but causing extra work for the caller. >> >> Other than that, it's just about adding default definitions for missing >> functions, and moving around some code in MacroAssembler to be in the >> correct `#ifdef` blocks. >> >> Testing: `make images` on Linux and Windows x86_32 platforms. > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Use the Unimplemented() macro instead of hlt() Hi Jorn, Sorry but I don't agree with the addition of JNI_ENTRY_CPP_NOENV. I think there is a flaw in the way these methods are exposed/structured. See comment below. Thanks, David src/hotspot/share/prims/universalUpcallHandler.cpp line 36: > 34: extern struct JavaVM_ main_vm; > 35: > 36: JNI_ENTRY_CPP_NOENV(void, > ProgrammableUpcallHandler::upcall_helper(jobject rec, address buff)) I do not like this. I think you have a design flaw here. We should not be directly calling member functions like this from other native code! The entry point should be a simple C-linkage function that is a normal JNI_ENTRY (assuming it actually should be a JNI_ENTRY and not JVM_ENTRY?) and that can then call the true target. ------------- Changes requested by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1266
