Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v13]
On Thu, 1 Feb 2024 10:31:14 GMT, Thomas Stuefe wrote: >> Suchismith Roy has updated the pull request incrementally with one >> additional commit since the last revision: >> >> spelling > > I'm busy with FOSDEM this week and probably next. Will look at this end of > next week or the week after. @tstuefe, @offamitkumar: You had requested changes. Should we wait for your feedback regarding the latest version? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1945348564
Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v13]
On Thu, 1 Feb 2024 09:42:00 GMT, Suchismith Roy wrote: >> In addition, the nullptr check is important to avoid undefined behavior when >> passing `pointer_to_dot` to any string function. > > Ok. So then may be we can return a nullptr and do a log_info(os) with the > correct error report ? @tstuefe @TheRealMDoerr I'd probably use `log_info(library)`. - PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1474449287
Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v13]
On Wed, 31 Jan 2024 13:17:21 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: > > spelling I'm busy with FOSDEM this week and probably next. Will look at this end of next week or the week after. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1921004805
Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v13]
On Thu, 1 Feb 2024 04:17:41 GMT, Martin Doerr wrote: >> An assertion is only used for debug builds. Such an error should be handled >> in product builds as well. I think an attempt to load an invalid library >> should simply fail. You may add logging if needed. >> @tstuefe: Do you agree or have another proposal to handle such errors? > > In addition, the nullptr check is important to avoid undefined behavior when > passing `pointer_to_dot` to any string function. Ok. So then may be we can return a nullptr and do a log_info(os) with the correct error report ? @tstuefe @TheRealMDoerr - PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1474128334
Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v13]
On Thu, 1 Feb 2024 04:13:52 GMT, Martin Doerr wrote: >> I didn't follow that. You mean i need to keep a check if it is null and >> print it out ? > > An assertion is only used for debug builds. Such an error should be handled > in product builds as well. I think an attempt to load an invalid library > should simply fail. You may add logging if needed. > @tstuefe: Do you agree or have another proposal to handle such errors? In addition, the nullptr check is important to avoid undefined behavior when passing `pointer_to_dot` to any string function. - PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1473774551
Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v13]
On Wed, 31 Jan 2024 15:06:54 GMT, Suchismith Roy wrote: >> src/hotspot/os/aix/os_aix.cpp line 1176: >> >>> 1174: strncpy(file_path,filename, buffer_length + 1); >>> 1175: char* const pointer_to_dot = strrchr(file_path, '.'); >>> 1176: assert(pointer_to_dot != nullptr, "Attempting to load a shared >>> object without extension? %s", filename); >> >> This should not only be an assertion. I think the check could be used >> instead of the strcmp below. > > I didn't follow that. You mean i need to keep a check if it is null and print > it out ? An assertion is only used for debug builds. Such an error should be handled in product builds as well. I think an attempt to load an invalid library should simply fail. You may add logging if needed. @tstuefe: Do you agree or have another proposal to handle such errors? - PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1473772905
Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v13]
On Wed, 31 Jan 2024 13:20:52 GMT, Martin Doerr wrote: >> Suchismith Roy has updated the pull request incrementally with one >> additional commit since the last revision: >> >> spelling > > src/hotspot/os/aix/os_aix.cpp line 1176: > >> 1174: strncpy(file_path,filename, buffer_length + 1); >> 1175: char* const pointer_to_dot = strrchr(file_path, '.'); >> 1176: assert(pointer_to_dot != nullptr, "Attempting to load a shared >> object without extension? %s", filename); > > This should not only be an assertion. I think the check could be used instead > of the strcmp below. I didn't follow that. You mean i need to keep a check if it is null and print it out ? - PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1472963709
Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v13]
On Wed, 31 Jan 2024 13:17:21 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: > > spelling src/hotspot/os/aix/os_aix.cpp line 1176: > 1174: strncpy(file_path,filename, buffer_length + 1); > 1175: char* const pointer_to_dot = strrchr(file_path, '.'); > 1176: assert(pointer_to_dot != nullptr, "Attempting to load a shared object > without extension? %s", filename); This should not only be an assertion. I think the check could be used instead of the strcmp below. - PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1472813890
Re: RFR: JDK-8320005 : Allow loading of shared objects with .a extension on AIX [v13]
> 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: spelling - Changes: - all: https://git.openjdk.org/jdk/pull/16604/files - new: https://git.openjdk.org/jdk/pull/16604/files/713e514b..af761abb Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16604&range=12 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16604&range=11-12 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 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
