8348663: [AIX] clang pollutes the burned-in library search paths of the generated executables #23313

JoKern65 · 2025-01-27T10:51:20Z

The clang compiler which we use since jdk21 pollutes the burned-in library search paths of the generated executables.
e.g. a dump -X64 -H ... /jdk-23.0.1+11-jre/lib/libnio.so

amongst others produces the following lines

                    ***Import File Strings***

INDEX PATH BASE MEMBER
0 /opt/IBM/openxlC/17.1.1/../../../../usr/lpp/xlC/lib:/opt/IBM/openxlC/17.1.1/lib:/opt/IBM/openxlC/17.1.1/bin/../../../../../usr/lpp/xlC/lib:/usr/opt/zlibNX/lib:/opt/freeware/lib/pthread:/opt/freeware/lib:/usr/lib

The long string is the burned in library search path which will be used after searching with LIBPATH envvar.
You see there are some compiler paths which customers will not have; also /opt/freeware is unwanted.
A proper executable should only have the burned in default search path
/usr/lib:lib

To ensure this we want to call the linker with the flag
-fuse-ld=our-wrapper-script-cleaning-LIBPATH- and-calling-original-ld-afterwards
This script we want to deliver in make/scripts/aix/ld.sh
just containing
#!/bin/bash
unset LIBPATH
exec /usr/bin/ld "$@"
return $?

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8348663: [AIX] clang pollutes the burned-in library search paths of the generated executables (Bug - P4)

Reviewers

Magnus Ihse Bursie (@magicus - Reviewer)
Matthias Baesken (@MBaesken - Reviewer) Review applies to 46a2e67e
Christoph Langer (@RealCLanger - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/23313/head:pull/23313
$ git checkout pull/23313

Update a local copy of the PR:
$ git checkout pull/23313
$ git pull https://git.openjdk.org/jdk.git pull/23313/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23313

View PR using the GUI difftool:
$ git pr show -t 23313

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23313.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-01-27T10:51:52Z

👋 Welcome back jkern! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-01-27T10:53:04Z

@JoKern65 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8348663: [AIX] clang pollutes the burned-in library search paths of the generated executables

Reviewed-by: ihse, clanger, mbaesken

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 46 new commits pushed to the master branch:

d985b31: 8342096: Popup menus that request focus are not shown on Linux with Wayland
cbe9ec5: 8348905: Add support to specify the JDK for compiling Jtreg tests
6b581d2: 8347997: assert(false) failed: EA: missing memory path
4662363: 8348687: [BACKOUT] C2: Non-fluid StringBuilder pattern bails out in OptoStringConcat
d266ca9: 8348752: Enable -XX:+AOTClassLinking by default when -XX:AOTMode is specified
cbc89a7: 8348898: Remove unused OctalDigits to clean up code
96fefed: 8319850: PrintInlining should print which methods are late inlines
51cce6e: 8318577: Windows Look-and-Feel JProgressBarUI does not render correctly on 2x UI scale
6bfae3a: 8333386: TestAbortOnVMOperationTimeout test fails for client VM
f98d9a3: 8348870: Eliminate array bound checks in DecimalDigits
... and 36 more: https://git.openjdk.org/jdk/compare/afcc2b03afc77f730300e1d92471466d56ed75fb...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-01-27T10:53:45Z

@JoKern65 The following label will be automatically applied to this pull request:

build

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

JoKern65 · 2025-01-28T14:59:22Z

@magicus Hi Magnus, I'm faced with the problem that I cannot check in an executable script. On the other side the compilers -fuse_ld parameter needs an executable script. Is there a way to circumvent this.
The only fallback I'm aware of would be to keep the script local and just commit a configure parameter which provides the path to the local script setting then -fuse-ld accordingly.
Are there other solutions possible?

magicus · 2025-01-29T09:35:05Z

This look like a very hacky workaround overall. Can't you just unset LIBPATH in spec.gmk? Or just before linking? Is this a documented behavior of the linker on AIX, or a bug?

mlbridge · 2025-01-29T09:50:57Z

Webrevs

JoKern65 · 2025-01-29T09:59:05Z

Unfortunately, there is no other way. The LIBPATH envvar during our build is already unset. The compiler itself, when called as the Linker is setting some paths into LIBPATH before he calls the real Linker by himself. This is the only way to get rid of the unwanted burned in paths. This was an advice from IBM to do so as a standard trick.
I now checked in the script without x-Bit. Then during configure call it is copied to the build directory, where the x-Bit is set. -fuse-ld= Parameter points now to this copied version.
If you are reluctant with this solution, we additionally can make it dependent from a new configure flag which we would set and nobody else.
By the way, if we try to address this as a possible bug and this is very unlikely accepted by IBM, it will take at least half a year to get a change. And because this would change the behavior of the compiler I'm pretty sure this will only be included in a new Compiler not yet shipped.

magicus · 2025-01-29T10:13:24Z

I still think you should report it as a bug, but I accept that we need to have a workaround in the meantime (or forever, if it doesn't get fixed).

However, you need to shape up the patch a bit first.

$OUTPUTDIR points to the current configure dir, use that instead.
You should use the detected versions of cp and chmod as $CP and $CMHOD.
You should not copy files in flags-ldflags.m4, that parts needs to find a better home.
The return $? part in the script is useless since exec will make the ld process replace the shell script.

magicus · 2025-01-29T10:14:04Z

I'm not sure what is a better place. Perhaps BASIC_POST_CONFIG_OUTPUT where we copy/chmod other files, even if this one is not strictly necessary to do late in the configure process.

JoKern65 · 2025-01-29T11:46:25Z

I followed all of Magnus proposals:
I used $OUTPUTDIR instead of ${TOPDIR}/build/${CONF_NAME}.
I replaced cp and chmod by $CP and $CMHOD.
I moved the copy and mode bit change from flags-ldflags.m4 to BASIC_POST_CONFIG_OUTPUT in basic.m4 .
I removed the useless return $? from my script.

magicus · 2025-01-29T11:47:23Z

make/autoconf/basic.m4

+
+  # Copy the linker wrapper script for AIX' clang and make it executable
+  if test "x$TOOLCHAIN_TYPE" = xclang && test "x$OPENJDK_TARGET_OS" = xaix &&
+     test -e "${TOPDIR}/make/scripts/aix/ld.sh"; then


You can assume that checked-in files exist and do not need to test for them.

magicus · 2025-01-29T11:48:46Z

make/autoconf/basic.m4

+  # Copy the linker wrapper script for AIX' clang and make it executable
+  if test "x$TOOLCHAIN_TYPE" = xclang && test "x$OPENJDK_TARGET_OS" = xaix &&
+     test -e "${TOPDIR}/make/scripts/aix/ld.sh"; then
+    $CP -f "${TOPDIR}/make/scripts/aix/ld.sh" "$OUTPUTDIR/ld.sh"


Suggested change

$CP -f "${TOPDIR}/make/scripts/aix/ld.sh" "$OUTPUTDIR/ld.sh"

$CP -f "$TOPDIR/make/scripts/aix/ld.sh" "$OUTPUTDIR/ld.sh"

We normally prefer to use $ without {} unless they are necessary, but in some parts of the code the ${} style is prevalent for historical reasons, and in those places it is better to keep with that style. But for code like this, you should stick to the {}-less style.

magicus · 2025-01-29T12:23:39Z

make/autoconf/basic.m4

  # Make the compare script executable
  $CHMOD +x $OUTPUTDIR/compare.sh
+
+  # Copy the linker wrapper script for AIX' clang and make it executable


Suggested change

# Copy the linker wrapper script for AIX' clang and make it executable

# Copy the linker wrapper script for clang on AIX and make it executable

magicus · 2025-01-29T12:24:18Z

Just one comment fix and then this is okay. Also please verify that the latest version still works on AIX.

JoKern65 · 2025-01-29T12:58:53Z

All done and also latest version still works on AIX.

magicus

Looks good now. Thanks!

RealCLanger · 2025-01-29T14:30:41Z

Looks good to me, too. However, you should update the copyright years in the modified files and also add a license header in the newly added script (like in other .sh files in that directory but with SAP copyright).

JoKern65 · 2025-01-29T14:41:02Z

Copyrights modified/added.

RealCLanger · 2025-01-29T15:13:11Z

make/scripts/aix/ld.sh

@@ -0,0 +1,28 @@
+#!/bin/bash
+#
+# Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.


You can leave out Oracle here.

RealCLanger

Close to perfect. 😄

JoKern65 · 2025-01-30T08:34:48Z

/integrate

openjdk · 2025-01-30T08:36:06Z

Going to push as commit e0c2cb4.
Since your change was applied there have been 48 commits pushed to the master branch:

14136f8: 8347564: ZGC: Crash in DependencyContext::clean_unloading_dependents
04c24f1: 8347779: sun/tools/jhsdb/HeapDumpTestWithActiveProcess.java fails with Unable to deduce type of thread from address
d985b31: 8342096: Popup menus that request focus are not shown on Linux with Wayland
cbe9ec5: 8348905: Add support to specify the JDK for compiling Jtreg tests
6b581d2: 8347997: assert(false) failed: EA: missing memory path
4662363: 8348687: [BACKOUT] C2: Non-fluid StringBuilder pattern bails out in OptoStringConcat
d266ca9: 8348752: Enable -XX:+AOTClassLinking by default when -XX:AOTMode is specified
cbc89a7: 8348898: Remove unused OctalDigits to clean up code
96fefed: 8319850: PrintInlining should print which methods are late inlines
51cce6e: 8318577: Windows Look-and-Feel JProgressBarUI does not render correctly on 2x UI scale
... and 38 more: https://git.openjdk.org/jdk/compare/afcc2b03afc77f730300e1d92471466d56ed75fb...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-01-30T08:36:21Z

@JoKern65 Pushed as commit e0c2cb4.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

JDK-8348663

a03af7f

openjdk bot changed the title ~~JDK-8348663: [AIX] clang pollutes the burned-in library search paths of the generated executables~~ 8348663: [AIX] clang pollutes the burned-in library search paths of the generated executables Jan 27, 2025

openjdk bot added the build build-dev@openjdk.org label Jan 27, 2025

check in script without x-Bit but copy it to build directory with x-Bit

d63cdb8

openjdk bot added the rfr Pull request is ready for review label Jan 29, 2025

following Magnus proposals

b5d402f

magicus reviewed Jan 29, 2025

View reviewed changes

cosmetic changes

d8f067c

magicus reviewed Jan 29, 2025

View reviewed changes

cosmetic changes 2

8d424cc

magicus approved these changes Jan 29, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 29, 2025

MBaesken approved these changes Jan 29, 2025

View reviewed changes

modified/added copyright headers

46a2e67

openjdk bot removed the ready Pull request is ready to be integrated label Jan 29, 2025

MBaesken approved these changes Jan 29, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 29, 2025

RealCLanger reviewed Jan 29, 2025

View reviewed changes

modified/added copyright headers again

5e6f4c0

openjdk bot removed the ready Pull request is ready to be integrated label Jan 29, 2025

RealCLanger approved these changes Jan 29, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 29, 2025

magicus approved these changes Jan 29, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Jan 30, 2025

openjdk bot closed this Jan 30, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 30, 2025

	$CP -f "${TOPDIR}/make/scripts/aix/ld.sh" "$OUTPUTDIR/ld.sh"
	$CP -f "$TOPDIR/make/scripts/aix/ld.sh" "$OUTPUTDIR/ld.sh"

	# Copy the linker wrapper script for AIX' clang and make it executable
	# Copy the linker wrapper script for clang on AIX and make it executable

8348663: [AIX] clang pollutes the burned-in library search paths of the generated executables #23313

8348663: [AIX] clang pollutes the burned-in library search paths of the generated executables #23313

Uh oh!

Conversation

JoKern65 commented Jan 27, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jan 27, 2025

Uh oh!

openjdk bot commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jan 27, 2025

Uh oh!

JoKern65 commented Jan 28, 2025

Uh oh!

magicus commented Jan 29, 2025

Uh oh!

mlbridge bot commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

JoKern65 commented Jan 29, 2025

Uh oh!

magicus commented Jan 29, 2025

Uh oh!

magicus commented Jan 29, 2025

Uh oh!

JoKern65 commented Jan 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

magicus commented Jan 29, 2025

Uh oh!

JoKern65 commented Jan 29, 2025

Uh oh!

magicus left a comment

Choose a reason for hiding this comment

Uh oh!

RealCLanger commented Jan 29, 2025

Uh oh!

JoKern65 commented Jan 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RealCLanger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoKern65 commented Jan 30, 2025

Uh oh!

openjdk bot commented Jan 30, 2025

Uh oh!

openjdk bot commented Jan 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

JoKern65 commented Jan 27, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jan 27, 2025 •

edited

Loading

mlbridge bot commented Jan 29, 2025 •

edited

Loading

RealCLanger left a comment •

edited

Loading