From 48d542d67c4958097552e7547761cb24e9d60311 Mon Sep 17 00:00:00 2001
From: Chris Fallin <chris@cfallin.org>
Date: Tue, 23 Feb 2021 15:01:01 -0800
Subject: [PATCH] Fix bad jumptable block ref when DCE removes a block.

When a block is unreachable, the `unreachable_code` pass will remove it,
which is perfectly sensible. Jump tables factor into unreachability in
an expected way: even if a block is listed in a jump table, the block
might be unreachable if the jump table itself is unused (or used in an
unreachable block). Unfortunately, the verifier still expects all
block refs in all jump tables to be valid, even after DCE, which will
not always be the case.

This makes a simple change to the pass: after removing blocks, it scans
jump tables. Any jump table that refers to an unreachable block must
itself be unused, and so we just clear its entries. We do not bother
removing it (and renumbering all later jumptables), and we do not bother
computing full unused-ness of all jumptables, as that would be more
expensive; it's sufficient to clear out the ones that refer to
unreachable blocks, which are a subset of all unused jumptables.

Fixes #2670.
---
 cranelift/codegen/src/ir/jumptable.rs         |  5 +++++
 cranelift/codegen/src/unreachable_code.rs     | 13 ++++++++++++
 .../isa/x64/unused_jt_unreachable_block.clif  | 21 +++++++++++++++++++
 3 files changed, 39 insertions(+)
 create mode 100644 cranelift/filetests/filetests/isa/x64/unused_jt_unreachable_block.clif

diff --git a/cranelift/codegen/src/ir/jumptable.rs b/cranelift/codegen/src/ir/jumptable.rs
index d102617df4..bf05169d36 100644
--- a/cranelift/codegen/src/ir/jumptable.rs
+++ b/cranelift/codegen/src/ir/jumptable.rs
@@ -68,6 +68,11 @@ impl JumpTableData {
     pub fn iter_mut(&mut self) -> IterMut<Block> {
         self.table.iter_mut()
     }
+
+    /// Clears all entries in this jump table.
+    pub fn clear(&mut self) {
+        self.table.clear();
+    }
 }
 
 impl Display for JumpTableData {
diff --git a/cranelift/codegen/src/unreachable_code.rs b/cranelift/codegen/src/unreachable_code.rs
index 63e3e230f8..3c3f4d9d6b 100644
--- a/cranelift/codegen/src/unreachable_code.rs
+++ b/cranelift/codegen/src/unreachable_code.rs
@@ -43,4 +43,17 @@ pub fn eliminate_unreachable_code(
         // Finally, remove the block from the layout.
         pos.func.layout.remove_block(block);
     }
+
+    // Remove all jumptable block-list contents that refer to unreachable
+    // blocks; the jumptable itself must have been unused (or used only in an
+    // unreachable block) if so. Note that we are not necessarily removing *all*
+    // unused jumptables, because that would require computing their
+    // reachability as well; we are just removing enough to clean up references
+    // to deleted blocks.
+    for jt_data in func.jump_tables.values_mut() {
+        let invalid_ref = jt_data.iter().any(|block| !domtree.is_reachable(*block));
+        if invalid_ref {
+            jt_data.clear();
+        }
+    }
 }
diff --git a/cranelift/filetests/filetests/isa/x64/unused_jt_unreachable_block.clif b/cranelift/filetests/filetests/isa/x64/unused_jt_unreachable_block.clif
new file mode 100644
index 0000000000..96478ba8ce
--- /dev/null
+++ b/cranelift/filetests/filetests/isa/x64/unused_jt_unreachable_block.clif
@@ -0,0 +1,21 @@
+test compile
+target x86_64
+feature "experimental_x64"
+
+;; From: https://github.com/bytecodealliance/wasmtime/issues/2670
+
+function %f() system_v {
+    jt0 = jump_table [block1]
+
+block0:
+    return
+
+block1:
+    trap unreachable
+}
+
+; check:  pushq   %rbp
+; nextln: movq    %rsp, %rbp
+; nextln: movq    %rbp, %rsp
+; nextln: popq    %rbp
+; nextln: ret