fix: Don't allow string references to be returned from run_expr

gluon-lang · May 1, 2018 · b133bf0 · b133bf0
1 parent 90390d8
commit b133bf0
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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, _: &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, _: &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, _: &T) {}
             equiv(&value, &$value);

diff --git 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
@@ -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
@@ -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<T> {
     }
 }
 
-
 pub struct Map<K, V>(PhantomData<(K, V)>);
 
-impl<K: VmType, V: VmType> VmType for Map<K, V> where K::Type: Sized, V::Type: Sized {
+impl<K: VmType, V: VmType> VmType for Map<K, V>
+where
+    K::Type: Sized,
+    V::Type: Sized,
+{
     type Type = Map<K::Type, V::Type>;
 
     fn make_type(vm: &Thread) -> ArcType {
@@ -2105,4 +2111,3 @@ impl<K: VmType, V: VmType> VmType for Map<K, V> where K::Type: Sized, V::Type: S
         Type::app(map_alias, collect![K::make_type(vm), V::make_type(vm)])
     }
 }
-