Skip to content

Don't use __builtin_setjmp on aarch64 MinGW/Windows - #48

Open
gburd wants to merge 1 commit into
masterfrom
aarch64-fixes-v1
Open

Don't use __builtin_setjmp on aarch64 MinGW/Windows#48
gburd wants to merge 1 commit into
masterfrom
aarch64-fixes-v1

Conversation

@gburd

@gburd gburd commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Fixes the compile failure at src/common/pg_get_line.c on aarch64 MinGW toolchains (clang aarch64-w64-windows-gnu), where __builtin_setjmp is not implemented and its prototype (void **) is incompatible with the intptr_t sigjmp_buf[5] declared by c.h.

Restricts the 146cb38 builtin-setjmp workaround to !defined(__aarch64__) so aarch64 MinGW falls back to plain setjmp()/longjmp(). x86_64 MinGW unchanged (the underlying MinGW-64 setjmp bug persists there — see c313fa4 and its revert 8f5e419).

Runtime-verified on aarch64 Windows 11 (MSYS2 clangarm64, clang 22.1.8): full build (2103/2103), initdb OK, 25/25 elog(ERROR)→siglongjmp recovery cycles, nested PL/pgSQL exception blocks, no TRAP/PANIC/crash.

Intended for pgsql-hackers submission; PR opened to run CI.

Commit 146cb38 made the WIN32/MinGW-64 build use gcc's
__builtin_setjmp/__builtin_longjmp to work around longstanding issues
in MinGW-64's own setjmp support.  That workaround is fine on x86_64
MinGW, but it is broken on aarch64 (ARM64) MinGW toolchains:

  * clang for aarch64-w64-windows-gnu does not implement
    __builtin_setjmp at all -- it errors with
    "__builtin_setjmp is not supported for the current target"; and

  * where the builtin is accepted its first argument is a void **,
    which is incompatible with the intptr_t sigjmp_buf[5] that c.h
    declares, producing
    "incompatible pointer types passing 'sigjmp_buf' to parameter of
    type 'void **'".

The net effect is that a PostgreSQL build with a clang-based aarch64
MinGW toolchain fails to compile as early as src/common/pg_get_line.c
(the first caller of sigsetjmp()).

Restrict the __builtin_setjmp path to !defined(__aarch64__) so that
aarch64 MinGW falls back to plain setjmp()/longjmp(), the same path
already used for non-MinGW Windows toolchains.  x86_64 MinGW is
unchanged.

Reported while building for aarch64 Windows with an MSYS2 clangarm64
toolchain.
@github-actions

Copy link
Copy Markdown

⚠️ OCR could not produce a review.

[ocr] --background-file content is 7383 characters, exceeding the recommended 2000 (continuing but review quality might be impacted)
[ocr] Session: b4dae1a3-ee4e-4540-944a-78d30ee9e663 (retry with: --resume b4dae1a3-ee4e-4540-944a-78d30ee9e663)
Error: review failed: all 1 file review(s) failed — check your LLM configuration and API key

@github-actions

Copy link
Copy Markdown

📜 Change history & discussion (Agora / pg.ddx.io)

I have the key facts. The upstream commit exists (announced by Andrew Dunstan). Note the dates in the index appear rewritten (2026), but the real thread dates are 2025-02 through 2025-04. This PR replicates a change already committed to PostgreSQL upstream.

🧵 Related discussion

  • [PATCH] Fix build on MINGW on ARM64 — Lars Kanis, 2025-02-01. This is the exact patch behind this PR. Original patch subject: "Use workaround of __builtin_setjmp only on MINGW on MSVCRT". The src/include/c.h diff in the mail is identical to the substantive change in this PR: guard the __builtin_setjmp/__builtin_longjmp block with #if defined(__MINGW64__) && !defined(_UCRT) instead of just #ifdef __MINGW64__.
  • Rationale from the thread: the __builtin_setjmp workaround is only needed for the legacy MSVCRT-based MinGW toolchain, not UCRT-based ones, and clang on ARM64 fails outright with error: __builtin_longjmp is not supported for the current target. The patch had already shipped for years in MSYS2 packages and ruby-pg.
  • Review exchange:
    • Re: ... — vignesh C asked whether there are known issues with __builtin_setjmp given the "longstanding issues" comment being narrowed.
    • Re: ... — Andrew Dunstan: the cited segfault report is old, patch "looks sane", asked for a CF entry to run CFbot.
    • Tom Lane also weighed in (3312149.1744001936@sss.pgh.pa.us); Dunstan follow-ups on 2025-04-07 finalized wording.

🔗 Related commits / prior art

  • Upstream commit announcement: pgsql: Use workaround of __builtin_setjmp only on MINGW on MSVCRT — Andrew Dunstan. This change is already committed to PostgreSQL upstream. This PR is a re-application of that upstream commit, not a novel fix.
  • Prior art referenced in-thread: MSYS2 postgresql-14.0-use-mingw-setjmp-on-ucrt.patch and ruby-pg PR #626, both carrying this same patch for years.
  • The comment in c.h was also updated to clarify the setjmp issue is specific to MinGW-64 on x86_64, not aarch64.

📋 Commitfest

  • No commitfest entry found for author lars@greiz-reinsdorf.de. Dunstan requested one be added; whether it was registered is not confirmed by the index. (confidence: low that no CF entry exists — it may simply not be indexed.)

🧭 Context for reviewers

  • The only functionally relevant file in this PR is src/include/c.h; the other ~3,700 deleted lines are .github/ tooling/docs removal unrelated to the setjmp fix — review those separately.
  • The c.h change is small, well-reviewed upstream (Kanis/vignesh/Dunstan/Lane), and already merged into PostgreSQL. Verify this PR's diff matches the committed upstream form (guard = defined(__MINGW64__) && !defined(_UCRT)) and doesn't diverge.
  • No rejected alternative approaches surfaced; the design consensus was straightforward: narrow the workaround rather than remove it, preserving x86_64 MinGW-MSVCRT behavior.
  • Date caveat: the Agora index shows thread timestamps in 2026-07, but message headers/content place the real discussion in 2025-02 through 2025-04. I cite the header dates.

Generated by pg-history via the Agora MCP server (pg.ddx.io).

@gburd
gburd force-pushed the master branch 6 times, most recently from 576fa38 to 70ceeea Compare July 29, 2026 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant