Skip to content

Fix JSG_TS_OVERRIDE macro corruption from <stdio.h> macros#6729

Merged
danlapid merged 1 commit into
cloudflare:mainfrom
zakcutner:fix-jsg-ts-override-stdio-macro-corruption
May 6, 2026
Merged

Fix JSG_TS_OVERRIDE macro corruption from <stdio.h> macros#6729
danlapid merged 1 commit into
cloudflare:mainfrom
zakcutner:fix-jsg-ts-override-stdio-macro-corruption

Conversation

@zakcutner

Copy link
Copy Markdown
Member

Problem

The JSG_TS_OVERRIDE, JSG_TS_DEFINE, JSG_STRUCT_TS_OVERRIDE, and JSG_STRUCT_TS_DEFINE macros stringify their varargs via JSG_STRING_LITERAL, a two-step helper that expands macro identifiers in the argument before stringification.

On Darwin, <stdio.h> defines stdin, stdout, and stderr as preprocessor macros (__stdinp, __stdoutp, __stderrp). Override blocks containing those identifiers are silently corrupted in the stringified TypeScript:

JSG_TS_OVERRIDE({ readonly stdin: WritableStream | null });
// macOS: "{ readonly __stdinp: WritableStream | null }"
// Linux: "{ readonly stdin: WritableStream | null }"

This is observable in src/workerd/api/container.h, where the ExecOptions and ExecProcess overrides reference these names. The platform divergence breaks bazel run //types for downstream consumers building on macOS — the generated worker-configuration.d.ts differs from the Linux-built committed snapshot.

Fix

Stringify directly with #__VA_ARGS__ at the outermost macro that captures the user's tokens, rather than forwarding through JSG_STRING_LITERAL. Single-step stringification doesn't trigger preprocessor expansion of the argument tokens.

JSG_STRING_LITERAL is preserved in macro-meta.h (it is still used by edgeworker/server/async-signal-debug.h for variable-name logging where two-step semantics are harmless). Its doc comment now warns against using it for user-supplied source-code blocks and points readers at the correct pattern.

Testing

Added a regression test (KJ_TEST("typescript macros with stdio identifiers") in rtti-test.c++) that asserts override and define blocks containing stdin/stdout/stderr survive stringification verbatim. Verified on macOS that:

  • The new test fails with the bug present, passes with the fix applied.
  • bazel test //src/workerd/jsg:all — 46/46 pass.
  • bazel build //types — generated experimental and latest types match the committed Linux snapshot exactly (diff -r types/generated-snapshot/ bazel-bin/types/definitions/ is empty).

`JSG_TS_OVERRIDE`, `JSG_TS_DEFINE`, `JSG_STRUCT_TS_OVERRIDE`, and
`JSG_STRUCT_TS_DEFINE` previously stringified their varargs via
`JSG_STRING_LITERAL`, which is a two-step "stringify after macro
expansion" helper. When a containing variadic macro forwards
`__VA_ARGS__` into the helper, the preprocessor expands any
identifiers in the argument before stringification. On Darwin,
`<stdio.h>` defines `stdin`/`stdout`/`stderr` as
`__stdinp`/`__stdoutp`/`__stderrp`, so overrides like
`{ readonly stdin: WritableStream }` were stringified as
`"{ readonly __stdinp: WritableStream }"`, corrupting the generated
TypeScript on macOS builds.

Stringify directly with `#__VA_ARGS__` at the outermost macro that
captures the user's tokens. Document the hazard on `JSG_STRING_LITERAL`
and add a regression test in `rtti-test.c++`.
@zakcutner zakcutner requested review from a team as code owners May 6, 2026 03:57
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@zakcutner

Copy link
Copy Markdown
Member Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request May 6, 2026
@npaun

npaun commented May 6, 2026

Copy link
Copy Markdown
Member

Damn it. This is the exact problem I was puzzled by last week. Nice catch!

@danlapid danlapid merged commit 6340578 into cloudflare:main May 6, 2026
21 of 22 checks passed
@zakcutner zakcutner deleted the fix-jsg-ts-override-stdio-macro-corruption branch May 6, 2026 13:02
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.

3 participants