Skip to content

fix(vm): harden async-generator/eval paths (#706, #707, #708, #709) #950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
replygirl wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(vm): harden async-generator/eval paths (#706, #707, #708, #709) #950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 39 additions & 49 deletions nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,6 @@ impl AsyncGenerator<'_> {
self.get(agent).executable.unwrap().bind(gc)
}

/// Returns true if the state of the AsyncGenerator is DRAINING-QUEUE or
/// EXECUTING.
///
/// > NOTE: In our implementation, EXECUTING is split into an extra
/// > EXECUTING-AWAIT state. This also checks for that.
pub(crate) fn is_active(self, agent: &Agent) -> bool {
self.get(agent)
.async_generator_state
.as_ref()
.unwrap()
.is_active()
}

pub(crate) fn is_draining_queue(self, agent: &Agent) -> bool {
self.get(agent)
.async_generator_state
Expand All @@ -58,14 +45,6 @@ impl AsyncGenerator<'_> {
.is_draining_queue()
}

pub(crate) fn is_executing(self, agent: &Agent) -> bool {
self.get(agent)
.async_generator_state
.as_ref()
.unwrap()
.is_executing()
}

pub(crate) fn is_suspended_start(self, agent: &Agent) -> bool {
self.get(agent)
.async_generator_state
Expand Down Expand Up @@ -155,8 +134,9 @@ impl AsyncGenerator<'_> {
AsyncGeneratorState::SuspendedStart { queue, .. }
| AsyncGeneratorState::SuspendedYield { queue, .. }
| AsyncGeneratorState::Executing(queue)
| AsyncGeneratorState::ExecutingAwait { queue, .. }
| AsyncGeneratorState::DrainingQueue(queue)
| AsyncGeneratorState::Completed(queue) => queue,
_ => unreachable!(),
};
async_generator_state.replace(AsyncGeneratorState::DrainingQueue(queue));
}
Expand All @@ -183,8 +163,13 @@ impl AsyncGenerator<'_> {
execution_context: ExecutionContext,
) {
let async_generator_state = &mut self.get_mut(agent).async_generator_state;
let AsyncGeneratorState::Executing(queue) = async_generator_state.take().unwrap() else {
unreachable!()
let state = async_generator_state.take().unwrap();
let queue = match state {
AsyncGeneratorState::Executing(queue) => queue,
state => {
async_generator_state.replace(state);
return;
}
};
async_generator_state.replace(AsyncGeneratorState::ExecutingAwait {
queue,
Expand All @@ -198,40 +183,50 @@ impl AsyncGenerator<'_> {
self,
agent: &mut Agent,
gc: NoGcScope<'gc, '_>,
) -> (SuspendedVm, ExecutionContext, Executable<'gc>) {
) -> Option<(SuspendedVm, ExecutionContext, Executable<'gc>)> {
let async_generator_state = &mut self.get_mut(agent).async_generator_state;
let (vm, execution_context, queue) = match async_generator_state.take() {
Some(AsyncGeneratorState::SuspendedStart {
let state = async_generator_state.take()?;
let (vm, execution_context, queue) = match state {
AsyncGeneratorState::SuspendedStart {
vm,
execution_context,
queue,
}) => (vm, execution_context, queue),
Some(AsyncGeneratorState::SuspendedYield {
} => (vm, execution_context, queue),
AsyncGeneratorState::SuspendedYield {
vm,
execution_context,
queue,
}) => (vm, execution_context, queue),
_ => unreachable!(),
} => (vm, execution_context, queue),
state => {
async_generator_state.replace(state);
return None;
}
};
async_generator_state.replace(AsyncGeneratorState::Executing(queue));
(vm, execution_context, self.get_executable(agent, gc))
Some((vm, execution_context, self.get_executable(agent, gc)))
}

pub(crate) fn transition_to_suspended(
self,
agent: &mut Agent,
vm: SuspendedVm,
execution_context: ExecutionContext,
) {
) -> bool {
let async_generator_state = &mut self.get_mut(agent).async_generator_state;
let AsyncGeneratorState::Executing(queue) = async_generator_state.take().unwrap() else {
unreachable!()
let state = async_generator_state.take().unwrap();
let queue = match state {
AsyncGeneratorState::Executing(queue) => queue,
state => {
async_generator_state.replace(state);
return false;
}
};
async_generator_state.replace(AsyncGeneratorState::SuspendedYield {
queue,
vm,
execution_context,
});
true
}

pub(crate) fn resume_await(
Expand All @@ -258,6 +253,11 @@ impl AsyncGenerator<'_> {
// 1. Assert: generator.[[AsyncGeneratorState]] is either suspended-start or suspended-yield.
let state = self.get_mut(agent).async_generator_state.take().unwrap();
let (vm, execution_context, queue, kind) = match state {
AsyncGeneratorState::SuspendedStart {
vm,
execution_context,
queue,
} => (vm, execution_context, queue, AsyncGeneratorAwaitKind::Await),
AsyncGeneratorState::SuspendedYield {
vm,
execution_context,
Expand All @@ -269,7 +269,10 @@ impl AsyncGenerator<'_> {
queue,
kind,
} => (vm, execution_context, queue, kind),
_ => unreachable!(),
state => {
self.get_mut(agent).async_generator_state.replace(state);
return;
}
};
agent.push_execution_context(execution_context);
self.get_mut(agent).async_generator_state = Some(AsyncGeneratorState::Executing(queue));
Expand Down Expand Up @@ -381,15 +384,6 @@ pub(crate) enum AsyncGeneratorState<'a> {
}

impl AsyncGeneratorState<'_> {
pub(crate) fn is_active(&self) -> bool {
matches!(
self,
AsyncGeneratorState::DrainingQueue(_)
| AsyncGeneratorState::Executing(_)
| AsyncGeneratorState::ExecutingAwait { .. }
)
}

pub(crate) fn is_completed(&self) -> bool {
matches!(self, Self::Completed(_))
}
Expand All @@ -398,10 +392,6 @@ impl AsyncGeneratorState<'_> {
matches!(self, AsyncGeneratorState::DrainingQueue(_))
}

pub(crate) fn is_executing(&self) -> bool {
matches!(self, AsyncGeneratorState::Executing(_))
}

pub(crate) fn is_suspended(&self) -> bool {
matches!(
self,
Expand Down
97 changes: 81 additions & 16 deletions ...ontrol_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

use crate::{
ecmascript::{
Agent, ECMAScriptFunction, ExceptionType, JsError, JsResult, Promise, PromiseCapability,
PromiseReactionHandler, Realm, Value, create_iter_result_object, inner_promise_then,
unwrap_try,
Agent, BUILTIN_STRING_MEMORY, ECMAScriptFunction, ExceptionType, JsError, JsResult,
Promise, PromiseCapability, PromiseReactionHandler, PromiseReactionType, Realm, Value,
create_iter_result_object, get, inner_promise_then, unwrap_try,
},
engine::{Bindable, ExecutionResult, GcScope, NoGcScope, Scopable, Scoped, SuspendedVm},
heap::ArenaAccessMut,
Expand Down Expand Up @@ -80,7 +80,9 @@ fn async_generator_complete_step(
) {
let completion = completion.bind(gc);
// 1. Assert: generator.[[AsyncGeneratorQueue]] is not empty.
assert!(!generator.queue_is_empty(agent));
if generator.queue_is_empty(agent) {
return;
}
// 2. Let next be the first element of generator.[[AsyncGeneratorQueue]].
// 3. Remove the first element from generator.[[AsyncGeneratorQueue]].
let next = generator.pop_first(agent, gc);
Expand All @@ -98,7 +100,7 @@ fn async_generator_complete_step(
}
// 7. Else,
// a. Assert: completion is a normal completion.
_ => unreachable!(),
AsyncGeneratorRequestCompletion::Return(value) => value,
};
// b. If realm is present, then
let iterator_result = if let Some(realm) = realm {
Expand Down Expand Up @@ -143,8 +145,10 @@ pub(crate) fn async_generator_resume(
// 1. Assert: generator.[[AsyncGeneratorState]] is either suspended-start or suspended-yield.
// 2. Let genContext be generator.[[AsyncGeneratorContext]].
// 5. Set generator.[[AsyncGeneratorState]] to executing.
assert!(generator.is_suspended_start(agent) || generator.is_suspended_yield(agent));
let (vm, gen_context, executable) = generator.transition_to_executing(agent, gc.nogc());
let Some((vm, gen_context, executable)) = generator.transition_to_executing(agent, gc.nogc())
else {
return;
};
let executable = executable.scope(agent, gc.nogc());

// 3. Let callerContext be the running execution context.
Expand Down Expand Up @@ -254,13 +258,33 @@ fn async_generator_perform_await(
mut gc: GcScope,
) {
// [27.7.5.3 Await ( value )](https://tc39.es/ecma262/#await)
let promise_resolve_error = if let Value::Promise(promise) = awaited_value {
match get(
agent,
promise,
BUILTIN_STRING_MEMORY.constructor.into(),
gc.reborrow(),
) {
Ok(_) => None,
Err(err) => Some(err.unbind()),
}
} else {
None
};

let execution_context = agent.pop_execution_context().unwrap();
let generator = scoped_generator.get(agent).bind(gc.nogc());
generator.transition_to_awaiting(agent, vm, kind, execution_context);
let generator = generator.unbind();
if let Some(err) = promise_resolve_error {
generator.resume_await(agent, PromiseReactionType::Reject, err.value().unbind(), gc);
return;
}

// 8. Remove asyncContext from the execution context stack and
// restore the execution context that is at the top of the
// execution context stack as the running execution context.
let handler = PromiseReactionHandler::AsyncGenerator(generator.unbind());
let handler = PromiseReactionHandler::AsyncGenerator(generator);
// 2. Let promise be ? PromiseResolve(%Promise%, value).
let promise = Promise::resolve(agent, awaited_value, gc.reborrow())
.unbind()
Expand Down Expand Up @@ -392,26 +416,63 @@ pub(crate) fn async_generator_await_return(
) {
let generator = scoped_generator.get(agent).bind(gc.nogc());
// 1. Assert: generator.[[AsyncGeneratorState]] is draining-queue.
assert!(generator.is_draining_queue(agent));
if !generator.is_draining_queue(agent) {
return;
}
// 2. Let queue be generator.[[AsyncGeneratorQueue]].
// 3. Assert: queue is not empty.
assert!(!generator.queue_is_empty(agent));
if generator.queue_is_empty(agent) {
return;
}
// 4. Let next be the first element of queue.
let next = generator.peek_first(agent, gc.nogc());
// 5. Let completion be Completion(next.[[Completion]]).
let completion = next.completion;
// 6. Assert: completion is a return completion.
let AsyncGeneratorRequestCompletion::Return(value) = completion else {
unreachable!()
async_generator_complete_step(agent, generator.unbind(), completion, true, None, gc.nogc());
async_generator_drain_queue(agent, scoped_generator, gc);
return;
};
let generator = generator.unbind();
let return_value = value.unbind();
// 7. Let promiseCompletion be Completion(PromiseResolve(%Promise%, completion.[[Value]])).
let constructor_error = if let Value::Promise(promise) = return_value {
match get(
agent,
promise,
BUILTIN_STRING_MEMORY.constructor.into(),
gc.reborrow(),
) {
Ok(_) => None,
Err(err) => Some(err.unbind()),
}
} else {
None
};

if let Some(err) = constructor_error {
let generator = generator.bind(gc.nogc());
generator.transition_to_complete(agent);
async_generator_complete_step(
agent,
generator.unbind(),
AsyncGeneratorRequestCompletion::Err(err),
true,
None,
gc.nogc(),
);
async_generator_drain_queue(agent, scoped_generator, gc);
return;
}

// 8. If promiseCompletion is an abrupt completion, then
// a. Perform AsyncGeneratorCompleteStep(generator, promiseCompletion, true).
// b. Perform AsyncGeneratorDrainQueue(generator).
// c. Return unused.
// 9. Assert: promiseCompletion is a normal completion.
// 10. Let promise be promiseCompletion.[[Value]].
let promise = Promise::resolve(agent, value.unbind(), gc.reborrow())
let promise = Promise::resolve(agent, return_value, gc.reborrow())
.unbind()
.bind(gc.nogc());
// 11. ... onFulfilled ...
Expand All @@ -437,7 +498,9 @@ pub(crate) fn async_generator_await_return_on_fulfilled(
// (value) that captures generator and performs the following steps
// when called:
// a. Assert: generator.[[AsyncGeneratorState]] is draining-queue.
assert!(generator.is_draining_queue(agent));
if !generator.is_draining_queue(agent) {
return;
}
// b. Let result be NormalCompletion(value).
// c. Perform AsyncGeneratorCompleteStep(generator, result, true).
let scoped_generator = generator.scope(agent, gc.nogc());
Expand Down Expand Up @@ -466,7 +529,9 @@ pub(crate) fn async_generator_await_return_on_rejected(
// (reason) that captures generator and performs the following steps
// when called:
// a. Assert: generator.[[AsyncGeneratorState]] is draining-queue.
assert!(generator.is_draining_queue(agent));
if !generator.is_draining_queue(agent) {
return;
}
// b. Let result be ThrowCompletion(reason).
let scoped_generator = generator.scope(agent, gc.nogc());
// c. Perform AsyncGeneratorCompleteStep(generator, result, true).
Expand Down Expand Up @@ -499,7 +564,7 @@ fn async_generator_drain_queue(
// Assert: generator.[[AsyncGeneratorState]] is draining-queue.
// 2. Let queue be generator.[[AsyncGeneratorQueue]].
let Some(AsyncGeneratorState::DrainingQueue(queue)) = &mut data.async_generator_state else {
unreachable!()
return;
};
// 3. If queue is empty, then
if queue.is_empty() {
Expand Down Expand Up @@ -538,7 +603,7 @@ fn async_generator_drain_queue(
// iii. If queue is empty, then
let Some(AsyncGeneratorState::DrainingQueue(queue)) = &mut data.async_generator_state
else {
unreachable!()
return;
};
if queue.is_empty() {
// 1. Set generator.[[AsyncGeneratorState]] to completed.
Expand Down
Loading