RFR: 8256486: Linux/Windows-x86 builds broken after JDK-8254231 [v4]

[David Holmes]

On Tue, 24 Nov 2020 01:14:20 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Use the Unimplemented() macro instead of hlt()
>
> 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.

Looking closer at this and the potential call-chain I don't think this is really either a JNI_ENTRY nor a JVM_ENTRY as we would normally define them. The upcall_stub seems to allow a thread to "tunnel" its way into the VM for a Java call, rather than going through the normal exported interfaces (i.e. JNI). I suspect that earlier in the review process it was requested that this be made some kind of ENTRY but I'm really not sure that fits at all. It may be cleaner, simpler and clearer to just declare a non-exported method that use `ThreadInVMFromNative` directly.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1266