Skip to content

Commit

Permalink
Fix bad jumptable block ref when DCE removes a block.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cfallin committed Feb 23, 2021
1 parent 98d3e68 commit 48d542d
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 0 deletions.
5 changes: 5 additions & 0 deletions cranelift/codegen/src/ir/jumptable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 13 additions & 0 deletions cranelift/codegen/src/unreachable_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 48d542d

Please sign in to comment.