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
@ELDiablO59152
Copy link
Copy Markdown

It does not do the trick unfortunately

../deps/v8/src/ast/ast-value-factory.cc:87:35: error: use of undeclared identifier 'GetReadOnlyRoots'; did you mean 'ReadOnlyRoots'?
   87 |         raw_hash_field_, HashSeed(GetReadOnlyRoots()));
      |                                   ^
../deps/v8/src/roots/roots.h:654:7: note: 'ReadOnlyRoots' declared here
  654 | class ReadOnlyRoots {
      |       ^
1 error generated.

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.

3 participants