Error for uninitialized register on asm blocks

sway-core/src/asm_lang/allocated_ops.rs

@@ -242,6 +242,7 @@ pub(crate) enum AllocatedOpcode {
 }
 
 impl AllocatedOpcode {
+    /// Returns a list of all registers *written* by instruction `self`.
     pub(crate) fn def_registers(&self) -> BTreeSet<&AllocatedRegister> {
         use AllocatedOpcode::*;
         (match self {

sway-core/src/asm_lang/virtual_ops.rs

@@ -341,7 +341,7 @@ impl VirtualOp {
             LT(_r1, r2, r3) => vec![r2, r3],
             MLOG(_r1, r2, r3) => vec![r2, r3],
             MOD(_r1, r2, r3) => vec![r2, r3],
-            MODI(r1, r2, _i) => vec![r1, r2],
+            MODI(_r1, r2, _i) => vec![r2],
             MOVE(_r1, r2) => vec![r2],
             MOVI(_r1, _i) => vec![],
             MROO(_r1, r2, r3) => vec![r2, r3],

sway-core/src/semantic_analysis/ast_node/expression/typed_expression.rs

@@ -842,7 +842,7 @@ impl ty::TyExpression {
         // this includes two checks:
         // 1. Check that no control flow opcodes are used.
         // 2. Check that initialized registers are not reassigned in the `asm` block.
-        check_asm_block_validity(handler, &asm)?;
+        check_asm_block_validity(handler, &asm, &ctx)?;
 
         let asm_span = asm
             .returns
@@ -2276,7 +2276,11 @@ mod tests {
     }
 }
 
-fn check_asm_block_validity(handler: &Handler, asm: &AsmExpression) -> Result<(), ErrorEmitted> {
+fn check_asm_block_validity(
+    handler: &Handler,
+    asm: &AsmExpression,
+    ctx: &TypeCheckContext,
+) -> Result<(), ErrorEmitted> {
     // Collect all asm block instructions in the form of `VirtualOp`s
     let mut opcodes = vec![];
     for op in &asm.body {
@@ -2365,5 +2369,58 @@ fn check_asm_block_validity(handler: &Handler, asm: &AsmExpression) -> Result<()
         handler.emit_err(err);
     }
 
+    // Check #3: Check if there are uninitialized registers that are read before being written
+    let mut uninitialized_registers = asm
+        .registers
+        .iter()
+        .filter(|reg| reg.initializer.is_none())
+        .map(|reg| {
+            let span = reg.name.span();
+
+            // Emit warning if this register shadows a variable
+            let temp_handler = Handler::default();
+            let decl = ctx.namespace.resolve_call_path(
+                &temp_handler,
+                ctx.engines,
+                &CallPath {
+                    prefixes: vec![],
+                    suffix: sway_types::BaseIdent::new(span.clone()),
+                    is_absolute: true,
+                },
+                None,
+            );
+
+            if let Ok(ty::TyDecl::VariableDecl(decl)) = decl {
+                handler.emit_warn(CompileWarning {
+                    span: span.clone(),
+                    warning_content: Warning::UninitializedAsmRegShadowsVariable {
+                        name: decl.name.clone(),
+                    },
+                });
+            }
+
+            (VirtualRegister::Virtual(reg.name.to_string()), span)
+        })
+        .collect::<HashMap<_, _>>();
+
+    for (op, _, _) in opcodes.iter() {
+        for being_read in op.use_registers() {
+            if let Some(span) = uninitialized_registers.remove(being_read) {
+                handler.emit_err(CompileError::UninitRegisterInAsmBlockBeingRead { span });
+            }
+        }
+
+        for being_written in op.def_registers() {
+            uninitialized_registers.remove(being_written);
+        }
+    }
+
+    if let Some((reg, _)) = asm.returns.as_ref() {
+        let reg = VirtualRegister::Virtual(reg.name.to_string());
+        if let Some(span) = uninitialized_registers.remove(&reg) {
+            handler.emit_err(CompileError::UninitRegisterInAsmBlockBeingRead { span });
+        }
+    }
+
     Ok(())
 }

sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs

@@ -3610,12 +3610,14 @@ fn asm_register_declaration_to_asm_register_declaration(
     engines: &Engines,
     asm_register_declaration: sway_ast::AsmRegisterDeclaration,
 ) -> Result<AsmRegisterDeclaration, ErrorEmitted> {
+    let initializer = asm_register_declaration
+        .value_opt
+        .map(|(_colon_token, expr)| expr_to_expression(context, handler, engines, *expr))
+        .transpose()?;
+
     Ok(AsmRegisterDeclaration {
         name: asm_register_declaration.register,
-        initializer: asm_register_declaration
-            .value_opt
-            .map(|(_colon_token, expr)| expr_to_expression(context, handler, engines, *expr))
-            .transpose()?,
+        initializer,
     })
 }
 

sway-error/src/error.rs

@@ -733,6 +733,9 @@ pub enum CompileError {
     AbiSupertraitMethodCallAsContractCall { fn_name: Ident, span: Span },
     #[error("\"Self\" is not valid in the self type of an impl block")]
     SelfIsNotValidAsImplementingFor { span: Span },
+
+    #[error("Unitialized register is being read before being written")]
+    UninitRegisterInAsmBlockBeingRead { span: Span },
 }
 
 impl std::convert::From<TypeError> for CompileError {
@@ -918,6 +921,7 @@ impl Spanned for CompileError {
             TypeNotAllowed { span, .. } => span.clone(),
             ExpectedStringLiteral { span } => span.clone(),
             SelfIsNotValidAsImplementingFor { span } => span.clone(),
+            UninitRegisterInAsmBlockBeingRead { span } => span.clone(),
         }
     }
 }

sway-error/src/warning.rs

@@ -67,6 +67,9 @@ pub enum Warning {
     ShadowsOtherSymbol {
         name: Ident,
     },
+    UninitializedAsmRegShadowsVariable {
+        name: Ident,
+    },
     OverridingTraitImplementation,
     DeadDeclaration,
     DeadEnumDeclaration,
@@ -202,6 +205,10 @@ impl fmt::Display for Warning {
                 f,
                 "This shadows another symbol in this scope with the same name \"{name}\"."
             ),
+            UninitializedAsmRegShadowsVariable { name } => write!(
+                f,
+                "This unitialized register is shadowing a variable, you probably meant to also initialize it like \"{name}: {name}\"."
+            ),
             OverridingTraitImplementation => write!(
                 f,
                 "This trait implementation overrides another one that was previously defined."

sway-ir/src/printer.rs

@@ -975,7 +975,7 @@ fn asm_block_to_doc(
             Doc::Empty,
             |doc, AsmArg { initializer, .. }| match initializer {
                 Some(init_val) if init_val.is_constant(context) => {
-                    doc.append(constant_to_doc(context, md_namer, namer, init_val))
+                    doc.append(maybe_constant_to_doc(context, md_namer, namer, init_val))
                 }
                 _otherwise => doc,
             },

test/src/e2e_vm_tests/test_programs/should_fail/asm_read_from_uninitialized_reg/Forc.lock

@@ -0,0 +1,3 @@
+[[package]]
+name = "asm_read_from_uninitialized_reg"
+source = "member"

test/src/e2e_vm_tests/test_programs/should_fail/asm_read_from_uninitialized_reg/Forc.toml

@@ -0,0 +1,6 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+entry = "main.sw"
+implicit-std = false
+license = "Apache-2.0"
+name = "asm_read_from_uninitialized_reg"

test/src/e2e_vm_tests/test_programs/should_fail/asm_read_from_uninitialized_reg/src/main.sw

@@ -0,0 +1,31 @@
+script;
+
+fn main() -> u64 {
+    // Returning an unitialized register is not ok
+    let _ = asm(r1) {
+        r1: u64
+    };
+
+    // Reading unitialized register is not ok
+    asm(r2) {
+        sw r2 r2 i0;
+    };
+
+    // Writing before reading unitialized register is ok
+    asm(r3) {
+        movi r3 i0;
+        sw r3 r3 i0;
+    };
+
+    // Writing before returning unitialized register is ok
+    let _ = asm(r4) {
+        movi r4 i0;
+        r4: u64
+    };
+
+    // Shadowing a variable is a warning
+    let r5 = 0;
+    asm(r5) {};
+
+    0
+}

test/src/e2e_vm_tests/test_programs/should_fail/asm_read_from_uninitialized_reg/test.toml

@@ -0,0 +1,9 @@
+category = "fail"
+
+# check: $()Returning an unitialized register is not ok
+# nextln: $()let _ = asm(r1)
+# nextln: $()Unitialized register is being read before being written
+
+# check: $()Reading unitialized register is not ok
+# nextln: $()asm(r2)
+# nextln: $()Unitialized register is being read before being written

test/src/e2e_vm_tests/test_programs/should_fail/storage_conflict/src/main.sw

@@ -101,21 +101,16 @@ fn do_pure_stuff_e() -> bool {
 const KEY: b256 = 0xfefefefefefefefefefefefefefefefefefefefefefefefefefefefefefefefe;
 
 fn read_storage_word() -> u64 {
-    asm (key: KEY, is_set, res, a, b, c) {
-        move a b;
+    asm (key: KEY, is_set, res) {
         srw res is_set key;
-        addi b c i1;
         res: u64
     }
 }
 
 fn read_storage_b256() -> b256 {
     let res = ZERO_B256;
-    asm (key: KEY, is_set, buf: res, count: 1, a, b, c) {
-        modi a b i10;
-        lt a b c;
+    asm (key: KEY, is_set, buf: res, count: 1) {
         srwq buf is_set key count;
-        and a b c;
     }
     res
 }

test/src/e2e_vm_tests/test_programs/should_fail/storage_conflict/test.toml

@@ -12,7 +12,7 @@ category = "fail"
 # check: storage_conflict/src/main.sw:13:5
 # check: $()This function performs a storage read but does not have the required attribute(s).  Try adding "#[storage(read)]" to the function declaration.
 
-# check: storage_conflict/src/main.sw:112:1
+# check: storage_conflict/src/main.sw:110:1
 # check: $()This function performs a storage read but does not have the required attribute(s).  Try adding "#[storage(read)]" to the function declaration.
 
 # check: storage_conflict/src/main.sw:57:1
@@ -24,7 +24,7 @@ category = "fail"
 # check: storage_conflict/src/main.sw:16:5
 # check: $()This function performs a storage read but does not have the required attribute(s).  Try adding "#[storage(read)]" to the function declaration.
 
-# check: storage_conflict/src/main.sw:123:1
+# check: storage_conflict/src/main.sw:118:1
 # check: $()This function performs a storage write but does not have the required attribute(s).  Try adding "#[storage(write)]" to the function declaration.
 
 # check: storage_conflict/src/main.sw:71:1
@@ -36,7 +36,7 @@ category = "fail"
 # check: storage_conflict/src/main.sw:19:5
 # check: $()This function performs a storage write but does not have the required attribute(s).  Try adding "#[storage(write)]" to the function declaration.
 
-# check: storage_conflict/src/main.sw:129:1
+# check: storage_conflict/src/main.sw:124:1
 # check: $()This function performs a storage write but does not have the required attribute(s).  Try adding "#[storage(write)]" to the function declaration.
 
 # check: storage_conflict/src/main.sw:87:1