Fixes to accessing the data section during bytecode gen (#5927)

## Description

This PR fixes a bunch of issues we had in generating access to the data
section. It also fixes #5876.

Reverts #5918.

---------

Co-authored-by: Joshua Batty <joshpbatty@gmail.com>

forc-pkg/src/manifest/build_profile.rs

@@ -24,6 +24,8 @@ pub struct BuildProfile {
     #[serde(default)]
     pub print_intermediate_asm: bool,
     #[serde(default)]
+    pub print_bytecode: bool,
+    #[serde(default)]
     pub terse: bool,
     #[serde(default)]
     pub time_phases: bool,
@@ -57,6 +59,7 @@ impl BuildProfile {
             print_ir: false,
             print_finalized_asm: false,
             print_intermediate_asm: false,
+            print_bytecode: false,
             terse: false,
             time_phases: false,
             metrics_outfile: None,
@@ -80,6 +83,7 @@ impl BuildProfile {
             print_ir: false,
             print_finalized_asm: false,
             print_intermediate_asm: false,
+            print_bytecode: false,
             terse: false,
             time_phases: false,
             metrics_outfile: None,
@@ -133,6 +137,7 @@ mod tests {
             print_ir: true,
             print_finalized_asm: true,
             print_intermediate_asm: true,
+            print_bytecode: true,
             terse: true,
             time_phases: true,
             metrics_outfile: Some("metrics_outfile".into()),

forc-pkg/src/pkg.rs

@@ -266,6 +266,8 @@ pub struct PrintOpts {
     /// This is the state of the ASM prior to performing register allocation and other ASM
     /// optimisations.
     pub intermediate_asm: bool,
+    /// Print the bytecode. This is the final output of the compiler.
+    pub bytecode: bool,
     /// Print the generated Sway IR (Intermediate Representation).
     pub ir: bool,
     /// Output build errors and warnings in reverse order.
@@ -1554,6 +1556,7 @@ pub fn sway_build_config(
     .with_print_dca_graph_url_format(build_profile.print_dca_graph_url_format.clone())
     .with_print_finalized_asm(build_profile.print_finalized_asm)
     .with_print_intermediate_asm(build_profile.print_intermediate_asm)
+    .with_print_bytecode(build_profile.print_bytecode)
     .with_print_ir(build_profile.print_ir)
     .with_include_tests(build_profile.include_tests)
     .with_time_phases(build_profile.time_phases)
@@ -1893,8 +1896,8 @@ pub fn compile(
     let bc_res = time_expr!(
         "compile asm to bytecode",
         "compile_asm_to_bytecode",
-        sway_core::asm_to_bytecode(&handler, asm, source_map, engines.se()),
-        Some(sway_build_config),
+        sway_core::asm_to_bytecode(&handler, asm, source_map, engines.se(), &sway_build_config),
+        Some(sway_build_config.clone()),
         metrics
     );
 
@@ -2079,6 +2082,7 @@ fn build_profile_from_opts(
     }
     profile.print_ir |= print.ir;
     profile.print_finalized_asm |= print.finalized_asm;
+    profile.print_bytecode |= print.bytecode;
     profile.print_intermediate_asm |= print.intermediate_asm;
     profile.terse |= pkg.terse;
     profile.time_phases |= time_phases;

forc-pkg/tests/sections/Forc.toml

@@ -13,6 +13,7 @@ print-dca-graph-url-format = "print_dca_graph_url_format"
 print-ir = true
 print-finalized-asm = true
 print-intermediate-asm = true
+print-bytecode = true
 terse = true
 time-phases = true
 metrics-outfile = "metrics_outfile"

forc-plugins/forc-client/src/op/deploy.rs

@@ -330,6 +330,7 @@ fn build_opts_from_cmd(cmd: &cmd::Deploy) -> pkg::BuildOpts {
             dca_graph_url_format: cmd.print.dca_graph_url_format.clone(),
             finalized_asm: cmd.print.finalized_asm,
             intermediate_asm: cmd.print.intermediate_asm,
+            bytecode: cmd.print.bytecode,
             ir: cmd.print.ir,
             reverse_order: cmd.print.reverse_order,
         },

forc-plugins/forc-client/src/op/run/mod.rs

@@ -222,6 +222,7 @@ fn build_opts_from_cmd(cmd: &cmd::Run) -> pkg::BuildOpts {
             dca_graph_url_format: cmd.print.dca_graph_url_format.clone(),
             finalized_asm: cmd.print.finalized_asm,
             intermediate_asm: cmd.print.intermediate_asm,
+            bytecode: cmd.print.bytecode,
             ir: cmd.print.ir,
             reverse_order: cmd.print.reverse_order,
         },

forc/src/cli/commands/test.rs

@@ -216,6 +216,7 @@ fn opts_from_cmd(cmd: Command) -> forc_test::TestOpts {
             dca_graph_url_format: cmd.build.print.dca_graph_url_format,
             finalized_asm: cmd.build.print.finalized_asm,
             intermediate_asm: cmd.build.print.intermediate_asm,
+            bytecode: cmd.build.print.bytecode,
             ir: cmd.build.print.ir,
             reverse_order: cmd.build.print.reverse_order,
         },

forc/src/cli/shared.rs

@@ -78,6 +78,9 @@ pub struct Print {
     /// optimisations.
     #[clap(long)]
     pub intermediate_asm: bool,
+    /// Print the bytecode. This is the final output of the compiler.
+    #[clap(long)]
+    pub bytecode: bool,
     /// Print the generated Sway IR (Intermediate Representation).
     #[clap(long)]
     pub ir: bool,

forc/src/ops/forc_build.rs

@@ -26,6 +26,7 @@ fn opts_from_cmd(cmd: BuildCommand) -> pkg::BuildOpts {
             dca_graph_url_format: cmd.build.print.dca_graph_url_format,
             finalized_asm: cmd.build.print.finalized_asm,
             intermediate_asm: cmd.build.print.intermediate_asm,
+            bytecode: cmd.build.print.bytecode,
             ir: cmd.build.print.ir,
             reverse_order: cmd.build.print.reverse_order,
         },

forc/src/ops/forc_contract_id.rs

@@ -61,6 +61,7 @@ fn build_opts_from_cmd(cmd: &ContractIdCommand) -> pkg::BuildOpts {
             dca_graph_url_format: cmd.print.dca_graph_url_format.clone(),
             finalized_asm: cmd.print.finalized_asm,
             intermediate_asm: cmd.print.intermediate_asm,
+            bytecode: cmd.print.bytecode,
             ir: cmd.print.ir,
             reverse_order: cmd.print.reverse_order,
         },

forc/src/ops/forc_predicate_root.rs

@@ -30,6 +30,7 @@ fn build_opts_from_cmd(cmd: PredicateRootCommand) -> pkg::BuildOpts {
             dca_graph_url_format: cmd.print.dca_graph_url_format.clone(),
             finalized_asm: cmd.print.finalized_asm,
             intermediate_asm: cmd.print.intermediate_asm,
+            bytecode: cmd.print.bytecode,
             ir: cmd.print.ir,
             reverse_order: cmd.print.reverse_order,
         },

sway-core/src/asm_generation/finalized_asm.rs

@@ -7,6 +7,7 @@ use super::{
 use crate::asm_lang::allocated_ops::{AllocatedOp, AllocatedOpcode};
 use crate::decl_engine::DeclRefFunction;
 use crate::source_map::SourceMap;
+use crate::BuildConfig;
 
 use etk_asm::asm::Assembler;
 use sway_error::error::CompileError;
@@ -54,15 +55,16 @@ impl FinalizedAsm {
         handler: &Handler,
         source_map: &mut SourceMap,
         source_engine: &SourceEngine,
+        build_config: &BuildConfig,
     ) -> 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,
-            ),
+                build_config,
+            )),
             InstructionSet::Evm { ops } => {
                 let mut assembler = Assembler::new();
                 if let Err(e) = assembler.push_all(ops.clone()) {
@@ -98,67 +100,99 @@ 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
+    build_config: &BuildConfig,
+) -> 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);
+
+    if build_config.print_bytecode {
+        println!(";; --- START OF TARGET BYTECODE ---\n");
+    }
 
     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];
+                if build_config.print_bytecode {
+                    println!("{:?}", data);
                 }
+                // 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 build_config.print_bytecode {
+                        println!("{:?}", op);
+                    }
                     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;
                 }
             }
         }
     }
+    if build_config.print_bytecode {
+        println!("{}", data_section);
+        println!(";; --- END OF TARGET BYTECODE ---\n");
+    }
+
+    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 +209,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 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.allocated_ops();
 
         // Collect the entry point offsets.
         let entries = self