[v26.x] v8: fix regexp modifier group handling by samsonradu · Pull Request #67207 · nodejs/node

samsonradu · 2026-11-02T20:56:28Z

Original commit message: [regexp] Fix modifiers for ChoiceNodes during Analysis During the analysis pass we didn't reset flags after visiting alternatives of a ChoiceNode, which is required as the sub-graph of an alternative might modify the flags. Fixed: 510487690 Change-Id: I6b2660a98b34cd12740538af4ce61685609934a0 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7826630 Commit-Queue: Patrick Thier <pthier@chromium.org> Auto-Submit: Patrick Thier <pthier@chromium.org> Reviewed-by: Jakob Linke <jgruber@chromium.org> Commit-Queue: Jakob Linke <jgruber@chromium.org> Cr-Commit-Position: refs/heads/main@{#107120} Adjusted for the V8 version in deps: use RegExpFlags instead of the Flags alias, which does not exist there. Refs: v8/v8@e540e11 Refs: nodejs#66277 Assisted-by: Claude Code Signed-off-by: Radu Samson <radu.samson@ext.ec.europa.eu>

Original commit message: [regexp] Reset flags after resetting state in Parser When restoring the previous state after parsing a closing parenthesis we also need to restore flags explicitly from the restored state. Fixed: 523687962 Change-Id: I63a5bd9fec2f9b7f6fb43b6893027c3c3d0755f0 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7950157 Auto-Submit: Patrick Thier <pthier@chromium.org> Commit-Queue: Jakob Linke <jgruber@chromium.org> Reviewed-by: Jakob Linke <jgruber@chromium.org> Cr-Commit-Position: refs/heads/main@{#108021} Refs: v8/v8@19e7a57 Refs: nodejs#66277 Assisted-by: Claude Code Signed-off-by: Radu Samson <radu.samson@ext.ec.europa.eu>

Original commit message: [regexp] Record flags on deferred worklist items ActionNode::Emit for MODIFY_FLAGS was previously changed to restore outer_flags on the way out to avoid leaking inner flags to trailing deferred nodes. However, this caused nodes deferred from within the modifier group to be compiled with the outer flags when processed by Compiler::Assemble. Instead of restoring flags in ActionNode::Emit, record the active flags at the point where a node is deferred in Compiler::AddWork, and restore them from the work-list item before emitting it. This partially reverts https://chromium-review.googlesource.com/c/v8/v8/+/8174589. Change-Id: I3a552f9223b88ba99889e4375b8d1ea761257b94 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8213283 Reviewed-by: Patrick Thier <pthier@chromium.org> Auto-Submit: Jakob Linke <jgruber@chromium.org> Commit-Queue: Patrick Thier <pthier@chromium.org> Cr-Commit-Position: refs/heads/main@{#109102} Adjusted for the V8 version in deps: use RegExpNode and RegExpFlags instead of the upstream Node class and Flags alias, and drop the MODIFY_FLAGS hunk, which reverts a workaround that was never part of this V8 version. The new tests are appended at the end of regexp-modifiers.js because the upstream context test does not exist here. Refs: v8/v8@b69a1a1 Refs: nodejs#66277 Assisted-by: Claude Code Signed-off-by: Radu Samson <radu.samson@ext.ec.europa.eu>

Original commit message: [regexp] Reset compiler flags between word boundary branches When compiling \b and \B word boundary checks in AssertionNode where the succeeding character type is unknown, two code paths (before_non_word and before_word) are generated sequentially. If the boundary assertion is placed inside a modifier group (e.g. (?i:\.|\bNULL)), emitting the first branch traverses through on_success and reaches the ActionNode that restores outer flags at group exit. This mutates flags without restoring it when returning to emit the second branch. Consequently, subsequent nodes in the second branch (before_word) were incorrectly compiled with outer flags instead of the modifier group's flags. This CL saves compiler->flags() and restores them before emitting the before_word branch in AssertionNode::EmitBoundaryCheck. TAG=agy Fixed: 544575761 Change-Id: I7f08bcb99c749d5e22636c8bd6eb44bd1d3e2e39 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8257209 Reviewed-by: Victor Gomes <victorgomes@chromium.org> Commit-Queue: Patrick Thier <pthier@chromium.org> Cr-Commit-Position: refs/heads/main@{#109236} Adjusted for the V8 version in deps: use RegExpFlags instead of the Flags alias, which does not exist there. Refs: v8/v8@d315af3 Refs: nodejs#66277 Assisted-by: Claude Code Signed-off-by: Radu Samson <radu.samson@ext.ec.europa.eu>

nodejs-github-bot · 2026-11-02T20:56:34Z

Review requested:

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

github-actions · 2026-11-02T20:56:54Z

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

the guide for first-time contributors
the contribution and automation policies
the pull request guide
the AI use policy
the Code of Conduct

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

inoway46 · 2026-11-03T04:16:34Z

Thanks for the updates! Could you add [v26.x] to the PR title and use the usual subsystem prefix (ex. #65746)?

V8 backports can depend on other upstream changes. This issue comment did not verify these patches on Node.js v26, so please run the reproduction cases before and after your changes and share the results. It also identifies 1e7ee91d as necessary alongside e540e116; could you check whether that fix is needed here?

Could you keep this PR as a draft until you've completed the verification?

Original commit message: [regexp] Reset flags between negative lookaround branches e540e1161b0 (bug 510487690) reset the analysis-time flags between the alternatives of a ChoiceNode, since an alternative's sub-graph may contain a MODIFY_FLAGS node. NegativeLookaroundChoiceNode has its own visitor and was left out, so a modifier group inside a negative lookaround still leaks its flags into the continuation. /(?!(?i:x|y))[K]/.exec('kz') // "k", should be null The lookaround body is visited first and leaves the group's flags behind; the continuation's class ranges are then made case independent under those flags rather than the pattern's, so [K] picks up 'k'. Only class ranges are affected: TextNode::MakeCaseIndependent materializes the equivalents during analysis, while atoms get their case closure at emission time from the restored flags. A single- alternative body does not reproduce either, since its restore node is emitted last. Change-Id: I4077076b9dbee6acf75918a2c4f7bf7d86cab059 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8175612 Auto-Submit: Jakob Linke <jgruber@chromium.org> Reviewed-by: Patrick Thier <pthier@chromium.org> Commit-Queue: Patrick Thier <pthier@chromium.org> Cr-Commit-Position: refs/heads/main@{#109043} Adjusted for the V8 version in deps: use RegExpFlags instead of the Flags alias, which does not exist there. Refs: v8/v8@1e7ee91 Refs: nodejs#66277 Assisted-by: Claude Code Signed-off-by: Radu Samson <radu.samson@ext.ec.europa.eu>

samsonradu · 2026-11-03T06:12:50Z

Results for Node v26.13.0:

console.log(/(?i:x|[a])/.test("A"));        // FALSE (Not OK)
console.log(/(?i:[a]|x)/.test("A"));        // TRUE (OK)
console.log(/(?i:x|a)/.test("A"));          // TRUE (OK)
console.log(/(?-i:x|[a])/i.test("A"));      // TRUE (Not OK)
console.log(/(?i:[a]??)x/.exec("Ax")[0]);               // `x` (Not OK, should return `Ax`)
console.log(/(?i:[a]{0,2}?)x/.exec("Ax")[0]);           // `x` (Not OK, should return `Ax`)
console.log(/(?i:[a]*?)x/.exec("Ax")[0]);          // `Ax` (OK)
console.log(/(?i:a??)x/.exec("Ax")[0]);            // `Ax` (OK)
console.log(/(?!(?i:x|y))[K]/.test("K"));        // TRUE (OK)
console.log(/(?!(?i:x|y))[K]/.test("k"));        // FALSE (OK)
console.log(/(?<!(?i:x|y))[K]/.test("K"));      // TRUE (OK)
console.log(/(?<!(?i:x|y))[K]/.test("k"));      // FALSE (OK)
console.log(/(?!(?-i:x|y))[K]/i.test("K"));     // TRUE (OK)
console.log(/(?!(?-i:x|y))[K]/i.test("k"));     // TRUE (OK)

samsonradu · 2026-11-03T07:08:05Z

Results for current branch:

console.log(/(?i:x|[a])/.test("A"));        // TRUE (OK)
console.log(/(?i:[a]|x)/.test("A"));        // TRUE (OK)
console.log(/(?i:x|a)/.test("A"));          // TRUE (OK)
console.log(/(?-i:x|[a])/i.test("A"));      // FALSE (OK)
console.log(/(?i:[a]??)x/.exec("Ax")[0]);               // `Ax` (OK)
console.log(/(?i:[a]{0,2}?)x/.exec("Ax")[0]);           // `Ax` (OK)
console.log(/(?i:[a]*?)x/.exec("Ax")[0]);          // `Ax` (OK)
console.log(/(?i:a??)x/.exec("Ax")[0]);            // `Ax` (OK)
console.log(/(?!(?i:x|y))[K]/.test("K"));        // TRUE (OK)
console.log(/(?!(?i:x|y))[K]/.test("k"));        // FALSE (OK)
console.log(/(?<!(?i:x|y))[K]/.test("K"));      // TRUE (OK)
console.log(/(?<!(?i:x|y))[K]/.test("k"));      // FALSE (OK)
console.log(/(?!(?-i:x|y))[K]/i.test("K"));     // TRUE (OK)
console.log(/(?!(?-i:x|y))[K]/i.test("k"));     // TRUE (OK)

samsonradu · 2026-11-03T07:47:48Z

@inoway46 Thanks, indeed there was a regression fixed by 1e7ee91d. Added now to the PR and posted the results.

inoway46 · 2026-11-03T07:57:42Z

Thanks for verifying the fixes locally!

I don’t have permission to trigger Jenkins CI myself, so let’s wait for a collaborator to run it.

Radu Samson added 4 commits November 2, 2026 21:51

samsonradu changed the title ~~Fix/66277 regexp modifier groups v2~~ Fix/66277 regexp modifier groups Nov 2, 2026

nodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. v26.x Issues that can be reproduced on v26.x or PRs targeting the v26.x-staging branch. v8 engine Issues and PRs related to the V8 dependency. labels Nov 2, 2026

samsonradu changed the title ~~Fix/66277 regexp modifier groups~~ [v26.x]: v8: fix regexp modifier groups #66277 Nov 3, 2026

samsonradu changed the title ~~[v26.x]: v8: fix regexp modifier groups #66277~~ [v26.x] v8: fix regexp modifier groups #66277 Nov 3, 2026

samsonradu marked this pull request as draft November 3, 2026 05:21

samsonradu marked this pull request as ready for review November 3, 2026 07:09

samsonradu changed the title ~~[v26.x] v8: fix regexp modifier groups #66277~~ [v26.x] v8: fix regexp modifier groups Nov 3, 2026

samsonradu changed the title ~~[v26.x] v8: fix regexp modifier groups~~ [v26.x] v8: fix regexp modifier group handling Nov 3, 2026

Navigation Menu

Repository navigation

[v26.x] v8: fix regexp modifier group handling - #67207

[v26.x] v8: fix regexp modifier group handling#67207
samsonradu wants to merge 5 commits into
nodejs:v26.x-stagingfrom
samsonradu:fix/66277-regexp-modifier-groups-v2

samsonradu commented Nov 2, 2026

Uh oh!

nodejs-github-bot commented Nov 2, 2026

Uh oh!

github-actions Bot commented Nov 2, 2026

Uh oh!

inoway46 commented Nov 3, 2026

Uh oh!

samsonradu commented Nov 3, 2026

Uh oh!

samsonradu commented Nov 3, 2026

Uh oh!

samsonradu commented Nov 3, 2026

Uh oh!

inoway46 commented Nov 3, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

samsonradu commented Nov 2, 2026

Uh oh!

nodejs-github-bot commented Nov 2, 2026

Uh oh!

github-actions Bot commented Nov 2, 2026

Uh oh!

inoway46 commented Nov 3, 2026

Uh oh!

samsonradu commented Nov 3, 2026

Uh oh!

samsonradu commented Nov 3, 2026

Uh oh!

samsonradu commented Nov 3, 2026

Uh oh!

inoway46 commented Nov 3, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants