Improve linking-related error messages (#3353)

Include more contextual information about why the link failed related to
why the types didn't match.

Closes #3172

crates/types/src/lib.rs

@@ -7,6 +7,7 @@ use cranelift_entity::entity_impl;
 
 use serde::{Deserialize, Serialize};
 use std::convert::{TryFrom, TryInto};
+use std::fmt;
 
 mod error;
 pub use error::*;
@@ -68,6 +69,21 @@ impl From<WasmType> for wasmparser::Type {
     }
 }
 
+impl fmt::Display for WasmType {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            WasmType::I32 => write!(f, "i32"),
+            WasmType::I64 => write!(f, "i64"),
+            WasmType::F32 => write!(f, "f32"),
+            WasmType::F64 => write!(f, "f64"),
+            WasmType::V128 => write!(f, "v128"),
+            WasmType::ExternRef => write!(f, "externref"),
+            WasmType::FuncRef => write!(f, "funcref"),
+            WasmType::ExnRef => write!(f, "exnref"),
+        }
+    }
+}
+
 /// WebAssembly function type -- equivalent of `wasmparser`'s FuncType.
 #[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
 pub struct WasmFuncType {

crates/wasmtime/src/types/matching.rs

@@ -5,6 +5,7 @@ use crate::{signatures::SignatureCollection, Engine, Extern};
 use anyhow::{bail, Context, Result};
 use wasmtime_environ::{
     EntityType, Global, InstanceTypeIndex, Memory, ModuleTypeIndex, SignatureIndex, Table,
+    WasmFuncType, WasmType,
 };
 use wasmtime_jit::TypeTables;
 use wasmtime_runtime::VMSharedSignatureIndex;
@@ -22,11 +23,15 @@ impl MatchCx<'_> {
     }
 
     fn global_ty(&self, expected: &Global, actual: &Global) -> Result<()> {
-        if expected.wasm_ty == actual.wasm_ty && expected.mutability == actual.mutability {
+        match_ty(expected.wasm_ty, actual.wasm_ty, "global")?;
+        match_bool(
+            expected.mutability,
+            actual.mutability,
+            "global",
+            "mutable",
+            "immutable",
+        )?;
         Ok(())
-        } else {
-            bail!("global types incompatible")
-        }
     }
 
     pub fn table(&self, expected: &Table, actual: &crate::Table) -> Result<()> {
@@ -34,20 +39,15 @@ impl MatchCx<'_> {
     }
 
     fn table_ty(&self, expected: &Table, actual: &Table) -> Result<()> {
-        if expected.wasm_ty == actual.wasm_ty
-            && expected.minimum <= actual.minimum
-            && match expected.maximum {
-                Some(expected) => match actual.maximum {
-                    Some(actual) => expected >= actual,
-                    None => false,
-                },
-                None => true,
-            }
-        {
+        match_ty(expected.wasm_ty, actual.wasm_ty, "table")?;
+        match_limits(
+            expected.minimum.into(),
+            expected.maximum.map(|i| i.into()),
+            actual.minimum.into(),
+            actual.maximum.map(|i| i.into()),
+            "table",
+        )?;
         Ok(())
-        } else {
-            bail!("table types incompatible")
-        }
     }
 
     pub fn memory(&self, expected: &Memory, actual: &crate::Memory) -> Result<()> {
@@ -55,21 +55,28 @@ impl MatchCx<'_> {
     }
 
     fn memory_ty(&self, expected: &Memory, actual: &Memory) -> Result<()> {
-        if expected.shared == actual.shared
-            && expected.memory64 == actual.memory64
-            && expected.minimum <= actual.minimum
-            && match expected.maximum {
-                Some(expected) => match actual.maximum {
-                    Some(actual) => expected >= actual,
-                    None => false,
-                },
-                None => true,
-            }
-        {
+        match_bool(
+            expected.shared,
+            actual.shared,
+            "memory",
+            "shared",
+            "non-shared",
+        )?;
+        match_bool(
+            expected.memory64,
+            actual.memory64,
+            "memory",
+            "64-bit",
+            "32-bit",
+        )?;
+        match_limits(
+            expected.minimum,
+            expected.maximum,
+            actual.minimum,
+            actual.maximum,
+            "memory",
+        )?;
         Ok(())
-        } else {
-            bail!("memory types incompatible")
-        }
     }
 
     pub fn func(&self, expected: SignatureIndex, actual: &crate::Func) -> Result<()> {
@@ -96,10 +103,39 @@ impl MatchCx<'_> {
             None => false,
         };
         if matches {
-            Ok(())
-        } else {
-            bail!("function types incompatible")
+            return Ok(());
         }
+        let msg = "function types incompatible";
+        let expected = &self.types.wasm_signatures[expected];
+        let actual = match self.engine.signatures().lookup_type(actual) {
+            Some(ty) => ty,
+            None => {
+                debug_assert!(false, "all signatures should be registered");
+                bail!("{}", msg);
+            }
+        };
+
+        let render = |ty: &WasmFuncType| {
+            let params = ty
+                .params
+                .iter()
+                .map(|s| s.to_string())
+                .collect::<Vec<_>>()
+                .join(", ");
+            let returns = ty
+                .returns
+                .iter()
+                .map(|s| s.to_string())
+                .collect::<Vec<_>>()
+                .join(", ");
+            format!("`({}) -> ({})`", params, returns)
+        };
+        bail!(
+            "{}: expected func of type {}, found func of type {}",
+            msg,
+            render(expected),
+            render(&actual)
+        )
     }
 
     pub fn instance(&self, expected: InstanceTypeIndex, actual: &crate::Instance) -> Result<()> {
@@ -362,6 +398,69 @@ impl MatchCx<'_> {
     }
 }
 
+fn match_ty(expected: WasmType, actual: WasmType, desc: &str) -> Result<()> {
+    if expected == actual {
+        return Ok(());
+    }
+    bail!(
+        "{} types incompatible: expected {0} of type `{}`, found {0} of type `{}`",
+        desc,
+        expected,
+        actual,
+    )
+}
+
+fn match_bool(
+    expected: bool,
+    actual: bool,
+    desc: &str,
+    if_true: &str,
+    if_false: &str,
+) -> Result<()> {
+    if expected == actual {
+        return Ok(());
+    }
+    bail!(
+        "{} types incompatible: expected {} {0}, found {} {0}",
+        desc,
+        if expected { if_true } else { if_false },
+        if actual { if_true } else { if_false },
+    )
+}
+
+fn match_limits(
+    expected_min: u64,
+    expected_max: Option<u64>,
+    actual_min: u64,
+    actual_max: Option<u64>,
+    desc: &str,
+) -> Result<()> {
+    if expected_min <= actual_min
+        && match expected_max {
+            Some(expected) => match actual_max {
+                Some(actual) => expected >= actual,
+                None => false,
+            },
+            None => true,
+        }
+    {
+        return Ok(());
+    }
+    let limits = |min: u64, max: Option<u64>| {
+        format!(
+            "min: {}, max: {}",
+            min,
+            max.map(|s| s.to_string()).unwrap_or(String::from("none"))
+        )
+    };
+    bail!(
+        "{} types incompatible: expected {0} limits ({}) doesn't match provided {0} limits ({})",
+        desc,
+        limits(expected_min, expected_max),
+        limits(actual_min, actual_max)
+    )
+}
+
 fn entity_desc(ty: &EntityType) -> &'static str {
     match ty {
         EntityType::Global(_) => "global",

tests/misc_testsuite/linking-errors.wast

@@ -0,0 +1,47 @@
+(module $m
+  (global (export "g i32") i32 (i32.const 0))
+  (global (export "g mut i32") (mut i32)  (i32.const 0))
+
+  (table (export "t funcref") 0 funcref)
+  (memory (export "mem") 0)
+
+  (func (export "f"))
+  (func (export "f p1r2") (param f32) (result i32 i64) unreachable)
+)
+
+;; make sure the name of the import is in the message
+(assert_unlinkable
+  (module (import "m" "g i32" (global i64)))
+  "incompatible import type for `m::g i32`")
+
+;; errors on globals
+(assert_unlinkable
+  (module (import "m" "g i32" (global i64)))
+  "expected global of type `i64`, found global of type `i32`")
+
+(assert_unlinkable
+  (module (import "m" "g i32" (global (mut i32))))
+  "expected mutable global, found immutable global")
+
+(assert_unlinkable
+  (module (import "m" "g mut i32" (global i32)))
+  "expected immutable global, found mutable global")
+
+;; errors on tables
+(assert_unlinkable
+  (module (import "m" "t funcref" (table 1 funcref)))
+  "expected table limits (min: 1, max: none) doesn't match provided table limits (min: 0, max: none)")
+
+;; errors on memories
+(assert_unlinkable
+  (module (import "m" "mem" (memory 1)))
+  "expected memory limits (min: 1, max: none) doesn't match provided memory limits (min: 0, max: none)")
+
+;; errors on functions
+(assert_unlinkable
+  (module (import "m" "f" (func (param i32))))
+  "expected func of type `(i32) -> ()`, found func of type `() -> ()`")
+
+(assert_unlinkable
+  (module (import "m" "f p1r2" (func (param i32 i32) (result f64))))
+  "expected func of type `(i32, i32) -> (f64)`, found func of type `(f32) -> (i32, i64)`")

tests/misc_testsuite/memory64/linking-errors.wast

@@ -0,0 +1,7 @@
+(module $m
+  (memory (export "mem") 0)
+)
+
+(assert_unlinkable
+  (module (import "m" "mem" (memory i64 0)))
+  "expected 64-bit memory, found 32-bit memory")

tests/misc_testsuite/reference-types/linking-errors.wast

@@ -0,0 +1,8 @@
+(module $m
+  (table (export "t externref") 0 externref)
+)
+
+(assert_unlinkable
+  (module (import "m" "t externref" (table 0 funcref)))
+  "expected table of type `funcref`, found table of type `externref`")
+