Re: Request for review- RFE 8005716
On 08/03/2013 02:22, BILL PITTORE wrote: Moved the string allocation into buildJniFunctionName as Alan suggested. Built and tested on windows and linux. Updated the webrev: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.02/ bill I see this updates the method descriptions to take on board Jeremy's comment on the possibility of the library being statically linked with the main executable with or without the VM. To be complete, I think this will require an update to the UnstatisfiedLinkError description too. Thanks for moving the sizing/allocation of the function name into buildJniFunctionName as that is cleaner and safer. Is FILENAME_MAX really the right limit to impose? A minor point, but the buffer is overrsized by 1 for the shared library case, I think it should be: len = strlen(sym) + (cname != NULL ? strlen(cname)+1 : 0) + 1 Otherwise, I think the implementation looks good to me. -Alan.
Re: Request for review- RFE 8005716
On 3/11/2013 9:40 AM, Alan Bateman wrote: On 08/03/2013 02:22, BILL PITTORE wrote: Moved the string allocation into buildJniFunctionName as Alan suggested. Built and tested on windows and linux. Updated the webrev: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.02/ bill I see this updates the method descriptions to take on board Jeremy's comment on the possibility of the library being statically linked with the main executable with or without the VM. To be complete, I think this will require an update to the UnstatisfiedLinkError description too. Thanks for moving the sizing/allocation of the function name into buildJniFunctionName as that is cleaner and safer. Is FILENAME_MAX really the right limit to impose? Maybe JVM_MAXPATHLEN is the better choice. A minor point, but the buffer is overrsized by 1 for the shared library case, I think it should be: len = strlen(sym) + (cname != NULL ? strlen(cname)+1 : 0) + 1 The +2 covered both cases; but re-wrote it to be the more explicit version above. bill Otherwise, I think the implementation looks good to me. -Alan.
Re: Request for review- RFE 8005716
Hi guys, I'm really glad to see you are doing this. We've done something similar to good effect within Google (and we'll probably drop our similar, internal patch and pick up this patch once it is pushed). Have you thought about support for having a JNI_OnLoad inside of a main executable that *doesn't* have a postfixed _lib? Our Google-internal patch treats the main executable as a regular JNI library if you execute a special System.loadFromLauncher() function. We also do the same thing with JVMTI. Nit: The comments that read like this: * for example, L, and a native library called L is statically linked * with the VM, then the JNI_OnLoad_L function exported by the library Should these read linked with the VM, or linked with the native application loading the VM? Jeremy On Wed, Mar 6, 2013 at 8:21 AM, Bob Vandette bob.vande...@oracle.comwrote: On Mar 5, 2013, at 7:36 PM, Dean Long wrote: If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you can't just turn that into _JNI_OnLoad@8_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 Good catch Dean. Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. Bill has already factored out the implementation of getProcessHandle. Although your approach is cleaner, I'm concerned about creating a VM version dependency. For a traditional JRE that doesn't even require static library support, we'd have to make sure to run on a VM that support JNI_VERSION_1_8. It looks like the JDK maintains their own copy of jni.h. If they didn't do that, we would have already gone down that path. The jdk sources already contain several instances of dlopen, dlysym and the Windows equivalents so I don't see a need to add a new VM function. Bob. dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Re: Request for review- RFE 8005716
On 3/7/2013 1:18 PM, Jeremy Manson wrote: Hi guys, I'm really glad to see you are doing this. We've done something similar to good effect within Google (and we'll probably drop our similar, internal patch and pick up this patch once it is pushed). Have you thought about support for having a JNI_OnLoad inside of a main executable that *doesn't* have a postfixed _lib? Our Google-internal patch treats the main executable as a regular JNI library if you execute a special System.loadFromLauncher() function. We also do the same thing with JVMTI. Nit: The comments that read like this: * for example, L, and a native library called L is statically linked * with the VM, then the JNI_OnLoad_L function exported by the library Hows this sound. * If the filename argument, when stripped of any platform-specific library * prefix, path, and file extension, indicates a library whose name is, * for example, L, and a native library called L is statically linked * with the native application or the VM (which itself may be statically * linked into the native application), then the JNI_OnLoad_L * function exported by the library is invoked rather than attempting * to load a dynamic library. bill Should these read linked with the VM, or linked with the native application loading the VM? Jeremy On Wed, Mar 6, 2013 at 8:21 AM, Bob Vandette bob.vande...@oracle.com mailto:bob.vande...@oracle.com wrote: On Mar 5, 2013, at 7:36 PM, Dean Long wrote: If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you can't just turn that into _JNI_OnLoad@8_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 Good catch Dean. Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. Bill has already factored out the implementation of getProcessHandle. Although your approach is cleaner, I'm concerned about creating a VM version dependency. For a traditional JRE that doesn't even require static library support, we'd have to make sure to run on a VM that support JNI_VERSION_1_8. It looks like the JDK maintains their own copy of jni.h. If they didn't do that, we would have already gone down that path. The jdk sources already contain several instances of dlopen, dlysym and the Windows equivalents so I don't see a need to add a new VM function. Bob. dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com mailto:bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/%7Ebpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ http://cr.openjdk.java.net/%7Ebpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Re: Request for review- RFE 8005716
On 07/03/2013 16:36, BILL PITTORE wrote: I updated the code based on the feedback. To fix the windows symbol name issue that Dean brought up I added a platform specific function, buildJniFunctionName. In windows it will properly convert _JNI_OnLoad@8 to _JNI_OnLoad_libname@8 as well as handle JNI_OnLoad symbol name. Fixed copyright headers as well. Tested on linux and windows Webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.01/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ bill Thanks for sorting out the decorator issue. One comment on that part of the implementation is that findJniFunction is sizing the buffer and buildJniFunctionName is using strcpy in the components. I wonder if this could be changed so that buildJniFunctionName sizes the buffer and uses strncpy. That way it will be obviously safe when just looking at one function. It makes it easier for static analysis tools to check too. -Alan
Re: Request for review- RFE 8005716
On 3/7/2013 3:50 PM, Alan Bateman wrote: On 07/03/2013 16:36, BILL PITTORE wrote: I updated the code based on the feedback. To fix the windows symbol name issue that Dean brought up I added a platform specific function, buildJniFunctionName. In windows it will properly convert _JNI_OnLoad@8 to _JNI_OnLoad_libname@8 as well as handle JNI_OnLoad symbol name. Fixed copyright headers as well. Tested on linux and windows Webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.01/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ bill Thanks for sorting out the decorator issue. One comment on that part of the implementation is that findJniFunction is sizing the buffer and buildJniFunctionName is using strcpy in the components. I wonder if this could be changed so that buildJniFunctionName sizes the buffer and uses strncpy. That way it will be obviously safe when just looking at one function. It makes it easier for static analysis tools to check too. I actually was passing in 'len' at one point and checking but it seemed redundant. The comment about static analysis tools having a fit makes sense. I'll tweak it as you suggest. bill -Alan
Re: Request for review- RFE 8005716
On 3/7/2013 10:18 AM, Jeremy Manson wrote: Hi guys, I'm really glad to see you are doing this. We've done something similar to good effect within Google (and we'll probably drop our similar, internal patch and pick up this patch once it is pushed). Have you thought about support for having a JNI_OnLoad inside of a main executable that *doesn't* have a postfixed _lib? Our Google-internal patch treats the main executable as a regular JNI library if you execute a special System.loadFromLauncher() function. We also do the same thing with JVMTI. Would that require additional changes to the JNI spec? Is there an advantage to treating it as an unnamed library rather than a named library (i.e. JNI_OnLoad_main)? dl Nit: The comments that read like this: * for example, L, and a native library called L is statically linked * with the VM, then the JNI_OnLoad_L function exported by the library Should these read linked with the VM, or linked with the native application loading the VM? Jeremy On Wed, Mar 6, 2013 at 8:21 AM, Bob Vandette bob.vande...@oracle.com mailto:bob.vande...@oracle.com wrote: On Mar 5, 2013, at 7:36 PM, Dean Long wrote: If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you can't just turn that into _JNI_OnLoad@8_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 Good catch Dean. Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. Bill has already factored out the implementation of getProcessHandle. Although your approach is cleaner, I'm concerned about creating a VM version dependency. For a traditional JRE that doesn't even require static library support, we'd have to make sure to run on a VM that support JNI_VERSION_1_8. It looks like the JDK maintains their own copy of jni.h. If they didn't do that, we would have already gone down that path. The jdk sources already contain several instances of dlopen, dlysym and the Windows equivalents so I don't see a need to add a new VM function. Bob. dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com mailto:bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/%7Ebpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ http://cr.openjdk.java.net/%7Ebpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Re: Request for review- RFE 8005716
On Thu, Mar 7, 2013 at 3:26 PM, Dean Long dean.l...@oracle.com wrote: On 3/7/2013 10:18 AM, Jeremy Manson wrote: Hi guys, I'm really glad to see you are doing this. We've done something similar to good effect within Google (and we'll probably drop our similar, internal patch and pick up this patch once it is pushed). Have you thought about support for having a JNI_OnLoad inside of a main executable that *doesn't* have a postfixed _lib? Our Google-internal patch treats the main executable as a regular JNI library if you execute a special System.loadFromLauncher() function. We also do the same thing with JVMTI. Would that require additional changes to the JNI spec? Is there an advantage to treating it as an unnamed library rather than a named library (i.e. JNI_OnLoad_main)? A couple of advantages, neither of which is a home run, but which are worth considering: First, it means you can write libraries that are intended to be both statically and dynamically compiled without making any code changes. Maybe all this means is that JNI_OnLoad_foo should work if you are trying to load libfoo.so (maybe it already does, and I didn't see that feature.) Second, you avoid potential name clashes with dynamic libraries named the same thing. This would be most useful in adding JNI_OnLoads to general-purpose launchers, where the author of the launcher doesn't have complete control over the code that gets loaded. The only one of these I know of in the wild is in Eclipse, so let's take that as an example. I could see Eclipse bundling all of its native code into a launcher, and making itself into a single executable. Let's say an Eclipse plugin wanted to load a native library called libfoo.so, but Eclipse itself had a JNI_OnLoad_foo in its launcher. Blech. Much better for the Eclipse launcher to have a single JNI_OnLoad that delegates to initialization code for foo. (The problem is compounded when you think about the fact that these will probably be (potentially different versions of) the same library. This implies a deep, deep need to worry about visibility / using Bdynamic when compiling your launcher and your static libraries.) Jeremy
Re: Request for review- RFE 8005716
Moved the string allocation into buildJniFunctionName as Alan suggested. Built and tested on windows and linux. Updated the webrev: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.02/ bill On 3/7/2013 3:50 PM, Alan Bateman wrote: On 07/03/2013 16:36, BILL PITTORE wrote: I updated the code based on the feedback. To fix the windows symbol name issue that Dean brought up I added a platform specific function, buildJniFunctionName. In windows it will properly convert _JNI_OnLoad@8 to _JNI_OnLoad_libname@8 as well as handle JNI_OnLoad symbol name. Fixed copyright headers as well. Tested on linux and windows Webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.01/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ bill Thanks for sorting out the decorator issue. One comment on that part of the implementation is that findJniFunction is sizing the buffer and buildJniFunctionName is using strcpy in the components. I wonder if this could be changed so that buildJniFunctionName sizes the buffer and uses strncpy. That way it will be obviously safe when just looking at one function. It makes it easier for static analysis tools to check too. -Alan
Re: Request for review- RFE 8005716
On 06/03/2013 04:13, BILL PITTORE wrote: On 3/5/2013 7:36 PM, Dean Long wrote: If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you can't just turn that into _JNI_OnLoad@8_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 I'll look into that. When I built for windows and ran our test, the JNI_OnLoad_TestStaticLib was exported without the decoration just based on the JNIEXPORT JNICALL specifiers on the function. I didn't do anything special to export it. But I recall this problem from another project. 10 1014 JNI_OnLoad_TestStaticLib = @ILT+15(?JNI_OnLoad_TestStaticLib@@YGHPAUJavaVM_@@PAX@Z) Dean makes a good point, the @8 need be at the end to match the decoration scheme. Also I think it's 32-bit only although it should just not be found on 64-bit so it will skip on JNI_OnLoad_L. -Alan.
Re: Request for review- RFE 8005716
On 05/03/2013 23:05, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. Overall I think this is quite good and follows the proposal in the JEP. I don't see anything obvious wrong with the logic and everything should just work for shared libraries as it does today. I assume you'll run all the appropriate tests. I see Dean's suggestion to add a JVM function to allow for a lookup table when the symbol information is available, If you do that then you'll need to get the hotspot changes in first. Alternatively, change what you have later once the hotspot changes are in.Just on the As findBuiltJniFunction can locate a built-in or a JNI_OnLoad/OnUnload in a library then the function name is probably not quite right (shouldn't have Builtin in the name). A minor nit in _findBuiltinLib is that if the OOME path should call JNU_ReleaseStringPlatformChars before returning. There are a few commented out statements in jni_util_md.c that I assume will be removed. Also you might want to check the indentation in both getProcessHandle implementations as they look like 2-spaces whereas the libs uses 4 (although this may be mute if you use a JVM function). Otherwise I think this is good and I can sponsor the jdk part to this and help get it into jdk8/tl. -Alan
Re: Request for review- RFE 8005716
On Mar 5, 2013, at 7:36 PM, Dean Long wrote: If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you can't just turn that into _JNI_OnLoad@8_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 Good catch Dean. Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. Bill has already factored out the implementation of getProcessHandle. Although your approach is cleaner, I'm concerned about creating a VM version dependency. For a traditional JRE that doesn't even require static library support, we'd have to make sure to run on a VM that support JNI_VERSION_1_8. It looks like the JDK maintains their own copy of jni.h. If they didn't do that, we would have already gone down that path. The jdk sources already contain several instances of dlopen, dlysym and the Windows equivalents so I don't see a need to add a new VM function. Bob. dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Re: Request for review- RFE 8005716
On Mar 6 2013, at 08:21 , Bob Vandette wrote: For a traditional JRE that doesn't even require static library support, we'd have to make sure to run on a VM that support JNI_VERSION_1_8. It looks like the JDK maintains their own copy of jni.h. In earlier days the jni.h file was copied from hotspot into jdk during the compilation process. The current build process doesn't validate that the two files match which opens up opportunities for inconsistencies and problems. If they didn't do that, we would have already gone down that path. If it's possible to improve upon having two jni.h instances then we should consider doing so. Mike
Re: Request for review- RFE 8005716
On 3/5/2013 8:13 PM, BILL PITTORE wrote: On 3/5/2013 7:36 PM, Dean Long wrote: If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you can't just turn that into _JNI_OnLoad@8_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 I'll look into that. When I built for windows and ran our test, the JNI_OnLoad_TestStaticLib was exported without the decoration just based on the JNIEXPORT JNICALL specifiers on the function. I didn't do anything special to export it. But I recall this problem from another project. 10 1014 JNI_OnLoad_TestStaticLib = @ILT+15(?JNI_OnLoad_TestStaticLib@@YGHPAUJavaVM_@@PAX@Z) Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). I'll remove that. I redid the call to findBuiltinJniFunction but forgot to remove that. Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. Not sure I'm following you on this. Make JVM_FindBuiltinLibraryEntry() an exported function in the VM? How does it find JNI_OnLoad_L? For Windows and Linux, it would use basically your same code, just refactored a little. By the way, did you consider using the special RTLD_DEFAULT handle instead of dlopen(NULL)? Via a table setup by the developer/build system when the library is linked in? Yes, in previous projects we did exactly that for more exotic OSes. dl bill dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Re: Request for review- RFE 8005716
Hi Bill;
Some notes from reviewing the JDK side changes.
System.java/Runtime.java:
The example which begins with the name If the filename argument, needs to
better identify that L is an example. (Italics?)
java/lang/ClassLoader.java:
NativeLibrary::fromClass could be final.
ClassLoader.c:
In Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib()
These two values are known at compile time.
int prefixLen = (int) strlen(JNI_LIB_PREFIX);
int suffixLen = (int) strlen(JNI_LIB_SUFFIX);
Some of the error conditions don't throw exceptions. Such as:
if (cname == NULL) {
return NULL;
}
The prefix and suffix are stripped from cname without checking that cname
actually contains the prefix or suffix.
if (len prefixLen) is invariant.
src/solaris/native/common/jni_util_md.c:
void* getProcessHandle() {
static void* procHandle = NULL;
if (procHandle == NULL) {
procHandle = (void*)dlopen(NULL, RTLD_LAZY);
}
return procHandle;
}
Why is the error handling code commented out?
Mike
On Mar 5 2013, at 15:05 , bill.pitt...@oracle.com wrote:
This request is tied to bugid 8005716 that deals with adding support for
statically linked JNI libraries.
The JEP is here: http://openjdk.java.net/jeps/178
The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716
The webrevs are here:
http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/
http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/
The main piece of functionality is to check for the presence of
JNI_OnLoad_libname to determine if the library specified by 'libname' has
been statically linked into the VM. If the symbol is found, it is assumed
that the library is linked in and will not be dynamically loaded.
thanks,
bill
Re: Request for review- RFE 8005716
If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you can't just turn that into _JNI_OnLoad@8_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Re: Request for review- RFE 8005716
Actually for windows I *did* export the undecorated name. I just didn't see where I did it in the VS IDE. If you don't export the undecorated name it doesn't work of course. bill On 3/5/2013 11:13 PM, BILL PITTORE wrote: On 3/5/2013 7:36 PM, Dean Long wrote: If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you can't just turn that into _JNI_OnLoad@8_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 I'll look into that. When I built for windows and ran our test, the JNI_OnLoad_TestStaticLib was exported without the decoration just based on the JNIEXPORT JNICALL specifiers on the function. I didn't do anything special to export it. But I recall this problem from another project. 10 1014 JNI_OnLoad_TestStaticLib = @ILT+15(?JNI_OnLoad_TestStaticLib@@YGHPAUJavaVM_@@PAX@Z) Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). I'll remove that. I redid the call to findBuiltinJniFunction but forgot to remove that. Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. Not sure I'm following you on this. Make JVM_FindBuiltinLibraryEntry() an exported function in the VM? How does it find JNI_OnLoad_L? Via a table setup by the developer/build system when the library is linked in? bill dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Re: Request for review- RFE 8005716
On 3/6/2013 12:50 PM, Mike Duigou wrote:
Hi Bill;
Some notes from reviewing the JDK side changes.
System.java/Runtime.java:
The example which begins with the name If the filename argument, needs to better
identify that L is an example. (Italics?)
Re-worded that a bit.
java/lang/ClassLoader.java:
NativeLibrary::fromClass could be final.
Ok.
ClassLoader.c:
In Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib()
These two values are known at compile time.
int prefixLen = (int) strlen(JNI_LIB_PREFIX);
int suffixLen = (int) strlen(JNI_LIB_SUFFIX);
Some of the error conditions don't throw exceptions. Such as:
if (cname == NULL) {
return NULL;
}
Fixed.
The prefix and suffix are stripped from cname without checking that cname
actually contains the prefix or suffix.
..._findBuiltinLib is only called after System.mapLibraryName has added
prefix and suffix.
if (len prefixLen) is invariant.
Based on System.mapLibraryName always called, then this could just be an
assert. Unless JNU_GetStringPlatformChars returns some bogus string.
src/solaris/native/common/jni_util_md.c:
void* getProcessHandle() {
static void* procHandle = NULL;
if (procHandle == NULL) {
procHandle = (void*)dlopen(NULL, RTLD_LAZY);
}
return procHandle;
}
Why is the error handling code commented out?
That was just for some debugging. I've removed it.
Thanks for commenting. I'll get a new webrev up shortly.
bill
Mike
On Mar 5 2013, at 15:05 , bill.pitt...@oracle.com wrote:
This request is tied to bugid 8005716 that deals with adding support for
statically linked JNI libraries.
The JEP is here: http://openjdk.java.net/jeps/178
The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716
The webrevs are here:
http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/
http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/
The main piece of functionality is to check for the presence of
JNI_OnLoad_libname to determine if the library specified by 'libname' has been
statically linked into the VM. If the symbol is found, it is assumed that the
library is linked in and will not be dynamically loaded.
thanks,
bill
Request for review- RFE 8005716
This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
Re: Request for review- RFE 8005716
On 3/5/2013 7:36 PM, Dean Long wrote: If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you can't just turn that into _JNI_OnLoad@8_ + libname. I think it needs to be _JNI_OnLoad_ + libname + @8 I'll look into that. When I built for windows and ran our test, the JNI_OnLoad_TestStaticLib was exported without the decoration just based on the JNIEXPORT JNICALL specifiers on the function. I didn't do anything special to export it. But I recall this problem from another project. 10 1014 JNI_OnLoad_TestStaticLib = @ILT+15(?JNI_OnLoad_TestStaticLib@@YGHPAUJavaVM_@@PAX@Z) Looks like onLoadSymbols[] is unused in Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(). I'll remove that. I redid the call to findBuiltinJniFunction but forgot to remove that. Instead of adding getProcessHandle(), why not add JVM_FindBuiltinLibraryEntry() instead? This would make it easier to support table-lookup when runtime symbol information is missing or not supported by the platform. Not sure I'm following you on this. Make JVM_FindBuiltinLibraryEntry() an exported function in the VM? How does it find JNI_OnLoad_L? Via a table setup by the developer/build system when the library is linked in? bill dl On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote: This request is tied to bugid 8005716 that deals with adding support for statically linked JNI libraries. The JEP is here: http://openjdk.java.net/jeps/178 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716 The webrevs are here: http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/ http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/ The main piece of functionality is to check for the presence of JNI_OnLoad_libname to determine if the library specified by 'libname' has been statically linked into the VM. If the symbol is found, it is assumed that the library is linked in and will not be dynamically loaded. thanks, bill
