From 9bab9d4af103d979ea2b5c309a993c85b70383f5 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Tue, 19 Feb 2019 08:17:14 -0800
Subject: [PATCH] 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         |  1 +
 crates/cli-support/src/js/closures.rs | 11 +++--------
 crates/wasm-interpreter/src/lib.rs    |  6 +++---
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/crates/cli-support/Cargo.toml b/crates/cli-support/Cargo.toml
index 81479563506..85c56055222 100644
--- a/crates/cli-support/Cargo.toml
+++ b/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"
diff --git a/crates/cli-support/src/js/closures.rs b/crates/cli-support/src/js/closures.rs
index 8cc8c13866f..925753076af 100644
--- a/crates/cli-support/src/js/closures.rs
+++ b/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;
         }
diff --git a/crates/wasm-interpreter/src/lib.rs b/crates/wasm-interpreter/src/lib.rs
index 55a2165c825..645de6fb297 100644
--- a/crates/wasm-interpreter/src/lib.rs
+++ b/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)