RFR: JDK-8315706: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java real fix for failure on AIX [v3]

Christoph Langer clanger at openjdk.org
Thu Sep 14 10:37:45 UTC 2023


On Thu, 14 Sep 2023 09:20:24 GMT, Joachim Kern <jkern at openjdk.org> wrote:

>> After push of [JDK-8307478](https://bugs.openjdk.org/browse/JDK-8307478) , the following test started to fail on AIX :
>> com/sun/tools/attach/warnings/DynamicLoadWarningTest.java;
>> The problem was described in [JDK-8309549](https://bugs.openjdk.org/browse/JDK-8309549) with a first try of a fix.
>> A second fix via [JDK-8310191](https://bugs.openjdk.org/browse/JDK-8310191) was necessary.
>> Both fixes just disable the specific subtest on AIX, without correction of the root cause.
>> The root cause is, that dlopen() on AIX returns different handles every time, even if you load a library twice. There is no official AIX API available to get this information on a different way.
>> My proposal is, to use the stat64x API with the fields st_device and st_inode. After a dlopen() the stat64x() API is called additionally to get this information which is then stored parallel to the library handle in the jvmtiAgent. For AIX we then can compare these values instead of the library handle and get the same functionality as on linux.
>
> Joachim Kern has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8315706
>  - try to improve code following Davids suggestions and do some cosmetic changes
>  - JDK-8315706

Just a few indentation remarks.

Could you please also update the SAP copyright year in src/hotspot/os/aix/os_aix.hpp?

src/hotspot/os/aix/os_aix.cpp line 3062:

> 3060:   size_t libpathlen = strlen(env);
> 3061:   char* libpath = NEW_C_HEAP_ARRAY(char, libpathlen + 1, mtServiceability);
> 3062:   char* combined = NEW_C_HEAP_ARRAY(char, libpathlen + strlen(path) +1, mtServiceability);

Space between +1
Suggestion:

  char* combined = NEW_C_HEAP_ARRAY(char, libpathlen + strlen(path) + 1, mtServiceability);

src/hotspot/os/aix/os_aix.cpp line 3065:

> 3063:   char *saveptr, *token;
> 3064:   strcpy(libpath, env);
> 3065:   for( token = strtok_r(libpath, ":", &saveptr); token != nullptr; token = strtok_r(nullptr, ":", &saveptr) ) {

spaces:
Suggestion:

  for (token = strtok_r(libpath, ":", &saveptr); token != nullptr; token = strtok_r(nullptr, ":", &saveptr)) {

src/hotspot/share/prims/jvmtiAgent.cpp line 306:

> 304:     agent->set_device(libstat.st_dev);
> 305:   }
> 306:   else {

Fix style: 
Suggestion:

  } else {

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

PR Review: https://git.openjdk.org/jdk/pull/15583#pullrequestreview-1626565423
PR Review Comment: https://git.openjdk.org/jdk/pull/15583#discussion_r1325746854
PR Review Comment: https://git.openjdk.org/jdk/pull/15583#discussion_r1325746078
PR Review Comment: https://git.openjdk.org/jdk/pull/15583#discussion_r1325747780


More information about the serviceability-dev mailing list