Fix an assert while deleting table elements

LLVM's mergefunc pass may mean that the same descriptor function is used
for different closure invocation sites even when the closure itself is
different. This typically only happens with LTO but in theory could
happen at any time!

The assert was tripping when we tried to delete the same function table
entry twice, so instead of a `Vec<usize>` of entries to delete this
commit switches to a `HashSet<usize>` which should do the deduplication
for us and enusre that we delete each descriptor only once.

Closes #1264

crates/cli-support/Cargo.toml

@@ -14,6 +14,7 @@ edition = '2018'
 [dependencies]
 base64 = "0.9"
 failure = "0.1.2"
+log = "0.4"
 rustc-demangle = "0.1.13"
 tempfile = "3.0"
 walrus = "0.2.1"

crates/cli-support/src/js/closures.rs

@@ -13,7 +13,7 @@ use crate::descriptor::Descriptor;
 use crate::js::js2rust::Js2Rust;
 use crate::js::Context;
 use failure::Error;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashSet};
 use std::mem;
 use walrus::ir::{Expr, ExprId};
 use walrus::{FunctionId, LocalFunction};
@@ -21,12 +21,6 @@ use walrus::{FunctionId, LocalFunction};
 pub fn rewrite(input: &mut Context) -> Result<(), Error> {
     let info = ClosureDescriptors::new(input);
 
-    // Sanity check to make sure things look ok and skip everything below if
-    // there's not calls to `Closure::new`.
-    assert_eq!(
-        info.element_removal_list.len(),
-        info.func_to_descriptor.len(),
-    );
     if info.element_removal_list.len() == 0 {
         return Ok(());
     }
@@ -41,7 +35,7 @@ struct ClosureDescriptors {
     /// A list of elements to remove from the function table. The first element
     /// of the pair is the index of the entry in the element section, and the
     /// second element of the pair is the index within that entry to remove.
-    element_removal_list: Vec<usize>,
+    element_removal_list: HashSet<usize>,
 
     /// A map from local functions which contain calls to
     /// `__wbindgen_describe_closure` to the information about the closure
@@ -150,6 +144,7 @@ impl ClosureDescriptors {
             walrus::TableKind::Function(f) => f,
         };
         for idx in self.element_removal_list.iter().cloned() {
+            log::trace!("delete element {}", idx);
             assert!(table.elements[idx].is_some());
             table.elements[idx] = None;
         }

crates/wasm-interpreter/src/lib.rs

@@ -18,7 +18,7 @@
 
 #![deny(missing_docs)]
 
-use std::collections::{BTreeMap, HashMap};
+use std::collections::{BTreeMap, HashMap, HashSet};
 use walrus::ir::ExprId;
 use walrus::{FunctionId, LocalFunction, LocalId, Module, TableId};
 
@@ -168,7 +168,7 @@ impl Interpreter {
         &mut self,
         id: FunctionId,
         module: &Module,
-        entry_removal_list: &mut Vec<usize>,
+        entry_removal_list: &mut HashSet<usize>,
     ) -> Option<&[u32]> {
         // Call the `id` function. This is an internal `#[inline(never)]`
         // whose code is completely controlled by the `wasm-bindgen` crate, so
@@ -213,7 +213,7 @@ impl Interpreter {
 
         // This is used later to actually remove the entry from the table, but
         // we don't do the removal just yet
-        entry_removal_list.push(descriptor_table_idx);
+        entry_removal_list.insert(descriptor_table_idx);
 
         // And now execute the descriptor!
         self.interpret_descriptor_id(descriptor_id, module)