Skip to content

fix: guard abi::__forced_unwind catch behind __GLIBCXX__#505

Merged
jbachorik merged 2 commits intomainfrom
fix/j9-forced-unwind-macos
Apr 30, 2026
Merged

fix: guard abi::__forced_unwind catch behind __GLIBCXX__#505
jbachorik merged 2 commits intomainfrom
fix/j9-forced-unwind-macos

Conversation

@jbachorik
Copy link
Copy Markdown
Collaborator

@jbachorik jbachorik commented Apr 30, 2026

What does this PR do?:

Wraps the try { ... } catch (abi::__forced_unwind&) { ... } block in j9WallClock.cpp::timerLoop() with #if defined(__GLIBCXX__) so the file compiles on libc++ targets (macOS clang+libc++, Alpine clang+libc++).

Motivation:

abi::__forced_unwind is a libstdc++ extension — declared in libstdc++'s <cxxabi.h> regardless of which libc is in use. libc++ (LLVM's C++ stdlib, the default on macOS clang) does not declare it. Commit 25c702ba4 (#492) introduced the catch without any guard, breaking builds on libc++ targets:

j9WallClock.cpp:141:17: error: no type named '__forced_unwind' in namespace '__cxxabiv1'
} catch (abi::__forced_unwind&) {
         ~~~~~^

Why GLIBCXX and not GLIBC

The compilation constraint is "is the C++ stdlib libstdc++?", not "is libc glibc?". The two are independent:

Target C++ stdlib abi::__forced_unwind defined? Catch fires?
Linux glibc + libstdc++ libstdc++ yes yes (glibc pthread_cancel raises it)
Linux musl + libstdc++ libstdc++ yes no (musl uses a non-exception cancellation path)
Linux musl + libc++ libc++ no n/a
macOS clang + libc++ libc++ no n/a

__GLIBCXX__ is the precise predicate — it's defined by libstdc++ regardless of libc, undefined under libc++. The catch is harmless dead code on musl/macOS+libstdc++ but compiles cleanly there.

The same pattern in libraryPatcher_linux.cpp is already inside #if defined(__linux__) so it compiles correctly. This change brings j9WallClock.cpp into line.

Additional Notes:

#include <cxxabi.h> is left unguarded — the header itself exists on libc++, it just doesn't declare the abi::__forced_unwind member. Including it is harmless.

OpenJ9 supports macOS, AIX, z/OS, Linux and Windows — but on every non-glibc platform J9's pthread_cancel uses a non-exception mechanism, so the catch is a no-op there regardless of whether it compiles. The guard is purely about compilation, not behaviour.

How to test the change?:

  • macOS / arm64 (clang + libc++): ./gradlew :ddprof-lib:compileRelease — verified to succeed (Successfully compiled 59 files).
  • Linux / glibc (libstdc++): existing forced_unwind_ut.cpp unit test still exercises the catch path; behaviour unchanged.

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.
  • JIRA: [JIRA-XXXX]

🤖 Generated with Claude Code

@jbachorik jbachorik added the AI label Apr 30, 2026
abi::__forced_unwind is declared by libstdc++ (any libc) but not by
libc++, so the catch only compiles where libstdc++ is the active
C++ stdlib.  Guard on __GLIBCXX__ — defined by libstdc++ regardless
of libc, undefined under libc++ — so the file compiles on macOS
clang+libc++ (and any libc++ build) while keeping the cancellation
cleanup wherever libstdc++ is in use.

The exception itself is raised by glibc's pthread_cancel; on platforms
with a different cancellation mechanism (musl, macOS, AIX) the catch
is harmless dead code but compiles cleanly under libstdc++.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jbachorik jbachorik force-pushed the fix/j9-forced-unwind-macos branch from 6c66bfe to 7583b8d Compare April 30, 2026 13:40
@jbachorik jbachorik changed the title fix: guard abi::__forced_unwind catch behind __GLIBC__ fix: guard abi::__forced_unwind catch behind __GLIBCXX__ Apr 30, 2026
@jbachorik jbachorik marked this pull request as ready for review April 30, 2026 13:40
@jbachorik jbachorik requested a review from a team as a code owner April 30, 2026 13:40
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7583b8dffa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/j9/j9WallClock.cpp Outdated
Drop the libstdc++-only try/catch around timerLoop's body and replace it
with an RAII Cleanup struct.  Destructors run during forced unwinding
under any C++ stdlib, so the cleanup path is no longer compiled out on
libc++ (macOS) and is preserved on glibc + libc++ — addressing the
review comment from chatgpt-codex-connector on PR #505.

Also drops the unused <cxxabi.h> include.

Co-Authored-By: muse <muse@noreply>
@dd-octo-sts
Copy link
Copy Markdown
Contributor

dd-octo-sts Bot commented Apr 30, 2026

CI Test Results

Run: #25170124398 | Commit: cddde3c | Duration: 27m 12s (longest job)

All 32 test jobs passed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - - -
8-ibm - - -
8-j9 - -
8-librca - -
8-orcl - - -
11 - - -
11-j9 - -
11-librca - -
17 - -
17-graal - -
17-j9 - -
17-librca - -
21 - -
21-graal - -
21-librca - -
25 - -
25-graal - -
25-librca - -

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 32 | Failed: 0


Updated: 2026-04-30 18:28:42 UTC

@jbachorik jbachorik merged commit 10816f8 into main Apr 30, 2026
173 of 174 checks passed
@jbachorik jbachorik deleted the fix/j9-forced-unwind-macos branch April 30, 2026 18:33
@github-actions github-actions Bot added this to the 1.43.0 milestone Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant