Error for uninitialized register on asm blocks (#5231)

## Description

This PR closes #4997. The problem
here is that "asm blocks registers" without initializers are considered
uninitialized. There is no fallback to "capture" an available variable.
This could be easily done, but that would leave us with no obvious
syntax for unitialized registers.

With this in mind this PR solves the compiler bug with two diagnostics:

1 - uninitialized registers cannot be read before they are written. This
generates an error:

```
 5 |     let _ = asm(r1) {
   |                 ^^ Unitialized register is being read before being written
 6 |         r1: u64
 7 |     };
```

2 - a warning when a unitialized register shadows an available variable
with the same name. This is almost for sure a problem, and it is better
the name the registe with something else and avoid all the confusion.

```
 27 |     let r5 = 0;
 28 |     asm(r5) {};
    |         -- This unitialized register is shadowing a variable, you probably meant to also initialize it like "r5: r5".
 29 | 
 30 |     0
```

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

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