Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v7]
On Wed, 20 Dec 2023 11:16:03 GMT, Suchismith Roy wrote: >> J2SE agent does not start and throws error when it tries to find the shared >> library ibm_16_am. >> After searching for ibm_16_am.so ,the jvm agent throws and error as dll_load >> fails.It fails to identify the shared library ibm_16_am.a shared archive >> file on AIX. >> Hence we are providing a function which will additionally search for .a file >> on AIX ,when the search for .so file fails. > > Suchismith Roy has updated the pull request incrementally with one additional > commit since the last revision: > > Spaces fix Hi, some requests and questions: - Please modify the JBS title, PR title, and JBS issue text to reflect that this adds an alternative shared object loading path for shared objects on AIX. Something like "Allow loading shared objects with .a extension on AIX". Please describe the new logic in the JBS issue text. - Does this really have to be handled in the OpenJDK? What does J9 on AIX do? Could this be done in a simpler way outside OpenJDK, e.g. by providing an *.so variant of the library in question? Where does this library come from? - What happens if we accidentally attempt to load a "real" static library, which is also named *.a? Would dlopen() then crash? What would happen? - What happens if the original path handed to os::dll_load is already a *.a file? Should the logic then be reversed? - We really need regression tests for this. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1864572287
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v7]
On Wed, 20 Dec 2023 11:16:03 GMT, Suchismith Roy wrote:
>> J2SE agent does not start and throws error when it tries to find the shared
>> library ibm_16_am.
>> After searching for ibm_16_am.so ,the jvm agent throws and error as dll_load
>> fails.It fails to identify the shared library ibm_16_am.a shared archive
>> file on AIX.
>> Hence we are providing a function which will additionally search for .a file
>> on AIX ,when the search for .so file fails.
>
> Suchismith Roy has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Spaces fix
Only some minor suggestions.
src/hotspot/os/aix/os_aix.cpp line 1168:
> 1166: int extension_length = 3;
> 1167: char* file_path = NEW_C_HEAP_ARRAY(char, buffer_length +
> extension_length + 1, mtInternal);
> 1168: strncpy(file_path,filename, buffer_length + 1);
Why not using
`char* file_path = os::strdup (filename);`
which would replace lines 1167+1168
and use the corresponding
`os::free (file_path);`
at the end
src/hotspot/os/aix/os_aix.cpp line 1174:
> 1172: result = dll_load_library(file_path, ebuf, ebuflen);
> 1173: // If the load fails,we try to reload by changing the extension to .a
> for .so files only.
> 1174: if(result == nullptr) {
Space between if and (
also next line
-
Changes requested by jkern (Author).
PR Review: https://git.openjdk.org/jdk/pull/16604#pullrequestreview-1790895382
PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1432716207
PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1432738451
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v7]
> J2SE agent does not start and throws error when it tries to find the shared > library ibm_16_am. > After searching for ibm_16_am.so ,the jvm agent throws and error as dll_load > fails.It fails to identify the shared library ibm_16_am.a shared archive file > on AIX. > Hence we are providing a function which will additionally search for .a file > on AIX ,when the search for .so file fails. Suchismith Roy has updated the pull request incrementally with one additional commit since the last revision: Spaces fix - Changes: - all: https://git.openjdk.org/jdk/pull/16604/files - new: https://git.openjdk.org/jdk/pull/16604/files/9df8c2c8..ffcbf786 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16604=06 - incr: https://webrevs.openjdk.org/?repo=jdk=16604=05-06 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16604.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16604/head:pull/16604 PR: https://git.openjdk.org/jdk/pull/16604
