Merge a47afce into b495d0d

sway-core/src/asm_generation/finalized_asm.rs

@@ -56,13 +56,12 @@ impl FinalizedAsm {
         source_engine: &SourceEngine,
     ) -> Result<CompiledBytecode, ErrorEmitted> {
         match &self.program_section {
-            InstructionSet::Fuel { ops } => to_bytecode_mut(
-                handler,
+            InstructionSet::Fuel { ops } => Ok(to_bytecode_mut(
                 ops,
                 &mut self.data_section,
                 source_map,
                 source_engine,
-            ),
+            )),
             InstructionSet::Evm { ops } => {
                 let mut assembler = Assembler::new();
                 if let Err(e) = assembler.push_all(ops.clone()) {
@@ -98,67 +97,83 @@ impl fmt::Display for FinalizedAsm {
 }
 
 fn to_bytecode_mut(
-    handler: &Handler,
     ops: &[AllocatedOp],
     data_section: &mut DataSection,
     source_map: &mut SourceMap,
     source_engine: &SourceEngine,
-) -> Result<CompiledBytecode, ErrorEmitted> {
-    if ops.len() & 1 != 0 {
-        tracing::info!("ops len: {}", ops.len());
-        return Err(handler.emit_err(CompileError::Internal(
-            "Non-word-aligned (odd-number) ops generated. This is an invariant violation.",
-            Span::new(" ".into(), 0, 0, None).unwrap(),
-        )));
-    }
-    // The below invariant is introduced to word-align the data section.
-    // A noop is inserted in ASM generation if there is an odd number of ops.
-    assert_eq!(ops.len() & 1, 0);
-    // this points at the byte (*4*8) address immediately following (+1) the last instruction
-    // Some LWs are expanded into two ops to allow for data larger than one word, so we calculate
-    // exactly how many ops will be generated to calculate the offset.
-    let offset_to_data_section_in_bytes = ops.iter().fold(0, |acc, item| match &item.opcode {
-        AllocatedOpcode::LoadDataId(_reg, data_label)
-            if !data_section
-                .has_copy_type(data_label)
-                .expect("data label references non existent data -- internal error") =>
-        {
-            acc + 8
+) -> CompiledBytecode {
+    fn op_size_in_bytes(data_section: &DataSection, item: &AllocatedOp) -> u64 {
+        match &item.opcode {
+            AllocatedOpcode::LoadDataId(_reg, data_label)
+                if !data_section
+                    .has_copy_type(data_label)
+                    .expect("data label references non existent data -- internal error") =>
+            {
+                8
+            }
+            AllocatedOpcode::DataSectionOffsetPlaceholder => 8,
+            AllocatedOpcode::BLOB(count) => count.value as u64 * 4,
+            AllocatedOpcode::CFEI(i) | AllocatedOpcode::CFSI(i) if i.value == 0 => 0,
+            _ => 4,
         }
-        AllocatedOpcode::BLOB(count) => acc + count.value as u64 * 4,
-        _ => acc + 4,
-    }) + 4;
+    }
 
-    // each op is four bytes, so the length of the buf is the number of ops times four.
-    let mut buf = vec![0; (ops.len() * 4) + 4];
+    // Some instructions may be omitted or expanded into multiple instructions, so we compute,
+    // using `op_size_in_bytes`, exactly how many ops will be generated to calculate the offset.
+    let mut offset_to_data_section_in_bytes = ops
+        .iter()
+        .fold(0, |acc, item| acc + op_size_in_bytes(data_section, item));
+
+    // A noop is inserted in ASM generation if required, to word-align the data section.
+    let mut ops_padded = Vec::new();
+    let ops = if offset_to_data_section_in_bytes & 7 == 0 {
+        ops
+    } else {
+        ops_padded.reserve(ops.len() + 1);
+        ops_padded.extend(ops.iter().cloned());
+        ops_padded.push(AllocatedOp {
+            opcode: AllocatedOpcode::NOOP,
+            comment: "word-alignment of data section".into(),
+            owning_span: None,
+        });
+        offset_to_data_section_in_bytes += 4;
+        &ops_padded
+    };
+
+    let mut buf = Vec::with_capacity(offset_to_data_section_in_bytes as usize);
 
     let mut half_word_ix = 0;
+    let mut offset_from_instr_start = 0;
     for op in ops.iter() {
         let span = op.owning_span.clone();
-        let op = op.to_fuel_asm(offset_to_data_section_in_bytes, data_section);
-        match op {
+        let fuel_op = op.to_fuel_asm(
+            offset_to_data_section_in_bytes,
+            offset_from_instr_start,
+            data_section,
+        );
+        offset_from_instr_start += op_size_in_bytes(data_section, op);
+
+        match fuel_op {
             Either::Right(data) => {
-                for i in 0..data.len() {
-                    buf[(half_word_ix * 4) + i] = data[i];
-                }
+                // Static assert to ensure that we're only dealing with DataSectionOffsetPlaceholder,
+                // a one-word (8 bytes) data within the code. No other uses are known.
+                let _: [u8; 8] = data;
+                buf.extend(data.iter().cloned());
                 half_word_ix += 2;
             }
             Either::Left(ops) => {
-                if ops.len() > 1 {
-                    buf.resize(buf.len() + ((ops.len() - 1) * 4), 0);
-                }
                 for op in ops {
                     if let Some(span) = &span {
                         source_map.insert(source_engine, half_word_ix, span);
                     }
-                    let read_range_upper_bound =
-                        core::cmp::min(half_word_ix * 4 + std::mem::size_of_val(&op), buf.len());
-                    buf[half_word_ix * 4..read_range_upper_bound].copy_from_slice(&op.to_bytes());
+                    buf.extend(op.to_bytes().iter());
                     half_word_ix += 1;
                 }
             }
         }
     }
+    assert_eq!(half_word_ix * 4, offset_to_data_section_in_bytes as usize);
+    assert_eq!(buf.len(), offset_to_data_section_in_bytes as usize);
 
     let config_offsets = data_section
         .config_map
@@ -175,10 +190,10 @@ fn to_bytecode_mut(
 
     buf.append(&mut data_section);
 
-    Ok(CompiledBytecode {
+    CompiledBytecode {
         bytecode: buf,
         config_const_offsets: config_offsets,
-    })
+    }
 }
 
 /// Checks for disallowed opcodes in non-contract code.

sway-core/src/asm_generation/fuel/abstract_instruction_set.rs

@@ -3,8 +3,7 @@ use crate::{
         allocated_abstract_instruction_set::AllocatedAbstractInstructionSet, register_allocator,
     },
     asm_lang::{
-        allocated_ops::{AllocatedOp, AllocatedOpcode},
-        Op, OrganizationalOp, RealizedOp, VirtualOp, VirtualRegister,
+        allocated_ops::AllocatedOp, Op, OrganizationalOp, RealizedOp, VirtualOp, VirtualRegister,
     },
 };
 
@@ -192,9 +191,8 @@ pub struct RealizedAbstractInstructionSet {
 }
 
 impl RealizedAbstractInstructionSet {
-    pub(crate) fn pad_to_even(self) -> Vec<AllocatedOp> {
-        let mut ops = self
-            .ops
+    pub(crate) fn to_allocated_ops(self) -> Vec<AllocatedOp> {
+        self.ops
             .into_iter()
             .map(
                 |RealizedOp {
@@ -209,16 +207,6 @@ impl RealizedAbstractInstructionSet {
                     }
                 },
             )
-            .collect::<Vec<_>>();
-
-        if ops.len() & 1 != 0 {
-            ops.push(AllocatedOp {
-                opcode: AllocatedOpcode::NOOP,
-                comment: "word-alignment of data section".into(),
-                owning_span: None,
-            });
-        }
-
-        ops
+            .collect::<Vec<_>>()
     }
 }

sway-core/src/asm_generation/fuel/data_section.rs

@@ -252,8 +252,8 @@ impl DataSection {
     /// static values that have a length longer than one word.
     /// This method appends pointers to the end of the data section (thus, not altering the data
     /// offsets of previous data).
-    /// `pointer_value` is in _bytes_ and refers to the offset from instruction start to the data
-    /// in question.
+    /// `pointer_value` is in _bytes_ and refers to the offset from instruction start or
+    /// relative to the current (load) instruction.
     pub(crate) fn append_pointer(&mut self, pointer_value: u64) -> DataId {
         // The 'pointer' is just a literal 64 bit address.
         self.insert_data_value(Entry::new_word(pointer_value, None, None))

sway-core/src/asm_generation/programs/allocated.rs

@@ -14,7 +14,7 @@ impl AllocatedProgram {
 
         let (realized_ops, mut label_offsets) =
             abstract_ops.realize_labels(&mut self.data_section)?;
-        let ops = realized_ops.pad_to_even();
+        let ops = realized_ops.to_allocated_ops();
 
         // Collect the entry point offsets.
         let entries = self

sway-core/src/asm_lang/allocated_ops.rs

@@ -523,6 +523,7 @@ impl AllocatedOp {
     pub(crate) fn to_fuel_asm(
         &self,
         offset_to_data_section: u64,
+        offset_from_instr_start: u64,
         data_section: &mut DataSection,
     ) -> Either<Vec<fuel_asm::Instruction>, DoubleWideData> {
         use AllocatedOpcode::*;
@@ -677,7 +678,13 @@ impl AllocatedOp {
                 return Either::Right(offset_to_data_section.to_be_bytes())
             }
             LoadDataId(a, b) => {
-                return Either::Left(realize_load(a, b, data_section, offset_to_data_section))
+                return Either::Left(realize_load(
+                    a,
+                    b,
+                    data_section,
+                    offset_to_data_section,
+                    offset_from_instr_start,
+                ))
             }
             Undefined => unreachable!("Sway cannot generate undefined ASM opcodes"),
         }])
@@ -693,6 +700,7 @@ fn realize_load(
     data_id: &DataId,
     data_section: &mut DataSection,
     offset_to_data_section: u64,
+    offset_from_instr_start: u64,
 ) -> Vec<fuel_asm::Instruction> {
     // if this data is larger than a word, instead of loading the data directly
     // into the register, we want to load a pointer to the data into the register
@@ -723,27 +731,29 @@ fn realize_load(
     };
 
     if !has_copy_type {
-        // load the pointer itself into the register
-        // `offset_to_data_section` is in bytes. We want a byte
-        // address here
-        let pointer_offset_from_instruction_start = offset_to_data_section + offset_bytes;
+        // load the pointer itself into the register. `offset_to_data_section` is in bytes.
+        // The -4 is because $pc is added in the *next* instruction.
+        let pointer_offset_from_current_instr =
+            offset_to_data_section - offset_from_instr_start + offset_bytes - 4;
+
         // insert the pointer as bytes as a new data section entry at the end of the data
-        let data_id_for_pointer =
-            data_section.append_pointer(pointer_offset_from_instruction_start);
+        let data_id_for_pointer = data_section.append_pointer(pointer_offset_from_current_instr);
+
         // now load the pointer we just created into the `dest`ination
         let mut buf = Vec::with_capacity(2);
         buf.append(&mut realize_load(
             dest,
             &data_id_for_pointer,
             data_section,
             offset_to_data_section,
+            offset_from_instr_start,
         ));
-        // add $is to the pointer since it is relative to the data section
+        // add $pc to the pointer since it is relative to the current instruction.
         buf.push(
             fuel_asm::op::ADD::new(
                 dest.to_reg_id(),
                 dest.to_reg_id(),
-                ConstantRegister::InstructionStart.to_reg_id(),
+                ConstantRegister::ProgramCounter.to_reg_id(),
             )
             .into(),
         );

test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle.json

@@ -7,7 +7,7 @@
         "typeArguments": null
       },
       "name": "C0",
-      "offset": 4724
+      "offset": 4688
     },
     {
       "configurableType": {
@@ -16,7 +16,7 @@
         "typeArguments": null
       },
       "name": "C1",
-      "offset": 4740
+      "offset": 4704
     },
     {
       "configurableType": {
@@ -25,7 +25,7 @@
         "typeArguments": null
       },
       "name": "C2",
-      "offset": 4756
+      "offset": 4720
     },
     {
       "configurableType": {
@@ -34,7 +34,7 @@
         "typeArguments": []
       },
       "name": "C3",
-      "offset": 4788
+      "offset": 4752
     },
     {
       "configurableType": {
@@ -43,7 +43,7 @@
         "typeArguments": []
       },
       "name": "C4",
-      "offset": 4804
+      "offset": 4768
     },
     {
       "configurableType": {
@@ -52,7 +52,7 @@
         "typeArguments": []
       },
       "name": "C5",
-      "offset": 4820
+      "offset": 4784
     },
     {
       "configurableType": {
@@ -61,7 +61,7 @@
         "typeArguments": null
       },
       "name": "C6",
-      "offset": 4836
+      "offset": 4800
     },
     {
       "configurableType": {
@@ -70,7 +70,7 @@
         "typeArguments": null
       },
       "name": "C7",
-      "offset": 4852
+      "offset": 4816
     },
     {
       "configurableType": {
@@ -79,7 +79,7 @@
         "typeArguments": null
       },
       "name": "C7_2",
-      "offset": 4956
+      "offset": 4920
     },
     {
       "configurableType": {
@@ -88,7 +88,7 @@
         "typeArguments": null
       },
       "name": "C9",
-      "offset": 4900
+      "offset": 4864
     }
   ],
   "functions": [

test/src/e2e_vm_tests/test_programs/should_pass/unit_tests/workspace_test/contract_multi_test/src/main.sw

@@ -17,15 +17,15 @@ fn test_foo() {
 
 #[test(should_revert)]
 fn test_fail() {
-    let contract_id = 0x9609f25a506fa3a692aee8e86458132e2909d866f4adcad3d60ce7550795c717;
+    let contract_id = 0x22f840cd4af724e7257dd01ce315d907d694e417778b5f16b205a918baee092e;
     let caller = abi(MyContract, contract_id);
     let result = caller.test_function {}();
     assert(result == false)
 }
 
 #[test]
 fn test_success() {
-    let contract_id = 0x9609f25a506fa3a692aee8e86458132e2909d866f4adcad3d60ce7550795c717;
+    let contract_id = 0x22f840cd4af724e7257dd01ce315d907d694e417778b5f16b205a918baee092e;
     let caller = abi(MyContract, contract_id);
     let result = caller.test_function {}();
     assert(result == true)