Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions crates/bytecode/src/legacy/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,16 @@ pub fn analyze_legacy(bytecode: Bytes) -> (JumpTable, Bytes) {
iterator = unsafe { iterator.add(1) };
} else {
let push_offset = opcode.wrapping_sub(opcode::PUSH1);
if push_offset < 32 {
// SAFETY: Iterator access range is checked in the while loop
iterator = unsafe { iterator.add(push_offset as usize + 2) };
let dupn_offset = opcode.wrapping_sub(opcode::DUPN);
let skip = if push_offset < 32 {
2 + push_offset as usize
} else if dupn_offset < 3 {
2
} else {
// SAFETY: Iterator access range is checked in the while loop
iterator = unsafe { iterator.add(1) };
}
1
};
// SAFETY: Iterator access range is checked in the while loop
iterator = unsafe { iterator.add(skip) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this can generate a different jumptable for previous bytecode, as we are now jumping over one more immediate

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, will need to rework this so jump table is same as previous bytecodes.

}
}

Expand Down
14 changes: 9 additions & 5 deletions crates/bytecode/src/opcode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,9 +623,9 @@ opcodes! {
// 0xE3
// 0xE4
// 0xE5
// 0xE6
// 0xE7
// 0xE8
0xE6 => DUPN => stack_io(0, 1), immediate_size(1);
0xE7 => SWAPN => stack_io(0, 0), immediate_size(1);
0xE8 => EXCHANGE => stack_io(0, 0), immediate_size(1);
Comment on lines +626 to +628
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stack_io values are not accurate here, they depend on the immediate, unless we want to put the worst case values here. I don't know what this is used for.

// 0xE9
// 0xEA
// 0xEB
Expand Down Expand Up @@ -668,11 +668,15 @@ mod tests {
#[test]
fn test_immediate_size() {
let mut expected = [0u8; 256];
// PUSH opcodes

for push in PUSH1..=PUSH32 {
expected[push as usize] = push - PUSH1 + 1;
}

for stack_op in [DUPN, SWAPN, EXCHANGE] {
expected[stack_op as usize] = 1;
}

for (i, opcode) in OPCODE_INFO.iter().enumerate() {
if let Some(opcode) = opcode {
assert_eq!(
Expand Down Expand Up @@ -718,7 +722,7 @@ mod tests {
for _ in OPCODE_INFO.into_iter().flatten() {
opcode_num += 1;
}
assert_eq!(opcode_num, 150);
assert_eq!(opcode_num, 153);
}

#[test]
Expand Down
6 changes: 6 additions & 0 deletions crates/interpreter/src/instruction_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ pub enum InstructionResult {
CreateInitCodeSizeLimit,
/// Fatal external error. Returned by database.
FatalExternalError,
/// Invalid encoding of an instruction's immediate operand.
InvalidImmediateEncoding,
}

impl From<TransferError> for InstructionResult {
Expand Down Expand Up @@ -190,6 +192,7 @@ macro_rules! return_error {
| $crate::InstructionResult::CreateContractStartingWithEF
| $crate::InstructionResult::CreateInitCodeSizeLimit
| $crate::InstructionResult::FatalExternalError
| $crate::InstructionResult::InvalidImmediateEncoding
};
}

Expand Down Expand Up @@ -348,6 +351,9 @@ impl<HaltReasonTr: From<HaltReason>> From<InstructionResult> for SuccessOrHalt<H
InstructionResult::InvalidExtDelegateCallTarget => {
Self::Internal(InternalResult::InvalidExtDelegateCallTarget)
}
InstructionResult::InvalidImmediateEncoding => {
Self::Halt(HaltReason::OpcodeNotFound.into())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to add a new HaltReason for this because IIUC it would be a breaking change for implementers of From<HaltReason>.

}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/interpreter/src/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,10 @@ const fn instruction_table_impl<WIRE: InterpreterTypes, H: Host>() -> [Instructi
table[SWAP15 as usize] = Instruction::new(stack::swap::<15, _, _>, 3);
table[SWAP16 as usize] = Instruction::new(stack::swap::<16, _, _>, 3);

table[DUPN as usize] = Instruction::new(stack::dupn, 3);
table[SWAPN as usize] = Instruction::new(stack::swapn, 3);
table[EXCHANGE as usize] = Instruction::new(stack::exchange, 3);

table[LOG0 as usize] = Instruction::new(host::log::<0, _>, gas::LOG);
table[LOG1 as usize] = Instruction::new(host::log::<1, _>, gas::LOG);
table[LOG2 as usize] = Instruction::new(host::log::<2, _>, gas::LOG);
Expand Down
174 changes: 173 additions & 1 deletion crates/interpreter/src/instructions/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ pub fn push<const N: usize, WIRE: InterpreterTypes, H: ?Sized>(
return;
}

// Can ignore return. as relative N jump is safe operation
context.interpreter.bytecode.relative_jump(N as isize);
}

Expand All @@ -60,3 +59,176 @@ pub fn swap<const N: usize, WIRE: InterpreterTypes, H: ?Sized>(
context.interpreter.halt(InstructionResult::StackOverflow);
}
}

/// Implements the DUPN instruction.
///
/// Duplicates the Nth stack item to the top of the stack, with N given by an immediate.
pub fn dupn<WIRE: InterpreterTypes, H: ?Sized>(context: InstructionContext<'_, H, WIRE>) {
check!(context.interpreter, AMSTERDAM);
let x: usize = context.interpreter.bytecode.read_u8().into();
if let Some(n) = decode_single(x) {
if !context.interpreter.stack.dup(n) {
context.interpreter.halt(InstructionResult::StackOverflow);
}
context.interpreter.bytecode.relative_jump(1);
} else {
context
.interpreter
.halt(InstructionResult::InvalidImmediateEncoding);
}
}

/// Implements the SWAPN instruction.
///
/// Swaps the top stack item with the N+1th stack item, with N given by an immediate.
pub fn swapn<WIRE: InterpreterTypes, H: ?Sized>(context: InstructionContext<'_, H, WIRE>) {
check!(context.interpreter, AMSTERDAM);
let x: usize = context.interpreter.bytecode.read_u8().into();
if let Some(n) = decode_single(x) {
if !context.interpreter.stack.exchange(0, n) {
context.interpreter.halt(InstructionResult::StackOverflow);
}
context.interpreter.bytecode.relative_jump(1);
} else {
context
.interpreter
.halt(InstructionResult::InvalidImmediateEncoding);
}
}

/// Implements the EXCHANGE instruction.
///
/// Swaps the N+1th stack item with the M+1th stack item, with N, M given by an immediate.
pub fn exchange<WIRE: InterpreterTypes, H: ?Sized>(context: InstructionContext<'_, H, WIRE>) {
check!(context.interpreter, AMSTERDAM);
let x: usize = context.interpreter.bytecode.read_u8().into();
if let Some((n, m)) = decode_pair(x) {
if !context.interpreter.stack.exchange(n, m - n) {
context.interpreter.halt(InstructionResult::StackOverflow);
}
context.interpreter.bytecode.relative_jump(1);
} else {
context
.interpreter
.halt(InstructionResult::InvalidImmediateEncoding);
}
}

fn decode_single(x: usize) -> Option<usize> {
if x <= 90 {
Some(x + 17)
} else if x >= 128 {
Some(x - 20)
} else {
None
}
}

fn decode_pair(x: usize) -> Option<(usize, usize)> {
let k = if x <= 79 {
x
} else if x >= 128 {
x - 48
} else {
return None;
};
let q = k / 16;
let r = k % 16;
if q < r {
Some((q + 1, r + 1))
} else {
Some((r + 1, 29 - q))
}
}

#[cfg(test)]
mod tests {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test cases are the same listed in the EIP.

use crate::{
gas::params::GasParams,
host::DummyHost,
instructions::instruction_table,
interpreter::{EthInterpreter, ExtBytecode, InputsImpl, SharedMemory},
interpreter_types::LoopControl,
Interpreter,
};
use bytecode::opcode::*;
use bytecode::Bytecode;
use primitives::{hardfork::SpecId, Bytes, U256};

fn run_bytecode(code: &[u8]) -> Interpreter {
let bytecode = Bytecode::new_raw(Bytes::copy_from_slice(code));
let mut interpreter = Interpreter::<EthInterpreter>::new(
SharedMemory::new(),
ExtBytecode::new(bytecode),
InputsImpl::default(),
false,
SpecId::AMSTERDAM,
u64::MAX,
GasParams::default(),
);
let table = instruction_table::<EthInterpreter, DummyHost>();
let mut host = DummyHost;
interpreter.run_plain(&table, &mut host);
interpreter
}

#[test]
fn test_dupn() {
let interpreter = run_bytecode(&[
PUSH1, 0x01, PUSH1, 0x00, DUP1, DUP1, DUP1, DUP1, DUP1, DUP1, DUP1, DUP1, DUP1, DUP1,
DUP1, DUP1, DUP1, DUP1, DUP1, DUPN, 0x00,
]);
assert_eq!(interpreter.stack.len(), 18);
assert_eq!(interpreter.stack.data()[17], U256::from(1));
assert_eq!(interpreter.stack.data()[0], U256::from(1));
for i in 1..17 {
assert_eq!(interpreter.stack.data()[i], U256::ZERO);
}
}

#[test]
fn test_swapn() {
let interpreter = run_bytecode(&[
PUSH1, 0x01, PUSH1, 0x00, DUP1, DUP1, DUP1, DUP1, DUP1, DUP1, DUP1, DUP1, DUP1, DUP1,
DUP1, DUP1, DUP1, DUP1, DUP1, PUSH1, 0x02, SWAPN, 0x00,
]);
assert_eq!(interpreter.stack.len(), 18);
assert_eq!(interpreter.stack.data()[17], U256::from(1));
assert_eq!(interpreter.stack.data()[0], U256::from(2));
for i in 1..17 {
assert_eq!(interpreter.stack.data()[i], U256::ZERO);
}
}

#[test]
fn test_exchange() {
let interpreter = run_bytecode(&[PUSH1, 0x00, PUSH1, 0x01, PUSH1, 0x02, EXCHANGE, 0x01]);
assert_eq!(interpreter.stack.len(), 3);
assert_eq!(interpreter.stack.data()[2], U256::from(2));
assert_eq!(interpreter.stack.data()[1], U256::from(0));
assert_eq!(interpreter.stack.data()[0], U256::from(1));
}

#[test]
fn test_swapn_invalid_immediate() {
let mut interpreter = run_bytecode(&[SWAPN, JUMPDEST]);
assert!(interpreter.bytecode.instruction_result().is_none());
}

#[test]
fn test_jump_over_invalid_dupn() {
let interpreter = run_bytecode(&[PUSH1, 0x04, JUMP, DUPN, JUMPDEST]);
assert!(interpreter.bytecode.is_not_end());
}

#[test]
fn test_exchange_with_iszero() {
let interpreter = run_bytecode(&[
PUSH1, 0x00, PUSH1, 0x00, PUSH1, 0x00, EXCHANGE, 0x01, ISZERO,
]);
assert_eq!(interpreter.stack.len(), 3);
assert_eq!(interpreter.stack.data()[2], U256::from(1));
assert_eq!(interpreter.stack.data()[1], U256::ZERO);
assert_eq!(interpreter.stack.data()[0], U256::ZERO);
}
}
1 change: 1 addition & 0 deletions crates/interpreter/src/interpreter_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ pub trait StackTr {
/// Exchanges two values on the stack.
///
/// Indexes are based from the top of the stack.
/// `n` is the first index, and the second index is calculated as `n + m`.
///
/// Returns `true` if swap was successful, `false` if stack underflow.
#[must_use]
Expand Down
Loading