On Wed, 23 Apr 2025 00:56:18 GMT, Jiangli Zhou <[email protected]> wrote:
>> Please review this PR that changes to use `NativeLibraries.loadLibrary()` >> for loading the `libsyslookup` in `jdk.internal.foreign.SystemLookup` class. >> >> `NativeLibraries.loadLibrary()` handles both the shared library and (static) >> built-in library loading properly. On `static-jdk`, calling >> `NativeLibraries.loadLibrary()` for `systlookup` library can find the >> built-in library by looking up using `JNI_OnLoad_syslookup`. The current >> change adds `DEF_STATIC_JNI_OnLoad` in syslookup.c (in shared, windows and >> aix versions) for that purpose. >> >> In addition to GHA testing, I tested the change on static-jdk with jdk tier1 >> tests on linux-x64 locally. All java/foreign/* jdk tier1 tests pass with the >> change. Without the change, there are about 60 java/foreign/* jdk tier1 >> tests fail on static-jdk. > > Jiangli Zhou has updated the pull request incrementally with two additional > commits since the last revision: > > - Merge branch 'JDK-8355080' of ssh://github.com/jianglizhou/jdk into > JDK-8355080 > - Address henryjen@ comment: > - Remove '#include <jni.h>'. Hi, I know this PR/issue has been closed, but I want to ask on this thread because experts seems to follow this. I wonder why `SystemLookup` depends on `libsyslookup` on Linux. I understand syslookup.dll is needed for Windows because some functions might not be lookup'ed. OTOH on Linux, `dlsym` can lookup symbols from library dependencies. In `SystemLookup`, handle of `libsyslookup` would be passed to `dlsym` eventually, but I think it is better to pass `RTLD_DEFAULT` in this case. I know it works when the handle of `libsyslookup` is passed, but `RTLD_DEFAULT` is better because Javadoc of `Linker::defaultLookup` says it returns a set of commonly used libraries. I guess the reson of use `libsyslookup` is to use `NativeLibraryImpl`. I think we can fix not to use `libsyslookup` like https://github.com/YaSuenag/jdk/commit/b125cc164d60ac14316549e59d18544d75f6fcb2 . It works on Linux (including static image of course). If it does not have a problem, I want to create JBS issue and PR for this. Do you have any comments? In addition, I guess we can apply this change to all of POSIX platforms because `dlsym` is defined in POSIX, but I'm not sure we can do (especially AIX - it has own syslookup.c in JDK source tree). ------------- PR Comment: https://git.openjdk.org/jdk/pull/24801#issuecomment-4191297164
