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

Jorn Vernee jvernee at openjdk.java.net
Tue Nov 24 11:38:59 UTC 2020


On Tue, 24 Nov 2020 06:12:55 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> 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.

Ok, that seems like a good idea to me as well.

Since this code is different from normal JNI/JVM entries, I'd rather add things like exception marks or vm entry traces as needed manually, than relying on the existing macros, so that the need for each item can be re-evaluated and explained in this context.

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

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


More information about the hotspot-runtime-dev mailing list