Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v7]

2021-10-12 Thread Jorn Vernee
On Thu, 3 Jun 2021 20:49:44 GMT, Maurizio Cimadamore  
wrote:

>> This patch overhauls the library loading mechanism used by the Foreign 
>> Linker API. We realized that, while handy, the *default* lookup abstraction 
>> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
>> 
>> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
>> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
>> concern with library loading, only symbol lookup. For this reason, two 
>> factories are added:
>> 
>> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
>> lookup symbols in libraries loaded by current loader
>> * `CLinker::systemLookup` - a more stable replacement for the *default* 
>> lookup, which looks for symbols in libc.so (or its equivalent in other 
>> platforms). The contents of this lookup are unspecified.
>> 
>> Both factories are *restricted*, so they can only be called when 
>> `--enable-native-access` is set.
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address review comments on shim lib makefile

I think the reason it worked in JDK 16 is because all the symbols in the JVM 
were visible through the system lookup, and the JVM statically links the 
standard library (AFAIU). So, just because the VM code depended on something, 
it could be loaded by the system lookup. But, that would mean that not all 
symbols in the standard library were visible... and also, being able to find 
_any_ symbol in the JVM was a bug.

I think the only solution here - assuming that libc is not available as a 
dynamic library on typical AIX systems - is to figure out how to repackage 
these static libraries as a dynamic library, retaining all the symbols, and 
then bundle that dynamic library with the JDK.

-

PR: https://git.openjdk.java.net/jdk/pull/4316


Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v7]

2021-10-12 Thread Cheng Jin
On Thu, 3 Jun 2021 20:49:44 GMT, Maurizio Cimadamore  
wrote:

>> This patch overhauls the library loading mechanism used by the Foreign 
>> Linker API. We realized that, while handy, the *default* lookup abstraction 
>> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
>> 
>> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
>> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
>> concern with library loading, only symbol lookup. For this reason, two 
>> factories are added:
>> 
>> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
>> lookup symbols in libraries loaded by current loader
>> * `CLinker::systemLookup` - a more stable replacement for the *default* 
>> lookup, which looks for symbols in libc.so (or its equivalent in other 
>> platforms). The contents of this lookup are unspecified.
>> 
>> Both factories are *restricted*, so they can only be called when 
>> `--enable-native-access` is set.
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address review comments on shim lib makefile

If so,  I am wondering whether there is anything else left (inherited from 
JDK16) in JDK17 we can leverage to support `libc.a` on AIX except the way 
similar to Windows.

-

PR: https://git.openjdk.java.net/jdk/pull/4316


Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v7]

2021-10-12 Thread Maurizio Cimadamore
On Tue, 12 Oct 2021 18:11:56 GMT, Cheng Jin  wrote:

> Just tried with `System.load()` but still ended up with pretty much the same 
> failure given both of them eventually invokes `ClassLoader.loadLibrary` to 
> load the library in which case there is no big difference at this point.

Yes and no. System::loadLibrary wants a library name (no extension). It will 
add a library prefix (e.g. `lib` on linux) and a library suffix (e.g. `.so` on 
linux). So, if you do:


System.loadLibrary("c")


You will end up with `libc.so`. The `System::loadLibrary` logic will then try 
to find that file in any of the known library paths.

`System.load` avoids this by accepting the full path of the library. So there's 
no guessing the path, nor guessing of prefix/suffix. But it seems like loading 
still fails, likely because we try to load this library with `dlopen` but this 
is a static library, so for `dlopen` it just doesn't make sense.

-

PR: https://git.openjdk.java.net/jdk/pull/4316


Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v7]

2021-10-12 Thread Cheng Jin
On Thu, 3 Jun 2021 20:49:44 GMT, Maurizio Cimadamore  
wrote:

>> This patch overhauls the library loading mechanism used by the Foreign 
>> Linker API. We realized that, while handy, the *default* lookup abstraction 
>> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
>> 
>> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
>> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
>> concern with library loading, only symbol lookup. For this reason, two 
>> factories are added:
>> 
>> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
>> lookup symbols in libraries loaded by current loader
>> * `CLinker::systemLookup` - a more stable replacement for the *default* 
>> lookup, which looks for symbols in libc.so (or its equivalent in other 
>> platforms). The contents of this lookup are unspecified.
>> 
>> Both factories are *restricted*, so they can only be called when 
>> `--enable-native-access` is set.
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address review comments on shim lib makefile

Just tried with `System.load()` but still ended up with pretty much the same 
failure given both of them eventually invokes `ClassLoader.loadLibrary` to load 
the library in which case there is no big difference at this point.

static {
System.load("/usr/lib/libc.a");
}

Exception in thread "main" java.lang.UnsatisfiedLinkError: Can't load 
/usr/lib/libc.a <
at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:1793)
at java.base/java.lang.System.load(System.java:672)
at StdLibTest.(StdLibTest.java:24)

-

PR: https://git.openjdk.java.net/jdk/pull/4316


Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v7]

2021-06-03 Thread Maurizio Cimadamore
> This patch overhauls the library loading mechanism used by the Foreign Linker 
> API. We realized that, while handy, the *default* lookup abstraction 
> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
> 
> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
> concern with library loading, only symbol lookup. For this reason, two 
> factories are added:
> 
> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
> lookup symbols in libraries loaded by current loader
> * `CLinker::systemLookup` - a more stable replacement for the *default* 
> lookup, which looks for symbols in libc.so (or its equivalent in other 
> platforms). The contents of this lookup are unspecified.
> 
> Both factories are *restricted*, so they can only be called when 
> `--enable-native-access` is set.

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  Address review comments on shim lib makefile

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4316/files
  - new: https://git.openjdk.java.net/jdk/pull/4316/files/2545e2b6..23d66faf

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4316=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4316=05-06

  Stats: 15 lines in 1 file changed: 0 ins; 0 del; 15 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4316.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4316/head:pull/4316

PR: https://git.openjdk.java.net/jdk/pull/4316