From ce10ebc5be51f4750225e64a86fb57c991f7eac1 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Tue, 10 Oct 2023 20:03:10 +0200 Subject: [PATCH] fix: disallow returning constant values (#2978) Co-authored-by: kevaundray --- compiler/noirc_errors/src/position.rs | 4 ++++ .../src/brillig/brillig_gen/brillig_block.rs | 2 +- compiler/noirc_evaluator/src/errors.rs | 14 ++++++++--- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 5 ++++ .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 20 +++++++++------- .../src/ssa/function_builder/mod.rs | 3 ++- .../noirc_evaluator/src/ssa/ir/basic_block.rs | 9 +++++++- compiler/noirc_evaluator/src/ssa/ir/cfg.rs | 20 ++++++++++++---- compiler/noirc_evaluator/src/ssa/ir/dom.rs | 3 ++- .../noirc_evaluator/src/ssa/ir/function.rs | 2 +- .../noirc_evaluator/src/ssa/ir/instruction.rs | 13 ++++++----- .../noirc_evaluator/src/ssa/ir/printer.rs | 2 +- .../src/ssa/opt/constant_folding.rs | 4 ++-- .../src/ssa/opt/flatten_cfg.rs | 10 ++++---- .../noirc_evaluator/src/ssa/opt/inlining.rs | 11 ++++++--- .../noirc_evaluator/src/ssa/opt/mem2reg.rs | 8 +++---- .../src/ssa/opt/simplify_cfg.rs | 4 ++-- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 23 ++++++++++++++++++- .../src/monomorphization/ast.rs | 4 +++- .../src/monomorphization/mod.rs | 14 +++++++++-- .../constant_return/Nargo.toml | 0 .../constant_return/Prover.toml | 0 .../constant_return/src/main.nr | 0 .../conditional_regression_547/Prover.toml | 1 + .../conditional_regression_547/src/main.nr | 4 ++-- .../higher_order_functions/Prover.toml | 1 + .../higher_order_functions/src/main.nr | 4 ++-- .../execution_success/modulus/src/main.nr | 4 +--- .../execution_success/to_be_bytes/src/main.nr | 6 ++--- .../tuple_inputs/src/main.nr | 3 --- 30 files changed, 138 insertions(+), 60 deletions(-) rename tooling/nargo_cli/tests/{execution_success => compile_failure}/constant_return/Nargo.toml (100%) rename tooling/nargo_cli/tests/{execution_success => compile_failure}/constant_return/Prover.toml (100%) rename tooling/nargo_cli/tests/{execution_success => compile_failure}/constant_return/src/main.nr (100%) diff --git a/compiler/noirc_errors/src/position.rs b/compiler/noirc_errors/src/position.rs index 4cbf934138c..e068ca319ea 100644 --- a/compiler/noirc_errors/src/position.rs +++ b/compiler/noirc_errors/src/position.rs @@ -119,4 +119,8 @@ impl Location { pub fn new(span: Span, file: FileId) -> Self { Self { span, file } } + + pub fn dummy() -> Self { + Self { span: Span::single_char(0), file: FileId::dummy() } + } } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 9d2a7245e87..4b83d21a702 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -153,7 +153,7 @@ impl<'block> BrilligBlock<'block> { self.create_block_label_for_current_function(*destination_block), ); } - TerminatorInstruction::Return { return_values } => { + TerminatorInstruction::Return { return_values, .. } => { let return_registers: Vec<_> = return_values .iter() .flat_map(|value_id| { diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index 2d0d73e9c87..3dc0194c8be 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -67,6 +67,8 @@ pub enum InternalError { UndeclaredAcirVar { call_stack: CallStack }, #[error("ICE: Expected {expected:?}, found {found:?}")] UnExpected { expected: String, found: String, call_stack: CallStack }, + #[error("Returning a constant value is not allowed")] + ReturnConstant { call_stack: CallStack }, } impl RuntimeError { @@ -79,7 +81,8 @@ impl RuntimeError { | InternalError::MissingArg { call_stack, .. } | InternalError::NotAConstant { call_stack, .. } | InternalError::UndeclaredAcirVar { call_stack } - | InternalError::UnExpected { call_stack, .. }, + | InternalError::UnExpected { call_stack, .. } + | InternalError::ReturnConstant { call_stack, .. }, ) | RuntimeError::FailedConstraint { call_stack, .. } | RuntimeError::IndexOutOfBounds { call_stack, .. } @@ -96,9 +99,8 @@ impl RuntimeError { impl From for FileDiagnostic { fn from(error: RuntimeError) -> FileDiagnostic { let call_stack = vecmap(error.call_stack(), |location| *location); - let diagnostic = error.into_diagnostic(); let file_id = call_stack.last().map(|location| location.file).unwrap_or_default(); - + let diagnostic = error.into_diagnostic(); diagnostic.in_file(file_id).with_call_stack(call_stack) } } @@ -106,6 +108,12 @@ impl From for FileDiagnostic { impl RuntimeError { fn into_diagnostic(self) -> Diagnostic { match self { + RuntimeError::InternalError(InternalError::ReturnConstant { ref call_stack }) => { + let message = self.to_string(); + let location = + call_stack.back().expect("Expected RuntimeError to have a location"); + Diagnostic::simple_error(message, "constant value".to_string(), location.span) + } RuntimeError::InternalError(cause) => { Diagnostic::simple_error( "Internal Consistency Evaluators Errors: \n diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 6803c614f0b..325088dbf0b 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -249,6 +249,11 @@ impl AcirContext { } } + /// True if the given AcirVar refers to a constant value + pub(crate) fn is_constant(&self, var: &AcirVar) -> bool { + matches!(self.vars[var], AcirVarData::Const(_)) + } + /// Adds a new Variable to context whose value will /// be constrained to be the negation of `var`. /// diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 36e54132a38..ed317f69a16 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1248,8 +1248,10 @@ impl Context { terminator: &TerminatorInstruction, dfg: &DataFlowGraph, ) -> Result<(), InternalError> { - let return_values = match terminator { - TerminatorInstruction::Return { return_values } => return_values, + let (return_values, call_stack) = match terminator { + TerminatorInstruction::Return { return_values, call_stack } => { + (return_values, call_stack) + } _ => unreachable!("ICE: Program must have a singular return"), }; @@ -1257,6 +1259,9 @@ impl Context { // will expand the array if there is one. let return_acir_vars = self.flatten_value_list(return_values, dfg); for acir_var in return_acir_vars { + if self.acir_context.is_constant(&acir_var) { + return Err(InternalError::ReturnConstant { call_stack: call_stack.clone() }); + } self.acir_context.return_var(acir_var)?; } Ok(()) @@ -1869,6 +1874,7 @@ mod tests { use crate::{ brillig::Brillig, + errors::{InternalError, RuntimeError}, ssa::{ function_builder::FunctionBuilder, ir::{function::RuntimeType, map::Id, types::Type}, @@ -1897,12 +1903,10 @@ mod tests { let ssa = builder.finish(); let context = Context::new(); - let mut acir = context.convert_ssa(ssa, Brillig::default(), &HashMap::default()).unwrap(); - - let expected_opcodes = - vec![Opcode::Arithmetic(&Expression::one() - &Expression::from(Witness(1)))]; - assert_eq!(acir.take_opcodes(), expected_opcodes); - assert_eq!(acir.return_witnesses, vec![Witness(1)]); + let acir = context + .convert_ssa(ssa, Brillig::default(), &HashMap::default()) + .expect_err("Return constant value"); + assert!(matches!(acir, RuntimeError::InternalError(InternalError::ReturnConstant { .. }))); } } // diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 3aa95e9f6e6..546a614a27f 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -316,7 +316,8 @@ impl FunctionBuilder { /// Terminate the current block with a return instruction pub(crate) fn terminate_with_return(&mut self, return_values: Vec) { - self.terminate_block_with(TerminatorInstruction::Return { return_values }); + let call_stack = self.call_stack.clone(); + self.terminate_block_with(TerminatorInstruction::Return { return_values, call_stack }); } /// Returns a ValueId pointing to the given function or imports the function diff --git a/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs b/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs index 9ca73bea762..981afa3e380 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs @@ -1,4 +1,5 @@ use super::{ + dfg::CallStack, instruction::{InstructionId, TerminatorInstruction}, map::Id, value::ValueId, @@ -117,7 +118,13 @@ impl BasicBlock { /// being kept, which is likely unwanted. pub(crate) fn take_terminator(&mut self) -> TerminatorInstruction { let terminator = self.terminator.as_mut().expect("Expected block to have a terminator"); - std::mem::replace(terminator, TerminatorInstruction::Return { return_values: Vec::new() }) + std::mem::replace( + terminator, + TerminatorInstruction::Return { + return_values: Vec::new(), + call_stack: CallStack::new(), + }, + ) } /// Return the jmp arguments, if any, of this block's TerminatorInstruction. diff --git a/compiler/noirc_evaluator/src/ssa/ir/cfg.rs b/compiler/noirc_evaluator/src/ssa/ir/cfg.rs index dbc4c29183e..ebfbf003ec4 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/cfg.rs @@ -131,7 +131,9 @@ impl ControlFlowGraph { #[cfg(test)] mod tests { - use crate::ssa::ir::{instruction::TerminatorInstruction, map::Id, types::Type}; + use crate::ssa::ir::{ + dfg::CallStack, instruction::TerminatorInstruction, map::Id, types::Type, + }; use super::{super::function::Function, ControlFlowGraph}; @@ -140,7 +142,10 @@ mod tests { let func_id = Id::test_new(0); let mut func = Function::new("func".into(), func_id); let block_id = func.entry_block(); - func.dfg[block_id].set_terminator(TerminatorInstruction::Return { return_values: vec![] }); + func.dfg[block_id].set_terminator(TerminatorInstruction::Return { + return_values: vec![], + call_stack: CallStack::new(), + }); ControlFlowGraph::with_function(&func); } @@ -173,7 +178,10 @@ mod tests { then_destination: block1_id, else_destination: block2_id, }); - func.dfg[block2_id].set_terminator(TerminatorInstruction::Return { return_values: vec![] }); + func.dfg[block2_id].set_terminator(TerminatorInstruction::Return { + return_values: vec![], + call_stack: CallStack::new(), + }); let mut cfg = ControlFlowGraph::with_function(&func); @@ -218,8 +226,10 @@ mod tests { // return () // } let ret_block_id = func.dfg.make_block(); - func.dfg[ret_block_id] - .set_terminator(TerminatorInstruction::Return { return_values: vec![] }); + func.dfg[ret_block_id].set_terminator(TerminatorInstruction::Return { + return_values: vec![], + call_stack: CallStack::new(), + }); func.dfg[block2_id].set_terminator(TerminatorInstruction::Jmp { destination: ret_block_id, arguments: vec![], diff --git a/compiler/noirc_evaluator/src/ssa/ir/dom.rs b/compiler/noirc_evaluator/src/ssa/ir/dom.rs index da55a593f9e..c1df0428c64 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dom.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dom.rs @@ -250,6 +250,7 @@ mod tests { function_builder::FunctionBuilder, ir::{ basic_block::BasicBlockId, + dfg::CallStack, dom::DominatorTree, function::{Function, RuntimeType}, instruction::TerminatorInstruction, @@ -265,7 +266,7 @@ mod tests { let block0_id = func.entry_block(); func.dfg.set_block_terminator( block0_id, - TerminatorInstruction::Return { return_values: vec![] }, + TerminatorInstruction::Return { return_values: vec![], call_stack: CallStack::new() }, ); let mut dom_tree = DominatorTree::with_function(&func); assert!(dom_tree.dominates(block0_id, block0_id)); diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index ab97d418783..360f5710113 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -93,7 +93,7 @@ impl Function { let mut function_return_values = None; for block in blocks { let terminator = self.dfg[block].terminator(); - if let Some(TerminatorInstruction::Return { return_values }) = terminator { + if let Some(TerminatorInstruction::Return { return_values, .. }) = terminator { function_return_values = Some(return_values); break; } diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 45b84cc97d9..f748aa040a1 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -536,7 +536,7 @@ pub(crate) enum TerminatorInstruction { /// unconditionally jump to a single exit block with the return values /// as the block arguments. Then the exit block can terminate in a return /// instruction returning these values. - Return { return_values: Vec }, + Return { return_values: Vec, call_stack: CallStack }, } impl TerminatorInstruction { @@ -557,9 +557,10 @@ impl TerminatorInstruction { arguments: vecmap(arguments, |value| f(*value)), call_stack: call_stack.clone(), }, - Return { return_values } => { - Return { return_values: vecmap(return_values, |value| f(*value)) } - } + Return { return_values, call_stack } => Return { + return_values: vecmap(return_values, |value| f(*value)), + call_stack: call_stack.clone(), + }, } } @@ -575,7 +576,7 @@ impl TerminatorInstruction { *argument = f(*argument); } } - Return { return_values } => { + Return { return_values, .. } => { for return_value in return_values { *return_value = f(*return_value); } @@ -595,7 +596,7 @@ impl TerminatorInstruction { f(*argument); } } - Return { return_values } => { + Return { return_values, .. } => { for return_value in return_values { f(*return_value); } diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 037f8aaac2b..aee8e456b13 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -112,7 +112,7 @@ pub(crate) fn display_terminator( else_destination ) } - Some(TerminatorInstruction::Return { return_values }) => { + Some(TerminatorInstruction::Return { return_values, .. }) => { writeln!(f, " return {}", value_list(function, return_values)) } None => writeln!(f, " (no terminator instruction)"), diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index a96a8d70e04..f48e6f2a129 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -234,7 +234,7 @@ mod test { assert_eq!(block.instructions().len(), 0); match block.terminator() { - Some(TerminatorInstruction::Return { return_values }) => { + Some(TerminatorInstruction::Return { return_values, .. }) => { let value = main .dfg .get_numeric_constant(return_values[0]) @@ -278,7 +278,7 @@ mod test { assert_ne!(new_add_instr_result, v1); let return_value_id = match entry_block.unwrap_terminator() { - TerminatorInstruction::Return { return_values } => return_values[0], + TerminatorInstruction::Return { return_values, .. } => return_values[0], _ => unreachable!(), }; let return_element = match &main.dfg[return_value_id] { diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 8d3fda3cfe8..44dc8b098e7 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -325,11 +325,13 @@ impl<'f> Context<'f> { let arguments = vecmap(arguments.clone(), |value| self.inserter.resolve(value)); self.inline_block(destination, &arguments) } - TerminatorInstruction::Return { return_values } => { + TerminatorInstruction::Return { return_values, call_stack } => { + let call_stack = call_stack.clone(); let return_values = vecmap(return_values.clone(), |value| self.inserter.resolve(value)); + let new_return = TerminatorInstruction::Return { return_values, call_stack }; let entry = self.inserter.function.entry_block(); - let new_return = TerminatorInstruction::Return { return_values }; + self.inserter.function.dfg.set_block_terminator(entry, new_return); block } @@ -1079,7 +1081,7 @@ mod test { let main = ssa.main(); let ret = match main.dfg[main.entry_block()].terminator() { - Some(TerminatorInstruction::Return { return_values }) => return_values[0], + Some(TerminatorInstruction::Return { return_values, .. }) => return_values[0], _ => unreachable!(), }; @@ -1442,7 +1444,7 @@ mod test { // The return value should be 200, not 310 match main.dfg[main.entry_block()].terminator() { - Some(TerminatorInstruction::Return { return_values }) => { + Some(TerminatorInstruction::Return { return_values, .. }) => { match main.dfg.get_numeric_constant(return_values[0]) { Some(constant) => { let value = constant.to_u128(); diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 617712c9912..ed2484febac 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -480,10 +480,15 @@ impl<'function> PerFunctionContext<'function> { } None } - TerminatorInstruction::Return { return_values } => { + TerminatorInstruction::Return { return_values, call_stack } => { let return_values = vecmap(return_values, |value| self.translate_value(*value)); if self.inlining_entry { - self.context.builder.terminate_with_return(return_values.clone()); + let mut new_call_stack = self.context.call_stack.clone(); + new_call_stack.append(call_stack.clone()); + self.context + .builder + .set_call_stack(new_call_stack) + .terminate_with_return(return_values.clone()); } // Note that `translate_block` would take us back to the point at which the // inlining of this source block began. Since additional blocks may have been @@ -686,7 +691,7 @@ mod test { let b6 = &main.dfg[b6_id]; match b6.terminator() { - Some(TerminatorInstruction::Return { return_values }) => { + Some(TerminatorInstruction::Return { return_values, .. }) => { assert_eq!(return_values.len(), 1); let value = main .dfg diff --git a/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs b/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs index 21c1797ca96..342a47160d0 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs @@ -385,7 +385,7 @@ impl<'f> PerFunctionContext<'f> { } } } - TerminatorInstruction::Return { return_values } => { + TerminatorInstruction::Return { return_values, .. } => { // Removing all `last_stores` for each returned reference is more important here // than setting them all to ReferenceValue::Unknown since no other block should // have a block with a Return terminator as a predecessor anyway. @@ -449,7 +449,7 @@ mod tests { assert_eq!(count_stores(block_id, &func.dfg), 0); let ret_val_id = match func.dfg[block_id].terminator().unwrap() { - TerminatorInstruction::Return { return_values } => return_values.first().unwrap(), + TerminatorInstruction::Return { return_values, .. } => return_values.first().unwrap(), _ => unreachable!(), }; assert_eq!(func.dfg[*ret_val_id], func.dfg[two]); @@ -485,7 +485,7 @@ mod tests { assert_eq!(count_stores(block_id, &func.dfg), 1); let ret_val_id = match func.dfg[block_id].terminator().unwrap() { - TerminatorInstruction::Return { return_values } => return_values.first().unwrap(), + TerminatorInstruction::Return { return_values, .. } => return_values.first().unwrap(), _ => unreachable!(), }; assert_eq!(func.dfg[*ret_val_id], func.dfg[one]); @@ -518,7 +518,7 @@ mod tests { assert_eq!(instructions.len(), 2); let ret_val_id = match func.dfg[block_id].terminator().unwrap() { - TerminatorInstruction::Return { return_values } => *return_values.first().unwrap(), + TerminatorInstruction::Return { return_values, .. } => *return_values.first().unwrap(), _ => unreachable!(), }; diff --git a/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs index da6f0af2484..d491afc3d26 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs @@ -202,7 +202,7 @@ mod test { assert_eq!(main.reachable_blocks().len(), 1); match main.dfg[main.entry_block()].terminator() { - Some(TerminatorInstruction::Return { return_values }) => { + Some(TerminatorInstruction::Return { return_values, .. }) => { assert_eq!(return_values.len(), 1); let return_value = main .dfg @@ -258,7 +258,7 @@ mod test { assert_eq!(main.reachable_blocks().len(), 1); match main.dfg[main.entry_block()].terminator() { - Some(TerminatorInstruction::Return { return_values }) => { + Some(TerminatorInstruction::Return { return_values, .. }) => { assert_eq!(return_values.len(), 1); let return_value = main .dfg diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index f3acf534d34..d990a95c540 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -19,12 +19,18 @@ use self::{ value::{Tree, Values}, }; -use super::ir::{function::RuntimeType, instruction::BinaryOp, types::Type, value::ValueId}; +use super::ir::{ + function::RuntimeType, + instruction::{BinaryOp, TerminatorInstruction}, + types::Type, + value::ValueId, +}; /// Generates SSA for the given monomorphized program. /// /// This function will generate the SSA but does not perform any optimizations on it. pub(crate) fn generate_ssa(program: Program) -> Ssa { + let return_location = program.return_location; let context = SharedContext::new(program); let main_id = Program::main_id(); @@ -41,6 +47,21 @@ pub(crate) fn generate_ssa(program: Program) -> Ssa { ); function_context.codegen_function_body(&main.body); + if let Some(return_location) = return_location { + let block = function_context.builder.current_block(); + if function_context.builder.current_function.dfg[block].terminator().is_some() { + let return_instruction = + function_context.builder.current_function.dfg[block].unwrap_terminator_mut(); + match return_instruction { + TerminatorInstruction::Return { call_stack, .. } => { + call_stack.clear(); + call_stack.push_back(return_location); + } + _ => unreachable!("ICE - expect return on the last block"), + } + } + } + // Main has now been compiled and any other functions referenced within have been added to the // function queue as they were found in codegen_ident. This queueing will happen each time a // previously-unseen function is found so we need now only continue popping from this queue diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index 5124e6767f3..c67b8f8bcec 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -242,6 +242,7 @@ pub struct Program { /// Note: this has no impact on monomorphization, and is simply attached here for ease of /// forwarding to the next phase. pub return_distinctness: Distinctness, + pub return_location: Option, } impl Program { @@ -249,8 +250,9 @@ impl Program { functions: Vec, main_function_signature: FunctionSignature, return_distinctness: Distinctness, + return_location: Option, ) -> Program { - Program { functions, main_function_signature, return_distinctness } + Program { functions, main_function_signature, return_distinctness, return_location } } pub fn main(&self) -> &Function { diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index e33910b12bb..b2c12746a8c 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -10,6 +10,7 @@ //! function, will monomorphize the entire reachable program. use acvm::FieldElement; use iter_extended::{btree_map, vecmap}; +use noirc_errors::Location; use noirc_printable_type::PrintableType; use std::{ collections::{BTreeMap, HashMap, VecDeque}, @@ -73,6 +74,8 @@ struct Monomorphizer<'interner> { next_function_id: u32, is_range_loop: bool, + + return_location: Option, } type HirType = crate::Type; @@ -103,7 +106,7 @@ pub fn monomorphize(main: node_interner::FuncId, interner: &NodeInterner) -> Pro let functions = vecmap(monomorphizer.finished_functions, |(_, f)| f); let FuncMeta { return_distinctness, .. } = interner.function_meta(&main); - Program::new(functions, function_sig, return_distinctness) + Program::new(functions, function_sig, return_distinctness, monomorphizer.return_location) } impl<'interner> Monomorphizer<'interner> { @@ -118,6 +121,7 @@ impl<'interner> Monomorphizer<'interner> { interner, lambda_envs_stack: Vec::new(), is_range_loop: false, + return_location: None, } } @@ -197,7 +201,13 @@ impl<'interner> Monomorphizer<'interner> { let new_main_id = self.next_function_id(); assert_eq!(new_main_id, Program::main_id()); self.function(main_id, new_main_id); - + self.return_location = + self.interner.function(&main_id).block(self.interner).statements().last().and_then( + |x| match self.interner.statement(x) { + HirStatement::Expression(id) => Some(self.interner.id_location(id)), + _ => None, + }, + ); let main_meta = self.interner.function_meta(&main_id); main_meta.into_function_signature() } diff --git a/tooling/nargo_cli/tests/execution_success/constant_return/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/constant_return/Nargo.toml similarity index 100% rename from tooling/nargo_cli/tests/execution_success/constant_return/Nargo.toml rename to tooling/nargo_cli/tests/compile_failure/constant_return/Nargo.toml diff --git a/tooling/nargo_cli/tests/execution_success/constant_return/Prover.toml b/tooling/nargo_cli/tests/compile_failure/constant_return/Prover.toml similarity index 100% rename from tooling/nargo_cli/tests/execution_success/constant_return/Prover.toml rename to tooling/nargo_cli/tests/compile_failure/constant_return/Prover.toml diff --git a/tooling/nargo_cli/tests/execution_success/constant_return/src/main.nr b/tooling/nargo_cli/tests/compile_failure/constant_return/src/main.nr similarity index 100% rename from tooling/nargo_cli/tests/execution_success/constant_return/src/main.nr rename to tooling/nargo_cli/tests/compile_failure/constant_return/src/main.nr diff --git a/tooling/nargo_cli/tests/execution_success/conditional_regression_547/Prover.toml b/tooling/nargo_cli/tests/execution_success/conditional_regression_547/Prover.toml index e69de29bb2d..3d2b4b14efe 100644 --- a/tooling/nargo_cli/tests/execution_success/conditional_regression_547/Prover.toml +++ b/tooling/nargo_cli/tests/execution_success/conditional_regression_547/Prover.toml @@ -0,0 +1 @@ +x = 1 \ No newline at end of file diff --git a/tooling/nargo_cli/tests/execution_success/conditional_regression_547/src/main.nr b/tooling/nargo_cli/tests/execution_success/conditional_regression_547/src/main.nr index e5df4cbf8da..1bd69f03ea7 100644 --- a/tooling/nargo_cli/tests/execution_success/conditional_regression_547/src/main.nr +++ b/tooling/nargo_cli/tests/execution_success/conditional_regression_547/src/main.nr @@ -1,4 +1,4 @@ -fn main() -> pub Field { +fn main(x: Field) -> pub Field { // Regression test for issue #547 // Warning: it must be kept at the start of main let arr: [u8; 2] = [1, 2]; @@ -9,7 +9,7 @@ fn main() -> pub Field { } // Regression for predicate simplification - safe_inverse(0) + x + safe_inverse(0) } fn safe_inverse(n: Field) -> Field diff --git a/tooling/nargo_cli/tests/execution_success/higher_order_functions/Prover.toml b/tooling/nargo_cli/tests/execution_success/higher_order_functions/Prover.toml index e69de29bb2d..b373bb827c4 100644 --- a/tooling/nargo_cli/tests/execution_success/higher_order_functions/Prover.toml +++ b/tooling/nargo_cli/tests/execution_success/higher_order_functions/Prover.toml @@ -0,0 +1 @@ +w = 1 \ No newline at end of file diff --git a/tooling/nargo_cli/tests/execution_success/higher_order_functions/src/main.nr b/tooling/nargo_cli/tests/execution_success/higher_order_functions/src/main.nr index ce61a4d572d..71eacfe4da5 100644 --- a/tooling/nargo_cli/tests/execution_success/higher_order_functions/src/main.nr +++ b/tooling/nargo_cli/tests/execution_success/higher_order_functions/src/main.nr @@ -1,4 +1,4 @@ -fn main() -> pub Field { +fn main(w: Field) -> pub Field { let f = if 3 * 7 > 200 as u32 { foo } else { bar }; assert(f()[1] == 2); // Lambdas: @@ -39,7 +39,7 @@ fn main() -> pub Field { let ret = twice(add1, 3); test_array_functions(); - ret + w + ret } /// Test the array functions in std::array diff --git a/tooling/nargo_cli/tests/execution_success/modulus/src/main.nr b/tooling/nargo_cli/tests/execution_success/modulus/src/main.nr index bb1a0ff5478..6dda450df5d 100644 --- a/tooling/nargo_cli/tests/execution_success/modulus/src/main.nr +++ b/tooling/nargo_cli/tests/execution_success/modulus/src/main.nr @@ -1,6 +1,6 @@ use dep::std; -fn main(bn254_modulus_be_bytes : [u8; 32], bn254_modulus_be_bits : [u1; 254]) -> pub Field { +fn main(bn254_modulus_be_bytes : [u8; 32], bn254_modulus_be_bits : [u1; 254]) { let modulus_size = std::field::modulus_num_bits(); // NOTE: The constraints used in this circuit will only work when testing nargo with the plonk bn254 backend assert(modulus_size == 254); @@ -22,6 +22,4 @@ fn main(bn254_modulus_be_bytes : [u8; 32], bn254_modulus_be_bits : [u1; 254]) -> for i in 0..254 { assert(modulus_le_bits[i] == bn254_modulus_be_bits[253-i]); } - - modulus_size } diff --git a/tooling/nargo_cli/tests/execution_success/to_be_bytes/src/main.nr b/tooling/nargo_cli/tests/execution_success/to_be_bytes/src/main.nr index 20e932c5073..81b08ed1785 100644 --- a/tooling/nargo_cli/tests/execution_success/to_be_bytes/src/main.nr +++ b/tooling/nargo_cli/tests/execution_success/to_be_bytes/src/main.nr @@ -5,8 +5,8 @@ fn main(x : Field) -> pub [u8; 31] { for i in 0..31 { bytes[i] = byte_array[i]; } - assert(bytes[30] == 60); - assert(bytes[29] == 33); - assert(bytes[28] == 31); + if ( bytes[30] != 60) | (bytes[29] != 33) | (bytes[28] != 31) { + assert(false); + } bytes } diff --git a/tooling/nargo_cli/tests/execution_success/tuple_inputs/src/main.nr b/tooling/nargo_cli/tests/execution_success/tuple_inputs/src/main.nr index 37160d5a0e6..2fc44a8fc75 100644 --- a/tooling/nargo_cli/tests/execution_success/tuple_inputs/src/main.nr +++ b/tooling/nargo_cli/tests/execution_success/tuple_inputs/src/main.nr @@ -9,9 +9,6 @@ struct Foo { } fn main(pair : (Field, Field), x: [(u8, u8, u8); 2], struct_pair: (Foo, Bar)) -> pub (Field, u8) { - assert(pair.0 == 1); - assert(pair.1 == 0); - let mut start_val = 0; for i in 0..2 { assert(x[i].0 == start_val);