Merge #1218

1218: Enable clif verifier in debug mode, fix issue with verifier r=MarkMcCaskey a=MarkMcCaskey Ensures we're using I32s to index into things that are indexed by I32s # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Mark McCaskey <mark@wasmer.io>
wasmerio · Feb 14, 2020 · 32991e6 · 32991e6
2 parents 8faac62 + 23275d0
commit 32991e6
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## **[Unreleased]**
 
+- [#1218](https://github.com/wasmerio/wasmer/pull/1218) Enable Cranelift verifier in debug mode. Fix bug with table indices being the wrong type.
 - [#787](https://github.com/wasmerio/wasmer/pull/787) New crate `wasmer-interface-types` to implement WebAssembly Interface Types.
 - [#1213](https://github.com/wasmerio/wasmer/pull/1213) Fixed WASI `fdstat` to detect `isatty` properly.
 - [#1192](https://github.com/wasmerio/wasmer/pull/1192) Use `ExceptionCode` for error representation.

diff --git a/lib/clif-backend/src/code.rs b/lib/clif-backend/src/code.rs
@@ -443,7 +443,7 @@ impl FuncEnvironment for FunctionEnvironment {
                 let local_memory_bound = func.create_global_value(ir::GlobalValueData::Load {
                     base: local_memory_ptr,
                     offset: (vm::LocalMemory::offset_bound() as i32).into(),
-                    global_type: ptr_type,
+                    global_type: ir::types::I32,
                     readonly: false,
                 });
 
@@ -551,7 +551,7 @@ impl FuncEnvironment for FunctionEnvironment {
         let table_count = func.create_global_value(ir::GlobalValueData::Load {
             base: table_struct_ptr,
             offset: (vm::LocalTable::offset_count() as i32).into(),
-            global_type: ptr_type,
+            global_type: ir::types::I32,
             // The table length can change, so it can't be readonly.
             readonly: false,
         });
@@ -673,7 +673,8 @@ impl FuncEnvironment for FunctionEnvironment {
                 colocated: false,
             });
 
-            pos.ins().symbol_value(ir::types::I64, sig_index_global)
+            let val = pos.ins().symbol_value(ir::types::I64, sig_index_global);
+            pos.ins().ireduce(ir::types::I32, val)
 
             // let dynamic_sigindices_array_ptr = pos.ins().load(
             //     ptr_type,

diff --git a/lib/clif-backend/src/lib.rs b/lib/clif-backend/src/lib.rs
@@ -42,7 +42,9 @@ fn get_isa() -> Box<dyn isa::TargetIsa> {
         builder.set("opt_level", "speed_and_size").unwrap();
         builder.set("jump_tables_enabled", "false").unwrap();
 
-        if cfg!(not(test)) {
+        if cfg!(test) || cfg!(debug_assertions) {
+            builder.set("enable_verifier", "true").unwrap();
+        } else {
             builder.set("enable_verifier", "false").unwrap();
         }
 

diff --git a/lib/clif-backend/src/resolver.rs b/lib/clif-backend/src/resolver.rs
@@ -11,7 +11,7 @@ use crate::{
 use byteorder::{ByteOrder, LittleEndian};
 use cranelift_codegen::{
     binemit::{Stackmap, StackmapSink},
-    ir, isa, Context,
+    ir, isa, CodegenError, Context,
 };
 use rayon::prelude::*;
 use std::{
@@ -124,7 +124,12 @@ impl FuncResolverBuilder {
                             &mut local_trap_sink,
                             &mut stackmap_sink,
                         )
-                        .map_err(|e| CompileError::InternalError { msg: e.to_string() })?;
+                        .map_err(|e| match e {
+                            CodegenError::Verifier(v) => CompileError::InternalError {
+                                msg: format!("Verifier error: {}", v),
+                            },
+                            _ => CompileError::InternalError { msg: e.to_string() },
+                        })?;
                         ctx.clear();
                         Ok((code_buf, (reloc_sink, local_trap_sink)))
                     },