From f33aab4216383220884665e6076b0d9f0ac52812 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Fri, 24 Dec 2021 14:00:58 -0800 Subject: [PATCH] fuzzbug fix and more robust undef handling --- src/backend/final.rs | 5 ----- src/backend/locations.rs | 2 +- src/backend/schedule.rs | 4 ---- src/backend/use_count.rs | 28 +++++++++------------------- src/frontend.rs | 29 ++++++++++++++--------------- src/ir.rs | 12 ------------ 6 files changed, 24 insertions(+), 56 deletions(-) diff --git a/src/backend/final.rs b/src/backend/final.rs index 78df26c..a06ef09 100644 --- a/src/backend/final.rs +++ b/src/backend/final.rs @@ -169,11 +169,6 @@ pub fn produce_func_wasm( for operator in &body.operators { ctx.translate(operator, locations); } - - // Fixup: add unreachable just before last `end`; there must be an explicit return. -// assert!(matches!(wasm.operators.pop(), Some(wasm_encoder::Instruction::End))); -// wasm.operators.push(wasm_encoder::Instruction::Unreachable); -// wasm.operators.push(wasm_encoder::Instruction::End); wasm } diff --git a/src/backend/locations.rs b/src/backend/locations.rs index 632aa04..78e3a72 100644 --- a/src/backend/locations.rs +++ b/src/backend/locations.rs @@ -21,7 +21,7 @@ pub struct Allocator<'a> { freelist: FxHashMap>, } -#[derive(Clone, Copy, Debug, Default)] +#[derive(Clone, Copy, Debug)] pub struct ValueSpan { value: Value, multi_value_index: usize, diff --git a/src/backend/schedule.rs b/src/backend/schedule.rs index 796177c..c35bf43 100644 --- a/src/backend/schedule.rs +++ b/src/backend/schedule.rs @@ -82,10 +82,6 @@ impl Schedule { } match value_def { &ValueDef::Operator(op, ref operands) => { - if operands.iter().any(|&value| value == Value::undef()) { - continue; - } - if operands.len() == 0 { if !op_rematerialize(&op) { log::trace!("immediately ready: v{}", value.index()); diff --git a/src/backend/use_count.rs b/src/backend/use_count.rs index ed7fe09..710acc5 100644 --- a/src/backend/use_count.rs +++ b/src/backend/use_count.rs @@ -23,22 +23,18 @@ impl UseCountAnalysis { let mut workqueue_set = FxHashSet::default(); for block in 0..f.blocks.len() { for &value in &f.blocks[block].insts { - if value != Value::undef() { - let value = f.resolve_alias(value); - counts.add(value); - if workqueue_set.insert(value) { - workqueue.push_back(value); - } - counts.toplevel.insert(value); + let value = f.resolve_alias(value); + counts.add(value); + if workqueue_set.insert(value) { + workqueue.push_back(value); } + counts.toplevel.insert(value); } f.blocks[block].terminator.visit_uses(|value| { - if value != Value::undef() { - let value = f.resolve_alias(value); - counts.add(value); - if workqueue_set.insert(value) { - workqueue.push_back(value); - } + let value = f.resolve_alias(value); + counts.add(value); + if workqueue_set.insert(value) { + workqueue.push_back(value); } }); @@ -48,9 +44,6 @@ impl UseCountAnalysis { &ValueDef::Alias(..) | &ValueDef::Arg(..) | &ValueDef::BlockParam(..) => {} &ValueDef::Operator(_op, ref args) => { for &arg in args { - if arg == Value::undef() { - continue; - } let arg = f.resolve_alias(arg); counts.add(arg); if counts.use_count[arg.index()] == 1 { @@ -61,9 +54,6 @@ impl UseCountAnalysis { } } &ValueDef::PickOutput(value, _) => { - if value == Value::undef() { - continue; - } let value = f.resolve_alias(value); counts.add(value); if counts.use_count[value.index()] == 1 { diff --git a/src/frontend.rs b/src/frontend.rs index 8f1254f..2b17c24 100644 --- a/src/frontend.rs +++ b/src/frontend.rs @@ -290,7 +290,8 @@ impl LocalTracker { assert!((local as usize) < body.locals.len()); self.get_in_block(body, block, local) } else { - Value::undef() + let ty = body.locals[local as usize]; + self.create_default_value(body, ty) } } @@ -541,11 +542,13 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { self.op_stack.pop().unwrap().1 } - fn block_results(&mut self, n: usize, start_depth: usize) -> Vec { - if self.op_stack.len() < start_depth + n { - vec![Value::undef(); n] + fn block_results(&mut self, tys: &[Type], start_depth: usize) -> Vec { + if self.op_stack.len() < start_depth + tys.len() { + tys.iter() + .map(|&ty| self.locals.create_default_value(&mut self.body, ty)) + .collect() } else { - self.pop_n(n) + self.pop_n(tys.len()) } } @@ -571,16 +574,12 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { wasmparser::Operator::LocalSet { local_index } => { let (_, value) = self.op_stack.pop().unwrap(); - if value != Value::undef() { - self.locals.set(*local_index, value); - } + self.locals.set(*local_index, value); } wasmparser::Operator::LocalTee { local_index } => { let (_ty, value) = *self.op_stack.last().unwrap(); - if value != Value::undef() { - self.locals.set(*local_index, value); - } + self.locals.set(*local_index, value); } wasmparser::Operator::Call { .. } @@ -787,7 +786,7 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { }) => { // Generate a branch to the out-block with // blockparams for the results. - let result_values = self.block_results(results.len(), *start_depth); + let result_values = self.block_results(&results[..], *start_depth); self.emit_branch(*out, &result_values[..]); self.op_stack.truncate(*start_depth); // Seal the out-block: no more edges will be @@ -812,7 +811,7 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { }) => { // Generate a branch to the out-block with // blockparams for the results. - let result_values = self.block_results(results.len(), *start_depth); + let result_values = self.block_results(&results[..], *start_depth); self.emit_branch(*out, &result_values[..]); self.op_stack.truncate(*start_depth); // No `else`, so we need to generate a trivial @@ -841,7 +840,7 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { }) => { // Generate a branch to the out-block with // blockparams for the results. - let result_values = self.block_results(results.len(), *start_depth); + let result_values = self.block_results(&results[..], *start_depth); self.emit_branch(*out, &result_values[..]); self.op_stack.truncate(*start_depth); self.cur_block = Some(*out); @@ -920,7 +919,7 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { results, } = self.ctrl_stack.pop().unwrap() { - let if_results = self.block_results(results.len(), start_depth); + let if_results = self.block_results(&results[..], start_depth); self.emit_branch(out, &if_results[..]); self.op_stack.truncate(start_depth); self.op_stack.extend(param_values); diff --git a/src/ir.rs b/src/ir.rs index 5aa43d0..c4d88ff 100644 --- a/src/ir.rs +++ b/src/ir.rs @@ -100,7 +100,6 @@ impl FunctionBody { } pub fn set_alias(&mut self, value: Value, to: Value) { - assert_ne!(to, Value::undef()); log::trace!("set_alias: value {:?} to {:?}", value, to); // Resolve the `to` value through all existing aliases. let to = self.resolve_and_update_alias(to); @@ -112,7 +111,6 @@ impl FunctionBody { } pub fn resolve_alias(&self, value: Value) -> Value { - assert_ne!(value, Value::undef()); let mut result = value; loop { if let &ValueDef::Alias(to) = &self.values[result.index()] { @@ -238,10 +236,6 @@ impl std::fmt::Display for Value { } impl Value { - pub fn undef() -> Self { - Value(u32::MAX) - } - pub fn index(self) -> usize { self.0 as usize } @@ -251,12 +245,6 @@ impl Value { } } -impl std::default::Default for Value { - fn default() -> Self { - Value::undef() - } -} - #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum ValueDef { Arg(usize),