From 6ac0fabb56ea20a4ec778d014eedb7f8a3bc0ec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 12 May 2023 02:46:42 +0200 Subject: [PATCH] test: Add tests showing incorrect exception behavior in debug build (#1227) Reproduces #1226 and denoland/deno#19021 ``` // Fails $ V8_FROM_SOURCE=1 cargo test exception // Passes $ V8_FROM_SOURCE=1 cargo test --release exception ``` We bisected this and this problem first appeared with V8 v11.2 upgrade. After further bisects we established that v8/v8@1f349da#diff-de75fc3e7b84373d94e18b4531827e8b749f9bbe05b59707e894e4e0ce2a1535 is the first V8 commit that causes this failure. However after more investigation we can't find any reason why that particular commit might cause this problem. It is only reproducible in debug build, but not release build. Current working theory is that it is a Rust compiler bug as changing the optimization level for this code makes the bug go away. This commit should be considered a band-aid that works around the problem, but doesn't fix it completely. We are gonna go with it as it unblocks un on day-to-day work, but ultimately we should track it down (or wait for another Rust upgrade which might fix it?). --------- Co-authored-by: Bert Belder --- src/script.rs | 2 +- tests/test_api.rs | 118 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/src/script.rs b/src/script.rs index 8d15119e..bdb5e1c5 100644 --- a/src/script.rs +++ b/src/script.rs @@ -80,7 +80,7 @@ impl Script { /// Runs the script returning the resulting value. It will be run in the /// context in which it was created (ScriptCompiler::CompileBound or /// UnboundScript::BindToCurrentContext()). - #[inline(always)] + #[inline] pub fn run<'s>( &self, scope: &mut HandleScope<'s>, diff --git a/tests/test_api.rs b/tests/test_api.rs index 2d186663..7b84ae74 100644 --- a/tests/test_api.rs +++ b/tests/test_api.rs @@ -9657,3 +9657,121 @@ fn object_define_property() { assert!(expected.strict_equals(actual)); } } + +// Regression test for https://github.com/denoland/deno/issues/19021 +#[test] +fn bubbling_up_exception() { + let _setup_guard = setup::parallel_test(); + let isolate = &mut v8::Isolate::new(Default::default()); + let scope = &mut v8::HandleScope::new(isolate); + let context = v8::Context::new(scope); + let scope = &mut v8::ContextScope::new(scope, context); + + fn boom_fn( + scope: &mut v8::HandleScope, + _args: v8::FunctionCallbackArguments, + _retval: v8::ReturnValue, + ) { + let msg = v8::String::new(scope, "boom").unwrap(); + let exception = v8::Exception::type_error(scope, msg); + scope.throw_exception(exception); + } + + let global_proxy = scope.get_current_context().global(scope); + let identifier = v8::String::new(scope, "boom").unwrap(); + let value = v8::FunctionTemplate::new(scope, boom_fn) + .get_function(scope) + .unwrap(); + global_proxy.set(scope, identifier.into(), value.into()); + + let code = r#" +try { + boom() +} catch (e) { + // +} +"#; + + let source = v8::String::new(scope, code).unwrap(); + let script = v8::Script::compile(scope, source, None).unwrap(); + + let scope = &mut v8::TryCatch::new(scope); + let _result = script.run(scope); + // This fails in debug build, but passes in release build. + assert!(!scope.has_caught()); + assert!(scope.exception().is_none()); +} + +// Regression test for https://github.com/denoland/rusty_v8/issues/1226 +#[test] +fn exception_thrown_but_continues_execution() { + let _setup_guard = setup::parallel_test(); + let isolate = &mut v8::Isolate::new(Default::default()); + let scope = &mut v8::HandleScope::new(isolate); + let context = v8::Context::new(scope); + let scope = &mut v8::ContextScope::new(scope, context); + + static CALL_COUNT: AtomicUsize = AtomicUsize::new(0); + + fn call_object_property<'s>( + scope: &mut v8::HandleScope<'s>, + value: v8::Local, + property: &str, + ) -> Option> { + let object: v8::Local = value.try_into().unwrap(); + let ident = v8::String::new(scope, property).unwrap().into(); + let prop = object.get(scope, ident).unwrap(); + let func: v8::Local = prop.try_into().unwrap(); + let recv = scope.get_current_context().global(scope).into(); + + let retval = func.call(scope, recv, &[]); + retval + } + + fn print_fn( + scope: &mut v8::HandleScope, + args: v8::FunctionCallbackArguments, + _retval: v8::ReturnValue, + ) { + let local_arg = args.get(0); + CALL_COUNT.fetch_add(1, Ordering::SeqCst); + let print_str = if local_arg.is_string() { + local_arg.to_rust_string_lossy(scope) + } else if local_arg.is_object() { + let obj_repr = call_object_property(scope, local_arg, "repr"); + if obj_repr.is_none() { + return; + } + "[".to_owned() + &obj_repr.unwrap().to_rust_string_lossy(scope) + "]" + } else { + "Unknown type".to_owned() + }; + println!("{print_str}"); + } + + let global_proxy = scope.get_current_context().global(scope); + let identifier = v8::String::new(scope, "print").unwrap(); + let value = v8::FunctionTemplate::new(scope, print_fn) + .get_function(scope) + .unwrap(); + global_proxy.set(scope, identifier.into(), value.into()); + + let code = r#" + let object_with_ok_repr = { + repr: function() { return "object_with_ok_repr"; } + }; + print(object_with_ok_repr); + let object_with_broken_repr = { + repr: function() { boom } + }; + print(object_with_broken_repr); + print("this should not be reachable"); + "#; + + let source = v8::String::new(scope, code).unwrap(); + let script = v8::Script::compile(scope, source, None).unwrap(); + + let scope = &mut v8::TryCatch::new(scope); + let _result = script.run(scope); + assert_eq!(CALL_COUNT.load(Ordering::SeqCst), 2); +}