Skip to content

fix(vm/opcode/push,environment,templates,generator,await,new): convert panics to EngineError::Panic using js_expect #5232

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
KaustubhOG wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(vm/opcode/push,environment,templates,generator,await,new): convert panics to EngineError::Panic using js_expect #5232

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
21 changes: 14 additions & 7 deletions core/engine/src/vm/opcode/await/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
Promise, async_generator::AsyncGenerator, generator::GeneratorContext,
promise::PromiseCapability,
},
error::PanicError,
js_string,
native_function::NativeFunction,
object::FunctionObjectBuilder,
Expand Down Expand Up @@ -34,9 +35,15 @@ impl Await {
value.clone(),
context,
) {
Ok(promise) => promise
.downcast::<Promise>()
.expect("%Promise% constructor must return a `Promise` object"),
Ok(promise) => match promise.downcast::<Promise>().ok() {
Some(v) => v,
None => {
return context.handle_error(
PanicError::new("%Promise% constructor must return a `Promise` object")
.into(),
);
}
},
Err(err) => return context.handle_error(err),
};

Expand All @@ -61,7 +68,7 @@ impl Await {
// b. Suspend prevContext.
// c. Push asyncContext onto the execution context stack; asyncContext is now the running execution context.
// d. Resume the suspended evaluation of asyncContext using NormalCompletion(value) as the result of the operation that suspended it.
let mut r#gen = captures.take().expect("should only run once");
let mut r#gen = captures.take().js_expect("should only run once")?;

// NOTE: We need to get the object before resuming, since it could clear the stack.
let async_generator = r#gen.async_generator_object()?;
Expand All @@ -75,7 +82,7 @@ impl Await {
if let Some(async_generator) = async_generator {
async_generator
.downcast_mut::<AsyncGenerator>()
.expect("must be async generator")
.js_expect("must be async generator")?
.context = Some(r#gen);
}

Expand All @@ -102,7 +109,7 @@ impl Await {
// d. Resume the suspended evaluation of asyncContext using ThrowCompletion(reason) as the result of the operation that suspended it.
// e. Assert: When we reach this step, asyncContext has already been removed from the execution context stack and prevContext is the currently running execution context.
// f. Return undefined.
let mut r#gen = captures.take().expect("should only run once");
let mut r#gen = captures.take().js_expect("should only run once")?;

// NOTE: We need to get the object before resuming, since it could clear the stack.
let async_generator = r#gen.async_generator_object()?;
Expand All @@ -116,7 +123,7 @@ impl Await {
if let Some(async_generator) = async_generator {
async_generator
.downcast_mut::<AsyncGenerator>()
.expect("must be async generator")
.js_expect("must be async generator")?
.context = Some(r#gen);
}

Expand Down
26 changes: 13 additions & 13 deletions core/engine/src/vm/opcode/environment/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl This {
.unwrap_or(context.realm().global_this().clone().into());
context.vm.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED;
context.vm.stack.set_this(
context.vm.frames.last().expect("frame must exist"),
context.vm.frames.last().js_expect("frame must exist")?,
this.clone(),
);
context.vm.set_register(dst.into(), this);
Expand Down Expand Up @@ -141,13 +141,13 @@ impl SuperCall {
.environments
.get_this_environment(frame.realm.environment())
.as_function()
.expect("super call must be in function environment")
.js_expect("super call must be in function environment")?
};

let new_target = this_env
.slots()
.new_target()
.expect("must have new.target")
.js_expect("must have new.target")?
.clone();

context.vm.stack.push(new_target);
Expand Down Expand Up @@ -179,12 +179,12 @@ impl SuperCallSpread {
let arguments_array = context.vm.stack.pop();
let arguments_array_object = arguments_array
.as_object()
.expect("arguments array in call spread function must be an object");
.js_expect("arguments array in call spread function must be an object")?;
let arguments = arguments_array_object
.borrow()
.properties()
.to_dense_indexed_properties()
.expect("arguments array in call spread function must be dense");
.js_expect("arguments array in call spread function must be dense")?;

let super_constructor = context.vm.stack.pop();

Expand All @@ -207,13 +207,13 @@ impl SuperCallSpread {
.environments
.get_this_environment(frame.realm.environment())
.as_function()
.expect("super call must be in function environment")
.js_expect("super call must be in function environment")?
};

let new_target = this_env
.slots()
.new_target()
.expect("must have new.target")
.js_expect("must have new.target")?
.clone();

context.vm.stack.push(new_target);
Expand Down Expand Up @@ -247,18 +247,18 @@ impl SuperCallDerived {
.environments
.get_this_environment(frame.realm.environment())
.as_function()
.expect("super call must be in function environment")
.js_expect("super call must be in function environment")?
};
let new_target = this_env
.slots()
.new_target()
.expect("must have new target")
.js_expect("must have new target")?
.clone();
let active_function = this_env.slots().function_object().clone();
let super_constructor = active_function
.__get_prototype_of__(&mut InternalMethodPropertyContext::new(context))
.expect("function object must have prototype")
.expect("function object must have prototype");
.js_expect("function object must have prototype")?
.js_expect("function object must have prototype")?;

if !super_constructor.is_constructor() {
return Err(JsNativeError::typ()
Expand Down Expand Up @@ -304,7 +304,7 @@ impl BindThisValue {
.vm
.get_register(value.into())
.as_object()
.expect("construct result should be an object")
.js_expect("construct result should be an object")?
.clone();

// 7. Let thisER be GetThisEnvironment().
Expand All @@ -314,7 +314,7 @@ impl BindThisValue {
.environments
.get_this_environment(frame.realm.environment())
.as_function()
.expect("super call must be in function environment")
.js_expect("super call must be in function environment")?
};

// 8. Perform ? thisER.BindThisValue(result).
Expand Down
32 changes: 19 additions & 13 deletions core/engine/src/vm/opcode/generator/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
pub(crate) mod yield_stm;

use crate::{
Context, JsObject, JsResult,
Context, JsExpect, JsObject, JsResult,
builtins::{
async_generator::{AsyncGenerator as NativeAsyncGenerator, AsyncGeneratorState},
generator::{Generator as NativeGenerator, GeneratorContext, GeneratorState},
},
error::PanicError,
object::PROTOTYPE,
vm::{CompletionRecord, opcode::Operation},
};
Expand All @@ -23,14 +24,16 @@ pub(crate) struct Generator;
impl Generator {
#[inline(always)]
pub(super) fn operation((): (), context: &mut Context) -> ControlFlow<CompletionRecord> {
let active_function = context.vm.stack.get_function(context.vm.frame());
let this_function_object =
active_function.expect("active function should be set to the generator");
let Some(this_function_object) = context.vm.stack.get_function(context.vm.frame()) else {
return context.handle_error(
PanicError::new("active function should be set to the generator").into(),
);
};

let proto = this_function_object
.get(PROTOTYPE, context)
.expect("generator must have a prototype property")
.as_object()
.ok()
.and_then(|v| v.as_object())
.unwrap_or_else(|| context.intrinsics().objects().generator());

let generator = JsObject::from_proto_and_data_with_shared_shape(
Expand Down Expand Up @@ -67,14 +70,16 @@ pub(crate) struct AsyncGenerator;
impl AsyncGenerator {
#[inline(always)]
pub(super) fn operation((): (), context: &mut Context) -> ControlFlow<CompletionRecord> {
let active_function = context.vm.stack.get_function(context.vm.frame());
let this_function_object =
active_function.expect("active function should be set to the generator");
let Some(this_function_object) = context.vm.stack.get_function(context.vm.frame()) else {
return context.handle_error(
PanicError::new("active function should be set to the generator").into(),
);
};

let proto = this_function_object
.get(PROTOTYPE, context)
.expect("generator must have a prototype property")
.as_object()
.ok()
.and_then(|v| v.as_object())
.unwrap_or_else(|| context.intrinsics().objects().async_generator());

let generator = JsObject::from_proto_and_data_with_shared_shape(
Expand Down Expand Up @@ -115,9 +120,10 @@ impl AsyncGeneratorClose {
let generator = context
.vm
.async_generator_object()
.expect("There should be a object")
.js_expect("There should be a object")?
.downcast::<NativeAsyncGenerator>()
.expect("must be async generator");
.ok()
.js_expect("must be async generator")?;

let mut r#gen = generator.borrow_mut();

Expand Down
19 changes: 12 additions & 7 deletions core/engine/src/vm/opcode/generator/yield_stm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::ops::ControlFlow;
use crate::{
Context, JsValue,
builtins::async_generator::{AsyncGenerator, AsyncGeneratorState},
error::PanicError,
vm::{
CompletionRecord, GeneratorResumeKind,
opcode::{Operation, RegisterOperand},
Expand Down Expand Up @@ -54,13 +55,17 @@ impl AsyncGeneratorYield {
// 2. Assert: genContext is the execution context of a generator.
// 3. Let generator be the value of the Generator component of genContext.
// 4. Assert: GetGeneratorKind() is async.
let async_generator_object = context
.vm
.async_generator_object()
.expect("`AsyncGeneratorYield` must only be called inside async generators");
let async_generator_object = async_generator_object
.downcast::<AsyncGenerator>()
.expect("must be async generator object");
let Some(async_generator_object) = context.vm.async_generator_object() else {
return context.handle_error(
PanicError::new(
"`AsyncGeneratorYield` must only be called inside async generators",
)
.into(),
);
};
let Ok(async_generator_object) = async_generator_object.downcast::<AsyncGenerator>() else {
return context.handle_error(PanicError::new("must be async generator object").into());
};

// 5. Let completion be NormalCompletion(value).
let value = context.vm.get_register(value.into());
Expand Down
6 changes: 3 additions & 3 deletions core/engine/src/vm/opcode/new/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::IndexOperand;
use crate::{Context, JsResult, error::JsNativeError, vm::opcode::Operation};
use crate::{Context, JsExpect, JsResult, error::JsNativeError, vm::opcode::Operation};

/// `New` implements the Opcode Operation for `Opcode::New`
///
Expand Down Expand Up @@ -48,12 +48,12 @@ impl NewSpread {
let arguments_array = context.vm.stack.pop();
let arguments_array_object = arguments_array
.as_object()
.expect("arguments array in call spread function must be an object");
.js_expect("arguments array in call spread function must be an object")?;
let arguments = arguments_array_object
.borrow()
.properties()
.to_dense_indexed_properties()
.expect("arguments array in call spread function must be dense");
.js_expect("arguments array in call spread function must be dense")?;

let func = context.vm.stack.pop();

Expand Down
19 changes: 10 additions & 9 deletions core/engine/src/vm/opcode/push/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
Context, JsResult, JsValue,
Context, JsExpect, JsResult, JsValue,
builtins::Array,
string::StaticJsStrings,
vm::opcode::{Operation, RegisterOperand},
Expand Down Expand Up @@ -42,13 +42,13 @@ impl PushValueToArray {
pub(crate) fn operation(
(value, array): (RegisterOperand, RegisterOperand),
context: &mut Context,
) {
) -> JsResult<()> {
let value = context.vm.get_register(value.into()).clone();
let o = context
.vm
.get_register(array.into())
.as_object()
.expect("should be an object");
.js_expect("should be an object")?;

// Fast path: push directly to dense indexed storage.
{
Expand All @@ -58,16 +58,17 @@ impl PushValueToArray {
&& o_mut.properties_mut().indexed_properties.push_dense(&value)
{
o_mut.properties_mut().storage[0] = JsValue::new(len + 1);
return;
return Ok(());
}
}

// Slow path: fall through to the generic property machinery.
let len = o
.length_of_array_like(context)
.expect("should have 'length' property");
.js_expect("should have 'length' property")?;
o.create_data_property_or_throw(len, value, context)
.expect("should be able to create new data property");
.js_expect("should be able to create new data property")?;
Ok(())
}
}

Expand All @@ -88,10 +89,10 @@ impl PushElisionToArray {
#[inline(always)]
pub(crate) fn operation(array: RegisterOperand, context: &mut Context) -> JsResult<()> {
let array = context.vm.get_register(array.into()).clone();
let o = array.as_object().expect("should always be an object");
let o = array.as_object().js_expect("should always be an object")?;
let len = o
.length_of_array_like(context)
.expect("arrays should always have a 'length' property");
.js_expect("arrays should always have a 'length' property")?;
o.set(StaticJsStrings::LENGTH, len + 1, true, context)?;
o.borrow_mut()
.properties_mut()
Expand Down Expand Up @@ -123,7 +124,7 @@ impl PushIteratorToArray {
.frame_mut()
.iterators
.pop()
.expect("iterator stack should have at least an iterator");
.js_expect("iterator stack should have at least an iterator")?;
while let Some(next) = iterator.step_value(context)? {
Array::push(&array, &[next], context)?;
}
Expand Down
Loading
Loading