Skip to content

deps: fix missing <cstdint> include in re2 pcre.h - #566

Open
riosje wants to merge 64 commits into
nsolid-v26from
jeff/fix-re2-cstdint
Open

riosje wants to merge 64 commits into
nsolid-v26from
jeff/fix-re2-cstdint

Conversation

@riosje

@riosje riosje commented Oct 9, 2026

Copy link
Copy Markdown

What

Adds the missing #include <cstdint> to deps/grpc/third_party/re2/util/pcre.h, which uses int32_t (line 503) without including the header that defines it.

Why

The file has always relied on transitive header leaks:

  • GCC 12 libstdc++ leaks <cstdint> through <string> — removed in GCC 13 (see GCC 13 porting notes).
  • glibc keeps leaking the global int32_t via <stdlib.h> → <sys/types.h> → bits/stdint-intn.h, even on GCC 13+.
  • musl (Alpine) leaks neither — so the Alpine binary build of Node.js 26 fails with:
deps/grpc/third_party/re2/util/pcre.h:503:11: error: 'int32_t' does not name a type

This blocks the NSolid lithium (Node 26) Alpine build in packages-repositories. Older codenames pass only because their Alpine build images are from 2023 (gcc 12); any image rebuild would break them identically.

Validation

Minimal repro of the same include chain (<string> then bare int32_t), compiled in Docker:

Environment libc gcc before after
alpine:3.18 musl 12.2.1 pass pass
alpine:3.20 musl 13.2.1 fail pass
alpine:3.21 musl 14.2.0 fail pass
ubuntu:24.04 glibc 2.39 13.3.0 pass pass

The change is a pure additive standard include — no behavior or ABI change on any toolchain.

Note

Per doc/contributing/pull-requests.md, deps/ changes normally go to their upstream projects. This shim is legacy re2 code vendored by gRPC as a pinned snapshot (bumping the vendored gRPC did not pick up a fix), so this lands directly in our fork to unblock the Node 26 pipeline. Upstream re2 has long since removed/rewritten this compatibility layer.

Specifically `Unused eslint-disable directive (no problems were reported
from 'jsdoc/require-returns-check')`.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Add a size-aware for_each overload to TSList so callbacks can observe
the list size while iterating. Return the updated size from erase and
extend coverage for object and pointer specializations.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
PR-URL: #517
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Add an OpenVEX generator, exception source, tests, and package the
generated document under share/doc/nsolid.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
PR-URL: #515
Reviewed-By: Jefferson <jefferson.rios.caro@gmail.com>
Add fixed-status statements and versioned products with subcomponents.
Document regenerating the VEX file as part of the release commit.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
PR-URL: #528
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
PR-URL: #529
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
santigimeno and others added 27 commits October 5, 2026 18:41
Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Use pointer access for the additional package fields after the upstream
package configuration refactor.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
Register print_nsolid_version through the upstream boolean field helpers.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
Initialize N|Solid async hooks during runtime preparation instead of
caching worker identity when the startup snapshot is created.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
Restore environment-local counter and tracing buffers before loading
N|Solid async hooks in both main and worker environments.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
The timer carries its owning environment and enters its context before
calling JavaScript. Another environment may be current on the isolate.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
Document ownership assumptions, required fixes and regression coverage
for multiple environments sharing an isolate.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
Keep the early diagnostics publication used by N|Solid tracing and
remove the duplicate publication later in stream destruction.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
Pass filenames to nearest-parent package lookup so N|Solid tracking
shares the upstream directory cache and starts from the correct directory.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
Record successful opens and closes on the event-loop thread before
callbacks run. Cover reads, writes, chunked fallback and failed opens.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
Avoid optional package and dependency scans that introduce permission
denials and unrelated audit events during deferred N|Solid startup.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
The edits introduced by 48283c7 replaced explicit Tuple template
arguments with a typed input in LowerClz and LowerCtz. These older V8
compatibility edits were carried in the Node 24 N|Solid branch.

The v26.7.0 port imported them in a8102f3. Conflict resolution in
6f98f72 incorrectly kept Tuple instead of the upstream MakeTuple
calls, and the v26.10.0 merge retained that resolution. The current
assembler no longer provides Tuple, so armhf compilation fails when
the 32-bit Int64LoweringReducer is instantiated.

Restore both calls exactly as shipped in Node.js v26.10.0. This removes
an obsolete compatibility patch without changing N|Solid functionality.
It is unrelated to Chromium issue 502030575, which concerns address
calculation when splitting 64-bit loads and stores.

Validation: the header matches v26.10.0 and git diff --check passes.
An armhf build was not run locally.

Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Assisted-by: Codex
Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
PR-URL: #180
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
PR-URL: #371
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
PR-URL: #422
Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com>
The vendored re2 PCRE compatibility shim (deps/grpc/third_party/re2)
uses int32_t in pcre.h without including <cstdint>. Until now this
compiled only because the toolchain leaked the type transitively:

- GCC 12's libstdc++ leaks <cstdint> through <string> (removed in
  GCC 13, see the GCC 13 porting notes).
- glibc's <stdlib.h> pulls <sys/types.h>, which still leaks the
  global int32_t typedef on GCC 13+.

musl (Alpine) provides neither leak, so the Alpine binary build of
Node.js 26 (gcc 13/14, musl) fails with:

  deps/grpc/third_party/re2/util/pcre.h:503:11:
  error: 'int32_t' does not name a type

Validated by compiling a minimal reproduction of the same include
chain: fails on alpine:3.20 (gcc 13.2) and alpine:3.21 (gcc 14.2),
passes on alpine:3.18 (gcc 12.2) and glibc + gcc 13.3; with the
include added it compiles on all of them.
@ns-control-tower

ns-control-tower commented Oct 9, 2026 •

Copy link
Copy Markdown

Walkthrough

This PR adds #include <cstdint> to deps/grpc/third_party/re2/util/pcre.h (line 163), a vendored gRPC/re2 compatibility header. The file declares mutable int32_t hit_limit_; (class PCRE) without including the standard header that defines int32_t. On glibc/libstdc++ toolchains the type leaks transitively through <string> (GCC ≤12) or <stdlib.h>→<sys/types.h>→bits/stdint-intn.h, so the omission was latent. On musl (Alpine) with GCC 13+ neither leak occurs, causing a hard compile failure that blocks the NSolid lithium (Node 26) Alpine build. The fix is a pure additive standard-library include — no behavior or ABI change on any toolchain.

Assessment

  • Correctness ✅ — <cstdint> is the correct C++ header for int32_t, and int32_t hit_limit_ is the only fixed-width integer usage in the file. The include is placed before the project-specific headers (util/util.h, re2/stringpiece.h), consistent with the system-headers-first convention.
  • Compatibility ✅ — Additive include; cannot break any existing build (at worst redundant where <cstdint> was already transitively available). The PR description includes a cross-platform validation matrix (Alpine 3.18/3.20/3.21, Ubuntu 24.04) confirming the fix resolves the musl/GCC 13+ failure without regressions.
  • Security ✅ — No security implications; a header include fix in a vendored dependency.
  • Supply chain ✅ — This is a minimal patch to a vendored, pinned gRPC/re2 snapshot. The PR description correctly notes that deps/ changes normally go upstream, but explains why this shim lands directly in the fork (legacy re2 code, bumping the vendored gRPC did not pick up a fix, upstream re2 has removed/rewritten this compatibility layer). Reasonable for unblocking the Node 26 pipeline.

No blocking findings.

Verdict: APPROVE — clean, minimal, well-justified header fix that resolves a real musl/Alpine build failure with no risk of regression.

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.

4 participants