From c4c88f8af55a9faa3b47e409e2d0779ab97b2c8d Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan.enderlin@hoa-project.net>
Date: Wed, 6 Nov 2019 13:58:49 +0100
Subject: [PATCH] fix(runtime-core) Remove undefined behavior with
 `mem::transmute`.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In the `wrap` functions, we use `std::mem::transmute(&())` to get the
pointer to the value “around” `wrap` (`Fn` has a method `to_raw` which
declares a `wrap` function, which uses `transmute` to retrieve
`Fn`). This is an undefined behavior. It was working until the
`FuncCtx` is introduced. Since then, the undefined behavior was
causing an error with the Singlepass backend.

This patch stores the pointer to `Fn` in `func_env`, so that the
pointer to the user-defined host function is always predictable.
---
 lib/runtime-core/src/typed_func.rs | 28 ++++++++++++++--------------
 lib/runtime-core/src/vm.rs         |  8 +++++++-
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/lib/runtime-core/src/typed_func.rs b/lib/runtime-core/src/typed_func.rs
index 6d7f79e4c..9c798460d 100644
--- a/lib/runtime-core/src/typed_func.rs
+++ b/lib/runtime-core/src/typed_func.rs
@@ -517,18 +517,18 @@ macro_rules! impl_traits {
                     let func_env = func_ctx.func_env;
 
                     let func: &FN = match func_env {
-                        // The imported function is a closure with a
-                        // captured environment.
+                        // The imported function is a regular
+                        // function, a closure without a captured
+                        // environmet, or a closure with a captured
+                        // environment.
                         Some(func_env) => unsafe {
                             let func: NonNull<FN> = func_env.cast();
 
                             &*func.as_ptr()
                         },
 
-                        // The imported function is a regular function
-                        // or a closure without a captured
-                        // environment.
-                        None => unsafe { mem::transmute_copy(&()) }
+                        // This branch is supposed to be unreachable.
+                        None => unreachable!()
                     };
 
                     // Catch unwind in case of errors.
@@ -563,7 +563,7 @@ macro_rules! impl_traits {
                     // `FN` is a function pointer, or a closure
                     // _without_ a captured environment.
                     if mem::size_of::<Self>() == 0 {
-                        None
+                        NonNull::new(&self as *const _ as *mut vm::FuncEnv)
                     }
                     // `FN` is a closure _with_ a captured
                     // environment. Grab it.
@@ -613,18 +613,18 @@ macro_rules! impl_traits {
                     let func_env = func_ctx.func_env;
 
                     let func: &FN = match func_env {
-                        // The imported function is a closure with a
-                        // captured environment.
+                        // The imported function is a regular
+                        // function, a closure without a captured
+                        // environmet, or a closure with a captured
+                        // environment.
                         Some(func_env) => unsafe {
                             let func: NonNull<FN> = func_env.cast();
 
                             &*func.as_ptr()
                         },
 
-                        // The imported function is a regular function
-                        // or a closure without a captured
-                        // environment.
-                        None => unsafe { mem::transmute_copy(&()) }
+                        // This branch is supposed to be unreachable.
+                        None => unreachable!()
                     };
 
                     // Catch unwind in case of errors.
@@ -656,7 +656,7 @@ macro_rules! impl_traits {
                     // `FN` is a function pointer, or a closure
                     // _without_ a captured environment.
                     if mem::size_of::<Self>() == 0 {
-                        None
+                        NonNull::new(&self as *const _ as *mut vm::FuncEnv)
                     }
                     // `FN` is a closure _with_ a captured
                     // environment. Grab it.
diff --git a/lib/runtime-core/src/vm.rs b/lib/runtime-core/src/vm.rs
index 240f71e1c..1c9a8b7ad 100644
--- a/lib/runtime-core/src/vm.rs
+++ b/lib/runtime-core/src/vm.rs
@@ -514,12 +514,18 @@ pub struct FuncEnv {
     _private: [u8; 0],
 }
 
-/// Represents a function context. It is used by imported function
+/// Represents a function context. It is used by imported functions
 /// only.
 #[derive(Debug)]
 #[repr(C)]
 pub(crate) struct FuncCtx {
+    /// The `Ctx` pointer.
     pub(crate) vmctx: NonNull<Ctx>,
+
+    /// A pointer to the function environment. It is used by imported
+    /// functions only to store the pointer to the real host function,
+    /// whether it is a regular function, or a closure with or without
+    /// a captured environment.
     pub(crate) func_env: Option<NonNull<FuncEnv>>,
 }