[move] Fixed locals disassembly (#20747)

## Description It looks like locals are currently not disassembled correctly when running Move disassembler and this PR contains a fix. In particular, consider the following Move code: ```move module test::M1; public enum SomeEnum has drop, copy { NamedVariant { field: u64 }, PositionalVariant(u64), } public fun foo(e: SomeEnum, p1: u64, p2: u64): (u64, u64) { match (e) { SomeEnum::NamedVariant { field } => { let mut res = field + p1; res = res + p2; (res, res) }, SomeEnum::PositionalVariant(field) => (field, field) } } #[test] public fun test() { let e = SomeEnum::NamedVariant { field: 42 }; foo(e, 42, 7); } ``` When running disassembler on the `M1` module, the following 3 locals are reported in the disassembly: ``` L0: __match_tmp%#unpack_subject#1#1#0: SomeEnum L1: field#3#0: u64 L2: res#1#0: u64 ``` However, 6 locals are reported in the source map: ``` %#7 %#8 __match_tmp%#match_subject#2#2#0 __match_tmp%#unpack_subject#1#1#0 field#3#0 res#1#0 ``` The same number of locals is also reported in the trace generated out of the test function in `M1`. It looks like the disassembler incorrectly assumes that function parameters are included in the locals and only locals beyond the (optional) initial number of parameters should be counted. After making the adjustment to ignore the parameters when disassembling locals, the following is reported in the disassembly which is consistent with the number (and names) of the locals in the source map and with the types of the locals in the trace: ``` L0: %#7: u64 L1: %#8: u64 L2: __match_tmp%#match_subject#2#2#0: &SomeEnum L3: __match_tmp%#unpack_subject#1#1#0: SomeEnum L4: field#3#0: u64 L5: res#1#0: u64 ``` ## Test plan All tests should pass
MystenLabs · Jan 6, 2025 · a56deae · a56deae
1 parent ac54b0c
commit a56deae
Show file tree

Hide file tree

Showing 13 changed files with 40 additions and 26 deletions.
diff --git a/external-crates/move/crates/move-cli/tests/sandbox_tests/module_disassemble/args.exp b/external-crates/move/crates/move-cli/tests/sandbox_tests/module_disassemble/args.exp
@@ -13,6 +13,7 @@ struct S {
 }
 
 public foo(Arg0: u64): vector<u64> {
+L1:	loc0: u64
 B0:
 	0: Call bar(): u64
 	1: StLoc[1](loc0: u64)

diff --git a/external-crates/move/crates/move-disassembler/src/disassembler.rs b/external-crates/move/crates/move-disassembler/src/disassembler.rs
@@ -572,24 +572,10 @@ impl<'a> Disassembler<'a> {
             return Ok(());
         }
 
-        if function_source_map.locals.len() <= parameter_len {
-            return Ok(());
-        }
-
         let signature = self.source_mapper.bytecode.signature_at(locals_idx);
-        for (local_idx, (name, _)) in function_source_map
-            .locals
-            .iter()
-            .skip(parameter_len)
-            .enumerate()
-        {
-            any_write!(buffer, "L{local_idx}:\t{name}: ")?;
-            self.disassemble_type_for_local(
-                buffer,
-                function_source_map,
-                parameter_len + local_idx,
-                signature,
-            )?;
+        for (local_idx, (name, _)) in function_source_map.locals.iter().enumerate() {
+            any_write!(buffer, "L{}:\t{}: ", local_idx + parameter_len, name)?;
+            self.disassemble_type_for_local(buffer, function_source_map, local_idx, signature)?;
             any_writeln!(buffer)?;
         }
 

diff --git a/.../move-ir-compiler-transactional-tests/tests/bytecode-generation/declarations/function.exp b/.../move-ir-compiler-transactional-tests/tests/bytecode-generation/declarations/function.exp
@@ -10,6 +10,7 @@ struct Coin {
 }
 
 public value(Arg0: &Coin): u64 {
+L1:	loc0: &u64
 B0:
 	0: MoveLoc[0](Arg0: &Coin)
 	1: ImmBorrowField[0](Coin.value: u64)
@@ -20,10 +21,12 @@ B0:
 }
 
 public deposit(Arg0: &mut Coin, Arg1: Coin) {
-L0:	loc2: &Coin
-L1:	loc3: u64
-L2:	loc4: u64
-L3:	loc5: u64
+L2:	loc0: &mut u64
+L3:	loc1: u64
+L4:	loc2: &Coin
+L5:	loc3: u64
+L6:	loc4: u64
+L7:	loc5: u64
 B0:
 	0: MoveLoc[0](Arg0: &mut Coin)
 	1: MutBorrowField[0](Coin.value: u64)

diff --git a/...tes/move-ir-compiler-transactional-tests/tests/bytecode-generation/expressions/borrow.exp b/...tes/move-ir-compiler-transactional-tests/tests/bytecode-generation/expressions/borrow.exp
@@ -53,6 +53,7 @@ struct T {
 }
 
 f(Arg0: &T) {
+L1:	loc0: &u64
 B0:
 	0: MoveLoc[0](Arg0: &T)
 	1: ImmBorrowField[0](T.u: u64)
@@ -63,6 +64,7 @@ B0:
 }
 
 g(Arg0: &mut T) {
+L1:	loc0: &u64
 B0:
 	0: MoveLoc[0](Arg0: &mut T)
 	1: ImmBorrowField[0](T.u: u64)
@@ -73,6 +75,7 @@ B0:
 }
 
 public h(Arg0: &mut T) {
+L1:	loc0: &mut u64
 B0:
 	0: MoveLoc[0](Arg0: &mut T)
 	1: MutBorrowField[0](T.u: u64)
@@ -95,6 +98,7 @@ struct T<Ty0> {
 }
 
 f(Arg0: &T<u64>) {
+L1:	loc0: &u64
 B0:
 	0: MoveLoc[0](Arg0: &T<u64>)
 	1: ImmBorrowFieldGeneric[0](T.u: u64)
@@ -105,6 +109,7 @@ B0:
 }
 
 g(Arg0: &mut T<u128>) {
+L1:	loc0: &u128
 B0:
 	0: MoveLoc[0](Arg0: &mut T<u128>)
 	1: ImmBorrowFieldGeneric[1](T.u: u128)

diff --git a/...move-ir-compiler-transactional-tests/tests/bytecode-generation/expressions/borrow_mut.exp b/...move-ir-compiler-transactional-tests/tests/bytecode-generation/expressions/borrow_mut.exp
@@ -32,6 +32,7 @@ struct FooCoin {
 }
 
 public borrow_mut_field(Arg0: &mut FooCoin) {
+L1:	loc0: &mut u64
 B0:
 	0: MoveLoc[0](Arg0: &mut FooCoin)
 	1: MutBorrowField[0](FooCoin.value: u64)
@@ -54,6 +55,7 @@ struct FooCoin<Ty0> {
 }
 
 public borrow_mut_field(Arg0: &mut FooCoin<address>) {
+L1:	loc0: &mut u64
 B0:
 	0: MoveLoc[0](Arg0: &mut FooCoin<address>)
 	1: MutBorrowFieldGeneric[0](FooCoin.value: u64)

diff --git a/...tes/move/crates/move-vm-transactional-tests/tests/enums/basic_poly_enum_type_mismatch.exp b/...tes/move/crates/move-vm-transactional-tests/tests/enums/basic_poly_enum_type_mismatch.exp
@@ -11,6 +11,8 @@ enum EnumWithTwoVariants<Ty0> {
 }
 
 public f<Ty0>(Arg0: Ty0, Arg1: Ty0): Ty0 {
+L2:	loc0: EnumWithTwoVariants<Ty0>
+L3:	loc1: Ty0
 B0:
 	0: MoveLoc[0](Arg0: Ty0)
 	1: PackVariantGeneric(VariantInstantiationHandleIndex(0))

diff --git a/...ates/move/crates/move-vm-transactional-tests/tests/enums/enum_variant_jump_same_label.exp b/...ates/move/crates/move-vm-transactional-tests/tests/enums/enum_variant_jump_same_label.exp
@@ -12,6 +12,7 @@ enum EnumWithThreeVariants<Ty0> {
 }
 
 public call_success<Ty0>(Arg0: Ty0): Ty0 {
+L1:	loc0: EnumWithThreeVariants<Ty0>
 B0:
 	0: MoveLoc[0](Arg0: Ty0)
 	1: PackVariantGeneric(VariantInstantiationHandleIndex(0))
@@ -42,6 +43,7 @@ B0:
 }
 
 f<Ty0>(Arg0: EnumWithThreeVariants<Ty0>): Ty0 {
+L1:	loc0: Ty0
 B0:
 	0: ImmBorrowLoc[0](Arg0: EnumWithThreeVariants<Ty0>)
 	1: VariantSwitch(VariantJumpTableIndex(0))

diff --git a/...rnal-crates/move/crates/move-vm-transactional-tests/tests/enums/enum_variant_mismatch.exp b/...rnal-crates/move/crates/move-vm-transactional-tests/tests/enums/enum_variant_mismatch.exp
@@ -11,6 +11,8 @@ enum EnumWithTwoVariants<Ty0> {
 }
 
 public f<Ty0>(Arg0: Ty0, Arg1: Ty0): Ty0 {
+L2:	loc0: EnumWithTwoVariants<Ty0>
+L3:	loc1: Ty0
 B0:
 	0: MoveLoc[0](Arg0: Ty0)
 	1: PackVariantGeneric(VariantInstantiationHandleIndex(0))

diff --git a/external-crates/move/crates/move-vm-transactional-tests/tests/enums/mono/mut_ref_update.exp b/external-crates/move/crates/move-vm-transactional-tests/tests/enums/mono/mut_ref_update.exp
@@ -11,6 +11,7 @@ enum EnumWithTwoVariants has drop {
 }
 
 public f(Arg0: u64): EnumWithTwoVariants {
+L1:	loc0: u64
 B0:
 	0: LdU64(0)
 	1: StLoc[1](loc0: u64)
@@ -21,6 +22,8 @@ B0:
 }
 
 public mutate(Arg0: &mut EnumWithTwoVariants, Arg1: u64) {
+L2:	loc0: &mut u64
+L3:	loc1: &mut u64
 B0:
 	0: CopyLoc[0](Arg0: &mut EnumWithTwoVariants)
 	1: FreezeRef
@@ -50,7 +53,8 @@ Jump tables:
 }
 
 public get(Arg0: &EnumWithTwoVariants): &u64 {
-L0:	loc1: &u64
+L1:	loc0: &u64
+L2:	loc1: &u64
 B0:
 	0: CopyLoc[0](Arg0: &EnumWithTwoVariants)
 	1: VariantSwitch(VariantJumpTableIndex(0))

diff --git a/...rates/move/crates/move-vm-transactional-tests/tests/enums/mono/mut_ref_update_variant.exp b/...rates/move/crates/move-vm-transactional-tests/tests/enums/mono/mut_ref_update_variant.exp
@@ -28,7 +28,8 @@ B0:
 }
 
 public is_variant_two(Arg0: &EnumWithTwoVariants): bool {
-L0:	loc1: &u64
+L1:	loc0: &u64
+L2:	loc1: &u64
 B0:
 	0: CopyLoc[0](Arg0: &EnumWithTwoVariants)
 	1: VariantSwitch(VariantJumpTableIndex(0))

diff --git a/external-crates/move/crates/move-vm-transactional-tests/tests/enums/poly/basic_poly_enum.exp b/external-crates/move/crates/move-vm-transactional-tests/tests/enums/poly/basic_poly_enum.exp
@@ -11,7 +11,8 @@ enum EnumWithTwoVariants<Ty0> {
 }
 
 public f<Ty0>(Arg0: Ty0): Ty0 {
-L0:	loc1: Ty0
+L1:	loc0: EnumWithTwoVariants<Ty0>
+L2:	loc1: Ty0
 B0:
 	0: MoveLoc[0](Arg0: Ty0)
 	1: PackVariantGeneric(VariantInstantiationHandleIndex(0))

diff --git a/...l-crates/move/crates/move-vm-transactional-tests/tests/enums/poly/poly_mut_ref_update.exp b/...l-crates/move/crates/move-vm-transactional-tests/tests/enums/poly/poly_mut_ref_update.exp
@@ -11,6 +11,7 @@ enum EnumWithTwoVariants<Ty0: drop> has drop {
 }
 
 public f<Ty0: drop>(Arg0: Ty0): EnumWithTwoVariants<Ty0> {
+L1:	loc0: u64
 B0:
 	0: LdU64(0)
 	1: StLoc[1](loc0: u64)
@@ -21,6 +22,8 @@ B0:
 }
 
 public mutate<Ty0: drop>(Arg0: &mut EnumWithTwoVariants<Ty0>, Arg1: Ty0) {
+L2:	loc0: &mut Ty0
+L3:	loc1: &mut u64
 B0:
 	0: CopyLoc[0](Arg0: &mut EnumWithTwoVariants<Ty0>)
 	1: FreezeRef
@@ -50,7 +53,8 @@ Jump tables:
 }
 
 public get<Ty0: drop>(Arg0: &EnumWithTwoVariants<Ty0>): &Ty0 {
-L0:	loc1: &u64
+L1:	loc0: &Ty0
+L2:	loc1: &u64
 B0:
 	0: CopyLoc[0](Arg0: &EnumWithTwoVariants<Ty0>)
 	1: VariantSwitch(VariantJumpTableIndex(0))

diff --git a/.../move/crates/move-vm-transactional-tests/tests/enums/poly/poly_mut_ref_update_variant.exp b/.../move/crates/move-vm-transactional-tests/tests/enums/poly/poly_mut_ref_update_variant.exp
@@ -28,7 +28,8 @@ B0:
 }
 
 public is_variant_two(Arg0: &EnumWithTwoVariants<u64>): bool {
-L0:	loc1: &u64
+L1:	loc0: &u64
+L2:	loc1: &u64
 B0:
 	0: CopyLoc[0](Arg0: &EnumWithTwoVariants<u64>)
 	1: VariantSwitch(VariantJumpTableIndex(0))