Repository: bytecodealliance/wasmtime
Pull request #2392 (2020-11-11 16:18:34)

Fix module initialization with externref element segments
This commit fixes an issue with reference-types-using-modules where they
panicked on instantiation if any element segments had an externref null
specified.



Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 7e22a7d85..156fd62c9 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -542,7 +542,7 @@ dependencies = [
  "smallvec",
  "target-lexicon",
  "thiserror",
- "wasmparser 0.66.0",
+ "wasmparser 0.67.0",
  "wat",
 ]
 
@@ -1090,7 +1090,7 @@ dependencies = [
  "smallvec",
  "thiserror",
  "typemap",
- "wasmparser 0.66.0",
+ "wasmparser 0.67.0",
  "wat",
 ]
 
@@ -2223,6 +2223,12 @@ version = "0.66.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "c97fc0456d6d09ca7b64bc33c34f4e8f29d5ccfa8e5595ef7a8957818339e6f6"
 
+[[package]]
+name = "wasmparser"
+version = "0.67.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9f091cf3849e5fe76a60255bff169277459f2201435bc583b6656880553f0ad0"
+
 [[package]]
 name = "wasmprinter"
 version = "0.2.13"
@@ -2250,7 +2256,7 @@ dependencies = [
  "smallvec",
  "target-lexicon",
  "tempfile",
- "wasmparser 0.66.0",
+ "wasmparser 0.67.0",
  "wasmtime-cache",
  "wasmtime-environ",
  "wasmtime-jit",
@@ -2328,7 +2334,7 @@ dependencies = [
  "test-programs",
  "tracing-subscriber",
  "wasi-common",
- "wasmparser 0.66.0",
+ "wasmparser 0.67.0",
  "wasmtime",
  "wasmtime-cache",
  "wasmtime-debug",
@@ -2363,7 +2369,7 @@ dependencies = [
  "object",
  "target-lexicon",
  "thiserror",
- "wasmparser 0.66.0",
+ "wasmparser 0.67.0",
  "wasmtime-environ",
 ]
 
@@ -2382,7 +2388,7 @@ dependencies = [
  "more-asserts",
  "serde",
  "thiserror",
- "wasmparser 0.66.0",
+ "wasmparser 0.67.0",
 ]
 
 [[package]]
@@ -2410,7 +2416,7 @@ dependencies = [
  "log",
  "rayon",
  "wasm-smith",
- "wasmparser 0.66.0",
+ "wasmparser 0.67.0",
  "wasmprinter",
  "wasmtime",
  "wasmtime-wast",
@@ -2437,7 +2443,7 @@ dependencies = [
  "serde",
  "target-lexicon",
  "thiserror",
- "wasmparser 0.66.0",
+ "wasmparser 0.67.0",
  "wasmtime-cranelift",
  "wasmtime-debug",
  "wasmtime-environ",
@@ -2454,7 +2460,7 @@ version = "0.21.0"
 dependencies = [
  "cranelift-codegen",
  "lightbeam",
- "wasmparser 0.66.0",
+ "wasmparser 0.67.0",
  "wasmtime-environ",
 ]
 
diff --git a/Cargo.toml b/Cargo.toml
index e2fed7a0e..f4adcf55e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -42,7 +42,7 @@ libc = "0.2.60"
 log = "0.4.8"
 rayon = "1.2.1"
 humantime = "2.0.0"
-wasmparser = "0.66"
+wasmparser = "0.67"
 
 [dev-dependencies]
 env_logger = "0.8.1"
diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit.rs b/cranelift/codegen/src/isa/aarch64/inst/emit.rs
index 2d7c02f21..d5aec6f1f 100644
--- a/cranelift/codegen/src/isa/aarch64/inst/emit.rs
+++ b/cranelift/codegen/src/isa/aarch64/inst/emit.rs
@@ -1909,6 +1909,46 @@ impl MachInstEmit for Inst {
 
                 sink.put4(enc_ldst_vec(q, size, rn, rd));
             }
+            &Inst::VecCSel { rd, rn, rm, cond } => {
+                /* Emit this:
+                      b.cond  else
+                      mov     rd, rm
+                      b       out
+                     else:
+                      mov     rd, rn
+                     out:
+
+                   Note, we could do better in the cases where rd == rn or rd == rm.
+                */
+                let else_label = sink.get_label();
+                let out_label = sink.get_label();
+
+                // b.cond else
+                let br_else_offset = sink.cur_offset();
+                sink.put4(enc_conditional_br(
+                    BranchTarget::Label(else_label),
+                    CondBrKind::Cond(cond),
+                ));
+                sink.use_label_at_offset(br_else_offset, else_label, LabelUse::Branch19);
+
+                // mov rd, rm
+                sink.put4(enc_vecmov(/* 16b = */ true, rd, rm));
+
+                // b out
+                let b_out_offset = sink.cur_offset();
+                sink.use_label_at_offset(b_out_offset, out_label, LabelUse::Branch26);
+                sink.add_uncond_branch(b_out_offset, b_out_offset + 4, out_label);
+                sink.put4(enc_jump26(0b000101, 0 /* will be fixed up later */));
+
+                // else:
+                sink.bind_label(else_label);
+
+                // mov rd, rn
+                sink.put4(enc_vecmov(/* 16b = */ true, rd, rn));
+
+                // out:
+                sink.bind_label(out_label);
+            }
             &Inst::MovToNZCV { rn } => {
                 sink.put4(0xd51b4200 | machreg_to_gpr(rn));
             }
diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs
index c7112299f..05c922912 100644
--- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs
+++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs
@@ -1016,6 +1016,15 @@ pub enum Inst {
         size: VectorSize,
     },
 
+    /// Vector conditional select, 128 bit.  A synthetic instruction, which generates a 4-insn
+    /// control-flow diamond.
+    VecCSel {
+        rd: Writable<Reg>,
+        rn: Reg,
+        rm: Reg,
+        cond: Cond,
+    },
+
     /// Move to the NZCV flags (actually a `MSR NZCV, Xn` insn).
     MovToNZCV {
         rn: Reg,
@@ -1732,6 +1741,11 @@ fn aarch64_get_regs(inst: &Inst, collector: &mut RegUsageCollector) {
             collector.add_def(rd);
             collector.add_use(rn);
         }
+        &Inst::VecCSel { rd, rn, rm, .. } => {
+            collector.add_def(rd);
+            collector.add_use(rn);
+            collector.add_use(rm);
+        }
         &Inst::FpuCmp32 { rn, rm } | &Inst::FpuCmp64 { rn, rm } => {
             collector.add_use(rn);
             collector.add_use(rm);
@@ -2343,6 +2357,16 @@ fn aarch64_map_regs<RUM: RegUsageMapper>(inst: &mut Inst, mapper: &RUM) {
             map_def(mapper, rd);
             map_use(mapper, rn);
         }
+        &mut Inst::VecCSel {
+            ref mut rd,
+            ref mut rn,
+            ref mut rm,
+            ..
+        } => {
+            map_def(mapper, rd);
+            map_use(mapper, rn);
+            map_use(mapper, rm);
+        }
         &mut Inst::FpuCmp32 {
             ref mut rn,
             ref mut rm,
@@ -3591,6 +3615,13 @@ impl Inst {
 
                 format!("ld1r {{ {} }}, [{}]", rd, rn)
             }
+            &Inst::VecCSel { rd, rn, rm, cond } => {
+                let rd = show_vreg_vector(rd.to_reg(), mb_rru, VectorSize::Size8x16);
+                let rn = show_vreg_vector(rn, mb_rru, VectorSize::Size8x16);
+                let rm = show_vreg_vector(rm, mb_rru, VectorSize::Size8x16);
+                let cond = cond.show_rru(mb_rru);
+                format!("vcsel {}, {}, {}, {} (if-then-else diamond)", rd, rn, rm, cond)
+            }
             &Inst::MovToNZCV { rn } => {
                 let rn = rn.show_rru(mb_rru);
                 format!("msr nzcv, {}", rn)
diff --git a/cranelift/codegen/src/isa/aarch64/lower_inst.rs b/cranelift/codegen/src/isa/aarch64/lower_inst.rs
index 6c2e33504..90ee97b83 100644
--- a/cranelift/codegen/src/isa/aarch64/lower_inst.rs
+++ b/cranelift/codegen/src/isa/aarch64/lower_inst.rs
@@ -1412,6 +1412,8 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
                 ctx.emit(Inst::FpuCSel32 { cond, rd, rn, rm });
             } else if is_float && bits == 64 {
                 ctx.emit(Inst::FpuCSel64 { cond, rd, rn, rm });
+            } else if is_float && bits == 128 {
+                ctx.emit(Inst::VecCSel { cond, rd, rn, rm });
             } else {
                 ctx.emit(Inst::CSel { cond, rd, rn, rm });
             }
diff --git a/cranelift/wasm/Cargo.toml b/cranelift/wasm/Cargo.toml
index d79647790..57df14aaa 100644
--- a/cranelift/wasm/Cargo.toml
+++ b/cranelift/wasm/Cargo.toml
@@ -12,7 +12,7 @@ keywords = ["webassembly", "wasm"]
 edition = "2018"
 
 [dependencies]
-wasmparser = { version = "0.66.0", default-features = false }
+wasmparser = { version = "0.67.0", default-features = false }
 cranelift-codegen = { path = "../codegen", version = "0.68.0", default-features = false }
 cranelift-entity = { path = "../entity", version = "0.68.0" }
 cranelift-frontend = { path = "../frontend", version = "0.68.0", default-features = false }
diff --git a/crates/debug/Cargo.toml b/crates/debug/Cargo.toml
index 487968938..54414f9dc 100644
--- a/crates/debug/Cargo.toml
+++ b/crates/debug/Cargo.toml
@@ -13,7 +13,7 @@ edition = "2018"
 
 [dependencies]
 gimli = "0.23.0"
-wasmparser = "0.66.0"
+wasmparser = "0.67.0"
 object = { version = "0.22.0", default-features = false, features = ["read", "write"] }
 wasmtime-environ = { path = "../environ", version = "0.21.0" }
 target-lexicon = { version = "0.11.0", default-features = false }
diff --git a/crates/environ/Cargo.toml b/crates/environ/Cargo.toml
index 5cd56a58e..ae10c53f3 100644
--- a/crates/environ/Cargo.toml
+++ b/crates/environ/Cargo.toml
@@ -16,7 +16,7 @@ anyhow = "1.0"
 cranelift-codegen = { path = "../../cranelift/codegen", version = "0.68.0", features = ["enable-serde"] }
 cranelift-entity = { path = "../../cranelift/entity", version = "0.68.0", features = ["enable-serde"] }
 cranelift-wasm = { path = "../../cranelift/wasm", version = "0.68.0", features = ["enable-serde"] }
-wasmparser = "0.66.0"
+wasmparser = "0.67.0"
 indexmap = { version = "1.0.2", features = ["serde-1"] }
 thiserror = "1.0.4"
 serde = { version = "1.0.94", features = ["derive"] }
diff --git a/crates/fuzzing/Cargo.toml b/crates/fuzzing/Cargo.toml
index 525631c42..e620f5b08 100644
--- a/crates/fuzzing/Cargo.toml
+++ b/crates/fuzzing/Cargo.toml
@@ -12,7 +12,7 @@ arbitrary = { version = "0.4.1", features = ["derive"] }
 env_logger = "0.8.1"
 log = "0.4.8"
 rayon = "1.2.1"
-wasmparser = "0.66.0"
+wasmparser = "0.67.0"
 wasmprinter = "0.2.13"
 wasmtime = { path = "../wasmtime" }
 wasmtime-wast = { path = "../wast" }
diff --git a/crates/jit/Cargo.toml b/crates/jit/Cargo.toml
index 05b684466..a449ebc1b 100644
--- a/crates/jit/Cargo.toml
+++ b/crates/jit/Cargo.toml
@@ -28,7 +28,7 @@ rayon = { version = "1.0", optional = true }
 region = "2.1.0"
 thiserror = "1.0.4"
 target-lexicon = { version = "0.11.0", default-features = false }
-wasmparser = "0.66.0"
+wasmparser = "0.67.0"
 more-asserts = "0.2.1"
 anyhow = "1.0"
 cfg-if = "1.0"
diff --git a/crates/lightbeam/Cargo.toml b/crates/lightbeam/Cargo.toml
index 6be7be2ab..e2f3eedde 100644
--- a/crates/lightbeam/Cargo.toml
+++ b/crates/lightbeam/Cargo.toml
@@ -24,7 +24,7 @@ more-asserts = "0.2.1"
 smallvec = "1.0.0"
 thiserror = "1.0.9"
 typemap = "0.3"
-wasmparser = "0.66.0"
+wasmparser = "0.67.0"
 
 [dev-dependencies]
 lazy_static = "1.2"
diff --git a/crates/lightbeam/wasmtime/Cargo.toml b/crates/lightbeam/wasmtime/Cargo.toml
index 6cea3decb..e760e3cc1 100644
--- a/crates/lightbeam/wasmtime/Cargo.toml
+++ b/crates/lightbeam/wasmtime/Cargo.toml
@@ -13,6 +13,6 @@ edition = "2018"
 
 [dependencies]
 lightbeam = { path = "..", version = "0.21.0" }
-wasmparser = "0.66"
+wasmparser = "0.67"
 cranelift-codegen = { path = "../../../cranelift/codegen", version = "0.68.0" }
 wasmtime-environ = { path = "../../environ", version = "0.21.0" }
diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs
index 87e7c326f..a95a4b594 100644
--- a/crates/runtime/src/instance.rs
+++ b/crates/runtime/src/instance.rs
@@ -1270,15 +1270,19 @@ fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> {
         }
 
         for (i, func_idx) in init.elements.iter().enumerate() {
-            let anyfunc = instance.get_caller_checked_anyfunc(*func_idx).map_or(
-                ptr::null_mut(),
-                |f: &VMCallerCheckedAnyfunc| {
-                    f as *const VMCallerCheckedAnyfunc as *mut VMCallerCheckedAnyfunc
-                },
-            );
-            table
-                .set(u32::try_from(start + i).unwrap(), anyfunc.into())
-                .unwrap();
+            let item = match table.element_type() {
+                TableElementType::Func => instance
+                    .get_caller_checked_anyfunc(*func_idx)
+                    .map_or(ptr::null_mut(), |f: &VMCallerCheckedAnyfunc| {
+                        f as *const VMCallerCheckedAnyfunc as *mut VMCallerCheckedAnyfunc
+                    })
+                    .into(),
+                TableElementType::Val(_) => {
+                    assert!(*func_idx == FuncIndex::reserved_value());
+                    TableElement::ExternRef(None)
+                }
+            };
+            table.set(u32::try_from(start + i).unwrap(), item).unwrap();
         }
     }
 
diff --git a/crates/wasmtime/Cargo.toml b/crates/wasmtime/Cargo.toml
index c2b4e16b9..026d82826 100644
--- a/crates/wasmtime/Cargo.toml
+++ b/crates/wasmtime/Cargo.toml
@@ -16,7 +16,7 @@ wasmtime-jit = { path = "../jit", version = "0.21.0" }
 wasmtime-cache = { path = "../cache", version = "0.21.0", optional = true }
 wasmtime-profiling = { path = "../profiling", version = "0.21.0" }
 target-lexicon = { version = "0.11.0", default-features = false }
-wasmparser = "0.66.0"
+wasmparser = "0.67.0"
 anyhow = "1.0.19"
 region = "2.2.0"
 libc = "0.2"


Test patch:
diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs
index ca1eee46d..9248c2a19 100644
--- a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs
+++ b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs
@@ -3879,6 +3879,17 @@ fn test_aarch64_binemit() {
         "ld1r { v0.8b }, [x25]",
     ));
 
+    insns.push((
+        Inst::VecCSel {
+            rd: writable_vreg(5),
+            rn: vreg(10),
+            rm: vreg(19),
+            cond: Cond::Gt,
+        },
+        "6C000054651EB34E02000014451DAA4E",
+        "vcsel v5.16b, v10.16b, v19.16b, gt (if-then-else diamond)",
+    ));
+
     insns.push((
         Inst::Extend {
             rd: writable_xreg(1),
diff --git a/cranelift/filetests/filetests/isa/aarch64/simd.clif b/cranelift/filetests/filetests/isa/aarch64/simd.clif
index 3e47bbbda..a61dd1b8a 100644
--- a/cranelift/filetests/filetests/isa/aarch64/simd.clif
+++ b/cranelift/filetests/filetests/isa/aarch64/simd.clif
@@ -47,3 +47,17 @@ block0:
 ; nextln:  mov sp, fp
 ; nextln:  ldp fp, lr, [sp], #16
 ; nextln:  ret
+
+function %f4(i32, i8x16, i8x16) -> i8x16 {
+block0(v0: i32, v1: i8x16, v2: i8x16):
+   v3 = select v0, v1, v2
+   return v3
+}
+
+; check:  stp fp, lr, [sp, #-16]!
+; nextln:  mov fp, sp
+; nextln:  subs wzr, w0, wzr
+; nextln:  vcsel v0.16b, v0.16b, v1.16b, ne (if-then-else diamond)
+; nextln:  mov sp, fp
+; nextln:  ldp fp, lr, [sp], #16
+; nextln:  ret
diff --git a/tests/misc_testsuite/reference-types/externref-segment.wast b/tests/misc_testsuite/reference-types/externref-segment.wast
new file mode 100644
index 000000000..9da6ff667
--- /dev/null
+++ b/tests/misc_testsuite/reference-types/externref-segment.wast
@@ -0,0 +1,5 @@
+(module
+  (table 2 externref)
+  (elem (i32.const 0) externref (ref.null extern))
+  (elem (i32.const 1) externref (ref.null extern))
+)