On 27/11/2018 23:05, Henry Jen wrote:
Hi,
Please review a follow up webrev[1] based on Priyanka’s patch, it simply added
a test case for Mac only that will link with libjli.
Note that, to use invoke API, one should probably link with libjvm, which works
for all supported platforms, not just Mac.
Cheers,
Henry
[1] http://cr.openjdk.java.net/~henryjen/jdk12/8213362.0/webrev/
The changes to java_md_macosx.m looks okay although the issue as to how
Eclipse runs into this have not been established. If Eclipse dlopen's
libjvm then it should have no issue locating and calling JNI's
CreateJavaVM. It may be that Eclipse is using libjli directly but it
shouldn't do that because it's not a documented/supported interface. It
might be that it's using the CFBundleExecutable key in Info.plist.
Henry, Priyanka and I discussed this a bit and were not able to able to
establish what it is doing.
On the test: it's clear whether you've moved or copied
test/hotspot/jtreg/runtime/jni/CalleeSavedRegisters/FPRegs.java. The
webrev suggests you've moved it but it's doesn't handle hg copy. Either
way, it needs clean up, e.g. it shouldn't need @modules
java.base/jdk.internal.misc, the really long lines make it difficult to
look at side-by-side changes, why does it check for Windows when the
@requires means it runs on Mac only. I assume we can find a better name
for exeJNILauncher.c. It will also need a copyright header.
-Alan
