On Wed, 23 Apr 2025 00:56:18 GMT, Jiangli Zhou <jian...@openjdk.org> 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>'.

Marked as reviewed by jvernee (Reviewer).

I had a look at the implementation, and see how it works now. The 
JNI_OnLoad_XXX function is essentially being used as a marker to indicate that 
this library was statically linked into the VM binary, so we can create a 
`NativeLibrary` instance that is marked as 'builtin', which changes the way 
symbol lookups are handled.

I think the current solution is probably fine. I think alternatively we could 
keep track of a table with statically linked libraries to determine where/how 
symbols should be looked up for a particular library. Or, the symbol doesn't 
necessarily have to be JNI_OnLoad_XXX, but could be some other, 
non-JNI-specific, marker symbol.

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

PR Review: https://git.openjdk.org/jdk/pull/24801#pullrequestreview-2799287975
PR Comment: https://git.openjdk.org/jdk/pull/24801#issuecomment-2835232241

Reply via email to