Skip to content

build: fix AIX shared library exports generation - #67359

Open
sxa wants to merge 4 commits into
nodejs:mainfrom
sxa:aix_shared_patch
Open

sxa wants to merge 4 commits into
nodejs:mainfrom
sxa:aix_shared_patch

Conversation

@sxa

@sxa sxa commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

Modifications to make the exports file available so that the addons tests work in the shared library builds:

  • Ensure test/addons/.headersbuildstamp has a dependency on the export file
  • Copies libnode.exp to node.exp
  • Links the node stub with -Wl,-bE:<PRODUCT_DIR>/node.exp to re-export the symbol set from the shared library.
  • Symbols from objects linked into libnode (e.g. gen/node_snapshot.o) could get missed as the expfile action could run before it was in place. The awk script in create_expfile.sh is abstracted out and used twice - once for the .a files as before and once for .o files. This is

Symbols which would have been missing from the previous solution that come from the snapshot object file:

Mangled Readable version / meaning
_ZN4node15SnapshotBuilder23GetEmbeddedSnapshotDataEv node::SnapshotBuilder::GetEmbeddedSnapshotData()
_ZN4node8builtins11BuiltinInfo13parameter_mapE node::builtins::BuiltinInfo::parameter_map
_ZGVN4node8builtins11BuiltinInfo13parameter_mapE Guard for parameter_map (one-time static init)
_ZNSt3__1…__do_rehash… libc++ unordered_map rehash (from generated snapshot translation unit)
_ZNSt3__1…__emplace_unique_key_args… libc++ hash table insert for builtin metadata
_ZNSt3__1…basic_string…__init_copy_ctor_external… libc++ string helper used during static setup

If there was a desire to have a minimal change in this patch we could take this part out which would also remove the need for the extra parameter for the create_expfile.sh.

The script was updated to:

  • Still use AIX dump -tov (including HIDDEN filtering).
  • Scan obj.target/<target>/*.o for the current target (e.g. libnode).
  • Accept expfile_extra_inputs so make orders the expfile step after snapshot objects exist.

Files touched

File Patch
Makefile node.exp prereq + build rule for addon/doc headers on AIX/OS400
node.gyp generate_node_exp, thin node ldflags, expfile_extra_inputs on libnode
node.gypi Expfile action inputs / create_expfile.sh invocation
tools/create_expfile.sh .o files, snapshot ordering, dump -tov preserved
node.gyp Extra -Wl,-brtl on shared AIX node (often redundant)

Changes are also being verified on ibmi to ensure they do not break anything there: https://ci.nodejs.org/job/node-test-commit-ibmi/2534/

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Nov 9, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@sxa

sxa commented Nov 9, 2026 •

Copy link
Copy Markdown
Member Author

Shared library build at https://ci.nodejs.org/job/node-test-commit-aix-shared-lib/nodes=aix73-power9/2617 worked with one exception - but that's much better than previously.

not ok 7282 addons/dlopen-ping-pong/test
  ---
  duration_ms: 280.95800
  severity: crashed
  exitcode: -4
  stack: |-
    process.dlopen: /home/iojs/build/workspace/node-test-commit-aix-shared-lib/nodes/aix73-power9/test/addons/dlopen-ping-pong/build/Release/binding.node
    module.exports.load: /home/iojs/build/workspace/node-test-commit-aix-shared-lib/nodes/aix73-power9/test/addons/dlopen-ping-pong/build/Release/ping.so
  ...

Noting that the test does have a block for Windows:

if (common.isWindows)
  common.skip('dlopen global symbol loading is not supported on this os.');

but in our case it does pass on AIX in the non-shared builds.

Incoming force push to update commit message, but no extra code changes.

@sxa
sxa force-pushed the aix_shared_patch branch from 9a4d75c to 22b6f33 Compare November 9, 2026 18:35
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@sxa

sxa commented Nov 9, 2026 •

Copy link
Copy Markdown
Member Author

reason for the failure may be related to the runtimelin linking options (or lack of them!) in the shared library builds. Latest commit will try to resolve that by adding this to the shared library clause:

'-Wl,-brtl',

Related: #15286

Signed-off-by: Stewart X Addison <sxa@ibm.com>
Assisted-by: Cursor
@sxa
sxa force-pushed the aix_shared_patch branch from e00951e to 71e6723 Compare November 10, 2026 15:44
Signed-off-by: Stewart X Addison <sxa@ibm.com>
@sxa

sxa commented Nov 10, 2026

Copy link
Copy Markdown
Member Author

Second commit here updates the pingpong test so that it works as well. This could reasonably be a separate PR so if that is desired I can split it out. The functionality can be reviewed separately, which is why I have not squashed the PRs. This is the first time I've looked at this test but it seems quite unique in some of what it does, and happens to trip up on AIX's shared library scenario.

The test does three things: load binding.node with RTLD_GLOBAL, load ping.so, then call dlopen_pong from the binding via code in ping.so.

On AIX, ping.so was built with -Wl,-G, so it imported dlopen_pong at dlopen(ping.so) time (deferred binding). binding.node did export dlopen_pong in dump, but in the thin node + libnode shared build that import often did not resolve correctly at load time causing a (SIGILL, exit −4). In Release, assert in binding.cc is off [*], so a failed load could leave a null function pointer and you’d get the same kind of crash instead of a clear error.

With the fix:

  1. ping.c on AIX: Look up dlopen_pong with dlsym(RTLD_DEFAULT, …) when dlopen_ping runs, which matches what RTLD_GLOBAL is supposed to do, without relying on -G import fixup when ping.so loads. dlsym(RTLD_DEFAULT, "dlopen_pong") skips the import-at-dlopen(ping.so) path: ping.so no longer needs -G for this symbol on AIX, and lookup happens explicitly when dlopen_ping runs, using the global symbol set RTLD_GLOBAL was meant to publish.
  2. binding.gyp: no -Wl,-G on the AIX ping target, so ping.so doesn’t carry that fragile import.
  3. binding.cc - throw on failure instead of assert, so failures show up as test errors, not −4.

[*] - Replacing assert with explicit checks that throw into JavaScript makes Release behave like a real test failure instead of a silent native crash.

@sxa

sxa commented Nov 10, 2026

Copy link
Copy Markdown
Member Author

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@sxa sxa assigned abmusse and unassigned abmusse Nov 11, 2026
@sxa
sxa requested a review from abmusse November 11, 2026 14:09
@sxa sxa self-assigned this Nov 11, 2026
@sxa
sxa marked this pull request as ready for review November 11, 2026 14:10
Signed-off-by: Stewart X Addison <sxa@ibm.com>
@sxa

sxa commented Nov 11, 2026

Copy link
Copy Markdown
Member Author

The linter check is objecting to the single quotes being used for the inlined awk script. I do not believe its comments are valid as we do not want any parameter expansion in this case.

Signed-off-by: Stewart X Addison <sxa@ibm.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@sxa

sxa commented Nov 11, 2026 •

Copy link
Copy Markdown
Member Author

Looks like the github actions runs are all over the place:

[31](https://github.com/nodejs/node/actions/runs/43039337927/job/130586760457?pr=67359#step:3:133)
  Err:10 http://ports.ubuntu.com/ubuntu-ports noble-updates/universe arm64 llvm-19-dev arm64 1:19.1.1-1ubuntu1~24.04.2
    Temporary failure resolving 'ports.ubuntu.com'

but they were passing successfully prior to the linter fixes so I am not concerned about them.

EDIT: https://status.canonical.com/#/ shows that they are having problems at the moment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants