The promise job's fulfillment / rejection handlers may push an execution
context onto the VM, which will throw an internal error if our ad-hoc
call stack size limit has been reached. Thus, we cannot blindly VERIFY
that the result of invoking these handlers is non-abrupt.
This patch will propagate any internal error forward, and retains the
condition that any other error type is not thrown.
There are now no users of the MUST_OR_THROW_OOM macro. Let's rename this
macro to indicate it may be used to propagate any internal error (such
as the call stack limit error) in places that would otherwise crash due
to a MUST/VERIFY invocation.
Note there's no actual functional change here, as we weren't able to
ensure the internal error was an OOM error previously.
Instead of adding a flag for the two callers that need a pop of the
execution context stack when invoking continue_async_execution inline
the pop of the execution context.
This makes the management of these stacks and surrounding VERIFY calls
much more obvious.
This has no functional difference as run_queued_promise jobs does
nothing when LibWeb is used as it has a different implementation of
enqueuing and running promise jobs. But this change makes it more
obvious that run_queued_promise jobs does nothing when there is an
embedder, and adjusts the comment to reflect what the code is
actually achieving.
In the very common case that no special constructor options are provided
for the Intl.Collator when calling localeCompare() on a string, we can
cache and reuse a default-constructed Intl.Collator, saving lots of time
and space.
This shaves a fair bit of load time off of https://wpt.fyi/ where they
use Array.prototype.sort() and localeCompare() to sort a big JSON thing.
Time spent in sort():
- Before: 1656 ms
- After: 135 ms
Before this change, it was possible for a second GC to get triggered
in the middle of a first GC, due to allocations happening in the
FinalizationRegistry cleanup host hook. To avoid this causing problems,
we add a "post-GC task" mechanism and use that to invoke the host hook
once all other GC activity is finished, and we've unset the "collecting
garbage" flag.
Note that the test included here only fails reliably when running with
the -g flag (collect garbage after each allocation).
Fixes#3051
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.
It is currently implemented as a member of CyclicModule. However, as the
spec indicates, this must be invokable with non-CyclicModule modules. In
several of the call sites, we are blindly casting to a CyclicModule;
this will fail for e.g. JSON modules.
Linking a module has assertions about the module's state, namely that
the state is not "new". The state remains "new" if loading the module
has failed. See: https://tc39.es/ecma262/#figure-module-graph-missing
In any case, this exception causes a loading failure, which results
in A's [[Status]] remaining new.
So we must propagate that failure, instead of blindly moving on to the
linking steps.
This updates our local ICU overlay port to use ICU 76.1. This includes
Unicode 16 and CLDR 46.
Upstream vcpkg is not able to supply versions past 74 yet due to various
dependency issues, but we are able to use this version ourselves. The
overlay port now includes a patch to revert ICU's dependence on autoconf
2.72 for now, as this version is not yet available on all systems.
All of the test changes were cross-referenced with Firefox to ensure
correctness.
In ICU 76, the default was changed from "arab" to "latn". See:
c149724509
The whole point of these tests was to use a non-Latin numbering system.
This patch ensures that is the case to make following patches easier to
grok.
While we don't yet have a working `using` implementation with our byte
code, we can still keep our DisposableStack implementation up to date.
The changes brought in here are all editorial, and set us up to start
an AsyncDisposableStack implementation.
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.
Instead of clamping to the limits allowed by ISOYearMonthWithinLimits,
clamp to the limits allowed by the type we are converting to (i32). This
allows some callers to then reject years outside that range.
This is no longer done. One of the comments is also innacurate for a
second reason - the call stack is never empty in that case, and is
verified as such only a few lines above.
From what I understand, the suspension steps are not required now,
or in the future for our implementation, or any other. The intent
is already implemented in the spec pushing on another execution
context to the stack and leaving the running execution context as-is.
The resume steps are a slightly different story as there is some subtle
behavior which the spec is trying to convey where some custom logic may
need to be done when one execution context changes from one to another.
It may be worth implementing those steps at a later point in time so
that this behavior is a bit easier to follow in those cases.
To make the situation more confusing - from what I can gather from the
spec, not all cases that the spec mentions resume actually means
anything normative. Resume is only _actually_ needed in a limited set
of locations.
For now, let's just remove the unneeded FIXMEs that indicate that there
is something to be done for the suspension steps, as there is not, and
leave the resume steps as is.
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.
Even though calling delete on a super property will ultimately throw a
ReferenceError, we must return the allocated register for the result of
the delete operation (which would normally be a boolean). If the delete
operation is used in a return statement, the bytecode generator for the
return statement must be able to assume the statement had some output.
