Skip to content

deps: fix ast-value-factory build without shared RO heap#62664

Open
DaveT1991 wants to merge 1 commit intonodejs:v22.x-stagingfrom
DaveT1991:fix/disable-shared-ro-heap-build
Open

deps: fix ast-value-factory build without shared RO heap#62664
DaveT1991 wants to merge 1 commit intonodejs:v22.x-stagingfrom
DaveT1991:fix/disable-shared-ro-heap-build

Conversation

@DaveT1991
Copy link
Copy Markdown

Summary

This fixes a V8 build regression on the v22.x line when Node is configured with --disable-shared-readonly-heap.

Problem

Building Node 22.22.2 without V8_SHARED_RO_HEAP fails in deps/v8/src/ast/ast-value-factory.cc because the code tries to retrieve read-only roots with a shared-RO-heap accessor that is not available in that configuration.

Root cause

The backported V8 change switched AstRawString::AsArrayIndex() to decode cached array indexes using a hash seed from read-only roots, but this call site used the wrong accessor for non-shared-readonly-heap builds.

Fix

  • align deps/v8/src/ast/ast-value-factory.cc with the current main branch implementation
  • use HashSeed(GetReadOnlyRoots()) at the call site
  • update the include list accordingly

Testing

  • git diff --check
  • manual inspection against the current main branch implementation

Fixes: #62631

Signed-off-by: DaveT1991 <dawoodaljanaby@gmail.com>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. v8 engine Issues and PRs related to the V8 dependency. labels Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants