On Fri, 21 Mar 2025 11:44:49 GMT, Joachim Kern <[email protected]> wrote:
>> After "JDK-8339480: Build static-jdk image with a statically linked
>> launcher" AIX was not able to build the new target. Therefore with
>> "JDK-8345590 AIX 'make all' fails after JDK-8339480" the new target was
>> disabled again.
>>
>> Now with this change we can enable the statically linked launcher target
>> again.
>> There are 3 things to do.
>> 1. Modify `dladdr()`. Because this API does not exist on AIX it is
>> implemented based on the `loadquery()` API. Unfortunately, this API does
>> only return the name of the executable, but not its path. Beforehand this
>> was no problem, because we asked for a loaded library, for which the API
>> provides the path. But now we are asking for the executable itself.
>> 2. `dladdr()` is differently implemented three times in the openjdk code.
>> In the static case I supressed now the usage of the additional modules
>> containing version two and three. I know this shouldn't be the final
>> version. Magnus mentioned that they have discussed from time to time to have
>> a "basic JDK utility lib" that can be shared between hotspot and the JDK
>> libraries. I think this is a good idea for the future, but far beyond the
>> scope of this PR. The second best thing Magnus proposed is to only have the
>> `dladdr()` functionality in Hotspot and then export it. Let's see how the
>> community decides.
>> 3. Because we lack a linker flag like `whole-archive`, I had to force the
>> export of all symbols by creating export files containing all symbols of the
>> static libs. I introduced the new rule for the export file generation as
>> "raw" make recipes. Magnus claimed to use the `SetupExecute`. Unfortunately
>> I was not able to make it function. So I still have my "raw" solution in
>> place, but my last try with `SetupExecute` as comment beneath. Help is
>> welcome.
>
> Joachim Kern has updated the pull request incrementally with one additional
> commit since the last revision:
>
> follow Magnus
I'm not really familiar with it, but I have some code style requests.
src/hotspot/os/aix/loadlib_aix.cpp line 218:
> 216: procentry64 PInfo;
> 217: PInfo.pi_pid = ::getpid();
> 218: if ( 0 == ::getargs(&PInfo, sizeof(PInfo), (char*)pgmpath,PATH_MAX)
> && *pgmpath ) {
Coding style: spaces after `(` and before `)` are uncommon, but we should have
one before `PATH_MAX`.
I think hotspot style guide compliant would be:
`if (0 == ::getargs(&PInfo, sizeof(PInfo), (char*)pgmpath, PATH_MAX) &&
*pgmpath != 0)`
src/hotspot/os/aix/loadlib_aix.cpp line 221:
> 219: pgmpath[PATH_MAX] = '\0';
> 220: pgmbase = strrchr(pgmpath, '/');
> 221: if (pgmbase) {
Better: `if (pgmbase != nullptr)`
src/hotspot/os/aix/loadlib_aix.cpp line 222:
> 220: pgmbase = strrchr(pgmpath, '/');
> 221: if (pgmbase) {
> 222: pgmbase +=1;
Maybe add a space: "+= 1"?
src/hotspot/os/aix/loadlib_aix.cpp line 245:
> 243: lm->data_len = ldi->ldinfo_datasize;
> 244:
> 245: if (pgmbase && 0 == strcmp(pgmbase, ldi->ldinfo_filename)) {
`pmgbase != nullptr`
-------------
PR Review: https://git.openjdk.org/jdk/pull/24062#pullrequestreview-2706439265
PR Review Comment: https://git.openjdk.org/jdk/pull/24062#discussion_r2007864471
PR Review Comment: https://git.openjdk.org/jdk/pull/24062#discussion_r2007869385
PR Review Comment: https://git.openjdk.org/jdk/pull/24062#discussion_r2007866684
PR Review Comment: https://git.openjdk.org/jdk/pull/24062#discussion_r2007870732
