-
-
Notifications
You must be signed in to change notification settings - Fork 644
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(core) EspressoVM compatibility #1003
Conversation
`reserved0` is used by EspressoVM, thus it cannot serve as a "reference NULL" as on HotSpot: https://github.com/oracle/graal/blob/3149f62458029ffe92b8dcefe0b3e59612684cfa/espresso/src/com.oracle.truffle.espresso.mokapot/include/mokapot.h#L67-L73 The relevant config is `MOKA_LATTE`. `reserved3` is `NULL` though.
ad4e87e
to
2756241
Compare
modules/lwjgl/core/src/main/java/org/lwjgl/system/ThreadLocalUtil.java
Outdated
Show resolved
Hide resolved
modules/lwjgl/core/src/main/java/org/lwjgl/system/ThreadLocalUtil.java
Outdated
Show resolved
Hide resolved
modules/lwjgl/core/src/main/java/org/lwjgl/system/ThreadLocalUtil.java
Outdated
Show resolved
Hide resolved
d12cf34
to
c8a8187
Compare
modules/lwjgl/core/src/main/java/org/lwjgl/system/ThreadLocalUtil.java
Outdated
Show resolved
Hide resolved
I have ran into this issue myself, what if all reversed slots are in use? Won't it be better to copy JNIEnv with one more pointer in the function table? |
Hey @lewurm and @gilles-duboscq, I'm not convinced this is necessary. It looks like EspressoVM uses the reserved fields of the Is the current implementation actually broken on EspressoVM? Could the issue be related to something else? |
I've now done some testing with EspressoVM and it indeed populates
Based on the above, the following implementation seems to work: private static final int CAPABILITIES_OFFSET = 3 * POINTER_SIZE;
private static final long RESERVED_NULL = memGetAddress(JNI_NATIVE_INTERFACE + CAPABILITIES_OFFSET);
public static void setFunctionMissingAddresses(int functionCount) {
long ptr = JNI_NATIVE_INTERFACE + CAPABILITIES_OFFSET;
long currentTable = memGetAddress(ptr);
if (functionCount == 0) {
if (currentTable == FUNCTION_MISSING_ABORT_TABLE && FUNCTION_MISSING_ABORT_TABLE != NULL) {
FUNCTION_MISSING_ABORT_TABLE = NULL;
getAllocator().free(currentTable);
memPutAddress(ptr, RESERVED_NULL);
}
} else {
if (currentTable != RESERVED_NULL) {
throw new IllegalStateException("setFunctionMissingAddresses has been called already");
}
if (currentTable != NULL) {
// check reserved0 to see if this Native Image or EspressoVM. EspressoVM will not have the reserved NULL here.
if (memGetAddress(JNI_NATIVE_INTERFACE) == RESERVED_NULL) {
// silently abort on Native Image, the global JNIEnv object lives in read-only memory by default. (see #875)
return;
}
}
FUNCTION_MISSING_ABORT_TABLE = getAllocator().malloc(Integer.toUnsignedLong(functionCount) * POINTER_SIZE);
for (int i = 0; i < functionCount; i++) {
memPutAddress(FUNCTION_MISSING_ABORT_TABLE + Integer.toUnsignedLong(i) * POINTER_SIZE, FUNCTION_MISSING_ABORT);
}
memPutAddress(ptr, FUNCTION_MISSING_ABORT_TABLE);
}
} I can successfully run LWJGL applications with EspressoVM (with
The process simply dies after the last line (and before the native method returns). I have no idea what might be causing this. |
After more fixes and testing, it turns out that abusing
As a partial fix, I've replaced the thread-local storage of After the above commit, LWJGL tests pass and demos that do not use OpenGL work correctly on EspressoVM. It is a breaking change, but it should affect very few users, that make system calls under |
ahhh 😅 Thanks for the investigation and for the fix @Spasi! |
reserved0
is used by EspressoVM, thus it cannot serve as a "reference NULL" as on HotSpot: https://github.com/oracle/graal/blob/3149f62458029ffe92b8dcefe0b3e59612684cfa/espresso/src/com.oracle.truffle.espresso.mokapot/include/mokapot.h#L67-L73The relevant config is
MOKA_LATTE
.reserved3
isNULL
though.EspressoVM aka. Java on Truffle: https://www.graalvm.org/latest/reference-manual/java-on-truffle/
I tested the fix with Espresso, but I'm not 100% sure about the implications for other implementations. So please understand this PR more like a bug report 🙂 In general, I saw that in #875 (comment) it is mentioned that you want to get rid of (ab)using
reserved3
, so I think that's the better fix going forward.