From b133bf087c14c25f125e15e589b2406e91949795 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Tue, 1 May 2018 14:09:52 +0200 Subject: [PATCH] fix: Don't allow string references to be returned from run_expr --- c-api/src/lib.rs | 2 +- scripts/travis.sh | 2 +- tests/support/mod.rs | 8 ++++---- tests/vm.rs | 10 +++++++++- vm/src/api/de.rs | 3 ++- vm/src/api/mod.rs | 25 +++++++++++++++---------- 6 files changed, 32 insertions(+), 18 deletions(-) diff --git a/c-api/src/lib.rs b/c-api/src/lib.rs index 07be2051d8..bab988343a 100644 --- a/c-api/src/lib.rs +++ b/c-api/src/lib.rs @@ -212,7 +212,7 @@ pub unsafe extern "C" fn glu_get_string( let stack = context.stack.current_frame(); match stack .get_variant(index) - .map(|value| <&str>::from_value(vm, value)) + .map(|value| <&str>::from_value_unsafe(vm, value)) { Some(value) => { *out = &*value.as_ptr(); diff --git a/scripts/travis.sh b/scripts/travis.sh index 54e22bfb63..0c4762deaf 100755 --- a/scripts/travis.sh +++ b/scripts/travis.sh @@ -10,7 +10,7 @@ if [ -z $NO_NORMAL_TEST ]; then echo "" | cargo run --features "test" --example 24 echo "TRAVIS_RUST_VERSION=$TRAVIS_RUST_VERSION" - (echo $TRAVIS_RUST_VERSION | grep -v nightly) && cargo test --features "test nightly" -p gluon compile_test "$@" + (echo $TRAVIS_RUST_VERSION | grep nightly) && cargo test --features "test nightly" -p gluon compile_test "$@" fi if [ ! -z $BENCH_DEFAULT_FEATURES_CHECK ] || [ -z CI ]; then diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 1dcb36282b..5775964503 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -64,7 +64,7 @@ macro_rules! test_expr { let mut vm = $crate::support::make_vm(); let value = $crate::support::run_expr_(&mut vm, $expr, true); assert_eq!(value, $value); - + // Help out the type inference by forcing that left and right are the same types fn equiv(_: &T, _: &T) {} equiv(&value, &$value); @@ -74,7 +74,7 @@ macro_rules! test_expr { #[test] fn $name() { use gluon::vm::api::IO; - + let _ = ::env_logger::try_init(); let mut vm = $crate::support::make_vm(); let (value, _) = ::gluon::Compiler::new() @@ -85,7 +85,7 @@ macro_rules! test_expr { match value { IO::Value(value) => { assert_eq!(value, $value); - + // Help out the type inference by forcing that left and right are the same types fn equiv(_: &T, _: &T) {} equiv(&value, &$value); @@ -110,7 +110,7 @@ macro_rules! test_expr { let mut vm = $crate::support::make_vm(); let value = $crate::support::run_expr(&mut vm, $expr); assert_eq!(value, $value); - + // Help out the type inference by forcing that left and right are the same types fn equiv(_: &T, _: &T) {} equiv(&value, &$value); diff --git a/tests/vm.rs b/tests/vm.rs index 10507684d3..b38f5574ac 100644 --- a/tests/vm.rs +++ b/tests/vm.rs @@ -425,7 +425,7 @@ test_expr!{ record_base_duplicate_fields, r#" { x = "" .. { x = 1 } }.x "#, -"" +"".to_string() } test_expr!{ record_base_duplicate_fields2, @@ -900,3 +900,11 @@ fn deep_clone_partial_application() { global_memory_with_closures ); } + +#[test] +#[should_panic] +fn run_expr_to_string_reference_is_ice() { + let vm = make_vm(); + + let _ = Compiler::new().run_expr::<&str>(&vm, "", r#" "test" "#); +} diff --git a/vm/src/api/de.rs b/vm/src/api/de.rs index 494b7a4fa0..de2ce74cdd 100644 --- a/vm/src/api/de.rs +++ b/vm/src/api/de.rs @@ -239,7 +239,8 @@ impl<'de, 't> Deserializer<'de, 't> { { let typ = resolve::remove_aliases_cow(self.state.env, self.typ); if expected(&typ) { - visit(T::from_value(self.state.thread, self.input)) + // We can rely on `self.input` being rooted for `de` letting us use `from_value_unsafe` + unsafe { visit(T::from_value_unsafe(self.state.thread, self.input)) } } else { Err(VmError::Message(format!( "Unable to deserialize `{}`", diff --git a/vm/src/api/mod.rs b/vm/src/api/mod.rs index bd31a08657..791b983cfe 100644 --- a/vm/src/api/mod.rs +++ b/vm/src/api/mod.rs @@ -519,19 +519,22 @@ where } // Only allow the unsafe version to be used fn from_value(_vm: &'vm Thread, _value: Variants) -> Self { - ice!("Getable::from_value usage") + panic!("Getable::from_value on references is only allowed in unsafe contexts") } } impl<'vm> Getable<'vm> for &'vm str { - fn from_value(_vm: &'vm Thread, value: Variants) -> Self { - unsafe { - match value.as_ref() { - ValueRef::String(ref s) => forget_lifetime(s), - _ => ice!("ValueRef is not a String"), - } + unsafe fn from_value_unsafe(_vm: &'vm Thread, value: Variants) -> Self { + match value.as_ref() { + ValueRef::String(ref s) => forget_lifetime(s), + _ => ice!("ValueRef is not a String"), } } + + // Only allow the unsafe version to be used + fn from_value(_vm: &'vm Thread, _value: Variants) -> Self { + panic!("Getable::from_value on references is only allowed in unsafe contexts") + } } /// Wrapper type which passes acts as the type `T` but also passes the `VM` to the called function @@ -2091,10 +2094,13 @@ impl<'vm, T: VmType> Pushable<'vm> for TypedBytecode { } } - pub struct Map(PhantomData<(K, V)>); -impl VmType for Map where K::Type: Sized, V::Type: Sized { +impl VmType for Map +where + K::Type: Sized, + V::Type: Sized, +{ type Type = Map; fn make_type(vm: &Thread) -> ArcType { @@ -2105,4 +2111,3 @@ impl VmType for Map where K::Type: Sized, V::Type: S Type::app(map_alias, collect![K::make_type(vm), V::make_type(vm)]) } } -