diff --git a/sway-core/src/asm_generation/abstract_instruction_set.rs b/sway-core/src/asm_generation/abstract_instruction_set.rs index 6b3a2281fd6..14bee9a977c 100644 --- a/sway-core/src/asm_generation/abstract_instruction_set.rs +++ b/sway-core/src/asm_generation/abstract_instruction_set.rs @@ -30,7 +30,7 @@ impl AbstractInstructionSet { .remove_unused_ops() } - /// Removes any jumps that jump to the subsequent line + /// Removes any jumps to the subsequent line. fn remove_sequential_jumps(mut self) -> AbstractInstructionSet { let dead_jumps: Vec<_> = self .ops @@ -145,8 +145,16 @@ impl AbstractInstructionSet { if def_regs.is_superset(&use_regs) { Ok(self) } else { - Err(CompileError::Internal( - "Program erroneously uses uninitialized virtual registers.", + let bad_regs = use_regs + .difference(&def_regs) + .map(|reg| match reg { + VirtualRegister::Virtual(name) => format!("$r{name}"), + VirtualRegister::Constant(creg) => creg.to_string(), + }) + .collect::>() + .join(", "); + Err(CompileError::InternalOwned( + format!("Program erroneously uses uninitialized virtual registers: {bad_regs}"), Span::dummy(), )) } diff --git a/sway-core/src/ir_generation/function.rs b/sway-core/src/ir_generation/function.rs index 25658e83abf..42c8b788d69 100644 --- a/sway-core/src/ir_generation/function.rs +++ b/sway-core/src/ir_generation/function.rs @@ -1461,23 +1461,25 @@ impl<'te> FnCompiler<'te> { // We're dancing around a bit here to make the blocks sit in the right order. Ideally we // have the cond block, followed by the body block which may contain other blocks, and the // final block comes after any body block(s). + // + // NOTE: This is currently very important! There is a limitation in the register allocator + // which requires that all value uses are after the value definitions, where 'after' means + // later in the list of instructions, as opposed to in the control flow sense. + // + // Hence the need for a 'break' block which does nothing more than jump to the final block, + // as we need to construct the final block after the body block, but we need somewhere to + // break to during the body block construction. // Jump to the while cond block. let cond_block = self.function.create_block(context, Some("while".into())); - if !self.current_block.is_terminated(context) { self.current_block.ins(context).branch(cond_block, vec![]); } - // Fill in the body block now, jump unconditionally to the cond block at its end. - let body_block = self - .function - .create_block(context, Some("while_body".into())); - - // Create the final block after we're finished with the body. - let final_block = self + // Create the break block. + let break_block = self .function - .create_block(context, Some("end_while".into())); + .create_block(context, Some("while_break".into())); // Keep track of the previous blocks we have to jump to in case of a break or a continue. // This should be `None` if we're not in a loop already or the previous break or continue @@ -1486,11 +1488,13 @@ impl<'te> FnCompiler<'te> { let prev_block_to_continue_to = self.block_to_continue_to; // Keep track of the current blocks to jump to in case of a break or continue. - self.block_to_break_to = Some(final_block); + self.block_to_break_to = Some(break_block); self.block_to_continue_to = Some(cond_block); - // Compile the body and a branch to the condition block if no branch is already present in - // the body block + // Fill in the body block now, jump unconditionally to the cond block at its end. + let body_block = self + .function + .create_block(context, Some("while_body".into())); self.current_block = body_block; self.compile_code_block(context, md_mgr, body)?; if !self.current_block.is_terminated(context) { @@ -1501,7 +1505,16 @@ impl<'te> FnCompiler<'te> { self.block_to_break_to = prev_block_to_break_to; self.block_to_continue_to = prev_block_to_continue_to; - // Add the conditional which jumps into the body or out to the final block. + // Create the final block now we're finished with the body. + let final_block = self + .function + .create_block(context, Some("end_while".into())); + + // Add an unconditional jump from the break block to the final block. + break_block.ins(context).branch(final_block, vec![]); + + // Add the conditional in the cond block which jumps into the body or out to the final + // block. self.current_block = cond_block; let cond_value = self.compile_expression(context, md_mgr, condition)?; if !self.current_block.is_terminated(context) { diff --git a/sway-ir/src/error.rs b/sway-ir/src/error.rs index be44afc3b37..e6ecf2612cf 100644 --- a/sway-ir/src/error.rs +++ b/sway-ir/src/error.rs @@ -45,6 +45,7 @@ pub enum IrError { VerifyIntToPtrToCopyType(String), VerifyIntToPtrUnknownSourceType, VerifyLoadFromNonPointer, + VerifyMemcopyNonExistentPointer, VerifyMismatchedReturnTypes(String), VerifyBlockArgMalformed, VerifyPtrCastFromNonPointer, @@ -253,6 +254,12 @@ impl fmt::Display for IrError { IrError::VerifyLoadFromNonPointer => { write!(f, "Verification failed: Load must be from a pointer.") } + IrError::VerifyMemcopyNonExistentPointer => { + write!( + f, + "Verification failed: Attempt to use non pointer with `mem_copy`." + ) + } IrError::VerifyMismatchedReturnTypes(fn_str) => write!( f, "Verification failed: Function {fn_str} return type must match its RET \ diff --git a/sway-ir/src/optimize/inline.rs b/sway-ir/src/optimize/inline.rs index b2380d2831d..69a742f0c27 100644 --- a/sway-ir/src/optimize/inline.rs +++ b/sway-ir/src/optimize/inline.rs @@ -180,7 +180,7 @@ pub fn inline_function_call( context, call_site, post_block.get_arg(context, 0).unwrap(), - Some(post_block), + None, ); // Take the locals from the inlined function and add them to this function. `value_map` is a diff --git a/sway-ir/src/verify.rs b/sway-ir/src/verify.rs index 8933cea07fd..836f5097bb8 100644 --- a/sway-ir/src/verify.rs +++ b/sway-ir/src/verify.rs @@ -652,7 +652,7 @@ impl<'a> InstructionVerifier<'a> { fn verify_mem_copy( &self, - _dst_val: &Value, + dst_val: &Value, _src_val: &Value, _byte_len: &u64, ) -> Result<(), IrError> { @@ -662,18 +662,17 @@ impl<'a> InstructionVerifier<'a> { //| XXX Pointers are broken, pending https://github.com/FuelLabs/sway/issues/2819 //| So here we may still get non-pointers, but still ref-types, passed as the source for //| mem_copy, especially when dealing with constant b256s or similar. - //|if !dst_val.get_pointer(self.context).is_some() + if dst_val.get_pointer(self.context).is_none() //| || !(src_val.get_pointer(self.context).is_some() //| || matches!( //| src_val.get_instruction(self.context), //| Some(Instruction::GetStorageKey) | Some(Instruction::IntToPtr(..)) //| )) - //|{ - //| Err(IrError::VerifyGetNonExistentPointer) - //|} else { - //| Ok(()) - //|} - Ok(()) + { + Err(IrError::VerifyMemcopyNonExistentPointer) + } else { + Ok(()) + } } fn verify_ret(&self, val: &Value, ty: &Type) -> Result<(), IrError> { diff --git a/test/src/e2e_vm_tests/mod.rs b/test/src/e2e_vm_tests/mod.rs index 5a394b4eb25..6f9c721ccbb 100644 --- a/test/src/e2e_vm_tests/mod.rs +++ b/test/src/e2e_vm_tests/mod.rs @@ -241,17 +241,24 @@ impl TestContext { output.push_str(&out); contract_ids.push(result); } - let contract_ids: Result, anyhow::Error> = - contract_ids.into_iter().collect(); + let contract_ids = contract_ids.into_iter().collect::, _>>()?; let (result, out) = - harness::runs_on_node(&name, &context.run_config, &contract_ids?).await; + harness::runs_on_node(&name, &context.run_config, &contract_ids).await; output.push_str(&out); let receipt = result?; - assert!(receipt.iter().all(|res| !matches!( - res, - fuel_tx::Receipt::Revert { .. } | fuel_tx::Receipt::Panic { .. } - ))); + if !receipt.iter().all(|res| { + !matches!( + res, + fuel_tx::Receipt::Revert { .. } | fuel_tx::Receipt::Panic { .. } + ) + }) { + println!(); + for cid in contract_ids { + println!("Deployed contract: 0x{cid}"); + } + panic!("Receipts contain reverts or panics: {receipt:?}"); + } assert!(receipt.len() >= 2); assert_matches!(receipt[receipt.len() - 2], fuel_tx::Receipt::Return { .. }); assert_eq!(receipt[receipt.len() - 2].val().unwrap(), val); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw index 92b604f5b71..fef7c4cc053 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw @@ -3,7 +3,7 @@ use basic_storage_abi::{Quad, StoreU64}; use std::assert::assert; fn main() -> u64 { - let addr = abi(StoreU64, 0x3b4a1dcf91b22eb138d14dd4aab4b0727f9c6fa855617860e59b8ba7d0fc8a07); + let addr = abi(StoreU64, 0x8384cd87cb862d1c4e16943c6cd7c72892ce81e0ead639fd7b71bb45236d7228); let key = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; let value = 4242; diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/test_abis/basic_storage_abi/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/test_abis/basic_storage_abi/Forc.lock index e9baaabdcb2..13456185dd8 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/test_abis/basic_storage_abi/Forc.lock +++ b/test/src/e2e_vm_tests/test_programs/should_pass/test_abis/basic_storage_abi/Forc.lock @@ -1,9 +1,8 @@ [[package]] name = 'basic_storage_abi' -source = 'root' +source = 'member' dependencies = ['core'] [[package]] name = 'core' source = 'path+from-root-3777C332FD2E3D7A' -dependencies = [] diff --git a/test/src/ir_generation/tests/break.sw b/test/src/ir_generation/tests/break.sw index aedb6082f6b..a1cefe9f5c5 100644 --- a/test/src/ir_generation/tests/break.sw +++ b/test/src/ir_generation/tests/break.sw @@ -12,40 +12,49 @@ fn main() { } } -// check: br $(while=$ID)() +// We currently use a 'while_break' block to help enforce linear order, where value definitions are +// always before their uses, when read top to bottom. Required only due to a limitation in the +// register allocator which in turn will be fixed in the short term. -// OUTER LOOP: while / while_body / end_while -// Jump to first inner loop in body, `break` forces return when done rather than jump back to `while`. +// check: br $(outer_while_cond=$ID)() -// check: $while(): -// check: cbr $VAL, $(while_body=$ID)(), $(end_while=$ID)() +// OUTER WHILE LOOP +// check: $outer_while_cond(): +// check: cbr $VAL, $(outer_while_body=$ID)(), $(outer_while_end=$ID)() -// check: $while_body(): -// check: br $(while0=$ID)() +// check: $(outer_while_break=$ID)(): +// check: br $outer_while_end() -// check: $end_while(): -// check: ret () $VAL +// check: $outer_while_body(): +// check: br $(inner1_while_cond=$ID)() + + +// FIRST INNER LOOP +// check: $inner1_while_cond(): +// check: cbr $VAL, $(inner1_while_body=$ID)(), $(inner1_while_end=$ID)() -// FIRST INNER LOOP: while0 / while_body1 / end_while2 -// `break` forces jump to `end_while2` in body, jumps to second inner loop when done. +// check: $(inner1_while_break=$ID)(): +// check: br $inner1_while_end() -// check: $while0(): -// check: cbr $VAL, $(while_body1=$ID)(), $(end_while2=$ID)() +// check: $inner1_while_body(): +// check: br $inner1_while_break() -// check: $while_body1(): -// check: br $end_while2() +// check: $inner1_while_end(): +// check: br $(inner2_while_cond=$ID)() -// check: $end_while2(): -// check: br $(while3=$ID)() -// SECOND INNER LOOP: while3 / while_body4 / end_while5 -// `break` forces jump to `end_while5` in body, jumps to outer loop when done. +// SECOND INNER LOOP +// check: $inner2_while_cond(): +// check: cbr $VAL, $(inner2_while_body=$ID)(), $(inner2_while_end=$ID)() -// check: $while3(): -// check: cbr $VAL, $(while_body4=$ID)(), $(end_while5=$ID)() +// check: $(inner2_while_break=$ID)(): +// check: br $inner2_while_end() -// check: $while_body4(): -// check: br $end_while5() +// check: $inner2_while_body(): +// check: br $inner2_while_break() -// check: $end_while5(): -// check: br $end_while() +// check: $inner2_while_end(): +// check: br $outer_while_break() + +// check: $outer_while_end(): +// check: ret () $VAL diff --git a/test/src/ir_generation/tests/continue.sw b/test/src/ir_generation/tests/continue.sw index 2f10064f255..4dc2d327380 100644 --- a/test/src/ir_generation/tests/continue.sw +++ b/test/src/ir_generation/tests/continue.sw @@ -13,40 +13,46 @@ fn main() { } -// check: br $(while=$ID)() +// OUTER LOOP +// check: br $(outer_while_cond=$ID)() -// OUTER LOOP: while / while_body / end_while -// Jump to first inner loop in body, return when done. +// check: $outer_while_cond(): +// check: cbr $VAL, $(outer_while_body=$ID)(), $(outer_while_end=$ID)() -// check: $while(): -// check: cbr $VAL, $(while_body=$ID)(), $(end_while=$ID)() +// check: $(outer_while_break=$ID)(): +// check: br $outer_while_end() -// check: $while_body(): -// check: br $(while0=$ID) +// check: $outer_while_body(): +// check: br $(inner1_while_cond=$ID)() -// check: $end_while(): -// check: ret () $VAL -// FIRST INNER LOOP: while0 / while_body1 / end_while2 -// `continue` forces jump to `while0` in body, branch to second inner loop when done. +// FIRST INNER LOOP +// check: $inner1_while_cond(): +// check: cbr $VAL, $(inner1_while_body=$ID)(), $(inner1_while_end=$ID)() + +// check: $(inner1_while_break=$ID)(): +// check: br $inner1_while_end() + +// check: $inner1_while_body(): +// check: br $inner1_while_cond() -// check: $while0(): -// check: cbr $VAL, $(while_body1=$ID)(), $(end_while2=$ID)() +// check: $inner1_while_end(): +// check: br $(inner2_while_cond=$ID)() -// check: $while_body1(): -// check: br $while0() -// check: $end_while2(): -// check: br $(while3=$ID) +// SECOND INNER LOOP +// check: $inner2_while_cond(): +// check: cbr $VAL, $(inner_while_body=$ID)(), $(inner2_while_end=$ID)() -// SECOND INNER LOOP: while3 / while_body4 / end_while5 -// `continue` forces jump to `while3` in body, branch to outer loop when done. +// check: $(inner2_while_break=$ID)(): +// check: br $inner2_while_end() -// check: $while3(): -// check: cbr $VAL, $(while_body4=$ID)(), $(end_while5=$ID)() +// check: $inner_while_body(): +// check: br $inner2_while_cond() -// check: $while_body4(): -// check: br $while3() +// check: $inner2_while_end(): +// check: br $outer_while_cond() -// check: $end_while5(): -// check: br $while() + +// check: $outer_while_end(): +// check: ret () $VAL diff --git a/test/src/ir_generation/tests/let_reassign_while_loop.sw b/test/src/ir_generation/tests/let_reassign_while_loop.sw index c52ceec9cc2..20237a6bc3a 100644 --- a/test/src/ir_generation/tests/let_reassign_while_loop.sw +++ b/test/src/ir_generation/tests/let_reassign_while_loop.sw @@ -13,13 +13,16 @@ fn main() -> bool { // check: $while(): // check: cbr $VAL, $(while_body=$ID)(), $(end_while=$ID)() +// check: $(while_break=$ID)(): +// check: br $end_while() + // check: $while_body(): // check: cbr $VAL, $(block0=$ID)(), $(block1=$ID)($VAL) -// check: $end_while(): - // check: $block0(): -// check: br $(block1=$ID) +// check: br $(block1=$ID)($VAL) // check: $block1($VAL: bool): // check: br $while + +// check: $end_while():