From 608e3f6d7da90aa40a8e455a650a89883b7fca76 Mon Sep 17 00:00:00 2001 From: Marco Wang Date: Mon, 27 Apr 2026 16:41:05 -0700 Subject: [PATCH] Wrap for-using bindings in VariableDeclaratorNode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: #### From Human The fix makes sense to me especially looking at the test change in xplat/static_h/test/Parser/for-using.js from a bare identifier to a normal declaration block. Should be able to fix problems in https://github.com/facebook/hermes/issues/1981 #### From AI Hermes' parser builds a malformed AST for `for (using x of y)` and `for [await] (await using x of y)`: it pushes a bare `IdentifierNode` into `VariableDeclaration._declarations` instead of wrapping it in a `VariableDeclaratorNode`. Compare to `for (var x of y)`, which correctly produces `[ VariableDeclarator { init: null, id: Identifier } ]`. Downstream Sema code (`SemanticResolver::extractIdentsFromDecl`, `ScopedFunctionPromoter::extractDeclaredIdents`) calls `cast(&decl)` over the list. With an `IdentifierNode` in the slot, the cast reads `_init`/`_id` at offsets that actually contain `_name` (a `UniqueString*`) and `_typeAnnotation` (often null), then dereferences the `UniqueString*` as a `Node*`. In native debug this fires a `Casting.h` assertion; in release WASM it traps non-deterministically depending on heap layout — the bug reported in https://github.com/facebook/hermes/issues/1981, which Prettier tripped over while adding `using` support in https://github.com/prettier/prettier/pull/18961. It surfaces only via hermes-parser (not regular Hermes execution) because `resolveASTForParser` runs Sema with `compile=false`, bypassing the explicit `using declarations are not yet supported` gate at `SemanticResolver.cpp:326`. Tools using hermes-parser (Prettier, ESLint, Babel) walk past the gate into the bad cast. **Fix:** in the two `using` for-loop branches in `JSParserImpl.cpp` (`:1912` and `:1932`), wrap the binding identifier in a `VariableDeclaratorNode` (with `init = nullptr`) before pushing into the declarations list. Mirrors what `parseVariableDeclaration` already does at `:1257` for the no-initializer case. The non-for-loop `parseUsingDeclaration` path was already correct because it routes through `parseVariableDeclarationList`. **Tests:** - New `test/Parser/for-using-declarator.js`: asserts the AST contains `VariableDeclarator { init: null, id: Identifier }` for `for (using x of y)` and `for await (await using x of y)`. - New `test/Sema/for-using-not-supported.js`: asserts the compile-path Sema rejects `for (using ...)` and `for await (await using ...)` with the clean `"using declarations are not yet supported"` diagnostic instead of crashing in `cast`. Covers the regression at the layer where it manifested. - Update stale CHECK lines in pre-existing `test/Parser/for-using.js`, which encoded the buggy bare-Identifier shape and would have masked any future re-introduction of the bug. **Caveat:** verified end-to-end against the native debug binary (assertion no longer fires; AST shape matches `for (var x of y)`). The original WASM trap was diagnosed by code-reading — the cast site, the `compile=false` codepath in `hermes-parser-wasm.cpp:104`, and the field-offset overlap between `IdentifierNode` and `VariableDeclaratorNode` all line up — but the WASM was not rebuilt to empirically confirm the 1-in-10000 trap drop. Reviewed By: avp Differential Revision: D102691940 --- lib/Parser/JSParserImpl.cpp | 12 ++++++-- test/Parser/for-using-declarator.js | 45 ++++++++++++++++++++++++++++ test/Parser/for-using.js | 24 +++++++++++---- test/Sema/for-using-not-supported.js | 35 ++++++++++++++++++++++ 4 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 test/Parser/for-using-declarator.js create mode 100644 test/Sema/for-using-not-supported.js diff --git a/lib/Parser/JSParserImpl.cpp b/lib/Parser/JSParserImpl.cpp index bd15e2b3a4e..eb58862cb41 100644 --- a/lib/Parser/JSParserImpl.cpp +++ b/lib/Parser/JSParserImpl.cpp @@ -1909,7 +1909,11 @@ Optional JSParserImpl::parseForStatement(Param param) { if (!optIdent) return None; ESTree::NodeList declList; - declList.push_back(**optIdent); + auto *declarator = setLocation( + *optIdent, + *optIdent, + new (context_) ESTree::VariableDeclaratorNode(nullptr, *optIdent)); + declList.push_back(*declarator); decl = setLocation( varStartLoc, @@ -1929,7 +1933,11 @@ Optional JSParserImpl::parseForStatement(Param param) { if (!optIdent) return None; ESTree::NodeList declList; - declList.push_back(**optIdent); + auto *declarator = setLocation( + *optIdent, + *optIdent, + new (context_) ESTree::VariableDeclaratorNode(nullptr, *optIdent)); + declList.push_back(*declarator); decl = setLocation( varStartLoc, diff --git a/test/Parser/for-using-declarator.js b/test/Parser/for-using-declarator.js new file mode 100644 index 00000000000..b33fa585c6b --- /dev/null +++ b/test/Parser/for-using-declarator.js @@ -0,0 +1,45 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: %hermesc -dump-ast --pretty-json %s | %FileCheck %s --match-full-lines + +// Regression test: in `for (using x of y)` and `for [await] (await using x of +// y)`, the binding must be wrapped in a `VariableDeclarator` (with `init: +// null`) inside `VariableDeclaration._declarations`, matching what +// `for (var x of y)` produces. Pushing a bare Identifier corrupts later +// stages that `cast` over the list +// (https://github.com/facebook/hermes/issues/1981). + +for (using x of y); +// CHECK: "type": "VariableDeclaration", +// CHECK-NEXT: "kind": "using", +// CHECK-NEXT: "declarations": [ +// CHECK-NEXT: { +// CHECK-NEXT: "type": "VariableDeclarator", +// CHECK-NEXT: "init": null, +// CHECK-NEXT: "id": { +// CHECK-NEXT: "type": "Identifier", +// CHECK-NEXT: "name": "x" +// CHECK-NEXT: } +// CHECK-NEXT: } +// CHECK-NEXT: ] + +async function f() { + for await (await using x of y); +} +// CHECK: "type": "VariableDeclaration", +// CHECK-NEXT: "kind": "await using", +// CHECK-NEXT: "declarations": [ +// CHECK-NEXT: { +// CHECK-NEXT: "type": "VariableDeclarator", +// CHECK-NEXT: "init": null, +// CHECK-NEXT: "id": { +// CHECK-NEXT: "type": "Identifier", +// CHECK-NEXT: "name": "x" +// CHECK-NEXT: } +// CHECK-NEXT: } +// CHECK-NEXT: ] diff --git a/test/Parser/for-using.js b/test/Parser/for-using.js index 517c249c619..107dcf5f0e6 100644 --- a/test/Parser/for-using.js +++ b/test/Parser/for-using.js @@ -19,8 +19,12 @@ for (using x of y); // CHECK-NEXT: "kind": "using", // CHECK-NEXT: "declarations": [ // CHECK-NEXT: { -// CHECK-NEXT: "type": "Identifier", -// CHECK-NEXT: "name": "x" +// CHECK-NEXT: "type": "VariableDeclarator", +// CHECK-NEXT: "init": null, +// CHECK-NEXT: "id": { +// CHECK-NEXT: "type": "Identifier", +// CHECK-NEXT: "name": "x" +// CHECK-NEXT: } // CHECK-NEXT: } // CHECK-NEXT: ] // CHECK-NEXT: }, @@ -42,8 +46,12 @@ for (using x in y); // CHECK-NEXT: "kind": "using", // CHECK-NEXT: "declarations": [ // CHECK-NEXT: { -// CHECK-NEXT: "type": "Identifier", -// CHECK-NEXT: "name": "x" +// CHECK-NEXT: "type": "VariableDeclarator", +// CHECK-NEXT: "init": null, +// CHECK-NEXT: "id": { +// CHECK-NEXT: "type": "Identifier", +// CHECK-NEXT: "name": "x" +// CHECK-NEXT: } // CHECK-NEXT: } // CHECK-NEXT: ] // CHECK-NEXT: }, @@ -93,8 +101,12 @@ async function f() { // CHECK-NEXT: "kind": "await using", // CHECK-NEXT: "declarations": [ // CHECK-NEXT: { -// CHECK-NEXT: "type": "Identifier", -// CHECK-NEXT: "name": "x" +// CHECK-NEXT: "type": "VariableDeclarator", +// CHECK-NEXT: "init": null, +// CHECK-NEXT: "id": { +// CHECK-NEXT: "type": "Identifier", +// CHECK-NEXT: "name": "x" +// CHECK-NEXT: } // CHECK-NEXT: } // CHECK-NEXT: ] // CHECK-NEXT: }, diff --git a/test/Sema/for-using-not-supported.js b/test/Sema/for-using-not-supported.js new file mode 100644 index 00000000000..68b4473fa21 --- /dev/null +++ b/test/Sema/for-using-not-supported.js @@ -0,0 +1,35 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: (! %hermesc -emit-binary -out /dev/null -O0 %s 2>&1) | %FileCheck --match-full-lines %s +// RUN: (! %hermesc -emit-binary -out /dev/null -O %s 2>&1) | %FileCheck --match-full-lines %s + +// Regression test: parsing `for (using x of y)` and `for [await] (await using +// x of y)` used to push a bare IdentifierNode into VariableDeclaration's +// `_declarations`. Sema helpers (`ScopedFunctionPromoter::extractDeclaredIdents`, +// `SemanticResolver::extractIdentsFromDecl`) then aborted via `Casting.h` +// `cast` before reaching the explicit `using` +// rejection, masking the user-facing error and corrupting memory in release +// WASM (https://github.com/facebook/hermes/issues/1981). +// +// With the parser fix, the cast succeeds and Sema reaches the explicit +// rejection cleanly. + +function for_of_using(arr) { + for (using x of arr) {} +} +// CHECK: {{.*}}for-using-not-supported.js:[[@LINE-2]]:8: error: using declarations are not yet supported + +function for_in_using(obj) { + for (using x in obj) {} +} +// CHECK: {{.*}}for-using-not-supported.js:[[@LINE-2]]:8: error: using declarations are not yet supported + +async function for_await_of_await_using(arr) { + for await (await using x of arr) {} +} +// CHECK: {{.*}}for-using-not-supported.js:[[@LINE-2]]:14: error: using declarations are not yet supported