fix(codegen): String.match no-match returns null, not a boxed null pointer#4860
Merged
Merged
Conversation
…inter (#4858) Expr::StringMatch codegen NaN-boxed js_string_match's result unconditionally. On no-match the runtime returns null (0), and POINTER_TAG|0 is neither `null` nor a valid heap pointer: `"abc".match(/x/g) === null` evaluated to false and consumers that dereference the result (JSON.stringify's tree walk, .map) segfaulted. This was the root cause of #4841's Stripe failure — extractUrlParams runs path.match(/\{\w+\}/g) on every request path, and any path without {params} crashed the request. Fix: branchless null -> TAG_NULL select on the call result, exactly mirroring the Expr::RegExpExec arm directly above (which documents this same bug class). The js_string_match_value path (lower_string_method.rs) already had the select; the static Expr::StringMatch fast path was the only one missing it. The non-global no-match case hit the identical codegen path and also crashed under JSON.stringify (the issue's "non-global works" note only held for console.log, which guards small pointers). Verified: issue repro + a broad match-semantics sweep (global/ non-global, hit/no-match, capture groups, named groups, .index, regex-in-local) byte-match node --experimental-strip-types. New end-to-end regression test compiles and runs the repro via CARGO_BIN_EXE_perry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4858.
Root cause
Expr::StringMatchcodegen (crates/perry-codegen/src/expr/instance_misc1.rs) NaN-boxed thejs_string_matchresult unconditionally. On no-match the runtime returns null (0), andPOINTER_TAG|0is neithernullnor a valid heap pointer:"abc".match(/x/g) === nullevaluated tofalseJSON.stringify(a)crashed with SIGSEGV (exit 139), and the Stripe SDK'sextractUrlParams(path.match(/\{\w+\}/g)+.map) failed on every path without{params}(Stripe SDK: stripe.products.create throws 'replace is not a function' (request dispatch, flat body) after #4831 #4841)The sibling
Expr::RegExpExecarm already documents and guards this exact bug class; thejs_string_match_valuepath (lower_string_method.rs) also had the guard. The staticExpr::StringMatchfast path was the only one missing it.Note: the non-global no-match case hit the identical codegen path and also segfaulted under
JSON.stringify— the issue's "non-global works" observation only held forconsole.log, which guards small pointers.Fix
Branchless
result == 0 → TAG_NULLselect on the call result, mirroringRegExpExec.Validation
before/after: null.index, regex held in a local, the StripeextractUrlParamspattern) byte-matchesnode --experimental-strip-typescrates/perry/tests/issue_4858_global_match_no_match.rs) compiles + runs the repro viaCARGO_BIN_EXE_perryand asserts exact outputcargo test --release -p perry -p perry-codegengreen locallyNo version bump / changelog per maintainer-at-merge convention.