Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 19 Dec 2023 16:47:33 GMT, Goetz Lindenmaier wrote: > try it! I got the instructions to replicate in my local repo later, so wasn't sure to proceed. Thanks for the suggestion. I think this makes it easier to keep in sync with the other change. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1864127477
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 28 Nov 2023 11:27:33 GMT, Suchismith Roy wrote:
> > > i would have to repeat the line 1132 and 1139 in os_aix.cpp again , if
> > > the condition fails for .so files, because i have to reload it again and
> > > check if the .a exists. In the shared code i had repeat less number of
> > > lines i believe. Do you suggest moving lines 1132 to 1139 to another
> > > function then ?
> >
> >
> > @tstuefe Any suggestion on this ?
>
> ```
> --- a/src/hotspot/os/aix/os_aix.cpp
> +++ b/src/hotspot/os/aix/os_aix.cpp
> @@ -1108,7 +1108,7 @@ bool os::dll_address_to_library_name(address addr,
> char* buf,
>return true;
> }
>
> -void *os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> +static void* dll_load_inner(const char *filename, char *ebuf, int ebuflen) {
>
>log_info(os)("attempting shared library load of %s", filename);
>
> @@ -1158,6 +1158,35 @@ void *os::dll_load(const char *filename, char *ebuf,
> int ebuflen) {
>return nullptr;
> }
>
> +void* os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> +
> + void* result = nullptr;
> +
> + // First try using *.so suffix; failing that, retry with *.a suffix.
> + const size_t len = strlen(filename);
> + constexpr size_t safety = 3 + 1;
> + constexpr size_t bufsize = len + safety;
> + char* buf = NEW_C_HEAP_ARRAY(char, bufsize, mtInternal);
> + strcpy(buf, filename);
> + char* const dot = strrchr(buf, '.');
> +
> + assert(dot != nullptr, "Attempting to load a shared object without
> extension? %s", filename);
> + assert(strcmp(dot, ".a") == 0 || strcmp(dot, ".so") == 0,
> + "Attempting to load a shared object that is neither *.so nor *.a",
> filename);
> +
> + sprintf(dot, ".so");
> + result = dll_load_inner(buf, ebuf, ebuflen);
> +
> + if (result == nullptr) {
> +sprintf(dot, ".a");
> +result = dll_load_inner(buf, ebuf, ebuflen);
> + }
> +
> + FREE_C_HEAP_ARRAY(char, buf);
> +
> + return result;
> +}
> +
> ```
Hi Thomas
May I know what is the reason to use constexpr over regular datatypes ?
Also, I have used strcpy to avoid buffer overflow.(Though we have calculated
the exact length). Would that be fine ?
-
PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1864063261
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 19 Dec 2023 16:47:33 GMT, Goetz Lindenmaier wrote: > try it! error: failed to push some refs to 'github.com:suchismith1993/jdk.git' I tried the fork instructions . However I think I need to do a force push. Will that be fine since the PR is not in draft state ? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1863210891
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Wed, 22 Nov 2023 16:24:24 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: > > change macro position try it! - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1863123599
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 19 Dec 2023 13:55:09 GMT, Suchismith Roy wrote: >>> > > >>> > >>> > >>> > @tstuefe 3rd parameter to pass the either of 2 things: >>> > ``` >>> > 1. The JvmTiAgent object "agent", so that after shifting the >>> > save_library_signature to os_aix,we can still access the agent object.-> >>> > For this i tried importing jvm/prims header file, but i get segmentation >>> > faults during build . Not sure if i am doing it the right way. >>> > >>> > 2. Pass a character buffer(and not const char*) where we copy the >>> > modified filename back to it and then use it in jvmAgent. code. >>> > ``` >>> >>> Does not sound really appealing tbh. We pile one hack atop of another. >>> >>> Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler >>> code, which will make this point moot. See >>> https://bugs.openjdk.org/browse/JDK-8320890. >> >> Hi @tstuefe Should i then wait for this code to be integrated and then >> rewrite the .a handling ? >> I mean this PR shall remain open then right ? >> @JoKern65 Are you even handling the .a handling case ? i would like this PR >> to stay open. Maybe i can wait for the design change that you are working on. > >> Hi @suchismith1993 , you can place this change on top of #16920 by comparing >> it with branch origin/pr/16920 instead of master. This way you might be able >> to proceed with your change. But as Thomas says you can only push if you >> have appropriate reviews. > > Hi @GoeLin I am not sure how to do that . Could you tell me in brief ? > Do I run the checkout command on the other PR and then place my change of top > of it ? > > > Hi @suchismith1993 , you can place this change on top of #16920 by > > > comparing it with branch origin/pr/16920 instead of master. This way you > > > might be able to proceed with your change. But as Thomas says you can > > > only push if you have appropriate reviews. > > > > > > Hi @GoeLin I am not sure how to do that . Could you tell me in brief ? Do I > > run the checkout command on the other PR and then place my change of top of > > it ? > > Yes, you can do that. You can also change the branch in this pr. Click edit > on the top right. Choose an alternative for "openjdk:master" I see. So If I do that, will it reflect on my command line in local machine ? I mean I need run a rebase command with origin as pr/16920 ? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1863086746
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 19 Dec 2023 13:55:09 GMT, Suchismith Roy wrote: > > Hi @suchismith1993 , you can place this change on top of #16920 by > > comparing it with branch origin/pr/16920 instead of master. This way you > > might be able to proceed with your change. But as Thomas says you can only > > push if you have appropriate reviews. > > Hi @GoeLin I am not sure how to do that . Could you tell me in brief ? Do I > run the checkout command on the other PR and then place my change of top of > it ? Yes, you can do that. You can also change the branch in this pr. Click edit on the top right. Choose an alternative for "openjdk:master" - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1863051404
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 28 Nov 2023 12:59:01 GMT, Suchismith Roy wrote: >>> > >>> >>> @tstuefe 3rd parameter to pass the either of 2 things: >>> >>> 1. The JvmTiAgent object "agent", so that after shifting the >>> save_library_signature to os_aix,we can still access the agent object.-> >>> For this i tried importing jvm/prims header file, but i get segmentation >>> faults during build . Not sure if i am doing it the right way. >>> >>> 2. Pass a character buffer(and not const char*) where we copy the >>> modified filename back to it and then use it in jvmAgent. code. >> >> Does not sound really appealing tbh. We pile one hack atop of another. >> >> Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler >> code, which will make this point moot. See >> https://bugs.openjdk.org/browse/JDK-8320890. > >> > > >> > >> > >> > @tstuefe 3rd parameter to pass the either of 2 things: >> > ``` >> > 1. The JvmTiAgent object "agent", so that after shifting the >> > save_library_signature to os_aix,we can still access the agent object.-> >> > For this i tried importing jvm/prims header file, but i get segmentation >> > faults during build . Not sure if i am doing it the right way. >> > >> > 2. Pass a character buffer(and not const char*) where we copy the modified >> > filename back to it and then use it in jvmAgent. code. >> > ``` >> >> Does not sound really appealing tbh. We pile one hack atop of another. >> >> Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler >> code, which will make this point moot. See >> https://bugs.openjdk.org/browse/JDK-8320890. > > Hi @tstuefe Should i then wait for this code to be integrated and then > rewrite the .a handling ? > I mean this PR shall remain open then right ? > @JoKern65 Are you even handling the .a handling case ? i would like this PR > to stay open. Maybe i can wait for the design change that you are working on. > Hi @suchismith1993 , you can place this change on top of #16920 by comparing > it with branch origin/pr/16920 instead of master. This way you might be able > to proceed with your change. But as Thomas says you can only push if you have > appropriate reviews. Hi @GoeLin I am not sure how to do that . Could you tell me in brief ? Do I run the checkout command on the other PR and then place my change of top of it ? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1862802768
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 28 Nov 2023 12:59:01 GMT, Suchismith Roy wrote: >>> > >>> >>> @tstuefe 3rd parameter to pass the either of 2 things: >>> >>> 1. The JvmTiAgent object "agent", so that after shifting the >>> save_library_signature to os_aix,we can still access the agent object.-> >>> For this i tried importing jvm/prims header file, but i get segmentation >>> faults during build . Not sure if i am doing it the right way. >>> >>> 2. Pass a character buffer(and not const char*) where we copy the >>> modified filename back to it and then use it in jvmAgent. code. >> >> Does not sound really appealing tbh. We pile one hack atop of another. >> >> Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler >> code, which will make this point moot. See >> https://bugs.openjdk.org/browse/JDK-8320890. > >> > > >> > >> > >> > @tstuefe 3rd parameter to pass the either of 2 things: >> > ``` >> > 1. The JvmTiAgent object "agent", so that after shifting the >> > save_library_signature to os_aix,we can still access the agent object.-> >> > For this i tried importing jvm/prims header file, but i get segmentation >> > faults during build . Not sure if i am doing it the right way. >> > >> > 2. Pass a character buffer(and not const char*) where we copy the modified >> > filename back to it and then use it in jvmAgent. code. >> > ``` >> >> Does not sound really appealing tbh. We pile one hack atop of another. >> >> Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler >> code, which will make this point moot. See >> https://bugs.openjdk.org/browse/JDK-8320890. > > Hi @tstuefe Should i then wait for this code to be integrated and then > rewrite the .a handling ? > I mean this PR shall remain open then right ? > @JoKern65 Are you even handling the .a handling case ? i would like this PR > to stay open. Maybe i can wait for the design change that you are working on. Hi @suchismith1993 , you can place this change on top of #16920 by comparing it with branch origin/pr/16920 instead of master. This way you might be able to proceed with your change. But as Thomas says you can only push if you have appropriate reviews. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1862791022
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 28 Nov 2023 12:48:14 GMT, Thomas Stuefe wrote: > > > > > > > > > @tstuefe 3rd parameter to pass the either of 2 things: > > ``` > > 1. The JvmTiAgent object "agent", so that after shifting the > > save_library_signature to os_aix,we can still access the agent object.-> > > For this i tried importing jvm/prims header file, but i get segmentation > > faults during build . Not sure if i am doing it the right way. > > > > 2. Pass a character buffer(and not const char*) where we copy the modified > > filename back to it and then use it in jvmAgent. code. > > ``` > > Does not sound really appealing tbh. We pile one hack atop of another. > > Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler > code, which will make this point moot. See > https://bugs.openjdk.org/browse/JDK-8320890. Hi @tstuefe Should i then wait for this code to be integrated and then rewrite the .a handling ? I mean this PR shall remain open then right ? @JoKern65 Are you even handling the .a handling case ? i would like this PR to stay open. Maybe i can wait for the design change that you are working on. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1829793568
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 28 Nov 2023 11:27:33 GMT, suchismith1993 wrote: > > > > @tstuefe 3rd parameter to pass the either of 2 things: > > 1. The JvmTiAgent object "agent", so that after shifting the > save_library_signature to os_aix,we can still access the agent object.-> For > this i tried importing jvm/prims header file, but i get segmentation faults > during build . Not sure if i am doing it the right way. > > 2. Pass a character buffer(and not const char*) where we copy the > modified filename back to it and then use it in jvmAgent. code. Does not sound really appealing tbh. We pile one hack atop of another. Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler code, which will make this point moot. See https://bugs.openjdk.org/browse/JDK-8320890. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1829776298
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 28 Nov 2023 11:08:43 GMT, Thomas Stuefe wrote:
> > > > > i would have to repeat the line 1132 and 1139 in os_aix.cpp again ,
> > > > > if the condition fails for .so files, because i have to reload it
> > > > > again and check if the .a exists. In the shared code i had repeat
> > > > > less number of lines i believe. Do you suggest moving lines 1132 to
> > > > > 1139 to another function then ?
> > > >
> > > >
> > > > @tstuefe Any suggestion on this ?
> > >
> > >
> > > ```
> > > --- a/src/hotspot/os/aix/os_aix.cpp
> > > +++ b/src/hotspot/os/aix/os_aix.cpp
> > > @@ -1108,7 +1108,7 @@ bool os::dll_address_to_library_name(address addr,
> > > char* buf,
> > >return true;
> > > }
> > >
> > > -void *os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> > > +static void* dll_load_inner(const char *filename, char *ebuf, int
> > > ebuflen) {
> > >
> > >log_info(os)("attempting shared library load of %s", filename);
> > >
> > > @@ -1158,6 +1158,35 @@ void *os::dll_load(const char *filename, char
> > > *ebuf, int ebuflen) {
> > >return nullptr;
> > > }
> > >
> > > +void* os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> > > +
> > > + void* result = nullptr;
> > > +
> > > + // First try using *.so suffix; failing that, retry with *.a suffix.
> > > + const size_t len = strlen(filename);
> > > + constexpr size_t safety = 3 + 1;
> > > + constexpr size_t bufsize = len + safety;
> > > + char* buf = NEW_C_HEAP_ARRAY(char, bufsize, mtInternal);
> > > + strcpy(buf, filename);
> > > + char* const dot = strrchr(buf, '.');
> > > +
> > > + assert(dot != nullptr, "Attempting to load a shared object without
> > > extension? %s", filename);
> > > + assert(strcmp(dot, ".a") == 0 || strcmp(dot, ".so") == 0,
> > > + "Attempting to load a shared object that is neither *.so nor *.a",
> > > filename);
> > > +
> > > + sprintf(dot, ".so");
> > > + result = dll_load_inner(buf, ebuf, ebuflen);
> > > +
> > > + if (result == nullptr) {
> > > +sprintf(dot, ".a");
> > > +result = dll_load_inner(buf, ebuf, ebuflen);
> > > + }
> > > +
> > > + FREE_C_HEAP_ARRAY(char, buf);
> > > +
> > > + return result;
> > > +}
> > > +
> > > ```
> >
> >
> > @tstuefe as discussed with @TheRealMDoerr do you think using default
> > argument will help ? Either we pass agent object as 3rd parameter or an
> > empty character buffer(and not const chat*) which would be spcifically used
> > to copy the alternate filename to it using strcpy so that it is reflected
> > in the jvmagent code ?
>
> A third parameter for what?
@tstuefe
3rd parameter to pass the either of 2 things:
1. The JvmTiAgent object "agent", so that after shifting the
save_library_signature to os_aix,we can still access the agent object.-> For
this i tried importing jvm/prims header file, but i get segmentation faults
during build . Not sure if i am doing it the right way.
2. Pass a character buffer(and not const char*) where we copy the modified
filename back to it and then use it in jvmAgent. code.
-
PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1829634847
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Mon, 27 Nov 2023 15:44:58 GMT, suchismith1993 wrote:
> > > > i would have to repeat the line 1132 and 1139 in os_aix.cpp again , if
> > > > the condition fails for .so files, because i have to reload it again
> > > > and check if the .a exists. In the shared code i had repeat less number
> > > > of lines i believe. Do you suggest moving lines 1132 to 1139 to another
> > > > function then ?
> > >
> > >
> > > @tstuefe Any suggestion on this ?
> >
> >
> > ```
> > --- a/src/hotspot/os/aix/os_aix.cpp
> > +++ b/src/hotspot/os/aix/os_aix.cpp
> > @@ -1108,7 +1108,7 @@ bool os::dll_address_to_library_name(address addr,
> > char* buf,
> >return true;
> > }
> >
> > -void *os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> > +static void* dll_load_inner(const char *filename, char *ebuf, int ebuflen)
> > {
> >
> >log_info(os)("attempting shared library load of %s", filename);
> >
> > @@ -1158,6 +1158,35 @@ void *os::dll_load(const char *filename, char *ebuf,
> > int ebuflen) {
> >return nullptr;
> > }
> >
> > +void* os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> > +
> > + void* result = nullptr;
> > +
> > + // First try using *.so suffix; failing that, retry with *.a suffix.
> > + const size_t len = strlen(filename);
> > + constexpr size_t safety = 3 + 1;
> > + constexpr size_t bufsize = len + safety;
> > + char* buf = NEW_C_HEAP_ARRAY(char, bufsize, mtInternal);
> > + strcpy(buf, filename);
> > + char* const dot = strrchr(buf, '.');
> > +
> > + assert(dot != nullptr, "Attempting to load a shared object without
> > extension? %s", filename);
> > + assert(strcmp(dot, ".a") == 0 || strcmp(dot, ".so") == 0,
> > + "Attempting to load a shared object that is neither *.so nor *.a",
> > filename);
> > +
> > + sprintf(dot, ".so");
> > + result = dll_load_inner(buf, ebuf, ebuflen);
> > +
> > + if (result == nullptr) {
> > +sprintf(dot, ".a");
> > +result = dll_load_inner(buf, ebuf, ebuflen);
> > + }
> > +
> > + FREE_C_HEAP_ARRAY(char, buf);
> > +
> > + return result;
> > +}
> > +
> > ```
>
> @tstuefe as discussed with @TheRealMDoerr do you think using default argument
> will help ? Either we pass agent object as 3rd parameter or an empty
> character buffer(and not const chat*) which would be spcifically used to copy
> the alternate filename to it using strcpy so that it is reflected in the
> jvmagent code ?
A third parameter for what?
-
PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1829602040
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Mon, 27 Nov 2023 11:36:02 GMT, suchismith1993 wrote:
> > > i would have to repeat the line 1132 and 1139 in os_aix.cpp again , if
> > > the condition fails for .so files, because i have to reload it again and
> > > check if the .a exists. In the shared code i had repeat less number of
> > > lines i believe. Do you suggest moving lines 1132 to 1139 to another
> > > function then ?
> >
> >
> > @tstuefe Any suggestion on this ?
>
> ```
> --- a/src/hotspot/os/aix/os_aix.cpp
> +++ b/src/hotspot/os/aix/os_aix.cpp
> @@ -1108,7 +1108,7 @@ bool os::dll_address_to_library_name(address addr,
> char* buf,
>return true;
> }
>
> -void *os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> +static void* dll_load_inner(const char *filename, char *ebuf, int ebuflen) {
>
>log_info(os)("attempting shared library load of %s", filename);
>
> @@ -1158,6 +1158,35 @@ void *os::dll_load(const char *filename, char *ebuf,
> int ebuflen) {
>return nullptr;
> }
>
> +void* os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> +
> + void* result = nullptr;
> +
> + // First try using *.so suffix; failing that, retry with *.a suffix.
> + const size_t len = strlen(filename);
> + constexpr size_t safety = 3 + 1;
> + constexpr size_t bufsize = len + safety;
> + char* buf = NEW_C_HEAP_ARRAY(char, bufsize, mtInternal);
> + strcpy(buf, filename);
> + char* const dot = strrchr(buf, '.');
> +
> + assert(dot != nullptr, "Attempting to load a shared object without
> extension? %s", filename);
> + assert(strcmp(dot, ".a") == 0 || strcmp(dot, ".so") == 0,
> + "Attempting to load a shared object that is neither *.so nor *.a",
> filename);
> +
> + sprintf(dot, ".so");
> + result = dll_load_inner(buf, ebuf, ebuflen);
> +
> + if (result == nullptr) {
> +sprintf(dot, ".a");
> +result = dll_load_inner(buf, ebuf, ebuflen);
> + }
> +
> + FREE_C_HEAP_ARRAY(char, buf);
> +
> + return result;
> +}
> +
> ```
@tstuefe as discussed with @TheRealMDoerr do you think using default argument
will help ?
Either we pass agent object as 3rd parameter or an empty character buffer(and
not const chat*) which would be spcifically used to copy the alternate
filename to it using strcpy so that it is reflected in the jvmagent code ?
-
PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1828086180
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Fri, 24 Nov 2023 14:04:07 GMT, Thomas Stuefe wrote:
> > > i would have to repeat the line 1132 and 1139 in os_aix.cpp again , if
> > > the condition fails for .so files, because i have to reload it again and
> > > check if the .a exists. In the shared code i had repeat less number of
> > > lines i believe. Do you suggest moving lines 1132 to 1139 to another
> > > function then ?
> >
> >
> > @tstuefe Any suggestion on this ?
>
> ```
> --- a/src/hotspot/os/aix/os_aix.cpp
> +++ b/src/hotspot/os/aix/os_aix.cpp
> @@ -1108,7 +1108,7 @@ bool os::dll_address_to_library_name(address addr,
> char* buf,
>return true;
> }
>
> -void *os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> +static void* dll_load_inner(const char *filename, char *ebuf, int ebuflen) {
>
>log_info(os)("attempting shared library load of %s", filename);
>
> @@ -1158,6 +1158,35 @@ void *os::dll_load(const char *filename, char *ebuf,
> int ebuflen) {
>return nullptr;
> }
>
> +void* os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> +
> + void* result = nullptr;
> +
> + // First try using *.so suffix; failing that, retry with *.a suffix.
> + const size_t len = strlen(filename);
> + constexpr size_t safety = 3 + 1;
> + constexpr size_t bufsize = len + safety;
> + char* buf = NEW_C_HEAP_ARRAY(char, bufsize, mtInternal);
> + strcpy(buf, filename);
> + char* const dot = strrchr(buf, '.');
> +
> + assert(dot != nullptr, "Attempting to load a shared object without
> extension? %s", filename);
> + assert(strcmp(dot, ".a") == 0 || strcmp(dot, ".so") == 0,
> + "Attempting to load a shared object that is neither *.so nor *.a",
> filename);
> +
> + sprintf(dot, ".so");
> + result = dll_load_inner(buf, ebuf, ebuflen);
> +
> + if (result == nullptr) {
> +sprintf(dot, ".a");
> +result = dll_load_inner(buf, ebuf, ebuflen);
> + }
> +
> + FREE_C_HEAP_ARRAY(char, buf);
> +
> + return result;
> +}
> +
> ```
Hi @tstuefe
Thanks for sharing!.
i have worked on an alternate code taking insight from your code.
There are a couple of issues being faced, but the most improtant one is the
segmentation fault that is occuring.
1. The dll_load function makes copy of filename and appends .a and then tries
to load the library.
However , in jvmTiAgent.cpp, we call save_library_signature method.
Since the changes to filename are happening only inside dlll_load, it is not
reflected in jvmTiAgent, due to which stat64 command is run on .old ".so"
filename and not the ".a" filename.
So stat64 function call happens with invalid library name and hence it causes
segmentation fault.
To make the changes reflected, we might need to do strcpy of modified
filename(buffer) to filename inside dll_load , but that is not possible as it
is const char*. I dont think we can/should change the signature of dll_load
here.
2. I tried moving save_library_signature from jvmTiAgent to os_aix but then it
also needs the "agent" object to be passed as well, which will lead to change
in signature of dll_load.
What do you think could be a solution for this ?
-
PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1827661214
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Fri, 24 Nov 2023 14:04:07 GMT, Thomas Stuefe wrote:
> > > i would have to repeat the line 1132 and 1139 in os_aix.cpp again , if
> > > the condition fails for .so files, because i have to reload it again and
> > > check if the .a exists. In the shared code i had repeat less number of
> > > lines i believe. Do you suggest moving lines 1132 to 1139 to another
> > > function then ?
> >
> >
> > @tstuefe Any suggestion on this ?
>
> ```
> --- a/src/hotspot/os/aix/os_aix.cpp
> +++ b/src/hotspot/os/aix/os_aix.cpp
> @@ -1108,7 +1108,7 @@ bool os::dll_address_to_library_name(address addr,
> char* buf,
>return true;
> }
>
> -void *os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> +static void* dll_load_inner(const char *filename, char *ebuf, int ebuflen) {
>
>log_info(os)("attempting shared library load of %s", filename);
>
> @@ -1158,6 +1158,35 @@ void *os::dll_load(const char *filename, char *ebuf,
> int ebuflen) {
>return nullptr;
> }
>
> +void* os::dll_load(const char *filename, char *ebuf, int ebuflen) {
> +
> + void* result = nullptr;
> +
> + // First try using *.so suffix; failing that, retry with *.a suffix.
> + const size_t len = strlen(filename);
> + constexpr size_t safety = 3 + 1;
> + constexpr size_t bufsize = len + safety;
> + char* buf = NEW_C_HEAP_ARRAY(char, bufsize, mtInternal);
> + strcpy(buf, filename);
> + char* const dot = strrchr(buf, '.');
> +
> + assert(dot != nullptr, "Attempting to load a shared object without
> extension? %s", filename);
> + assert(strcmp(dot, ".a") == 0 || strcmp(dot, ".so") == 0,
> + "Attempting to load a shared object that is neither *.so nor *.a",
> filename);
> +
> + sprintf(dot, ".so");
> + result = dll_load_inner(buf, ebuf, ebuflen);
> +
> + if (result == nullptr) {
> +sprintf(dot, ".a");
> +result = dll_load_inner(buf, ebuf, ebuflen);
> + }
> +
> + FREE_C_HEAP_ARRAY(char, buf);
> +
> + return result;
> +}
> +
> ```
Thanks for sharing !
I have worked on an alternate code as well, takling some lessons from your
code.
I see a couple of issues
**Issue 1.**
assert(strcmp(dot, ".a") == 0 || strcmp(dot, ".so") == 0,
+ "Attempting to load a shared object that is neither *.so nor *.a",
filename);
This fails as we have paths such as /usr/lib/libc.a(shr_64.0) .
However this check is already done in dll_load_inner already.
**Issue 2 :**
After calling dll_load_inner for .so ,after appending ".a" to filename, i face
a segmentation fault. On looking into it further, i see the dlopen succeeds,
but it is failing in stat64 being done using save_signature method for AIX.
I even tried by just supplying a string "libam_ibm_16.a" without doing any
string operations and i still see the issue.
Does this look familiar ?
#
# A fatal error has been detected by the Java Runtime Environment:
#
# Internal Error
(/home/hotspot/openjdk/jdk-suchi/jdk/src/hotspot/share/prims/jvmtiAgent.cpp:307),
pid=31719930, tid=258
# assert(false) failed: stat64x failed
#
# JRE version: (22.0) (fastdebug build )
# Java VM: OpenJDK 64-Bit Server VM (fastdebug 22-internal-adhoc.hotspot.jdk,
mixed mode, tiered, compressed oops, compressed class ptrs, g1 gc, aix-ppc64)
# Core dump will be written. Default location:
/home/hotspot/openjdk/jdk-suchi/j2se/j2se_dc/.gdc/7.3.0.15.0/runtime/j2secheckapp.p8-java1-hs01.checkapp/core
or core.31719930
#
# An error report file with more information is saved as:
#
/home/hotspot/openjdk/jdk-suchi/j2se/j2se_dc/.gdc/7.3.0.15.0/runtime/j2secheckapp.p8-java1-hs01.checkapp/hs_err_pid31719930.log
#
#
-
PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1827513022
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Fri, 24 Nov 2023 11:45:25 GMT, suchismith1993 wrote:
> > i would have to repeat the line 1132 and 1139 in os_aix.cpp again , if the
> > condition fails for .so files, because i have to reload it again and check
> > if the .a exists. In the shared code i had repeat less number of lines i
> > believe. Do you suggest moving lines 1132 to 1139 to another function then ?
>
> @tstuefe Any suggestion on this ?
--- a/src/hotspot/os/aix/os_aix.cpp
+++ b/src/hotspot/os/aix/os_aix.cpp
@@ -1108,7 +1108,7 @@ bool os::dll_address_to_library_name(address addr, char*
buf,
return true;
}
-void *os::dll_load(const char *filename, char *ebuf, int ebuflen) {
+static void* dll_load_inner(const char *filename, char *ebuf, int ebuflen) {
log_info(os)("attempting shared library load of %s", filename);
@@ -1158,6 +1158,35 @@ void *os::dll_load(const char *filename, char *ebuf, int
ebuflen) {
return nullptr;
}
+void* os::dll_load(const char *filename, char *ebuf, int ebuflen) {
+
+ void* result = nullptr;
+
+ // First try using *.so suffix; failing that, retry with *.a suffix.
+ const size_t len = strlen(filename);
+ constexpr size_t safety = 3 + 1;
+ constexpr size_t bufsize = len + safety;
+ char* buf = NEW_C_HEAP_ARRAY(char, bufsize, mtInternal);
+ strcpy(buf, filename);
+ char* const dot = strrchr(buf, '.');
+
+ assert(dot != nullptr, "Attempting to load a shared object without
extension? %s", filename);
+ assert(strcmp(dot, ".a") == 0 || strcmp(dot, ".so") == 0,
+ "Attempting to load a shared object that is neither *.so nor *.a",
filename);
+
+ sprintf(dot, ".so");
+ result = dll_load_inner(buf, ebuf, ebuflen);
+
+ if (result == nullptr) {
+sprintf(dot, ".a");
+result = dll_load_inner(buf, ebuf, ebuflen);
+ }
+
+ FREE_C_HEAP_ARRAY(char, buf);
+
+ return result;
+}
+
-
PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1825721906
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Thu, 23 Nov 2023 06:03:15 GMT, Thomas Stuefe wrote:
>> suchismith1993 has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> change macro position
>
> src/hotspot/os/aix/os_aix.cpp line 3065:
>
>> 3063: //Replaces provided path with alternate path for the given file,if it
>> doesnt exist.
>> 3064: //For AIX,this replaces .so with .a.
>> 3065: void os::Aix::mapAlternateName(char* buffer, const char *extension) {
>
> The documentation is wrong:
>
> // Replaces the specified path with an alternative path for the
>given file if the original path doesn't exist
>
> It does no such thing; it replaces the extension unconditionally. The comment
> sounds like it does a file system check.
>
> The whole function is not that well named - "map alternate name" does not
> really tell me anything, I need to look at the implementation and the caller
> to understand what it is doing. There is no mapping here, this is just a
> string utility function.
>
> The function should not modify the original buffer but instead assemble a
> copy. That is the conventional way to do these things. You can work with
> immutable strings as input, e.g. literals, and don't risk buffer overflows.
>
> All of this should be handled inside os_aix.cpp; see my other comment. This
> should not live in the external os::aix interface, since it has nothing to do
> with AIX. But I think all of this should be confined to os_aix.cpp.
>
> Proposal for a clearer name, comment, and pseudocode
>
> // Given a filename with an extension, return a new string containing the
> filename with the new extension.
> // New string is allocated in resource area.
> static char* replace_extension_in_filename(const char* filename, const char*
> new_extension) {
> - allocate buffer in RA
> - assemble new path by contacting old path - old extension + new extension
> - return new path
> }
thank you for the explanation. I am working on it.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1404265530
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Thu, 23 Nov 2023 18:26:56 GMT, suchismith1993 wrote: > > > > I'm not a big fan of this approach. We accumulate more and more "#ifdef > > > > AIX" in shared code because of many recent AIX additions. No other > > > > platform has such a large ifdef footprint in shared code. > > > > I argue that all of this should be handled inside os_aix.cpp and not > > > > leak out into the external space: > > > > If .a is a valid shared object format on AIX, this should be handled in > > > > `os::dll_load()`, and be done for all shared objects. If not, why do we > > > > try to load a static archive via dlload in this case but not in other > > > > cases? > > > > _If_ this is needed in shared code, the string replacement function > > > > should be a generic utility function for all platforms, and it should > > > > be tested with a small gtest. A gtest would have likely uncovered the > > > > buffer overflow too. > > > > > > > > > So i tried to check how to move the code to os_aix file. A few problems > > > is see : > > > > > > 1. When i have to implemented the logic at dll_load function, i would > > > have to repeat a lot of code after dlopen, i.e i have to call dlopen > > > again for .so files and hence i have to copy the logic again for it. > > > 2. Currently using function before dll_load,in the shared code makes this > > > a bit easier. > > >I have an alternate suggestion . > > >Shall we declare the utlity function as part of os ? and implement it > > > platform wise. > > > > > > Not without any need. If this is an AIX specific issue, it should be handed > > in os::dll_load on AIX. > > > In that way we just keep the actual implentation and aix and in windows > > > and linux we keep it empty. > > > So that way we can just call the utility function in shared code and it > > > wouldnt affect other platform and will run the usecase for AIX. > > > If that is not acceptable, then is there a better way to avoid repeating > > > the dlopen again in os_aix file ? > > > > > > I don't understand the problem. What is preventing you from using a file > > local scope utility function inside os::dll_load? > > i would have to repeat the line 1132 and 1139 in os_aix.cpp again , if the > condition fails for .so files, because i have to reload it again and check if > the .a exists. In the shared code i had repeat less number of lines i > believe. Do you suggest moving lines 1132 to 1139 to another function then ? @tstuefe Any suggestion on this ? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1825557400
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Thu, 23 Nov 2023 18:03:33 GMT, Thomas Stuefe wrote: > > > I'm not a big fan of this approach. We accumulate more and more "#ifdef > > > AIX" in shared code because of many recent AIX additions. No other > > > platform has such a large ifdef footprint in shared code. > > > I argue that all of this should be handled inside os_aix.cpp and not leak > > > out into the external space: > > > If .a is a valid shared object format on AIX, this should be handled in > > > `os::dll_load()`, and be done for all shared objects. If not, why do we > > > try to load a static archive via dlload in this case but not in other > > > cases? > > > _If_ this is needed in shared code, the string replacement function > > > should be a generic utility function for all platforms, and it should be > > > tested with a small gtest. A gtest would have likely uncovered the buffer > > > overflow too. > > > > > > So i tried to check how to move the code to os_aix file. A few problems is > > see : > > > > 1. When i have to implemented the logic at dll_load function, i would have > > to repeat a lot of code after dlopen, i.e i have to call dlopen again for > > .so files and hence i have to copy the logic again for it. > > 2. Currently using function before dll_load,in the shared code makes this a > > bit easier. > >I have an alternate suggestion . > >Shall we declare the utlity function as part of os ? and implement it > > platform wise. > > Not without any need. If this is an AIX specific issue, it should be handed > in os::dll_load on AIX. > > > In that way we just keep the actual implentation and aix and in windows and > > linux we keep it empty. > > So that way we can just call the utility function in shared code and it > > wouldnt affect other platform and will run the usecase for AIX. > > If that is not acceptable, then is there a better way to avoid repeating > > the dlopen again in os_aix file ? > > I don't understand the problem. What is preventing you from using a file > local scope utility function inside os::dll_load? i would have to repeat the line 1132 and 1139 in os_aix.cpp again , if the condition fails for .so files, because i have to reload it again and check if the .a exists. In the shared code i had repeat less number of lines i believe. Do you suggest moving lines 1132 to 1139 to another function then ? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1824804477
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Thu, 23 Nov 2023 17:05:29 GMT, suchismith1993 wrote: > > I'm not a big fan of this approach. We accumulate more and more "#ifdef > > AIX" in shared code because of many recent AIX additions. No other platform > > has such a large ifdef footprint in shared code. > > I argue that all of this should be handled inside os_aix.cpp and not leak > > out into the external space: > > If .a is a valid shared object format on AIX, this should be handled in > > `os::dll_load()`, and be done for all shared objects. If not, why do we try > > to load a static archive via dlload in this case but not in other cases? > > _If_ this is needed in shared code, the string replacement function should > > be a generic utility function for all platforms, and it should be tested > > with a small gtest. A gtest would have likely uncovered the buffer overflow > > too. > > So i tried to check how to move the code to os_aix file. A few problems is > see : > > 1. When i have to implemented the logic at dll_load function, i would have to > repeat a lot of code after dlopen, i.e i have to call dlopen again for .so > files and hence i have to copy the logic again for it. > 2. Currently using function before dll_load,in the shared code makes this a > bit easier. >I have an alternate suggestion . >Shall we declare the utlity function as part of os ? and implement it > platform wise. Not without any need. If this is an AIX specific issue, it should be handed in os::dll_load on AIX. >In that way we just keep the actual implentation and aix and in windows > and linux we keep it empty. >So that way we can just call the utility function in shared code and it > wouldnt affect other platform and will run the usecase for AIX. >If that is not acceptable, then is there a better way to avoid repeating > the dlopen again in os_aix file ? I don't understand the problem. What is preventing you from using a file local scope utility function inside os::dll_load? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1824787258
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Wed, 22 Nov 2023 16:24:24 GMT, suchismith1993 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. > > suchismith1993 has updated the pull request incrementally with one additional > commit since the last revision: > > change macro position > I'm not a big fan of this approach. We accumulate more and more "#ifdef AIX" > in shared code because of many recent AIX additions. No other platform has > such a large ifdef footprint in shared code. > > I argue that all of this should be handled inside os_aix.cpp and not leak out > into the external space: > > If .a is a valid shared object format on AIX, this should be handled in > `os::dll_load()`, and be done for all shared objects. If not, why do we try > to load a static archive via dlload in this case but not in other cases? > > _If_ this is needed in shared code, the string replacement function should be > a generic utility function for all platforms, and it should be tested with a > small gtest. A gtest would have likely uncovered the buffer overflow too. So i tried to check how to move the code to os_aix file. A few problems is see : 1. When i have to implemented the logic at dll_load function, i would have to repeat a lot of code after dlopen, i.e i have to call dlopen again for .so files and hence i have to copy the logic again for it. 2. Currently using function before dll_load,in the shared code makes this a bit easier. I have an alternate suggestion . Shall we declare the utlity function as part of os ? and implement it platform wise. In that way we just keep the actual implentation and aix and in windows and linux we keep it empty. So that way we can just call the utility function in shared code and it wouldnt affect other platform and will run the usecase for AIX. If that is not acceptable, then is there a better way to avoid repeating the dlopen again in os_aix file ? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1824737158
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Thu, 23 Nov 2023 06:16:54 GMT, Thomas Stuefe wrote: > If .a is a valid shared object format on AIX, this should be handled in > `os::dll_load()`, and be done for all shared objects. If not, why do we try > to load a static archive via dlload in this case but not in other cases? In AIX, we have shared objects with .a extension and also static archives with .a extension.I think in other platforms the format for shared objects is fixed? . In that case does this become specific to AIX? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1824086655
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Thu, 23 Nov 2023 08:08:51 GMT, suchismith1993 wrote: > The JBS issue with respect to that has been closed. Need to check if that PR > is required. Currently putting it on hold. This response on the issue suggest otherwise: : The JDK does not support dynamically loaded archive files (.a files) and there are no plans to add this support. Closing as will not fix - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1824009424
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Thu, 23 Nov 2023 05:49:41 GMT, Amit Kumar wrote: > #16490 The JBS issue with respect to that has been closed. Need to check if that PR is required. Currently putting it on hold. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1823952398
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Thu, 23 Nov 2023 05:55:28 GMT, Thomas Stuefe wrote:
>> suchismith1993 has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> change macro position
>
> src/hotspot/os/aix/os_aix.cpp line 3069:
>
>> 3067: while (end > 0 && buffer[end] != '.') {
>> 3068: end = end - 1;
>> 3069: }
>
> Use strrchr.
Pls handle the case where the string contains no dot.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1402941387
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Wed, 22 Nov 2023 16:24:24 GMT, suchismith1993 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.
>
> suchismith1993 has updated the pull request incrementally with one additional
> commit since the last revision:
>
> change macro position
I'm not a big fan of this approach. We accumulate more and more "#ifdef AIX" in
shared code because of many recent AIX additions. No other platform has such a
large ifdef footprint in shared code.
I argue that all of this should be handled inside os_aix.cpp and not leak out
into the external space:
If .a is a valid shared object format on AIX, this should be handled in
`os::dll_load()`, and be done for all shared objects. If not, why do we try to
load a static archive via dlload in this case but not in other cases?
*If* this is needed in shared code, the string replacement function should be a
generic utility function for all platforms, and it should be tested with a
small gtest. A gtest would have likely uncovered the buffer overflow too.
src/hotspot/os/aix/os_aix.cpp line 3065:
> 3063: //Replaces provided path with alternate path for the given file,if it
> doesnt exist.
> 3064: //For AIX,this replaces .so with .a.
> 3065: void os::Aix::mapAlternateName(char* buffer, const char *extension) {
The documentation is wrong:
// Replaces the specified path with an alternative path for the given file if
the original path doesn't exist
It does no such thing, it replaces the extension unconditionally. The comment
sounds like it does a file system check. That does not happen here.
The whole function is not well named - "map alternate name" does not really
tell me anything, I need to look at the implementation and the caller to
understand what it is doing. There is no mapping here, this is just a string
utility function.
The function should not modify the original buffer but instead assemble a copy.
That is the conventional way to do these things. You can work with immutable
strings as input, e.g. literals, and don't risk buffer overflows.
All of this should be handled inside os_aix.cpp; see my other comment. This
should not live in the external os::aix interface, since it has nothing to do
with AIX. *If* this is needed in generic code, which I don't think, then this
should be made generic utility API, available on all platforms, and with a
small gtest.
But I think all of this should be confined to os_aix.cpp.
Proposal for a clearer name, comment, and pseudocode
// Given a filename with an extension, return a new string containing the
filename with the new extension.
// New string is allocated in resource area.
static char* replace_extension_in_filename(const char* filename, const char*
new_extension) {
- allocate buffer in RA
- assemble new path by contacting old path - old extension + new extension
- return new path
}
src/hotspot/os/aix/os_aix.cpp line 3069:
> 3067: while (end > 0 && buffer[end] != '.') {
> 3068: end = end - 1;
> 3069: }
Use strrchr.
src/hotspot/os/aix/os_aix.cpp line 3072:
> 3070: buffer[end] = '\0';
> 3071: strcat(buffer, extension);
> 3072: }
This is a buffer overrun waiting to happen if replacement is larger than
original extension.
-
Changes requested by stuefe (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/16604#pullrequestreview-1745743102
PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1402944936
PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1402941240
PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1402941730
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Wed, 22 Nov 2023 16:24:24 GMT, suchismith1993 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. > > suchismith1993 has updated the pull request incrementally with one additional > commit since the last revision: > > change macro position A couple of comments. First, if you always want to look for `libfoo.a` if you can't find `libfoo.so` then maybe that should be handled as the `os::dll_load` level? Otherwisae it is not clear why this is something you only do for agents. ?/ Second, the amount of AIX-specific code in the shared `jvmtiAgent.cpp` now seems unreasonable. As I think @tstuefe mentioned in another PR perhaps it is time to find better abstractions here to hide all these AIX specific quirks? src/hotspot/os/aix/os_aix.cpp line 3071: > 3069: } > 3070: buffer[end] = '\0'; > 3071: strcat(buffer, extension); At some point you need to check the length of extension won't overflow buffer. - PR Review: https://git.openjdk.org/jdk/pull/16604#pullrequestreview-1745744207 PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1402941846
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Wed, 22 Nov 2023 16:24:24 GMT, suchismith1993 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. > > suchismith1993 has updated the pull request incrementally with one additional > commit since the last revision: > > change macro position Also are you planning to close this one : https://github.com/openjdk/jdk/pull/16490 ? JBS issue is already closed. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1823849323
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Wed, 22 Nov 2023 16:24:24 GMT, suchismith1993 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.
>
> suchismith1993 has updated the pull request incrementally with one additional
> commit since the last revision:
>
> change macro position
some nits you might want to consider.
src/hotspot/os/aix/os_aix.cpp line 3064:
> 3062:
> 3063: //Replaces provided path with alternate path for the given file,if it
> doesnt exist.
> 3064: //For AIX,this replaces .so with .a.
Suggestion:
// Replaces the specified path with an alternative path for the given file if
the original path doesn't exist.
// For AIX, this replaces extension from ".so" to ".a".
src/hotspot/os/aix/os_aix.cpp line 3065:
> 3063: //Replaces provided path with alternate path for the given file,if it
> doesnt exist.
> 3064: //For AIX,this replaces .so with .a.
> 3065: void os::Aix::mapAlternateName(char* buffer, const char *extension) {
Suggestion:
void os::Aix::map_alternate_name(char* buffer, const char *extension) {
src/hotspot/os/aix/os_aix.hpp line 181:
> 179: static int stat64x_via_LIBPATH(const char* path, struct stat64x* stat);
> 180: // Provide alternate path name,if file does not exist.
> 181: static void mapAlternateName(char* buffer, const char *extension);
Suggestion:
// Provides alternate path name, if file does not exist.
static void map_alternate_name(char* buffer, const char *extension);
-
Changes requested by amitkumar (Committer).
PR Review: https://git.openjdk.org/jdk/pull/16604#pullrequestreview-1745734586
PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1402935976
PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1402936171
PR Review Comment: https://git.openjdk.org/jdk/pull/16604#discussion_r1402936497
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Wed, 22 Nov 2023 16:35:36 GMT, Thomas Stuefe wrote: > Hi, is this patch meant for review already? If yes, could you please describe > the problem you fix, and how you fix it? If no, I suggest working on it in > draft state till its ready for review. I have updated the description. Let me know if anything is missing - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1823834982
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Wed, 22 Nov 2023 16:24:24 GMT, suchismith1993 wrote: >> JDK-8320005 : Native library suffix impact on hotspot code in AIX > > suchismith1993 has updated the pull request incrementally with one additional > commit since the last revision: > > change macro position Hi, is this patch meant for review already? If yes, could you please describe the problem you fix, and how you fix it? If no, I suggest working on it in draft state till its ready for review. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1823105203
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
> JDK-8320005 : Native library suffix impact on hotspot code in AIX suchismith1993 has updated the pull request incrementally with one additional commit since the last revision: change macro position - Changes: - all: https://git.openjdk.org/jdk/pull/16604/files - new: https://git.openjdk.org/jdk/pull/16604/files/6fdfba81..077083d2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16604=01 - incr: https://webrevs.openjdk.org/?repo=jdk=16604=00-01 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
