The parser previously detected direct eval() calls at the end of
parse_expression(), by checking if the final expression was a
CallExpression with "eval" as the callee. This missed cases where
eval() appeared as a subexpression, e.g. `eval(code) | 0`, since
the final expression would be a BinaryExpression, not a
CallExpression.
Move the detection into parse_call_expression() where the
CallExpression is actually created. This ensures we always set the
contains_direct_call_to_eval flag regardless of surrounding
operators, so local variables are correctly placed in the
declarative environment where eval'd code can find them.
Otherwise we'll fail to recognise that the identifier token is invalid
(e.g. a keyword such as null) in a binding pattern and that it may not
be a binding pattern after all, but an object expression.
Fixes some scripts on Discord failing to parse.
Previously, when parsing a named function expression like
`Oops = function Oops() { Oops }`, the parser set a group-level flag
`might_be_variable_in_lexical_scope_in_named_function_assignment` that
propagated to the parent scope. This incorrectly prevented ALL `Oops`
identifiers from being marked as global, including those outside the
function expression.
Fix this by marking identifiers individually using
`set_is_inside_scope_with_eval()` only for identifiers inside the
function scope. This allows identifiers outside the function expression
to correctly use GetGlobal/SetGlobal while identifiers inside still
use GetBinding (since they may refer to the function's name binding).
Previously, when a nested function contained eval(), the parser would
mark all identifiers in parent functions as "inside scope with eval".
This prevented those identifiers from being marked as global, forcing
them to use GetBinding instead of GetGlobal.
However, eval() can only inject variables into its containing function's
scope, not into parent function scopes. So a parent function's reference
to a global like `Number` should still be able to use GetGlobal even if
a nested function contains eval().
This change adds a new flag `m_eval_in_current_function` that propagates
through block scopes within the same function but stops at function
boundaries. This flag is used for marking identifiers, while the
existing `m_screwed_by_eval_in_scope_chain` continues to propagate
across functions for local variable deoptimization (since eval can
access closure variables).
Before: `new Number(42)` in outer() with eval in inner() -> GetBinding
After: `new Number(42)` in outer() with eval in inner() -> GetGlobal
Cache necessary data during parsing to eliminate HashMap operations
in SharedFunctionInstanceData construction.
Before: 2 HashMap copies + N HashMap insertions with hash computations
After: Direct vector iteration with no hashing
Build FunctionScopeData for function scopes in the parser containing:
- functions_to_initialize: deduplicated var-scoped function decls
- vars_to_initialize: var decls with is_parameter/is_function_name
- var_names: HashTable for AnnexB extension checks
- Pre-computed counts for environment size calculation
- Flags for "arguments" handling
Add ScopeNode::ensure_function_scope_data() to compute the data
on-demand for edge cases that don't go through normal parser flow
(synthetic class constructors, static initializers, module wrappers).
Use this cached data directly in SFID with zero HashMap operations.
The previous fix prevented eval() in sibling function scopes from
affecting each other, but it still had a limitation: when identifiers
from multiple scopes were merged into the same identifier group at
Program scope, the presence of eval() anywhere would taint all
identifiers in the group.
This change tracks per-identifier whether it was inside a scope with
eval() in the scope chain. When a scope closes, if it contains eval()
or has eval() in its parent chain, each identifier in that scope is
marked with `is_inside_scope_with_eval`. At Program scope finalization,
only identifiers that are NOT marked can be optimized to global lookups.
This allows code like:
```js
var x = undefined; // Can be optimized (program scope)
(function() {
function walk() { undefined; } // Cannot be optimized
eval('');
})();
```
Before: Neither `undefined` could be optimized
After: The program-scope `undefined` is optimized, while the one inside
the function with eval() correctly uses dynamic lookup.
The parser was preventing identifiers like `undefined` from being
optimized to global constants when eval() was present in any function
scope, even when eval() could not affect the identifier's binding.
This change makes the parser smarter by preventing eval() in one
function scope from affecting global identifier optimization in sibling
function scopes. The key insight is that eval() in function A cannot
shadow globals in function B.
Instead, let functions have a view into the AST's SourceCode object's
underlying string data. The source string is kept alive by the AST, so
it's fine to have views into it as long as the AST exists.
Reduces memory footprint on my x.com home feed by 65 MiB.
This fixes an issue where we'd incorrectly retain objects via the
[[HomeObject]] slot. This common pattern was affected:
Object.defineProperty(o, "foo", {
get: function() { return 123; }
});
Above, the object literal would get assigned to the [[HomeObject]]
slot even though "get" is not a "method" per the spec.
This frees about 30,000 objects on my x.com home feed.
The lookahead lexer used by next_token() no longer needs to be kept
alive, since tokens created by Parser::next_token() now have any string
views guaranteed safe by the fact that they point into the one true
SourceCode provided by whoever set up the lexer.
This moves the responsibility of setting up a SourceCode object to the
users of JS::Lexer.
This means Lexer and Parser are free to use string views into the
SourceCode internally while working.
It also means Lexer no longer has to think about anything other than
UTF-16 (or ASCII) inputs. So the unit test for parsing various invalid
UTF-8 sequences is deleted here.
This commits puts the strict mode flag in the header of every bytecode
instruction. This allows us to check for strict mode without looking at
the currently running execution context.
This allows us to use the GetGlobal and SetGlobal bytecode instructions
for them, enabling cached accesses.
2.62x speed-up on this Fibonacci program:
function fib(n) {
return n < 2 ? n : fib(n - 1) + fib(n - 2);
}
for (let i = 0; i < 50_000; ++i)
fib(10);
This ports the lexer to UTF-16 and deals with the immediate fallout up
to the AST. The AST will be dealt with in upcoming commits.
The lexer will still accept UTF-8 strings as input, and will transcode
them to UTF-16 for lexing. This doesn't actually incur a new allocation,
as we were already converting the input StringView to a ByteString for
each lexer.
One immediate logical benefit here is that we do not need to know off-
hand how many UTF-8 bytes some special code points occupy. They all
happen to be a single UTF-16 code unit. So instead of advancing the
lexer by 3 positions in some cases, we can just always advance by 1.
Currently, the lexer holds a ByteString, which is always heap-allocated.
When we create a copy of the lexer for the lookahead token, that token
will outlive the lexer copy. The token holds a couple of string views
into the lexer's source string. This is fine for now, because the source
string will be kept alive by the original lexer.
But if the lexer were to hold a String or Utf16String, short strings
will be stored on the stack due to SSO. Thus the token will hold views
into released stack data. We need to keep the lookahead lexer alive to
prevent UAF on views into its source string.
This reverts commit c14173f651. We
should only annotate the minimum number of symbols that external
consumers actually use, so I am starting from scratch to do that
Instead, we can just use the scope type to determine if a scope is a
function scope.
This fixes using `this` for parameter default values in arrow functions
crashing. This happened by `uses_this_from_environment` was not set in
`set_uses_this`, as it didn't think it was in a function scope whilst
parsing parameters.
Fixes closing modal dialogs causing a crash on https://www.ikea.com/
No test262 diff.
Reverts the functional part of 08cfd5f, because it was a workaround for
this issue.
In the following example:
```js
const f = (i) => ({
obj: { a: { x: i }, b: { x: i } },
g: () => {},
});
```
The body of function `f` is initially parsed as an arrow function. As a
result, what is actually an object expression is interpreted as a formal
parameter with a binding pattern. Since duplicate identifiers are not
allowed in this context (`i` in the example), the parser generates an
error, causing the entire script to fail parsing.
This change ignores the "Duplicate parameter names in bindings" error
during arrow function parameter parsing, allowing the parser to continue
and recognize the object expression of the outer arrow function with an
implicit return.
Fixes error on https://chat.openai.com/
`var` bindings are never in the temporal dead zone (TDZ), and so we
know accessing them will not throw.
We now take advantage of this by having a specialized environment
binding value getter that doesn't check for exceptional cases.
1.08x speedup on JetStream.
This allows us to get rid of instructions that move arguments to locals
and allocate smaller JS::Value vector in ExecutionContext by reusing
slots that were already allocated for arguments.
With this change for following function:
```js
function f(x, y) {
return x + y;
}
```
we now produce following bytecode:
```
[ 0] 0: Add dst:reg6, lhs:arg0, rhs:arg1
[ 10] Return value:reg6
```
instead of:
```
[ 0] 0: GetArgument 0, dst:x~1
[ 10] GetArgument 1, dst:y~0
[ 20] Add dst:reg6, lhs:x~1, rhs:y~0
[ 30] Return value:reg6
```
Previously we blocked using locals for function arguments whenever
`arguments` was mentioned in function body, however, this is not
necessary in strict mode, where mutations to the arguments object are
not reflected in the function arguments and vice versa.
By doing that we consistently use Identifier node for identifiers and
also enable mechanism that registers identifiers in a corresponding
ScopePusher for catch parameters, which is necessary for work in the
upcoming changes.
This prevents the variables declared inside a class static initializer
to escape to the nearest containing function causing all sorts of memory
corruptions.
This commit removes the -Wno-unusued-private-field flag, thus
reenabling the warning. Unused field were either removed or marked
[[maybe_unused]] when unsure.
Instead of making a copy of the Vector<FunctionParameter> from the AST
every time we instantiate an ECMAScriptFunctionObject, we now keep the
parameters in a ref-counted FunctionParameters object.
This reduces memory usage, and also allows us to cache the bytecode
executables for default parameter expressions without recompiling them
for every instantiation. :^)
The "with" statement is its own token (TokenType::With), and thus would
fail to parse as an identifier. We've already asserted that the token
we are parsing is "with" or "assert", so just consume it.
Previously it only deoptimized the parent scope if the current scope
contains direct eval, which is incorrect because code ran in direct
eval mode has access to the entire scope chain it was executed in.
The fix is to also propagate direct eval's presence if the current
scope is marked as being screwed by direct eval.
This fixes Google's botguard failing to complete on Google sign in, as
it tried to access local variables outside of a direct parent function
with eval, causing it throw "unhandled" exceptions. Unhandled is in
quotes because their bytecode VM _technically_ caught it, but it was
considered an unhandled exception. This was determined by removing get
optimizations and then adding debug output for every get operation.
Using this, I noticed that for these errors, it would access the
'message' and 'stack' properties. This is because their error handler
function noticed this was not a synthesised error, which is never
expected to happen. That was determined by using Chrome Devtools 'pause
on handled exception' feature, and noticing it never threw a '[var] is
not defined' exception, but only synthesized error objects which
contained a sentinel value to let it know it was synthesized.
I added debug output to eval to print out what was being eval'd because
it makes heavy use of eval. This revealed that the exceptions only came
from eval.
I then dumped every generated executable and noticed the variables it
was trying to access were generated as local variables in the top
scope. This led to checking what makes a variable considered local or
not, which then lead to this block of code in ~ScopePusher that
propagates eval presence only to the immediate parent scope. This
variable directly controls whether to create all variables properly
with variable environments and bindings or allow them to be stored as
local registers tied to that function's executable.
Since this now lets botguard run to completion, it no longer considers
us to be an insecure/potential bot browser when signing in, now
allowing us to be able to sign in to Google.
For example, https://locals.com/site/discover has a script with an
object of the form:
var f = {
parser: {
sync() {},
async async() {},
}
};
We were previously throwing a syntax error on the async function, as we
specifically did not allow using "async" as a function name here.
