From 3b5d2ac34cbaec2768b82ce7eb2e4dabf67fa0cf Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Tue, 12 Sep 2023 15:41:08 -0300 Subject: [PATCH 01/19] Assembler simplification --- Cargo.lock | 5 +- etk-analyze/src/bin/ecfg.rs | 2 +- etk-asm/Cargo.toml | 17 +- etk-asm/src/asm.rs | 867 ++++++++---------- etk-asm/src/ast.rs | 5 +- etk-asm/src/ingest.rs | 255 ++---- etk-asm/src/parse/mod.rs | 1 + etk-asm/tests/asm.rs | 49 + etk-asm/tests/asm/every-op/test.etk | 5 + .../undefined-label-undefined-macro.etk | 6 + etk-asm/tests/asm/variable-push/included.etk | 594 ++++++++++++ etk-asm/tests/asm/variable-push/main.etk | 6 + etk-asm/tests/asm/variable-push2/main1.etk | 3 + etk-asm/tests/asm/variable-push2/main2.etk | 3 + etk-asm/tests/asm/variable-push2/main3.etk | 6 + etk-dasm/src/bin/disease.rs | 5 +- 16 files changed, 1132 insertions(+), 697 deletions(-) create mode 100644 etk-asm/tests/asm/every-op/test.etk create mode 100644 etk-asm/tests/asm/instruction-macro/undefined-label-undefined-macro.etk create mode 100644 etk-asm/tests/asm/variable-push/included.etk create mode 100644 etk-asm/tests/asm/variable-push/main.etk create mode 100644 etk-asm/tests/asm/variable-push2/main1.etk create mode 100644 etk-asm/tests/asm/variable-push2/main2.etk create mode 100644 etk-asm/tests/asm/variable-push2/main3.etk diff --git a/Cargo.lock b/Cargo.lock index d2a9264e..1f8d0a38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -439,6 +439,7 @@ dependencies = [ "hex", "hex-literal", "num-bigint", + "num-traits", "pest", "pest_derive", "rand", @@ -936,9 +937,9 @@ dependencies = [ [[package]] name = "num-traits" -version = "0.2.15" +version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "578ede34cf02f8924ab9447f50c28075b4d3e5b269972345e7e0372b38c6cdcd" +checksum = "39e3200413f237f41ab11ad6d161bc7239c84dcb631773ccd7de3dfe4b5c267c" dependencies = [ "autocfg", ] diff --git a/etk-analyze/src/bin/ecfg.rs b/etk-analyze/src/bin/ecfg.rs index a8fd7a37..7f963c3b 100644 --- a/etk-analyze/src/bin/ecfg.rs +++ b/etk-analyze/src/bin/ecfg.rs @@ -58,7 +58,7 @@ fn run() -> Result<(), Error> { let blocks = separator .take() .into_iter() - .chain(separator.finish().into_iter()) + .chain(separator.finish()) .map(|x| AnnotatedBlock::annotate(&x)); let mut cfg = ControlFlowGraph::new(blocks); diff --git a/etk-asm/Cargo.toml b/etk-asm/Cargo.toml index a479bd3e..e2b4622d 100644 --- a/etk-asm/Cargo.toml +++ b/etk-asm/Cargo.toml @@ -1,7 +1,10 @@ [package] name = "etk-asm" version = "0.4.0-dev" -authors = ["Sam Wilson ", "lightclient "] +authors = [ + "Sam Wilson ", + "lightclient ", +] license = "MIT OR Apache-2.0" edition = "2018" description = "EVM Toolkit assembler" @@ -9,23 +12,29 @@ homepage = "https://quilt.github.io/etk" repository = "https://github.com/quilt/etk" readme = "README.md" keywords = ["etk", "ethereum", "assembler"] -categories = ["cryptography::cryptocurrencies", "command-line-utilities", "development-tools", "compilers"] +categories = [ + "cryptography::cryptocurrencies", + "command-line-utilities", + "development-tools", + "compilers", +] [features] cli = ["clap", "etk-cli"] -backtraces = [ "snafu/backtraces", "etk-ops/backtraces" ] +backtraces = ["snafu/backtraces", "etk-ops/backtraces"] [dependencies] etk-ops = { path = "../etk-ops", version = "0.4.0-dev" } etk-cli = { optional = true, path = "../etk-cli", version = "0.4.0-dev" } hex = "0.4.3" num-bigint = "0.4" +num-traits = "0.2.17" pest = "2.1.3" pest_derive = "2.1" rand = "0.8.5" sha3 = "0.10.1" clap = { optional = true, version = "3.1", features = ["derive"] } -snafu = { version = "0.7.1", default-features = false, features = [ "std" ] } +snafu = { version = "0.7.1", default-features = false, features = ["std"] } [dev-dependencies] assert_matches = "1.5.0" diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index a0555961..195ee683 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -126,45 +126,42 @@ mod error { #[snafu(backtrace)] source: ParseError, }, + + /// An instruction macro was used without being defined. + #[snafu(display("variable `{}` inside macro, was never defined", var))] + #[non_exhaustive] + UndeclaredVariableMacro { + /// The variable that was used without being defined. + var: String, + + /// The location of the error. + backtrace: Backtrace, + }, } } pub use self::error::Error; -use crate::ops::expression::{self, Terminal}; -use crate::ops::{self, AbstractOp, Assemble, Expression, Imm, MacroDefinition}; -use etk_ops::cancun::Op; +use crate::ops::expression::Error::{UndefinedVariable, UnknownLabel, UnknownMacro}; +use crate::ops::{self, AbstractOp, Assemble, Expression, MacroDefinition}; +use num_traits::ToPrimitive; use rand::Rng; -use snafu::OptionExt; -use std::collections::{hash_map, HashMap, HashSet, VecDeque}; +use std::collections::{hash_map, HashMap, HashSet}; -/// An item to be assembled, which can be either an [`AbstractOp`] or a raw byte -/// sequence. +/// An item to be assembled, which can be either an [`AbstractOp`], +/// the inclusion of a new scope or a raw byte sequence. #[derive(Debug, Clone)] pub enum RawOp { /// An instruction to be assembled. Op(AbstractOp), + /// A new scope to be created with its corresponding list of operations. + Scope(Vec), + /// Raw bytes, for example from `%include_hex`, to be included verbatim in /// the output. Raw(Vec), } -impl RawOp { - fn size(&self) -> Option { - match self { - Self::Op(op) => op.size(), - Self::Raw(raw) => Some(raw.len()), - } - } - - fn expr(&self) -> Option<&Expression> { - match self { - Self::Op(op) => op.expr(), - Self::Raw(_) => None, - } - } -} - impl From for RawOp { fn from(op: AbstractOp) -> Self { Self::Op(op) @@ -177,6 +174,12 @@ impl From> for RawOp { } } +impl From<&AbstractOp> for RawOp { + fn from(op: &AbstractOp) -> Self { + Self::Op(op.clone()) + } +} + /// Assembles a series of [`RawOp`] into raw bytes, tracking and resolving macros and labels, /// and handling dynamic pushes. /// @@ -190,52 +193,28 @@ impl From> for RawOp { /// # /// # use hex_literal::hex; /// let mut asm = Assembler::new(); -/// asm.push_all(vec![ -/// AbstractOp::new(GetPc), -/// ])?; -/// let output = asm.take(); -/// asm.finish()?; -/// # assert_eq!(output, hex!("58")); +/// let code = vec![AbstractOp::new(GetPc)]; +/// let result = asm.assemble(&code)?; +/// # assert_eq!(result, hex!("58")); /// # Result::<(), Error>::Ok(()) /// ``` -#[derive(Debug)] +#[derive(Debug, Default)] pub struct Assembler { - /// Assembled ops, ready to be taken. - ready: Vec, + /// Assembled ops. + ready: Vec, - /// Ops that cannot be encoded yet. - pending: VecDeque, - - /// Sum of the size of all the ops in `pending`, or `None` if `pending` contains - /// an unsized op. - pending_len: Option, - - /// Total number of `u8` that have been appended to `ready`. + /// Number of bytes used by the operations in `ready``. concrete_len: usize, - /// Labels, in `pending`, associated with an `AbstractOp::Label`. + /// Labels associated with an `AbstractOp::Label`. declared_labels: HashMap>, - /// Macros, in `pending`, associated with an `AbstractOp::Macro`. + /// Macros associated with an `AbstractOp::Macro`. declared_macros: HashMap, - /// Labels, in `pending`, that have been referred to (ex. with push) but + /// Labels that have been referred to (ex. with push) but /// have not been declared with an `AbstractOp::Label`. - undeclared_labels: HashSet, -} - -impl Default for Assembler { - fn default() -> Self { - Self { - ready: Default::default(), - pending: Default::default(), - pending_len: Some(0), - concrete_len: 0, - declared_labels: Default::default(), - declared_macros: Default::default(), - undeclared_labels: Default::default(), - } - } + undeclared_labels: Vec, } impl Assembler { @@ -244,89 +223,34 @@ impl Assembler { Self::default() } - /// Indicate that the input sequence is complete. Returns any errors that - /// may remain. - pub fn finish(self) -> Result<(), Error> { - if let Some(undef) = self.pending.front() { - return match undef { - RawOp::Op(AbstractOp::Macro(invc)) => error::UndeclaredInstructionMacro { - name: invc.name.clone(), - } - .fail(), - RawOp::Op(op) => { - match op - .clone() - .concretize((&self.declared_labels, &self.declared_macros).into()) - { - Ok(_) => unreachable!(), - Err(ops::Error::ContextIncomplete { - source: expression::Error::UnknownMacro { name, .. }, - .. - }) => error::UndeclaredExpressionMacro { name }.fail(), - Err(ops::Error::ContextIncomplete { - source: expression::Error::UnknownLabel { .. }, - .. - }) => { - let labels = op.expr().unwrap().labels(&self.declared_macros).unwrap(); - let declared = self.declared_labels.into_keys().collect(); - let invoked: HashSet<_> = labels.into_iter().collect(); - let missing = invoked - .difference(&declared) - .cloned() - .collect::>(); - error::UndeclaredLabels { labels: missing }.fail() - } - _ => unreachable!(), - } - } - // bug: if a variable is used when it isn't available, e.g. push1 $size - _ => unreachable!(), - }; - } - - if !self.ready.is_empty() { - panic!("not all assembled bytecode has been taken"); - } - - Ok(()) - } - - /// Collect any assembled instructions that are ready to be output. - pub fn take(&mut self) -> Vec { - std::mem::take(&mut self.ready) - } - /// Feed instructions into the `Assembler`. /// - /// Returns the number of bytes that can be collected with [`Assembler::take`]. - pub fn push_all(&mut self, ops: I) -> Result + /// Returns the code of the assembled program. + pub fn assemble(&mut self, ops: &[O]) -> Result, Error> where - I: IntoIterator, - O: Into, + O: Into + Clone, { + self.declare_macros(ops)?; + for op in ops { - self.push(op)?; + self.push(op.clone().into())?; } - Ok(self.ready.len()) + let output = self.backpatch_and_emit()?; + self.ready.clear(); + Ok(output) } - /// Insert explicilty declared macros and labels, via `AbstractOp`, and implictly declared - /// macros and labels via usage in `Op`. - fn declare_content(&mut self, rop: &RawOp) -> Result<(), Error> { - match rop { - RawOp::Op(AbstractOp::Label(ref label)) => { - match self.declared_labels.entry(label.to_owned()) { - hash_map::Entry::Occupied(_) => { - return error::DuplicateLabel { label }.fail(); - } - hash_map::Entry::Vacant(v) => { - v.insert(None); - self.undeclared_labels.remove(label); - } - } - } - RawOp::Op(AbstractOp::MacroDefinition(ref defn)) => { + /// Pre-define macros, via `AbstractOp`, into the `Assembler`. + /// + /// This is used to define macros that are used in the same scope. + fn declare_macros(&mut self, ops: &[O]) -> Result<(), Error> + where + O: Into + Clone, + { + for op in ops { + let rop = op.clone().into(); + if let RawOp::Op(AbstractOp::MacroDefinition(ref defn)) = rop { match self.declared_macros.entry(defn.name().to_owned()) { hash_map::Entry::Occupied(_) => { return error::DuplicateMacro { name: defn.name() }.fail() @@ -336,75 +260,32 @@ impl Assembler { } } } - _ => (), - }; - - // Get all labels used by `rop`, check if they've been defined, and if not, note them as - // "undeclared". - if let Some(Ok(labels)) = rop.expr().map(|e| e.labels(&self.declared_macros)) { - for label in labels { - if !self.declared_labels.contains_key(&label) { - self.undeclared_labels.insert(label.to_owned()); - } - } } Ok(()) } /// Feed a single instruction into the `Assembler`. - /// - /// Returns the number of bytes that can be collected with [`Assembler::take`] - pub fn push(&mut self, rop: O) -> Result + fn push(&mut self, rop: O) -> Result where O: Into, { let rop = rop.into(); + self.declare_label(&rop)?; - self.declare_content(&rop)?; - - // Expand instruction macros immediately. We do this here because it's the same process - // regardless if we `push_read` or `push_pending` -- in fact, `expand_macro` pushes each op - // individually which calls the correct unchecked push. - if let RawOp::Op(AbstractOp::Macro(ref m)) = rop { - self.expand_macro(&m.name, &m.parameters)?; - return Ok(self.ready.len()); - } - - self.push_unchecked(rop)?; - Ok(self.ready.len()) - } - - fn push_unchecked(&mut self, rop: RawOp) -> Result<(), Error> { - if self.pending.is_empty() && self.pending_len.is_some() { - self.push_ready(rop) - } else { - self.push_pending(rop) - } - } - - fn push_ready(&mut self, rop: RawOp) -> Result<(), Error> { match rop { RawOp::Op(AbstractOp::Label(label)) => { + self.undeclared_labels.retain(|l| *l != label); + let old = self .declared_labels .insert(label, Some(self.concrete_len)) .expect("label should exist"); assert_eq!(old, None, "label should have been undefined"); - Ok(()) } - RawOp::Op(AbstractOp::MacroDefinition(_)) => Ok(()), + RawOp::Op(AbstractOp::MacroDefinition(_)) => {} RawOp::Op(AbstractOp::Macro(ref m)) => { - match self.declared_macros.get(&m.name) { - // Do nothing if the instruction macro has been defined. - Some(MacroDefinition::Instruction(_)) => (), - _ => { - assert_eq!(self.pending_len, Some(0)); - self.pending_len = None; - self.pending.push_back(rop); - } - } - Ok(()) + self.expand_macro(&m.name, &m.parameters)?; } RawOp::Op(ref op) => { match op @@ -413,7 +294,7 @@ impl Assembler { { Ok(cop) => { self.concrete_len += cop.size(); - cop.assemble(&mut self.ready); + self.ready.push(rop.clone()) } Err(ops::Error::ExpressionTooLarge { value, spec, .. }) => { return error::ExpressionTooLarge { @@ -430,202 +311,137 @@ impl Assembler { } .fail() } - Err(_) => { - assert_eq!(self.pending_len, Some(0)); - self.pending_len = rop.size(); - self.pending.push_back(rop); + Err(ops::Error::ContextIncomplete { + source: UnknownLabel { label, .. }, + }) => { + if let AbstractOp::Push(_) = op { + self.concrete_len += 2; + } else { + self.concrete_len += op.size().unwrap(); + } + + self.undeclared_labels.push(label); + self.ready.push(rop.clone()); } + Err(ops::Error::ContextIncomplete { + source: UnknownMacro { name, .. }, + }) => return error::UndeclaredInstructionMacro { name }.fail(), + Err(ops::Error::ContextIncomplete { + source: UndefinedVariable { name, .. }, + }) => return error::UndeclaredVariableMacro { var: name }.fail(), } - - Ok(()) } RawOp::Raw(raw) => { self.concrete_len += raw.len(); - self.ready.extend(raw); - Ok(()) + self.ready.push(RawOp::Raw(raw.to_vec())); + } + RawOp::Scope(scope) => { + let mut asm = Self::new(); + let scope_result = asm.assemble(&scope)?; + self.concrete_len += scope_result.len(); + self.ready.push(RawOp::Raw(scope_result)); } } + + Ok(self.concrete_len) } - fn push_pending(&mut self, rop: RawOp) -> Result<(), Error> { - // Update total size of pending ops. - if let Some(ref mut pending_len) = self.pending_len { - match rop.size() { - Some(size) => *pending_len += size, - None => self.pending_len = None, + /// Backpatch dynamic operations and emit the assembled program. + /// + /// Errors if there are any undeclared labels. + fn backpatch_and_emit(&mut self) -> Result, Error> { + if !self.undeclared_labels.is_empty() { + return error::UndeclaredLabels { + labels: self + .undeclared_labels + .iter() + .map(|l| l.to_owned()) + .collect::>(), } + .fail(); } + let mut output = Vec::new(); + for op in self.ready.iter() { + if let RawOp::Op(ref op) = op { + match op + .clone() + .concretize((&self.declared_labels, &self.declared_macros).into()) + { + Ok(mut cop) => { + if let AbstractOp::Push(imm) = op { + let exp = imm.tree.eval_with_context( + (&self.declared_labels, &self.declared_macros).into(), + ); + + let labels: HashSet = imm + .tree + .labels(&self.declared_macros) + .unwrap() + .into_iter() + .collect(); + if !labels.is_empty() { + if let Ok(val) = exp { + let extra = val.to_usize().unwrap() / 256; + if extra > 0 { + self.concrete_len += extra; + + for label in labels { + if let Some(position) = + self.declared_labels.get_mut(&label) + { + *position = position.map(|v| v + 1); + } + } + } + } + } - // Handle new label and macro definitions. - match (self.pending_len, rop) { - (Some(pending_len), RawOp::Op(AbstractOp::Label(lbl))) => { - // The label has a defined address. - let address = self.concrete_len + pending_len; - let item = self.declared_labels.get_mut(&*lbl).unwrap(); - *item = Some(address); - } - (None, rop @ RawOp::Op(AbstractOp::Label(_))) => { - self.pending.push_back(rop); - if self.undeclared_labels.is_empty() { - self.choose_sizes()?; - } - } - (_, RawOp::Op(AbstractOp::MacroDefinition(defn))) => { - if let Some(RawOp::Op(AbstractOp::Macro(invc))) = self.pending.front() { - if defn.name() == &invc.name { - let invc = invc.clone(); - self.pending.pop_front(); - self.expand_macro(&invc.name, &invc.parameters)?; + cop = op + .clone() + .concretize((&self.declared_labels, &self.declared_macros).into()) + .unwrap(); + } + cop.assemble(&mut output) } - } - } - (_, rop) => { - // Not a label. - self.pending.push_back(rop); - } - } - - // Repeatedly check if the front of the pending list is ready. - while let Some(next) = self.pending.front() { - let op = match next { - RawOp::Op(AbstractOp::Push(Imm { - tree: Expression::Terminal(Terminal::Label(_)), - .. - })) => { - if self.undeclared_labels.is_empty() { - unreachable!() - } else { - // Still waiting on more labels. - break; + Err(ops::Error::ContextIncomplete { + source: UnknownLabel { .. }, + }) => { + return error::UndeclaredLabels { + labels: self.undeclared_labels.to_vec(), + } + .fail(); } - } - RawOp::Op(AbstractOp::Label(_)) => unreachable!(), - RawOp::Op(AbstractOp::Macro(_)) => { - // Still waiting on more macros. - break; - } - RawOp::Op(op) => op, - RawOp::Raw(_) => { - self.pop_pending()?; - continue; - } - }; - - match op - .clone() - .concretize((&self.declared_labels, &self.declared_macros).into()) - { - Ok(cop) => { - let front = self.pending.front_mut().unwrap(); - *front = RawOp::Op(cop.into()); - } - Err(ops::Error::ExpressionTooLarge { value, spec, .. }) => { - return error::ExpressionTooLarge { - expr: op.expr().unwrap().clone(), - value, - spec, + Err(ops::Error::ContextIncomplete { + source: UnknownMacro { name, .. }, + }) => { + return error::UndeclaredInstructionMacro { name }.fail(); + } + Err(ops::Error::ContextIncomplete { + source: UndefinedVariable { name, .. }, + }) => { + return error::UndeclaredVariableMacro { var: name }.fail(); } - .fail(); - } - Err(_) => { - // Still waiting for some definition. - break; - } - } - - self.pop_pending()?; - } - - Ok(()) - } - - fn pop_pending(&mut self) -> Result<(), Error> { - let popped = self.pending.pop_front().unwrap(); - - let size; - match popped { - RawOp::Raw(raw) => { - size = raw.len(); - self.ready.extend(raw); - } - RawOp::Op(aop) => { - let cop = aop - .concretize((&self.declared_labels, &self.declared_macros).into()) - // Already able to concretize in `push_pending` loop. - .unwrap(); - size = cop.size(); - cop.assemble(&mut self.ready); + Err(_) => unreachable!("all ops should be concretizable"), + } + } else if let RawOp::Raw(raw) = op { + output.extend(raw); } } - self.concrete_len += size; - - if self.pending.is_empty() { - self.pending_len = Some(0); - } else if let Some(ref mut pending_len) = self.pending_len { - *pending_len -= size; - } - - Ok(()) + Ok(output) } - fn choose_sizes(&mut self) -> Result<(), Error> { - let mut sizes: HashMap> = self - .pending - .iter() - .filter(|op| matches!(op, RawOp::Op(AbstractOp::Push(_)))) - .map(|op| (op.expr().unwrap().clone(), Op::<()>::push_for(1).unwrap())) - .collect(); - - let mut subasm; - - loop { - // Create a sub-assembler to try assembling with the sizes in - // `undefined_labels`. - subasm = Self::default(); - subasm.concrete_len = self.concrete_len; - subasm.declared_labels = self.declared_labels.clone(); - - let result: Result, Error> = self - .pending - .iter() - .map(|op| { - let new = match op { - RawOp::Op(AbstractOp::Push(Imm { tree, .. })) => { - let new = sizes[tree].with(tree.clone()).unwrap(); - let aop = AbstractOp::new(new); - RawOp::Op(aop) - } - op => op.clone(), - }; - - subasm.push_pending(new) - }) - .collect(); - - match result { - Ok(_) => { - assert!(subasm.pending.is_empty()); - break; + fn declare_label(&mut self, rop: &RawOp) -> Result<(), Error> { + if let RawOp::Op(AbstractOp::Label(label)) = rop { + if self.declared_labels.contains_key(label) { + return error::DuplicateLabel { + label: label.to_owned(), } - Err(Error::ExpressionTooLarge { expr, .. }) => { - // If an expression is too large for an op, increase the width of that op. - let item = sizes.get_mut(&expr).unwrap(); - let new_size = item.upsize().context(error::UnsizedPushTooLarge)?; - *item = new_size; - } - Err(e) => return Err(e), + .fail(); } + self.declared_labels.insert(label.to_owned(), None); } - - // Insert the results of the sub-assembler into self. - let raw = subasm.take(); - self.pending_len = Some(raw.len()); - self.pending.clear(); - self.pending.push_back(RawOp::Raw(raw)); - self.declared_labels = subasm.declared_labels; - Ok(()) } @@ -686,19 +502,12 @@ impl Assembler { } } - Ok(Some(self.push_all(m.contents)?)) - } - _ => { - assert_eq!(self.pending_len, Some(0)); - self.pending_len = None; - self.pending.push_back(RawOp::Op(AbstractOp::Macro( - ops::InstructionMacroInvocation { - name: name.to_string(), - parameters: parameters.to_vec(), - }, - ))); - Ok(None) + for op in m.contents.iter() { + self.push(op)?; + } + Ok(Some(self.concrete_len)) } + _ => error::UndeclaredInstructionMacro { name }.fail(), } } } @@ -718,20 +527,20 @@ mod tests { #[test] fn assemble_variable_push_const_while_pending() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![ + let code = vec![ AbstractOp::Op(Push1(Imm::with_label("label1")).into()), AbstractOp::Push(Terminal::Number(0xaabb.into()).into()), AbstractOp::Label("label1".into()), - ])?; - assert_eq!(5, sz); - assert_eq!(asm.take(), hex!("600561aabb")); + ]; + let result = asm.assemble(&code)?; + assert_eq!(result, hex!("600561aabb")); Ok(()) } #[test] fn assemble_variable_pushes_abab() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![ + let code = vec![ AbstractOp::new(JumpDest), AbstractOp::Push(Imm::with_label("label1")), AbstractOp::Push(Imm::with_label("label2")), @@ -739,16 +548,16 @@ mod tests { AbstractOp::new(GetPc), AbstractOp::Label("label2".into()), AbstractOp::new(GetPc), - ])?; - assert_eq!(7, sz); - assert_eq!(asm.take(), hex!("5b600560065858")); + ]; + let result = asm.assemble(&code)?; + assert_eq!(result, hex!("5b600560065858")); Ok(()) } #[test] fn assemble_variable_pushes_abba() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![ + let code = vec![ AbstractOp::new(JumpDest), AbstractOp::Push(Imm::with_label("label1")), AbstractOp::Push(Imm::with_label("label2")), @@ -756,34 +565,34 @@ mod tests { AbstractOp::new(GetPc), AbstractOp::Label("label1".into()), AbstractOp::new(GetPc), - ])?; - assert_eq!(7, sz); - assert_eq!(asm.take(), hex!("5b600660055858")); + ]; + let result = asm.assemble(&code)?; + assert_eq!(result, hex!("5b600660055858")); Ok(()) } #[test] fn assemble_variable_push1_multiple() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![ + let code = vec![ AbstractOp::new(JumpDest), AbstractOp::Push(Imm::with_label("auto")), AbstractOp::Push(Imm::with_label("auto")), AbstractOp::Label("auto".into()), - ])?; - assert_eq!(5, sz); - assert_eq!(asm.take(), hex!("5b60056005")); + ]; + let result = asm.assemble(&code)?; + assert_eq!(result, hex!("5b60056005")); Ok(()) } #[test] fn assemble_variable_push_const() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![AbstractOp::Push( + let code = vec![AbstractOp::Push( Terminal::Number((0x00aaaaaaaaaaaaaaaaaaaaaaaa as u128).into()).into(), - )])?; - assert_eq!(13, sz); - assert_eq!(asm.take(), hex!("6baaaaaaaaaaaaaaaaaaaaaaaa")); + )]; + let result = asm.assemble(&code)?; + assert_eq!(result, hex!("6baaaaaaaaaaaaaaaaaaaaaaaa")); Ok(()) } @@ -792,9 +601,8 @@ mod tests { let v = BigInt::from_bytes_be(Sign::Plus, &[1u8; 33]); let mut asm = Assembler::new(); - let err = asm - .push_all(vec![AbstractOp::Push(Terminal::Number(v).into())]) - .unwrap_err(); + let code = vec![AbstractOp::Push(Terminal::Number(v).into())]; + let err = asm.assemble(&code).unwrap_err(); assert_matches!(err, Error::ExpressionTooLarge { .. }); } @@ -802,9 +610,8 @@ mod tests { #[test] fn assemble_variable_push_negative() { let mut asm = Assembler::new(); - let err = asm - .push_all(vec![AbstractOp::Push(Terminal::Number((-1).into()).into())]) - .unwrap_err(); + let code = vec![AbstractOp::Push(Terminal::Number((-1).into()).into())]; + let err = asm.assemble(&code).unwrap_err(); assert_matches!(err, Error::ExpressionNegative { .. }); } @@ -812,73 +619,72 @@ mod tests { #[test] fn assemble_variable_push_const0() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![AbstractOp::Push( + let code = vec![AbstractOp::Push( Terminal::Number((0x00 as u128).into()).into(), - )])?; - assert_eq!(2, sz); - assert_eq!(asm.take(), hex!("6000")); + )]; + let result = asm.assemble(&code)?; + assert_eq!(result, hex!("6000")); Ok(()) } #[test] fn assemble_variable_push1_known() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![ + let code = vec![ AbstractOp::new(JumpDest), AbstractOp::Label("auto".into()), AbstractOp::Push(Imm::with_label("auto")), - ])?; - assert_eq!(3, sz); - assert_eq!(asm.take(), hex!("5b6001")); + ]; + let result = asm.assemble(&code)?; + assert_eq!(result, hex!("5b6001")); Ok(()) } #[test] fn assemble_variable_push1() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![ + let code = vec![ AbstractOp::Push(Imm::with_label("auto")), AbstractOp::Label("auto".into()), AbstractOp::new(JumpDest), - ])?; - assert_eq!(3, sz); - assert_eq!(asm.take(), hex!("60025b")); + ]; + let result = asm.assemble(&code)?; + assert_eq!(result, hex!("60025b")); Ok(()) } #[test] fn assemble_variable_push1_reuse() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![ + let code = vec![ AbstractOp::Push(Imm::with_label("auto")), AbstractOp::Label("auto".into()), AbstractOp::new(JumpDest), AbstractOp::new(Push1(Imm::with_label("auto"))), - ])?; - assert_eq!(5, sz); - assert_eq!(asm.take(), hex!("60025b6002")); + ]; + let result = asm.assemble(&code)?; + assert_eq!(result, hex!("60025b6002")); Ok(()) } #[test] fn assemble_variable_push2() -> Result<(), Error> { - let mut asm = Assembler::new(); - asm.push(AbstractOp::Push(Imm::with_label("auto")))?; + let mut code = vec![]; + code.push(AbstractOp::Push(Imm::with_label("auto"))); for _ in 0..255 { - asm.push(AbstractOp::new(GetPc))?; + code.push(AbstractOp::new(GetPc)); } - asm.push_all(vec![ - AbstractOp::Label("auto".into()), - AbstractOp::new(JumpDest), - ])?; + code.push(AbstractOp::Label("auto".into())); + code.push(AbstractOp::new(JumpDest)); + + let mut asm = Assembler::new(); + let result = asm.assemble(&code)?; let mut expected = vec![0x61, 0x01, 0x02]; expected.extend_from_slice(&[0x58; 255]); expected.push(0x5b); - assert_eq!(asm.take(), expected); - - asm.finish()?; + assert_eq!(result, expected); Ok(()) } @@ -886,8 +692,8 @@ mod tests { #[test] fn assemble_undeclared_label() -> Result<(), Error> { let mut asm = Assembler::new(); - asm.push_all(vec![AbstractOp::new(Push1(Imm::with_label("hi")))])?; - let err = asm.finish().unwrap_err(); + let code = vec![AbstractOp::new(Push1(Imm::with_label("hi")))]; + let err = asm.assemble(&code).unwrap_err(); assert_matches!(err, Error::UndeclaredLabels { labels, .. } if labels == vec!["hi"]); Ok(()) } @@ -895,10 +701,10 @@ mod tests { #[test] fn assemble_jumpdest_no_label() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![AbstractOp::new(JumpDest)])?; - assert_eq!(1, sz); + let code = vec![AbstractOp::new(JumpDest)]; + let result = asm.assemble(&code)?; assert!(asm.declared_labels.is_empty()); - assert_eq!(asm.take(), hex!("5b")); + assert_eq!(result, hex!("5b")); Ok(()) } @@ -907,11 +713,10 @@ mod tests { let mut asm = Assembler::new(); let ops = vec![AbstractOp::Label("lbl".into()), AbstractOp::new(JumpDest)]; - let sz = asm.push_all(ops)?; - assert_eq!(1, sz); + let result = asm.assemble(&ops)?; assert_eq!(asm.declared_labels.len(), 1); assert_eq!(asm.declared_labels.get("lbl"), Some(&Some(0))); - assert_eq!(asm.take(), hex!("5b")); + assert_eq!(result, hex!("5b")); Ok(()) } @@ -924,9 +729,8 @@ mod tests { ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 3); - assert_eq!(asm.take(), hex!("5b6000")); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("5b6000")); Ok(()) } @@ -940,9 +744,8 @@ mod tests { ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 3); - assert_eq!(asm.take(), hex!("600258")); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("600258")); Ok(()) } @@ -956,9 +759,8 @@ mod tests { ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 3); - assert_eq!(asm.take(), hex!("60025b")); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("60025b")); Ok(()) } @@ -972,7 +774,7 @@ mod tests { ops.push(AbstractOp::new(JumpDest)); ops.push(AbstractOp::new(Push1(Imm::with_label("a")))); let mut asm = Assembler::new(); - let err = asm.push_all(ops).unwrap_err(); + let err = asm.assemble(&ops).unwrap_err(); assert_matches!(err, Error::ExpressionTooLarge { expr: Expression::Terminal(Terminal::Label(label)), .. } if label == "a"); } @@ -985,11 +787,7 @@ mod tests { ops.push(AbstractOp::new(JumpDest)); ops.push(AbstractOp::new(Push1(Imm::with_label("b")))); let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 259); - - let assembled = asm.take(); - asm.finish()?; + let result = asm.assemble(&ops)?; let mut expected = vec![0x58; 255]; expected.push(0x5b); @@ -997,7 +795,7 @@ mod tests { expected.push(0x60); expected.push(0xff); - assert_eq!(assembled, expected); + assert_eq!(result, expected); Ok(()) } @@ -1028,10 +826,8 @@ mod tests { ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 0); - let out = asm.take(); - assert_eq!(out, []); + let result = asm.assemble(&ops)?; + assert_eq!(result, []); Ok(()) } @@ -1064,10 +860,8 @@ mod tests { ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 13); - let out = asm.take(); - assert_eq!(out, hex!("5b60005b600360005b60086000")); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("5b60005b600360005b60086000")); Ok(()) } @@ -1096,10 +890,8 @@ mod tests { ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 8); - let out = asm.take(); - assert_eq!(out, hex!("5b60005b60036000")); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("5b60005b60036000")); Ok(()) } @@ -1128,10 +920,8 @@ mod tests { ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 8); - let out = asm.take(); - assert_eq!(out, hex!("5b60005b60036000")); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("5b60005b60036000")); Ok(()) } @@ -1160,9 +950,8 @@ mod tests { ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(7, sz); - assert_eq!(asm.take(), hex!("5b600560065858")); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("5b600560065858")); Ok(()) } @@ -1173,8 +962,7 @@ mod tests { InstructionMacroInvocation::with_zero_parameters("my_macro".into()), )]; let mut asm = Assembler::new(); - asm.push_all(ops)?; - let err = asm.finish().unwrap_err(); + let err = asm.assemble(&ops).unwrap_err(); assert_matches!(err, Error::UndeclaredInstructionMacro { name, .. } if name == "my_macro"); Ok(()) @@ -1197,7 +985,7 @@ mod tests { .into(), ]; let mut asm = Assembler::new(); - let err = asm.push_all(ops).unwrap_err(); + let err = asm.assemble(&ops).unwrap_err(); assert_matches!(err, Error::DuplicateMacro { name, .. } if name == "my_macro"); Ok(()) @@ -1217,7 +1005,7 @@ mod tests { )), ]; let mut asm = Assembler::new(); - let err = asm.push_all(ops).unwrap_err(); + let err = asm.assemble(&ops).unwrap_err(); assert_matches!(err, Error::DuplicateLabel { label, .. } if label == "a"); Ok(()) @@ -1244,13 +1032,9 @@ mod tests { AbstractOp::new(Push1(Imm::with_label("a"))), ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 5); - - let out = asm.take(); - asm.finish()?; + let result = asm.assemble(&ops)?; - assert_eq!(out, hex!("3360016000")); + assert_eq!(result, hex!("3360016000")); Ok(()) } @@ -1280,10 +1064,8 @@ mod tests { ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 7); - let out = asm.take(); - assert_eq!(out, hex!("5b600060426000")); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("5b600060426000")); Ok(()) } @@ -1295,10 +1077,8 @@ mod tests { )))]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 2); - let out = asm.take(); - assert_eq!(out, hex!("6002")); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("6002")); Ok(()) } @@ -1309,7 +1089,7 @@ mod tests { BigInt::from(-1).into(), )))]; let mut asm = Assembler::new(); - let err = asm.push_all(ops).unwrap_err(); + let err = asm.assemble(&ops).unwrap_err(); assert_matches!(err, Error::ExpressionNegative { value, .. } if value == BigInt::from(-1)); Ok(()) @@ -1318,10 +1098,10 @@ mod tests { #[test] fn assemble_expression_undeclared_label() -> Result<(), Error> { let mut asm = Assembler::new(); - asm.push_all(vec![AbstractOp::new(Push1(Imm::with_expression( + let ops = vec![AbstractOp::new(Push1(Imm::with_expression( Terminal::Label(String::from("hi")).into(), - )))])?; - let err = asm.finish().unwrap_err(); + )))]; + let err = asm.assemble(&ops).unwrap_err(); assert_matches!(err, Error::UndeclaredLabels { labels, .. } if labels == vec!["hi"]); Ok(()) } @@ -1329,39 +1109,40 @@ mod tests { #[test] fn assemble_variable_push_expression_with_undeclared_labels() -> Result<(), Error> { let mut asm = Assembler::new(); - asm.push_all(vec![ + let ops = vec![ AbstractOp::new(JumpDest), AbstractOp::Push(Imm::with_expression(Expression::Plus( Terminal::Label("foo".into()).into(), Terminal::Label("bar".into()).into(), ))), AbstractOp::new(Gas), - ])?; - let err = asm.finish().unwrap_err(); - assert_matches!(err, Error::UndeclaredLabels { labels, .. } if (labels.contains(&"foo".to_string())) && labels.contains(&"bar".to_string())); + ]; + let err = asm.assemble(&ops).unwrap_err(); + // The expressions have short-circuit evaluation, so only the first label is caught in the error. + assert_matches!(err, Error::UndeclaredLabels { labels, .. } if (labels.contains(&"foo".to_string()))); Ok(()) } #[test] fn assemble_variable_push1_expression() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![ + let ops = vec![ AbstractOp::new(JumpDest), AbstractOp::Label("auto".into()), AbstractOp::Push(Imm::with_expression(Expression::Plus( 1.into(), Terminal::Label(String::from("auto")).into(), ))), - ])?; - assert_eq!(3, sz); - assert_eq!(asm.take(), hex!("5b6002")); + ]; + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("5b6002")); Ok(()) } #[test] fn assemble_expression_with_labels() -> Result<(), Error> { let mut asm = Assembler::new(); - let sz = asm.push_all(vec![ + let ops = vec![ AbstractOp::new(JumpDest), AbstractOp::Push(Imm::with_expression(Expression::Plus( Terminal::Label(String::from("foo")).into(), @@ -1370,9 +1151,9 @@ mod tests { AbstractOp::new(Gas), AbstractOp::Label("foo".into()), AbstractOp::Label("bar".into()), - ])?; - assert_eq!(4, sz); - assert_eq!(asm.take(), hex!("5b60085a")); + ]; + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("5b60085a")); Ok(()) } @@ -1392,10 +1173,98 @@ mod tests { ]; let mut asm = Assembler::new(); - let sz = asm.push_all(ops)?; - assert_eq!(sz, 2); - let out = asm.take(); - assert_eq!(out, hex!("6002")); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("6002")); + + Ok(()) + } + + #[test] + fn assemble_instruction_macro_with_undeclared_variables() { + let ops = vec![ + InstructionMacroDefinition { + name: "my_macro".into(), + parameters: vec!["foo".into()], + contents: vec![AbstractOp::new(Push1(Imm::with_variable("bar")))], + } + .into(), + AbstractOp::Label("b".into()), + AbstractOp::new(JumpDest), + AbstractOp::new(Push1(Imm::with_label("b"))), + AbstractOp::Macro(InstructionMacroInvocation { + name: "my_macro".into(), + parameters: vec![BigInt::from_bytes_be(Sign::Plus, &vec![0x42]).into()], + }), + ]; + + let mut asm = Assembler::new(); + let err = asm.assemble(&ops).unwrap_err(); + + assert_matches!(err, Error::UndeclaredVariableMacro { var, .. } if var == "bar"); + } + + #[test] + fn assemble_instruction_macro_two_delayed_definitions_mirrored() -> Result<(), Error> { + let ops = vec![ + AbstractOp::new(GetPc), + AbstractOp::Macro(InstructionMacroInvocation { + name: "macro1".into(), + parameters: vec![], + }), + AbstractOp::Macro(InstructionMacroInvocation { + name: "macro0".into(), + parameters: vec![], + }), + InstructionMacroDefinition { + name: "macro0".into(), + parameters: vec![], + contents: vec![AbstractOp::new(JumpDest)], + } + .into(), + InstructionMacroDefinition { + name: "macro1".into(), + parameters: vec![], + contents: vec![AbstractOp::new(Caller)], + } + .into(), + ]; + + let mut asm = Assembler::new(); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("58335b")); + + Ok(()) + } + + #[test] + fn assemble_instruction_macro_two_delayed_definitions() -> Result<(), Error> { + let ops = vec![ + AbstractOp::new(GetPc), + AbstractOp::Macro(InstructionMacroInvocation { + name: "macro0".into(), + parameters: vec![], + }), + AbstractOp::Macro(InstructionMacroInvocation { + name: "macro1".into(), + parameters: vec![], + }), + InstructionMacroDefinition { + name: "macro0".into(), + parameters: vec![], + contents: vec![AbstractOp::new(JumpDest)], + } + .into(), + InstructionMacroDefinition { + name: "macro1".into(), + parameters: vec![], + contents: vec![AbstractOp::new(Caller)], + } + .into(), + ]; + + let mut asm = Assembler::new(); + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("585b33")); Ok(()) } diff --git a/etk-asm/src/ast.rs b/etk-asm/src/ast.rs index a4d2e15d..e3824c15 100644 --- a/etk-asm/src/ast.rs +++ b/etk-asm/src/ast.rs @@ -1,16 +1,15 @@ +use std::path::PathBuf; + use crate::ops::{Abstract, AbstractOp, ExpressionMacroDefinition, InstructionMacroDefinition}; use etk_ops::cancun::Op; -use std::path::PathBuf; #[derive(Debug, Clone, PartialEq)] pub(crate) enum Node { Op(AbstractOp), - Raw(Vec), Import(PathBuf), Include(PathBuf), IncludeHex(PathBuf), } - impl From> for Node { fn from(op: Op) -> Self { Node::Op(AbstractOp::Op(op)) diff --git a/etk-asm/src/ingest.rs b/etk-asm/src/ingest.rs index 05227609..0cef990d 100644 --- a/etk-asm/src/ingest.rs +++ b/etk-asm/src/ingest.rs @@ -51,13 +51,15 @@ mod error { }, /// An error that occurred while parsing a file. - #[snafu(context(false))] #[non_exhaustive] - #[snafu(display("parsing failed"))] + #[snafu(display("parsing failed on path `{}`", path.to_string_lossy()))] Parse { /// The underlying source of this error. #[snafu(backtrace)] source: ParseError, + + /// The location of the error. + path: PathBuf, }, /// An error that occurred while assembling a file. @@ -106,40 +108,7 @@ use std::fs::{read_to_string, File}; use std::io::{self, Read, Write}; use std::path::{Path, PathBuf}; -fn parse_file>(path: P) -> Result, Error> { - let asm = read_to_string(path.as_ref()).with_context(|_| error::Io { - message: "reading file before parsing", - path: path.as_ref().to_owned(), - })?; - let nodes = parse_asm(&asm)?; - - Ok(nodes) -} - -#[derive(Debug)] -enum Scope { - Same, - Independent(Box), -} - -impl Scope { - fn same() -> Self { - Self::Same - } - - fn independent() -> Self { - Self::Independent(Box::new(Assembler::new())) - } -} - -#[derive(Debug)] -struct Source { - path: PathBuf, - nodes: std::vec::IntoIter, - scope: Scope, -} - -#[derive(Debug)] +#[derive(Debug, Clone)] struct Root { original: PathBuf, canonicalized: PathBuf, @@ -211,135 +180,44 @@ impl Root { } } -#[must_use] -struct PartialSource<'a, W> { - stack: &'a mut SourceStack, - path: PathBuf, - scope: Scope, -} - -impl<'a, W> PartialSource<'a, W> { - fn path(&self) -> &Path { - &self.path - } - - fn push(self, nodes: Vec) -> &'a mut Source { - self.stack.sources.push(Source { - path: self.path, - nodes: nodes.into_iter(), - scope: self.scope, - }); - - self.stack.sources.last_mut().unwrap() - } -} - #[derive(Debug)] -struct SourceStack { - output: W, - sources: Vec, +struct Program { root: Option, + sources: Vec, } -impl SourceStack { - fn new(output: W) -> Self { +impl Program { + fn new(path: PathBuf) -> Self { Self { - output, - sources: Default::default(), - root: Default::default(), + root: Root::new(path.clone()).ok(), + sources: vec![path], } } - fn resolve(&mut self, path: PathBuf, scope: Scope) -> Result, Error> { + fn push_path(&mut self, path: &PathBuf) -> Result { ensure!(self.sources.len() <= 255, error::RecursionLimit); let path = if let Some(ref root) = self.root { let last = self.sources.last().unwrap(); - let dir = match last.path.parent() { + let dir = match last.parent() { Some(s) => s, None => Path::new("./"), }; let candidate = dir.join(path); root.check(&candidate)?; + self.sources.push(candidate.clone()); candidate } else { assert!(self.sources.is_empty()); - self.root = Some(Root::new(path.clone())?); - path + self.root = Some(Root::new(path.to_owned())?); + path.clone() }; - Ok(PartialSource { - stack: self, - path, - scope, - }) + Ok(path) } - fn peek(&mut self) -> Option<&mut Source> { - self.sources.last_mut() - } -} - -impl SourceStack -where - W: Write, -{ - fn pop(&mut self) -> Result<(), Error> { - let popped = self.sources.pop().unwrap(); - - if self.sources.is_empty() { - self.root = None; - } - - let mut asm = match popped.scope { - Scope::Independent(a) => a, - Scope::Same => return Ok(()), - }; - - let raw = asm.take(); - asm.finish()?; - - if raw.is_empty() { - return Ok(()); - } - - if self.sources.is_empty() { - self.output.write_all(&raw).context(error::Io { - message: "writing output", - path: None, - })?; - Ok(()) - } else { - self.write(RawOp::Raw(raw)) - } - } - - fn write(&mut self, mut op: RawOp) -> Result<(), Error> { - if self.sources.is_empty() { - panic!("no sources!"); - } - - for frame in self.sources[1..].iter_mut().rev() { - let asm = match frame.scope { - Scope::Same => continue, - Scope::Independent(ref mut a) => a, - }; - - if 0 == asm.push(op)? { - return Ok(()); - } else { - op = RawOp::Raw(asm.take()); - } - } - - let first_asm = match self.sources[0].scope { - Scope::Independent(ref mut a) => a, - Scope::Same => panic!("sources[0] must be independent"), - }; - - first_asm.push(op)?; - - Ok(()) + fn pop_path(&mut self) { + self.sources.pop(); } } @@ -370,15 +248,13 @@ where /// ``` #[derive(Debug)] pub struct Ingest { - sources: SourceStack, + output: W, } impl Ingest { /// Make a new `Ingest` that writes assembled bytes to `output`. pub fn new(output: W) -> Self { - Self { - sources: SourceStack::new(output), - } + Self { output } } } @@ -403,7 +279,8 @@ where path: path.clone(), })?; - self.ingest(path, &text) + self.ingest(path, &text)?; + Ok(()) } /// Assemble instructions from `src` as if they were read from a file located @@ -412,61 +289,70 @@ where where P: Into, { - let nodes = parse_asm(src)?; - let partial = self.sources.resolve(path.into(), Scope::independent())?; - partial.push(nodes); - - while let Some(source) = self.sources.peek() { - let node = match source.nodes.next() { - Some(n) => n, - None => { - self.sources.pop()?; - continue; - } - }; + let mut program = Program::new(path.into()); + let nodes = self.preprocess(&mut program, src)?; + let mut asm = Assembler::new(); + let raw = asm.assemble(&nodes)?; + + self.output.write_all(&raw).context(error::Io { + message: "writing output", + path: None, + })?; + + Ok(()) + } + fn preprocess(&mut self, program: &mut Program, src: &str) -> Result, Error> { + let nodes = parse_asm(src).with_context(|_| error::Parse { + path: program.sources.last().unwrap().clone(), + })?; + let mut raws = Vec::new(); + for node in nodes { match node { Node::Op(op) => { - self.sources.write(RawOp::Op(op))?; - } - Node::Raw(raw) => { - self.sources.write(RawOp::Raw(raw))?; + raws.push(RawOp::Op(op)); } - Node::Import(path) => { - let partial = self.sources.resolve(path, Scope::same())?; - let parsed = parse_file(partial.path())?; - partial.push(parsed); + Node::Import(imp_path) => { + let new_raws = self.resolve_and_ingest(program, imp_path)?; + raws.extend(new_raws); } - Node::Include(path) => { - let partial = self.sources.resolve(path, Scope::independent())?; - let parsed = parse_file(partial.path())?; - partial.push(parsed); + Node::Include(inc_path) => { + let inc_raws = self.resolve_and_ingest(program, inc_path)?; + raws.push(RawOp::Scope(inc_raws)); } - Node::IncludeHex(path) => { - let partial = self.sources.resolve(path, Scope::same())?; - - let file = - std::fs::read_to_string(partial.path()).with_context(|_| error::Io { - message: "reading hex include", - path: partial.path().to_owned(), - })?; + Node::IncludeHex(hex_path) => { + let file = std::fs::read_to_string(&hex_path).with_context(|_| error::Io { + message: "reading hex include", + path: hex_path.to_owned(), + })?; let raw = hex::decode(file.trim()) .map_err(|e| Box::new(e) as Box) .context(error::InvalidHex { - path: partial.path().to_owned(), + path: hex_path.to_owned(), })?; - partial.push(vec![Node::Raw(raw)]); + raws.push(RawOp::Raw(raw)) } } } - if !self.sources.sources.is_empty() { - panic!("extra sources?"); - } + Ok(raws) + } - Ok(()) + fn resolve_and_ingest( + &mut self, + program: &mut Program, + path: PathBuf, + ) -> Result, Error> { + let source = program.push_path(&path)?; + let code = read_to_string(source).with_context(|_| error::Io { + message: "reading file before parsing", + path: path.to_owned(), + })?; + let new_raws = self.preprocess(program, &code)?; + program.pop_path(); + Ok(new_raws) } } @@ -538,6 +424,7 @@ mod tests { let mut output = Vec::new(); let mut ingest = Ingest::new(&mut output); ingest.ingest(root, &text)?; + assert_eq!(output, hex!("60015b586000566002")); Ok(()) diff --git a/etk-asm/src/parse/mod.rs b/etk-asm/src/parse/mod.rs index dba643c7..4858abaa 100644 --- a/etk-asm/src/parse/mod.rs +++ b/etk-asm/src/parse/mod.rs @@ -19,6 +19,7 @@ use self::{ error::ParseError, parser::{AsmParser, Rule}, }; + use crate::ast::Node; use crate::ops::AbstractOp; use etk_ops::cancun::Op; diff --git a/etk-asm/tests/asm.rs b/etk-asm/tests/asm.rs index 36fc72cf..07302f6b 100644 --- a/etk-asm/tests/asm.rs +++ b/etk-asm/tests/asm.rs @@ -108,6 +108,22 @@ fn instruction_macro_with_two_instructions_per_line() { assert_matches!(err, Error::Parse { .. }); } +#[test] +fn undefined_label_undefined_macro() { + let mut output = Vec::new(); + let mut ingester = Ingest::new(&mut output); + let err = ingester + .ingest_file(source(&[ + "instruction-macro", + "undefined-label-undefined-macro.etk", + ])) + .unwrap_err(); + + assert_matches!(err, etk_asm::ingest::Error::Assemble { source: + etk_asm::asm::Error::UndeclaredInstructionMacro { name, .. }, .. + } if name == "revert".to_string()); +} + #[test] fn every_op() -> Result<(), Error> { let mut output = Vec::new(); @@ -276,3 +292,36 @@ fn every_op() -> Result<(), Error> { Ok(()) } + +#[test] +fn test_dynamic_push_and_include() -> Result<(), Error> { + let mut output = Vec::new(); + let mut ingester = Ingest::new(&mut output); + ingester.ingest_file(source(&["variable-push", "main.etk"]))?; + + let tmp: Vec = output.iter().map(|&num| format!("{:x?}", num)).collect(); + println!("{:?}", tmp.join("")); + assert_eq!(output, hex!("61025758585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585801010101010158585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858015b")); + + Ok(()) +} + +#[test] +fn test_dynamic_push2() -> Result<(), Error> { + let mut output = Vec::new(); + let mut ingester = Ingest::new(&mut output); + ingester.ingest_file(source(&["variable-push2", "main1.etk"]))?; + assert_eq!(output, hex!("61010158")); + + let mut output = Vec::new(); + let mut ingester = Ingest::new(&mut output); + ingester.ingest_file(source(&["variable-push2", "main2.etk"]))?; + assert_eq!(output, hex!("61010158")); + + let mut output = Vec::new(); + let mut ingester = Ingest::new(&mut output); + ingester.ingest_file(source(&["variable-push2", "main3.etk"]))?; + assert_eq!(output, hex!("610107015801")); + + Ok(()) +} diff --git a/etk-asm/tests/asm/every-op/test.etk b/etk-asm/tests/asm/every-op/test.etk new file mode 100644 index 00000000..e1df3285 --- /dev/null +++ b/etk-asm/tests/asm/every-op/test.etk @@ -0,0 +1,5 @@ +%push(hello) +jump + +hello: +jumpdest \ No newline at end of file diff --git a/etk-asm/tests/asm/instruction-macro/undefined-label-undefined-macro.etk b/etk-asm/tests/asm/instruction-macro/undefined-label-undefined-macro.etk new file mode 100644 index 00000000..4103fb6b --- /dev/null +++ b/etk-asm/tests/asm/instruction-macro/undefined-label-undefined-macro.etk @@ -0,0 +1,6 @@ +%macro revert_if_neq() + push1 revert +%end + +%revert_if_neq() +%revert() \ No newline at end of file diff --git a/etk-asm/tests/asm/variable-push/included.etk b/etk-asm/tests/asm/variable-push/included.etk new file mode 100644 index 00000000..db4c91e2 --- /dev/null +++ b/etk-asm/tests/asm/variable-push/included.etk @@ -0,0 +1,594 @@ +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +add +add +add +add +add +add +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +pc +add \ No newline at end of file diff --git a/etk-asm/tests/asm/variable-push/main.etk b/etk-asm/tests/asm/variable-push/main.etk new file mode 100644 index 00000000..08144f7c --- /dev/null +++ b/etk-asm/tests/asm/variable-push/main.etk @@ -0,0 +1,6 @@ +%push(label) +pc +pc +%include("included.etk") +label: +jumpdest \ No newline at end of file diff --git a/etk-asm/tests/asm/variable-push2/main1.etk b/etk-asm/tests/asm/variable-push2/main1.etk new file mode 100644 index 00000000..7c5eb360 --- /dev/null +++ b/etk-asm/tests/asm/variable-push2/main1.etk @@ -0,0 +1,3 @@ +push2 label + 254 +label: +pc \ No newline at end of file diff --git a/etk-asm/tests/asm/variable-push2/main2.etk b/etk-asm/tests/asm/variable-push2/main2.etk new file mode 100644 index 00000000..10fdca9d --- /dev/null +++ b/etk-asm/tests/asm/variable-push2/main2.etk @@ -0,0 +1,3 @@ +%push(label + 254) +label: +pc \ No newline at end of file diff --git a/etk-asm/tests/asm/variable-push2/main3.etk b/etk-asm/tests/asm/variable-push2/main3.etk new file mode 100644 index 00000000..7c1f1d0e --- /dev/null +++ b/etk-asm/tests/asm/variable-push2/main3.etk @@ -0,0 +1,6 @@ +%push(label1 + label2 + 254) +add +label1: + pc +label2: + add \ No newline at end of file diff --git a/etk-dasm/src/bin/disease.rs b/etk-dasm/src/bin/disease.rs index 7729d9e0..08afb442 100644 --- a/etk-dasm/src/bin/disease.rs +++ b/etk-dasm/src/bin/disease.rs @@ -55,10 +55,7 @@ fn run() -> Result<(), Error> { separator.push_all(disasm.ops()); - let basic_blocks = separator - .take() - .into_iter() - .chain(separator.finish().into_iter()); + let basic_blocks = separator.take().into_iter().chain(separator.finish()); for block in basic_blocks { let mut offset = block.offset; From 087f96f3e75cee8d7bfada055d5e6e375fa5955d Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Wed, 22 Nov 2023 17:54:09 -0300 Subject: [PATCH 02/19] Documentation, comments on backpatch function & new test (all related to dynamic push and Sam's comments) --- etk-asm/src/asm.rs | 171 ++++++++++++++++++++++++++++--------------- etk-asm/tests/asm.rs | 7 +- 2 files changed, 117 insertions(+), 61 deletions(-) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index 195ee683..441c5ac2 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -315,6 +315,8 @@ impl Assembler { source: UnknownLabel { label, .. }, }) => { if let AbstractOp::Push(_) = op { + // Here, we set the size of the push to 2 bytes (min possible value), + // as we don't know the final value of the label yet. self.concrete_len += 2; } else { self.concrete_len += op.size().unwrap(); @@ -348,7 +350,20 @@ impl Assembler { /// Backpatch dynamic operations and emit the assembled program. /// - /// Errors if there are any undeclared labels. + /// This function performs the final steps in the assembly process. It ensures that all labels + /// and dynamic elements in the code have been properly resolved and finalized. This includes + /// handling variable-sized push instructions, where the actual size of the push may not be known + /// until all labels and expressions have been evaluated. + /// + /// Handle Variable-sized Pushes: Specifically for push operations, this function plays a + /// critical role. The size of a push operation may depend on the value being pushed, especially + /// when labels are involved. As labels could be resolved to different addresses during the + /// assembly process, the final value of a label (and thus the size of the push) might only be + /// known at this stage. This function recalculates the size of each push operation based on the + /// final resolved values of labels and expressions. If a push operation requires more space than + /// initially estimated, the function adjusts the code accordingly. This step is essential for + /// ensuring that the generated bytecode is correct and adheres to the EVM's requirements for + /// push instructions. fn backpatch_and_emit(&mut self) -> Result, Error> { if !self.undeclared_labels.is_empty() { return error::UndeclaredLabels { @@ -362,70 +377,89 @@ impl Assembler { } let mut output = Vec::new(); for op in self.ready.iter() { - if let RawOp::Op(ref op) = op { - match op - .clone() - .concretize((&self.declared_labels, &self.declared_macros).into()) - { - Ok(mut cop) => { - if let AbstractOp::Push(imm) = op { - let exp = imm.tree.eval_with_context( - (&self.declared_labels, &self.declared_macros).into(), - ); - - let labels: HashSet = imm - .tree - .labels(&self.declared_macros) - .unwrap() - .into_iter() - .collect(); - if !labels.is_empty() { - if let Ok(val) = exp { - let extra = val.to_usize().unwrap() / 256; - if extra > 0 { - self.concrete_len += extra; - - for label in labels { - if let Some(position) = - self.declared_labels.get_mut(&label) - { - *position = position.map(|v| v + 1); - } + let op = match op { + RawOp::Op(ref op) => op, + RawOp::Raw(raw) => { + output.extend(raw); + continue; + } + RawOp::Scope(_) => unreachable!("scopes should be expanded"), + }; + + match op + .clone() + .concretize((&self.declared_labels, &self.declared_macros).into()) + { + Ok(mut cop) => { + // If the push is variable-sized, we need to recalculate (backpatch) the size of the + // push based on the final resolved value of the label/expression. + if let AbstractOp::Push(imm) = op { + // Evaluate the expression with the current context. + let exp = imm.tree.eval_with_context( + (&self.declared_labels, &self.declared_macros).into(), + ); + + let labels: HashSet = imm + .tree + .labels(&self.declared_macros) + .unwrap() + .into_iter() + .collect(); + + // If the expression contains any labels, we need to adjust the size of the + // push operation to account for the final resolved value of the expression. + if !labels.is_empty() { + if let Ok(val) = exp { + // We calculate the real size of the push operation based on the + // final resolved value of the expression. + let extra = (val.to_f64().unwrap().log2() / 8.0).ceil() as usize; + + // If the size of the push operation has changed (we already reserved + // size for a push1), we need to adjust the code accordingly. + if extra > 1 { + self.concrete_len += extra - 1; + + for label in labels { + // Update the position of the label to account for the extra bytes. + if let Some(position) = self.declared_labels.get_mut(&label) + { + // As the label's value is based on concrete_len, we need to + // add the extra bytes to the label's position. + *position = position.map(|v| v + extra - 1); } } } } - - cop = op - .clone() - .concretize((&self.declared_labels, &self.declared_macros).into()) - .unwrap(); - } - cop.assemble(&mut output) - } - Err(ops::Error::ContextIncomplete { - source: UnknownLabel { .. }, - }) => { - return error::UndeclaredLabels { - labels: self.undeclared_labels.to_vec(), } - .fail(); - } - Err(ops::Error::ContextIncomplete { - source: UnknownMacro { name, .. }, - }) => { - return error::UndeclaredInstructionMacro { name }.fail(); + + // Recreate the push operation with the new size. + cop = op + .clone() + .concretize((&self.declared_labels, &self.declared_macros).into()) + .unwrap(); } - Err(ops::Error::ContextIncomplete { - source: UndefinedVariable { name, .. }, - }) => { - return error::UndeclaredVariableMacro { var: name }.fail(); + cop.assemble(&mut output) + } + Err(ops::Error::ContextIncomplete { + source: UnknownLabel { .. }, + }) => { + return error::UndeclaredLabels { + labels: self.undeclared_labels.to_vec(), } - - Err(_) => unreachable!("all ops should be concretizable"), + .fail(); + } + Err(ops::Error::ContextIncomplete { + source: UnknownMacro { name, .. }, + }) => { + return error::UndeclaredInstructionMacro { name }.fail(); } - } else if let RawOp::Raw(raw) = op { - output.extend(raw); + Err(ops::Error::ContextIncomplete { + source: UndefinedVariable { name, .. }, + }) => { + return error::UndeclaredVariableMacro { var: name }.fail(); + } + + Err(_) => unreachable!("all ops should be concretizable"), } } @@ -689,6 +723,29 @@ mod tests { Ok(()) } + #[test] + fn assemble_variable_push3() -> Result<(), Error> { + let mut code = vec![]; + code.push(AbstractOp::Push(Imm::with_label("auto"))); + for _ in 0..65537 { + code.push(AbstractOp::new(GetPc)); + } + + code.push(AbstractOp::Label("auto".into())); + code.push(AbstractOp::new(JumpDest)); + + let mut asm = Assembler::new(); + let result = asm.assemble(&code)?; + + let mut expected = vec![0x62, 0x01, 0x00, 0x05]; + expected.extend_from_slice(&[0x58; 65537]); + expected.push(0x5b); + + assert_eq!(result, expected); + + Ok(()) + } + #[test] fn assemble_undeclared_label() -> Result<(), Error> { let mut asm = Assembler::new(); diff --git a/etk-asm/tests/asm.rs b/etk-asm/tests/asm.rs index 07302f6b..07385ba0 100644 --- a/etk-asm/tests/asm.rs +++ b/etk-asm/tests/asm.rs @@ -299,8 +299,6 @@ fn test_dynamic_push_and_include() -> Result<(), Error> { let mut ingester = Ingest::new(&mut output); ingester.ingest_file(source(&["variable-push", "main.etk"]))?; - let tmp: Vec = output.iter().map(|&num| format!("{:x?}", num)).collect(); - println!("{:?}", tmp.join("")); assert_eq!(output, hex!("61025758585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585801010101010158585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858585858015b")); Ok(()) @@ -316,12 +314,13 @@ fn test_dynamic_push2() -> Result<(), Error> { let mut output = Vec::new(); let mut ingester = Ingest::new(&mut output); ingester.ingest_file(source(&["variable-push2", "main2.etk"]))?; - assert_eq!(output, hex!("61010158")); + assert_eq!(output, hex!("61010058")); + let mut output = Vec::new(); let mut ingester = Ingest::new(&mut output); ingester.ingest_file(source(&["variable-push2", "main3.etk"]))?; assert_eq!(output, hex!("610107015801")); - + Ok(()) } From f7ffc4cddb954a9ce94d4009a3444c969771647a Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Wed, 22 Nov 2023 18:09:54 -0300 Subject: [PATCH 03/19] Rustfmt --- etk-asm/tests/asm.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/etk-asm/tests/asm.rs b/etk-asm/tests/asm.rs index 07385ba0..b38ca19f 100644 --- a/etk-asm/tests/asm.rs +++ b/etk-asm/tests/asm.rs @@ -316,11 +316,10 @@ fn test_dynamic_push2() -> Result<(), Error> { ingester.ingest_file(source(&["variable-push2", "main2.etk"]))?; assert_eq!(output, hex!("61010058")); - let mut output = Vec::new(); let mut ingester = Ingest::new(&mut output); ingester.ingest_file(source(&["variable-push2", "main3.etk"]))?; assert_eq!(output, hex!("610107015801")); - + Ok(()) } From d3e99bcf83d3a14d6fa189261bbe47dab6157790 Mon Sep 17 00:00:00 2001 From: Sam Wilson Date: Mon, 27 Nov 2023 12:29:26 -0500 Subject: [PATCH 04/19] Add an additional test for variable pushes --- etk-asm/src/asm.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index 441c5ac2..fe6f6a7c 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -1163,6 +1163,23 @@ mod tests { Ok(()) } + #[test] + fn assemble_variable_push_before_push2() -> Result<(), Error> { + let mut asm = Assembler::new(); + let ops = vec![ + AbstractOp::Push(Imm::with_expression(Expression::Plus( + Terminal::Label("foo".into()).into(), + BigInt::from(256).into(), + ))), + AbstractOp::new(Push2(Imm::with_label("foo1"))), + AbstractOp::Label("foo".into()), + AbstractOp::Label("foo1".into()), + ]; + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("610106610006")); + Ok(()) + } + #[test] fn assemble_variable_push_expression_with_undeclared_labels() -> Result<(), Error> { let mut asm = Assembler::new(); From 1296aee6dd2517d6806e43ed2b7924c98201e96f Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Wed, 29 Nov 2023 14:14:51 -0300 Subject: [PATCH 05/19] Adressing Sam's comments --- etk-asm/src/asm.rs | 146 +++++++++++++++++++++++++++++----- etk-asm/src/ops/expression.rs | 15 ++-- 2 files changed, 137 insertions(+), 24 deletions(-) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index fe6f6a7c..4d373127 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -145,7 +145,7 @@ use crate::ops::expression::Error::{UndefinedVariable, UnknownLabel, UnknownMacr use crate::ops::{self, AbstractOp, Assemble, Expression, MacroDefinition}; use num_traits::ToPrimitive; use rand::Rng; -use std::collections::{hash_map, HashMap, HashSet}; +use std::collections::{hash_map, BTreeMap, HashMap, HashSet}; /// An item to be assembled, which can be either an [`AbstractOp`], /// the inclusion of a new scope or a raw byte sequence. @@ -207,7 +207,7 @@ pub struct Assembler { concrete_len: usize, /// Labels associated with an `AbstractOp::Label`. - declared_labels: HashMap>, + declared_labels: BTreeMap>, /// Macros associated with an `AbstractOp::Macro`. declared_macros: HashMap, @@ -215,6 +215,31 @@ pub struct Assembler { /// Labels that have been referred to (ex. with push) but /// have not been declared with an `AbstractOp::Label`. undeclared_labels: Vec, + + /// Pushes that are dynamic and need to be backpatched. + labeled_dynamic_pushes: HashMap, +} + +/// A label definition. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct LabelDef { + position: usize, + updated: bool, +} + +impl LabelDef { + /// Create a new `LabelDef`. + pub fn new(position: usize) -> Self { + Self { + position, + updated: false, + } + } + + /// Get the position of the label. + pub fn position(&self) -> usize { + self.position + } } impl Assembler { @@ -279,7 +304,13 @@ impl Assembler { let old = self .declared_labels - .insert(label, Some(self.concrete_len)) + .insert( + label, + Some(LabelDef { + position: self.concrete_len, + updated: false, + }), + ) .expect("label should exist"); assert_eq!(old, None, "label should have been undefined"); } @@ -318,6 +349,11 @@ impl Assembler { // Here, we set the size of the push to 2 bytes (min possible value), // as we don't know the final value of the label yet. self.concrete_len += 2; + // We count the number of times this label appears in dynamic pushes. + *self + .labeled_dynamic_pushes + .entry(label.to_owned()) + .or_insert(0) += 1; } else { self.concrete_len += op.size().unwrap(); } @@ -361,9 +397,7 @@ impl Assembler { /// assembly process, the final value of a label (and thus the size of the push) might only be /// known at this stage. This function recalculates the size of each push operation based on the /// final resolved values of labels and expressions. If a push operation requires more space than - /// initially estimated, the function adjusts the code accordingly. This step is essential for - /// ensuring that the generated bytecode is correct and adheres to the EVM's requirements for - /// push instructions. + /// initially estimated, the function adjusts the code accordingly. fn backpatch_and_emit(&mut self) -> Result, Error> { if !self.undeclared_labels.is_empty() { return error::UndeclaredLabels { @@ -376,6 +410,7 @@ impl Assembler { .fail(); } let mut output = Vec::new(); + let mut acum = 0; for op in self.ready.iter() { let op = match op { RawOp::Op(ref op) => op, @@ -412,20 +447,44 @@ impl Assembler { if let Ok(val) = exp { // We calculate the real size of the push operation based on the // final resolved value of the expression. - let extra = (val.to_f64().unwrap().log2() / 8.0).ceil() as usize; - - // If the size of the push operation has changed (we already reserved - // size for a push1), we need to adjust the code accordingly. - if extra > 1 { - self.concrete_len += extra - 1; + let push_size = + (val.to_f64().unwrap().log2() / 8.0).ceil() as usize; + + if push_size > 1 { + // If the size of the push operation is greater than 1, we need + // to adjust the size to account for the final resolved value of the expression. + for (label, label_value) in self.declared_labels.iter_mut() { + let labeldef = label_value.as_ref().unwrap(); + // If the label has already been updated, we skip it. + if labeldef.updated { + continue; + } - for label in labels { - // Update the position of the label to account for the extra bytes. - if let Some(position) = self.declared_labels.get_mut(&label) + // We check if the label is used in a dynamic push. + if let Some(value) = + self.labeled_dynamic_pushes.get_mut(label) { - // As the label's value is based on concrete_len, we need to - // add the extra bytes to the label's position. - *position = position.map(|v| v + extra - 1); + // If the label is used in a dynamic push, we need to + // multiply the final resolved value of the label by + // the number of times the label is used in dynamic + // pushes. + self.concrete_len += (push_size - 1) * *value; + *label_value = Some(LabelDef { + position: labeldef.position + + *value * (push_size - 1), + updated: true, + }); + acum += *value * (push_size - 1); + } else { + // If the label is not used in a dynamic push, we + // simply update the size of the push operation. + // Since ´declared_labels' is ordered by insertion + // we can simply add the accumulated size to the + // position of the label. + *label_value = Some(LabelDef { + position: labeldef.position + acum, + updated: true, + }); } } } @@ -772,7 +831,13 @@ mod tests { let result = asm.assemble(&ops)?; assert_eq!(asm.declared_labels.len(), 1); - assert_eq!(asm.declared_labels.get("lbl"), Some(&Some(0))); + assert_eq!( + asm.declared_labels.get("lbl"), + Some(&Some(LabelDef { + position: 0, + updated: false + })) + ); assert_eq!(result, hex!("5b")); Ok(()) } @@ -1180,6 +1245,49 @@ mod tests { Ok(()) } + #[test] + fn assemble_double_update_variable_push() -> Result<(), Error> { + let mut asm = Assembler::new(); + let ops = vec![ + AbstractOp::Push(Imm::with_expression(Expression::Plus( + Terminal::Label("foo".into()).into(), + BigInt::from(256).into(), + ))), + AbstractOp::new(Push2(Imm::with_label("foo1"))), + AbstractOp::Push(Imm::with_expression(Expression::Plus( + Terminal::Label("foo".into()).into(), + BigInt::from(256).into(), + ))), + AbstractOp::Label("foo".into()), + AbstractOp::Label("foo1".into()), + ]; + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("610109610009610109")); + Ok(()) + } + + #[test] + fn assemble_double_update_variable_push2() -> Result<(), Error> { + let mut asm = Assembler::new(); + let ops = vec![ + AbstractOp::Push(Imm::with_expression(Expression::Plus( + Terminal::Label("foo".into()).into(), + BigInt::from(256).into(), + ))), + AbstractOp::new(Push2(Imm::with_label("foo1"))), + AbstractOp::Push(Imm::with_expression(Expression::Plus( + Terminal::Label("foo".into()).into(), + BigInt::from(256).into(), + ))), + AbstractOp::new(Push2(Imm::with_label("foo1"))), + AbstractOp::Label("foo".into()), + AbstractOp::Label("foo1".into()), + ]; + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("61010c61000c61010c61000c")); + Ok(()) + } + #[test] fn assemble_variable_push_expression_with_undeclared_labels() -> Result<(), Error> { let mut asm = Assembler::new(); diff --git a/etk-asm/src/ops/expression.rs b/etk-asm/src/ops/expression.rs index b5508606..7c21b22d 100644 --- a/etk-asm/src/ops/expression.rs +++ b/etk-asm/src/ops/expression.rs @@ -1,8 +1,10 @@ +use crate::asm::LabelDef; + use super::macros::{ExpressionMacroInvocation, MacroDefinition}; use num_bigint::BigInt; use snafu::OptionExt; use snafu::{Backtrace, Snafu}; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::fmt::{self, Debug}; /// An error that arises when an expression cannot be evaluated. @@ -22,7 +24,7 @@ pub enum Error { UndefinedVariable { name: String, backtrace: Backtrace }, } -type LabelsMap = HashMap>; +type LabelsMap = BTreeMap>; type VariablesMap = HashMap; type MacrosMap = HashMap; @@ -36,7 +38,7 @@ pub struct Context<'a> { impl<'a> Context<'a> { /// Looks up a label in the current context. - pub fn get_label(&self, key: &str) -> Option<&Option> { + pub fn get_label(&self, key: &str) -> Option<&Option> { match self.labels { Some(labels) => labels.get(key), None => None, @@ -172,6 +174,7 @@ impl Terminal { .get_label(label) .context(UnknownLabel { label })? .context(UnknownLabel { label })? + .position() .into(), Terminal::Variable(name) => ctx .get_variable(name) @@ -390,7 +393,9 @@ mod tests { fn expr_with_label() { // foo + 1 = 42 let expr = Expression::Plus(Terminal::Label(String::from("foo")).into(), 1.into()); - let labels: HashMap<_, _> = vec![("foo".to_string(), Some(41))].into_iter().collect(); + let labels: BTreeMap<_, _> = vec![("foo".to_string(), Some(LabelDef::new(41)))] + .into_iter() + .collect(); let out = expr.eval_with_context(Context::from(&labels)).unwrap(); assert_eq!(out, BigInt::from(42)); } @@ -404,7 +409,7 @@ mod tests { // label w/o defined address let expr = Expression::Plus(Terminal::Label(String::from("foo")).into(), 1.into()); - let labels: HashMap<_, _> = vec![("foo".to_string(), None)].into_iter().collect(); + let labels: BTreeMap<_, _> = vec![("foo".to_string(), None)].into_iter().collect(); let err = expr.eval_with_context(Context::from(&labels)).unwrap_err(); assert_matches!(err, Error::UnknownLabel { label, .. } if label == "foo"); } From 8a2f320e7319d6c8dfec067efcfd486ef821ac7f Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Wed, 29 Nov 2023 14:21:47 -0300 Subject: [PATCH 06/19] Adressing Sam's comments --- etk-asm/src/asm.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index 4d373127..df9aad91 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -391,8 +391,7 @@ impl Assembler { /// handling variable-sized push instructions, where the actual size of the push may not be known /// until all labels and expressions have been evaluated. /// - /// Handle Variable-sized Pushes: Specifically for push operations, this function plays a - /// critical role. The size of a push operation may depend on the value being pushed, especially + /// Handle Variable-sized Pushes: The size of a push operation may depend on the value being pushed, especially /// when labels are involved. As labels could be resolved to different addresses during the /// assembly process, the final value of a label (and thus the size of the push) might only be /// known at this stage. This function recalculates the size of each push operation based on the From f2a1317b7b8746edb894a08a6cc42e4f1dbec46e Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Wed, 29 Nov 2023 16:00:51 -0300 Subject: [PATCH 07/19] Adressing Sam's comments --- etk-asm/src/asm.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index df9aad91..1543d8f2 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -452,7 +452,11 @@ impl Assembler { if push_size > 1 { // If the size of the push operation is greater than 1, we need // to adjust the size to account for the final resolved value of the expression. - for (label, label_value) in self.declared_labels.iter_mut() { + for (label, label_value) in self + .declared_labels + .iter_mut() + .filter(|(_, def)| !def.unwrap().updated) + { let labeldef = label_value.as_ref().unwrap(); // If the label has already been updated, we skip it. if labeldef.updated { From 22e8d68f9e04e6eae4cf3cf905188199fb9015ad Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Wed, 29 Nov 2023 16:44:00 -0300 Subject: [PATCH 08/19] Better documentation --- etk-asm/src/asm.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index 1543d8f2..659ff7f2 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -470,7 +470,7 @@ impl Assembler { // If the label is used in a dynamic push, we need to // multiply the final resolved value of the label by // the number of times the label is used in dynamic - // pushes. + // pushes (to update the size of the bytecode accordingly). self.concrete_len += (push_size - 1) * *value; *label_value = Some(LabelDef { position: labeldef.position @@ -483,7 +483,7 @@ impl Assembler { // simply update the size of the push operation. // Since ´declared_labels' is ordered by insertion // we can simply add the accumulated size to the - // position of the label. + // actual position of the label. *label_value = Some(LabelDef { position: labeldef.position + acum, updated: true, From 6e5befc270b743c9940e7fc10b4faa5be639f996 Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Wed, 29 Nov 2023 17:39:34 -0300 Subject: [PATCH 09/19] Better documentation --- etk-asm/src/asm.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index 659ff7f2..de8212b3 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -452,11 +452,7 @@ impl Assembler { if push_size > 1 { // If the size of the push operation is greater than 1, we need // to adjust the size to account for the final resolved value of the expression. - for (label, label_value) in self - .declared_labels - .iter_mut() - .filter(|(_, def)| !def.unwrap().updated) - { + for (label, label_value) in self.declared_labels.iter_mut() { let labeldef = label_value.as_ref().unwrap(); // If the label has already been updated, we skip it. if labeldef.updated { From 6e1593333a7954073645e939474e0e45029c2583 Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Wed, 29 Nov 2023 18:03:18 -0300 Subject: [PATCH 10/19] Another test (variable push with mixed labels) --- etk-asm/src/asm.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index de8212b3..9573438c 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -1244,6 +1244,29 @@ mod tests { Ok(()) } + #[test] + fn assemble_variable_push_mixed_labels() -> Result<(), Error> { + let mut asm = Assembler::new(); + let ops = vec![ + AbstractOp::Push(Imm::with_expression(Expression::Plus( + Terminal::Label("foo".into()).into(), + BigInt::from(256).into(), + ))), + AbstractOp::new(Push2(Imm::with_label("foo"))), + AbstractOp::Push(Imm::with_expression(Expression::Plus( + Terminal::Label("foo1".into()).into(), + BigInt::from(256).into(), + ))), + AbstractOp::new(Gas), + AbstractOp::Label("foo".into()), + AbstractOp::new(Gas), + AbstractOp::Label("foo1".into()), + ]; + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("61010961000961010a5a5a")); + Ok(()) + } + #[test] fn assemble_double_update_variable_push() -> Result<(), Error> { let mut asm = Assembler::new(); From 27fae0e4036af7a4150abd39cbe29caf38e720a9 Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Thu, 30 Nov 2023 11:56:51 -0300 Subject: [PATCH 11/19] IndexMap and more tests --- etk-asm/Cargo.toml | 1 + etk-asm/src/asm.rs | 26 ++++++++++++++++++++++---- etk-asm/src/ops/expression.rs | 7 ++++--- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/etk-asm/Cargo.toml b/etk-asm/Cargo.toml index e2b4622d..d77a844f 100644 --- a/etk-asm/Cargo.toml +++ b/etk-asm/Cargo.toml @@ -35,6 +35,7 @@ rand = "0.8.5" sha3 = "0.10.1" clap = { optional = true, version = "3.1", features = ["derive"] } snafu = { version = "0.7.1", default-features = false, features = ["std"] } +indexmap = "2.1.0" [dev-dependencies] assert_matches = "1.5.0" diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index 9573438c..ab93b19e 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -143,9 +143,10 @@ mod error { pub use self::error::Error; use crate::ops::expression::Error::{UndefinedVariable, UnknownLabel, UnknownMacro}; use crate::ops::{self, AbstractOp, Assemble, Expression, MacroDefinition}; +use indexmap::IndexMap; use num_traits::ToPrimitive; use rand::Rng; -use std::collections::{hash_map, BTreeMap, HashMap, HashSet}; +use std::collections::{hash_map, HashMap, HashSet}; /// An item to be assembled, which can be either an [`AbstractOp`], /// the inclusion of a new scope or a raw byte sequence. @@ -207,7 +208,7 @@ pub struct Assembler { concrete_len: usize, /// Labels associated with an `AbstractOp::Label`. - declared_labels: BTreeMap>, + declared_labels: IndexMap>, /// Macros associated with an `AbstractOp::Macro`. declared_macros: HashMap, @@ -1244,6 +1245,23 @@ mod tests { Ok(()) } + #[test] + fn assemble_variable_push_before_push2_inverted() -> Result<(), Error> { + let mut asm = Assembler::new(); + let ops = vec![ + AbstractOp::Push(Imm::with_expression(Expression::Plus( + Terminal::Label("z".into()).into(), + BigInt::from(256).into(), + ))), + AbstractOp::new(Push2(Imm::with_label("a"))), + AbstractOp::Label("z".into()), + AbstractOp::Label("a".into()), + ]; + let result = asm.assemble(&ops)?; + assert_eq!(result, hex!("610106610006")); + Ok(()) + } + #[test] fn assemble_variable_push_mixed_labels() -> Result<(), Error> { let mut asm = Assembler::new(); @@ -1254,13 +1272,13 @@ mod tests { ))), AbstractOp::new(Push2(Imm::with_label("foo"))), AbstractOp::Push(Imm::with_expression(Expression::Plus( - Terminal::Label("foo1".into()).into(), + Terminal::Label("bar".into()).into(), BigInt::from(256).into(), ))), AbstractOp::new(Gas), AbstractOp::Label("foo".into()), AbstractOp::new(Gas), - AbstractOp::Label("foo1".into()), + AbstractOp::Label("bar".into()), ]; let result = asm.assemble(&ops)?; assert_eq!(result, hex!("61010961000961010a5a5a")); diff --git a/etk-asm/src/ops/expression.rs b/etk-asm/src/ops/expression.rs index 7c21b22d..c2f63dac 100644 --- a/etk-asm/src/ops/expression.rs +++ b/etk-asm/src/ops/expression.rs @@ -1,6 +1,7 @@ use crate::asm::LabelDef; use super::macros::{ExpressionMacroInvocation, MacroDefinition}; +use indexmap::IndexMap; use num_bigint::BigInt; use snafu::OptionExt; use snafu::{Backtrace, Snafu}; @@ -24,7 +25,7 @@ pub enum Error { UndefinedVariable { name: String, backtrace: Backtrace }, } -type LabelsMap = BTreeMap>; +type LabelsMap = IndexMap>; type VariablesMap = HashMap; type MacrosMap = HashMap; @@ -393,7 +394,7 @@ mod tests { fn expr_with_label() { // foo + 1 = 42 let expr = Expression::Plus(Terminal::Label(String::from("foo")).into(), 1.into()); - let labels: BTreeMap<_, _> = vec![("foo".to_string(), Some(LabelDef::new(41)))] + let labels: IndexMap<_, _> = vec![("foo".to_string(), Some(LabelDef::new(41)))] .into_iter() .collect(); let out = expr.eval_with_context(Context::from(&labels)).unwrap(); @@ -409,7 +410,7 @@ mod tests { // label w/o defined address let expr = Expression::Plus(Terminal::Label(String::from("foo")).into(), 1.into()); - let labels: BTreeMap<_, _> = vec![("foo".to_string(), None)].into_iter().collect(); + let labels: IndexMap<_, _> = vec![("foo".to_string(), None)].into_iter().collect(); let err = expr.eval_with_context(Context::from(&labels)).unwrap_err(); assert_matches!(err, Error::UnknownLabel { label, .. } if label == "foo"); } From beb28b8232837de56941cb2b0bd7c7f04019187e Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Thu, 30 Nov 2023 12:13:06 -0300 Subject: [PATCH 12/19] Rustfmt --- etk-asm/src/ops/expression.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etk-asm/src/ops/expression.rs b/etk-asm/src/ops/expression.rs index c2f63dac..7fc4a202 100644 --- a/etk-asm/src/ops/expression.rs +++ b/etk-asm/src/ops/expression.rs @@ -5,7 +5,7 @@ use indexmap::IndexMap; use num_bigint::BigInt; use snafu::OptionExt; use snafu::{Backtrace, Snafu}; -use std::collections::{BTreeMap, HashMap}; +use std::collections::HashMap; use std::fmt::{self, Debug}; /// An error that arises when an expression cannot be evaluated. From f0427f414734ade90fb3332639352d521e5f0bae Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Thu, 30 Nov 2023 12:19:16 -0300 Subject: [PATCH 13/19] Better names --- etk-asm/src/asm.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index ab93b19e..f0f21bd1 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -460,21 +460,21 @@ impl Assembler { continue; } + let pos = labeldef.position; // We check if the label is used in a dynamic push. - if let Some(value) = + if let Some(times) = self.labeled_dynamic_pushes.get_mut(label) { // If the label is used in a dynamic push, we need to // multiply the final resolved value of the label by // the number of times the label is used in dynamic // pushes (to update the size of the bytecode accordingly). - self.concrete_len += (push_size - 1) * *value; + self.concrete_len += (push_size - 1) * *times; *label_value = Some(LabelDef { - position: labeldef.position - + *value * (push_size - 1), + position: pos + *times * (push_size - 1), updated: true, }); - acum += *value * (push_size - 1); + acum += *times * (push_size - 1); } else { // If the label is not used in a dynamic push, we // simply update the size of the push operation. @@ -482,7 +482,7 @@ impl Assembler { // we can simply add the accumulated size to the // actual position of the label. *label_value = Some(LabelDef { - position: labeldef.position + acum, + position: pos + acum, updated: true, }); } From 75d72663a7a53058dfe7edccff5c46cf71b5d012 Mon Sep 17 00:00:00 2001 From: Sam Wilson Date: Thu, 14 Dec 2023 16:21:22 -0500 Subject: [PATCH 14/19] Add another test for weird expression+variable push interactions --- etk-asm/src/asm.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index f0f21bd1..6fe37d87 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -1345,6 +1345,54 @@ mod tests { Ok(()) } + #[test] + fn assemble_variable_push2_comparison_with_undeclared_labels() -> Result<(), Error> { + let mut asm = Assembler::new(); + + // %push(lbl1 - lbl2) + // push2 lbl1 + lbl2 + // pc # repeat 126 times. + // lbl1: + // lbl2: + let mut ops = vec![AbstractOp::new(GetPc); 130]; + ops[0] = AbstractOp::Push(Imm::with_expression(Expression::Minus( + Terminal::Label(String::from("lbl1")).into(), + Terminal::Label(String::from("lbl2")).into(), + ))); + ops[1] = AbstractOp::new(Push2( + Expression::Plus( + Terminal::Label(String::from("lbl1")).into(), + Terminal::Label(String::from("lbl2")).into(), + ) + .into(), + )); + ops[128] = AbstractOp::Label("lbl1".into()); + ops[129] = AbstractOp::Label("lbl2".into()); + + let expected = asm.assemble(&ops)?; + + let mut asm = Assembler::new(); + + // %push(lbl1 - lbl2) + // %push(lbl1 + lbl2) + // pc # repeat 126 times. + // lbl1: + // lbl2: + ops[1] = AbstractOp::Push(Imm::with_expression(Expression::Plus( + Terminal::Label(String::from("lbl1")).into(), + Terminal::Label(String::from("lbl2")).into(), + ))); + let result = asm.assemble(&ops)?; + + // Sanity check the expected result: should use push1 then push2. + assert_eq!(expected[0], 0x60); + assert_eq!(expected[2], 0x61); + + // Assert that the two results are identical. + assert_eq!(expected, result); + Ok(()) + } + #[test] fn assemble_variable_push1_expression() -> Result<(), Error> { let mut asm = Assembler::new(); From 40274b02d11f7b189e03031c1a00706b4b0144d7 Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Thu, 14 Dec 2023 18:28:08 -0300 Subject: [PATCH 15/19] Vec to Hashmap --- Cargo.lock | 33 ++++++++++++++++++++++++++++----- etk-asm/src/asm.rs | 6 +++--- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1f8d0a38..f4c9309a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -264,7 +264,7 @@ dependencies = [ "bitflags", "clap_derive", "clap_lex", - "indexmap", + "indexmap 1.8.1", "lazy_static", "strsim 0.10.0", "termcolor", @@ -401,6 +401,12 @@ dependencies = [ "termcolor", ] +[[package]] +name = "equivalent" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" + [[package]] name = "etk-4byte" version = "0.4.0-dev" @@ -438,6 +444,7 @@ dependencies = [ "etk-ops", "hex", "hex-literal", + "indexmap 2.1.0", "num-bigint", "num-traits", "pest", @@ -477,7 +484,7 @@ name = "etk-ops" version = "0.4.0-dev" dependencies = [ "educe", - "indexmap", + "indexmap 1.8.1", "quote", "serde", "snafu", @@ -629,7 +636,7 @@ dependencies = [ "futures-sink", "futures-util", "http", - "indexmap", + "indexmap 1.8.1", "slab", "tokio", "tokio-util", @@ -642,6 +649,12 @@ version = "0.11.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab5ef0d4909ef3724cc8cce6ccc8572c5c817592e9285f5464f8e86f8bd3726e" +[[package]] +name = "hashbrown" +version = "0.14.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "290f1a1d9242c78d09ce40a5e87e7554ee637af1351968159f4952f028f75604" + [[package]] name = "heck" version = "0.4.0" @@ -764,10 +777,20 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0f647032dfaa1f8b6dc29bd3edb7bbef4861b8b8007ebb118d6db284fd59f6ee" dependencies = [ "autocfg", - "hashbrown", + "hashbrown 0.11.2", "serde", ] +[[package]] +name = "indexmap" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d530e1a18b1cb4c484e6e34556a0d948706958449fca0cab753d649f2bce3d1f" +dependencies = [ + "equivalent", + "hashbrown 0.14.3", +] + [[package]] name = "instant" version = "0.1.12" @@ -1078,7 +1101,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a13a2fa9d0b63e5f22328828741e523766fff0ee9e779316902290dff3f824f" dependencies = [ "fixedbitset", - "indexmap", + "indexmap 1.8.1", ] [[package]] diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index 6fe37d87..e272f7a3 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -215,7 +215,7 @@ pub struct Assembler { /// Labels that have been referred to (ex. with push) but /// have not been declared with an `AbstractOp::Label`. - undeclared_labels: Vec, + undeclared_labels: HashSet, /// Pushes that are dynamic and need to be backpatched. labeled_dynamic_pushes: HashMap, @@ -359,7 +359,7 @@ impl Assembler { self.concrete_len += op.size().unwrap(); } - self.undeclared_labels.push(label); + self.undeclared_labels.insert(label); self.ready.push(rop.clone()); } Err(ops::Error::ContextIncomplete { @@ -503,7 +503,7 @@ impl Assembler { source: UnknownLabel { .. }, }) => { return error::UndeclaredLabels { - labels: self.undeclared_labels.to_vec(), + labels: self.undeclared_labels.iter().cloned().collect::>(), } .fail(); } From 5f9c5523d6084959b62aa20ee69e1f704c84896b Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Sat, 23 Dec 2023 10:02:23 -0300 Subject: [PATCH 16/19] Bug in push with operation between labels, fixed --- etk-asm/src/asm.rs | 154 +++++++++++++++++-------------------------- etk-asm/tests/asm.rs | 4 +- 2 files changed, 62 insertions(+), 96 deletions(-) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index e272f7a3..1f7c00db 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -182,7 +182,7 @@ impl From<&AbstractOp> for RawOp { } /// Assembles a series of [`RawOp`] into raw bytes, tracking and resolving macros and labels, -/// and handling dynamic pushes. +/// and handling variable-sized pushes. /// /// ## Example /// @@ -217,8 +217,8 @@ pub struct Assembler { /// have not been declared with an `AbstractOp::Label`. undeclared_labels: HashSet, - /// Pushes that are dynamic and need to be backpatched. - labeled_dynamic_pushes: HashMap, + /// Pushes that are variable-sized and need to be backpatched. + variable_sized_push: Vec, } /// A label definition. @@ -344,22 +344,26 @@ impl Assembler { .fail() } Err(ops::Error::ContextIncomplete { - source: UnknownLabel { label, .. }, + source: UnknownLabel { .. }, }) => { + let labels = op + .expr() + .unwrap() + .labels(&self.declared_macros) + .unwrap() + .into_iter() + .collect::>(); + if let AbstractOp::Push(_) = op { // Here, we set the size of the push to 2 bytes (min possible value), // as we don't know the final value of the label yet. self.concrete_len += 2; - // We count the number of times this label appears in dynamic pushes. - *self - .labeled_dynamic_pushes - .entry(label.to_owned()) - .or_insert(0) += 1; + self.variable_sized_push.push(op.clone()); } else { self.concrete_len += op.size().unwrap(); } - self.undeclared_labels.insert(label); + self.undeclared_labels.extend(labels.into_iter()); self.ready.push(rop.clone()); } Err(ops::Error::ContextIncomplete { @@ -385,10 +389,37 @@ impl Assembler { Ok(self.concrete_len) } - /// Backpatch dynamic operations and emit the assembled program. + fn backpatch_labels(&mut self) -> Result<(), Error> { + for op in self.variable_sized_push.iter() { + if let AbstractOp::Push(imm) = op { + let exp = imm + .tree + .eval_with_context((&self.declared_labels, &self.declared_macros).into()); + + if let Ok(val) = exp { + let imm_size = (val.to_f64().unwrap().log2() / 8.0).ceil() as usize; + + if imm_size > 1 { + for (_label, label_value) in self.declared_labels.iter_mut() { + let labeldef = label_value.as_ref().unwrap(); + self.concrete_len += imm_size - 1; + *label_value = Some(LabelDef { + position: labeldef.position + imm_size - 1, + updated: true, + }); + } + } + } + } + } + + Ok(()) + } + + /// Backpatch variable-sized operations and emit the assembled program. /// /// This function performs the final steps in the assembly process. It ensures that all labels - /// and dynamic elements in the code have been properly resolved and finalized. This includes + /// and variable-sized ops in the code have been properly resolved and finalized. This includes /// handling variable-sized push instructions, where the actual size of the push may not be known /// until all labels and expressions have been evaluated. /// @@ -409,8 +440,17 @@ impl Assembler { } .fail(); } + self.backpatch_labels()?; + let output = match self.emit_bytecode() { + Ok(value) => value, + Err(value) => return value, + }; + + Ok(output) + } + + fn emit_bytecode(&mut self) -> Result, Result, Error>> { let mut output = Vec::new(); - let mut acum = 0; for op in self.ready.iter() { let op = match op { RawOp::Op(ref op) => op, @@ -425,103 +465,28 @@ impl Assembler { .clone() .concretize((&self.declared_labels, &self.declared_macros).into()) { - Ok(mut cop) => { - // If the push is variable-sized, we need to recalculate (backpatch) the size of the - // push based on the final resolved value of the label/expression. - if let AbstractOp::Push(imm) = op { - // Evaluate the expression with the current context. - let exp = imm.tree.eval_with_context( - (&self.declared_labels, &self.declared_macros).into(), - ); - - let labels: HashSet = imm - .tree - .labels(&self.declared_macros) - .unwrap() - .into_iter() - .collect(); - - // If the expression contains any labels, we need to adjust the size of the - // push operation to account for the final resolved value of the expression. - if !labels.is_empty() { - if let Ok(val) = exp { - // We calculate the real size of the push operation based on the - // final resolved value of the expression. - let push_size = - (val.to_f64().unwrap().log2() / 8.0).ceil() as usize; - - if push_size > 1 { - // If the size of the push operation is greater than 1, we need - // to adjust the size to account for the final resolved value of the expression. - for (label, label_value) in self.declared_labels.iter_mut() { - let labeldef = label_value.as_ref().unwrap(); - // If the label has already been updated, we skip it. - if labeldef.updated { - continue; - } - - let pos = labeldef.position; - // We check if the label is used in a dynamic push. - if let Some(times) = - self.labeled_dynamic_pushes.get_mut(label) - { - // If the label is used in a dynamic push, we need to - // multiply the final resolved value of the label by - // the number of times the label is used in dynamic - // pushes (to update the size of the bytecode accordingly). - self.concrete_len += (push_size - 1) * *times; - *label_value = Some(LabelDef { - position: pos + *times * (push_size - 1), - updated: true, - }); - acum += *times * (push_size - 1); - } else { - // If the label is not used in a dynamic push, we - // simply update the size of the push operation. - // Since ´declared_labels' is ordered by insertion - // we can simply add the accumulated size to the - // actual position of the label. - *label_value = Some(LabelDef { - position: pos + acum, - updated: true, - }); - } - } - } - } - } - - // Recreate the push operation with the new size. - cop = op - .clone() - .concretize((&self.declared_labels, &self.declared_macros).into()) - .unwrap(); - } - cop.assemble(&mut output) - } + Ok(cop) => cop.assemble(&mut output), Err(ops::Error::ContextIncomplete { source: UnknownLabel { .. }, }) => { - return error::UndeclaredLabels { + return Err(error::UndeclaredLabels { labels: self.undeclared_labels.iter().cloned().collect::>(), } - .fail(); + .fail()); } Err(ops::Error::ContextIncomplete { source: UnknownMacro { name, .. }, }) => { - return error::UndeclaredInstructionMacro { name }.fail(); + return Err(error::UndeclaredInstructionMacro { name }.fail()); } Err(ops::Error::ContextIncomplete { source: UndefinedVariable { name, .. }, }) => { - return error::UndeclaredVariableMacro { var: name }.fail(); + return Err(error::UndeclaredVariableMacro { var: name }.fail()); } - Err(_) => unreachable!("all ops should be concretizable"), } } - Ok(output) } @@ -1281,7 +1246,8 @@ mod tests { AbstractOp::Label("bar".into()), ]; let result = asm.assemble(&ops)?; - assert_eq!(result, hex!("61010961000961010a5a5a")); + + assert_eq!(result, hex!("61010a61000a61010b5a5a")); Ok(()) } diff --git a/etk-asm/tests/asm.rs b/etk-asm/tests/asm.rs index b38ca19f..20ad4678 100644 --- a/etk-asm/tests/asm.rs +++ b/etk-asm/tests/asm.rs @@ -294,7 +294,7 @@ fn every_op() -> Result<(), Error> { } #[test] -fn test_dynamic_push_and_include() -> Result<(), Error> { +fn test_variable_sized_push_and_include() -> Result<(), Error> { let mut output = Vec::new(); let mut ingester = Ingest::new(&mut output); ingester.ingest_file(source(&["variable-push", "main.etk"]))?; @@ -305,7 +305,7 @@ fn test_dynamic_push_and_include() -> Result<(), Error> { } #[test] -fn test_dynamic_push2() -> Result<(), Error> { +fn test_variable_sized_push2() -> Result<(), Error> { let mut output = Vec::new(); let mut ingester = Ingest::new(&mut output); ingester.ingest_file(source(&["variable-push2", "main1.etk"]))?; From d9b193f6ee8454d9cc180c8c0efc31875ecc5012 Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Sat, 23 Dec 2023 10:13:34 -0300 Subject: [PATCH 17/19] rustfmt --- etk-asm/src/asm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index 1f7c00db..87949033 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -363,7 +363,7 @@ impl Assembler { self.concrete_len += op.size().unwrap(); } - self.undeclared_labels.extend(labels.into_iter()); + self.undeclared_labels.extend(labels); self.ready.push(rop.clone()); } Err(ops::Error::ContextIncomplete { From 0a468224d381d0aa764b5883ab2551bd6b783d43 Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Tue, 16 Jan 2024 15:30:30 -0300 Subject: [PATCH 18/19] BigInt in imm_size and better ceiling --- etk-asm/src/asm.rs | 11 ++++++----- etk-asm/tests/asm.rs | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/etk-asm/src/asm.rs b/etk-asm/src/asm.rs index 87949033..622308e3 100644 --- a/etk-asm/src/asm.rs +++ b/etk-asm/src/asm.rs @@ -144,7 +144,7 @@ pub use self::error::Error; use crate::ops::expression::Error::{UndefinedVariable, UnknownLabel, UnknownMacro}; use crate::ops::{self, AbstractOp, Assemble, Expression, MacroDefinition}; use indexmap::IndexMap; -use num_traits::ToPrimitive; +use num_bigint::BigInt; use rand::Rng; use std::collections::{hash_map, HashMap, HashSet}; @@ -397,14 +397,15 @@ impl Assembler { .eval_with_context((&self.declared_labels, &self.declared_macros).into()); if let Ok(val) = exp { - let imm_size = (val.to_f64().unwrap().log2() / 8.0).ceil() as usize; + let val_bits = BigInt::bits(&val).max(1); + let imm_size = 1 + ((val_bits - 1) / 8); if imm_size > 1 { - for (_label, label_value) in self.declared_labels.iter_mut() { + for label_value in self.declared_labels.values_mut() { let labeldef = label_value.as_ref().unwrap(); - self.concrete_len += imm_size - 1; + self.concrete_len += imm_size as usize - 1; *label_value = Some(LabelDef { - position: labeldef.position + imm_size - 1, + position: labeldef.position + imm_size as usize - 1, updated: true, }); } diff --git a/etk-asm/tests/asm.rs b/etk-asm/tests/asm.rs index 20ad4678..8ef77aa2 100644 --- a/etk-asm/tests/asm.rs +++ b/etk-asm/tests/asm.rs @@ -314,7 +314,7 @@ fn test_variable_sized_push2() -> Result<(), Error> { let mut output = Vec::new(); let mut ingester = Ingest::new(&mut output); ingester.ingest_file(source(&["variable-push2", "main2.etk"]))?; - assert_eq!(output, hex!("61010058")); + assert_eq!(output, hex!("61010158")); let mut output = Vec::new(); let mut ingester = Ingest::new(&mut output); From 8a6ac3abe6d1deae461f2cac8d36a3887135100c Mon Sep 17 00:00:00 2001 From: Gaston Zanitti Date: Tue, 16 Jan 2024 15:49:13 -0300 Subject: [PATCH 19/19] Rustfmt --- etk-dasm/src/blocks/annotated.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etk-dasm/src/blocks/annotated.rs b/etk-dasm/src/blocks/annotated.rs index 382c17e3..368cd20f 100644 --- a/etk-dasm/src/blocks/annotated.rs +++ b/etk-dasm/src/blocks/annotated.rs @@ -127,7 +127,7 @@ impl AnnotatedBlock { pub fn annotate(basic: &BasicBlock) -> Self { let jump_target = basic .ops - .get(0) + .first() .map(Operation::is_jump_target) .unwrap_or_default();