Skip to content
This repository has been archived by the owner on Feb 21, 2024. It is now read-only.

Commit

Permalink
Fix bugs in jumpdest analysis (0xPolygonZero#1474)
Browse files Browse the repository at this point in the history
* Fix simulation for jumpdest analysis

* Fix bugs in jumpdest analysis

* Update evm/src/cpu/kernel/asm/core/jumpdest_analysis.asm

Co-authored-by: Robin Salen <[email protected]>

* Address reviews

---------

Co-authored-by: Robin Salen <[email protected]>
  • Loading branch information
4l0n50 and Nashtare authored Jan 24, 2024
1 parent c070006 commit ca2e56e
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 105 deletions.
126 changes: 66 additions & 60 deletions evm/src/cpu/kernel/asm/core/jumpdest_analysis.asm
Original file line number Diff line number Diff line change
Expand Up @@ -164,106 +164,108 @@ global write_table_if_jumpdest:
// stack: (is_1_at_1 or is_0_at_2_or_3|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
// stack: (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest

// Compute in_range =
// - (0xFF|X⁷)³² for the first 15 bytes
// - (has_prefix => is_0_at_4 |X⁷)³² for the next 15 bytes
// - (~has_prefix|X⁷)³² for the last byte
// Compute also ~has_prefix = ~has_prefix OR is_0_at_4 for all bytes. We don't need to update ~has_prefix
// for the second half but it takes less cycles if we do it.
// Compute in_range and has_prefix' =
// - in_range = (0xFF|X⁷)³² and ~has_prefix' = ~has_prefix OR is_0_at_4, for the first 15 bytes
// - in_range = (has_prefix => is_0_at_4 |X⁷)³² and ~has_prefix' = ~has_prefix, for the next 15 bytes
// - in_range = (~has_prefix|X⁷)³² and ~has_prefix' = ~has_prefix, for the last byte.
DUP2 %shl_const(3)
NOT
// stack: (is_0_at_4|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00
AND
// stack: (is_0_at_4|X⁷)³¹|0, (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP2
DUP2
OR
DUP1
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000
AND
// stack: (is_0_at_4|X⁷)¹⁵|(0⁸)¹⁷, (is_0_at_4|X⁷)³¹|0, (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP3
OR
// stack: (in_range|X⁷)³², (is_0_at_4|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
// (~has_prefix'|X⁷)³², (is_0_at_4|X⁷)³¹|0, (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
SWAP2
OR
// stack: (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000
OR
// stack: (in_range|X⁷)³², (~has_prefix'|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest

// Compute in_range' = in_range AND
// - (0xFF|X⁷)³² for bytes in positions 1-7 and 16-23
// - (has_prefix => is_0_at_5 |X⁷)³² on the rest
// Compute also ~has_prefix = ~has_prefix OR is_0_at_5 for all bytes.
// Compute in_range' and ~has_prefix as
// - in_range' = in_range and has_prefix' = ~has_prefix OR is_0_at_5, for bytes in positions 1-7 and 16-23
// - in_range' = in_range AND (has_prefix => is_0_at_5 |X⁷)³² and has_prefix' = ~has_prefix, for the rest.

DUP3 %shl_const(4)
NOT
// stack: (is_0_at_5|X⁷)³², (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP2
DUP2
OR
// stack: (is_0_at_5|X⁷)³², (in_range|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP1
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0xFFFFFFFFFFFFFF0000000000000000FFFFFFFFFFFFFFFF000000000000000000
AND
// stack: (is_0_at_5|X⁷)⁷|(0⁸)⁸|(is_0_at_5|X⁷)⁸|(0⁸)⁸, (is_0_at_5|X⁷)³², (in_range|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP4
OR
// stack: (in_range'|X⁷)³², (is_0_at_5|X⁷)³², (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
SWAP2
// stack: (~has_prefix'|X⁷)³², (is_0_at_5|X⁷)³², (in_range|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
SWAP3
OR
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0xFFFFFFFFFFFFFF0000000000000000FFFFFFFFFFFFFFFF000000000000000000
OR
// stack: (~has_prefix|X⁷)³², (in_range'|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
SWAP2
AND
SWAP1

// Compute in_range' = in_range AND
// - (0xFF|X⁷)³² for bytes in positions 1-3, 8-11, 16-19, and 24-27
// - (has_prefix => is_0_at_6 |X⁷)³² on the rest
// Compute also that ~has_prefix = ~has_prefix OR is_0_at_4 for all bytes.
// stack: (in_range'|X⁷)³², (~has_prefix'|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest

// stack: (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
// Compute in_range' and ~has_prefix' as
// - in_range' = in_range and ~has_prefix' = ~has_prefix OR is_0_at_6, for bytes in positions 1-3, 8-11, 16-19, and 24-27
// - in_range' = in_range AND (has_prefix => is_0_at_6 |X⁷)³² and ~has_prefix' = has_prefix, for the rest.
DUP3 %shl_const(5)
NOT
// stack: (is_0_at_6|X⁷)³², (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP2
DUP2
OR
// stack: (is_0_at_6|X⁷)³², (in_range|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP1
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0xFFFFFF00000000FFFFFFFF00000000FFFFFFFF00000000FFFFFFFF0000000000
AND
// stack: (is_0_at_6|X⁷)³|(0⁸)⁴|((is_0_at_6|X⁷)⁴|(0⁸)⁴)³, (is_0_at_6|X⁷)³², (in_range|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP4
OR
// stack: (in_range'|X⁷)³², (is_0_at_6|X⁷)³², (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
SWAP2
// stack: (~has_prefix'|X⁷)³², (is_0_at_6|X⁷)³², (in_range|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
SWAP3
OR
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0xFFFFFF00000000FFFFFFFF00000000FFFFFFFF00000000FFFFFFFF0000000000
OR
// stack: (~has_prefix|X⁷)³², (in_range'|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
SWAP2
AND
SWAP1

// Compute in_range' = in_range AND
// - (0xFF|X⁷)³² for bytes in 1, 4-5, 8-9, 12-13, 16-17, 20-21, 24-25, 28-29
// - (has_prefix => is_0_at_7 |X⁷)³² on the rest
// Compute also that ~has_prefix = ~has_prefix OR is_0_at_7 for all bytes.
// stack: (in_range'|X⁷)³², (~has_prefix'|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest

// stack: (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
// Compute in_range' and ~has_prefix' as
// - in_range' = in_range and ~has_prefix' = has_prefix OR is_0_at_7, for bytes in 1, 4-5, 8-9, 12-13, 16-17, 20-21, 24-25, 28-29
// - in_range' = in_range AND (has_prefix => is_0_at_7 |X⁷)³² and ~has_prefix' = ~has_prefix, for the rest.
DUP3 %shl_const(6)
NOT
// stack: (is_0_at_7|X⁷)³², (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP2
DUP2
OR
// stack: (is_0_at_7|X⁷)³², (in_range|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP1
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0xFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF000000
AND
// stack: is_0_at_7|X⁷|(0⁸)²|((is_0_at_7|X⁷)²|(0⁸)²)⁷, (is_0_at_7|X⁷)³², (in_range|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP4
OR
// stack: (in_range'|X⁷)³², (is_0_at_7|X⁷)³², (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
SWAP2
// (~has_prefix'|X⁷)³², (is_0_at_7|X⁷)³², (in_range|X⁷)³², (~has_prefix|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
SWAP3
OR
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0xFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF0000FFFF000000
OR
// stack: (~has_prefix|X⁷)³², (in_range'|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
SWAP2
AND
SWAP1
// stack: (in_range'|X⁷)³², (~has_prefix'|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest

// Compute in_range' = in_range AND
// - (0xFF|X⁷)³² for bytes in odd positions
// - (has_prefix => is_0_at_8 |X⁷)³² on the rest
// Compute in_range' as
// - in_range' = in_range, for odd positions
// - in_range' = in_range AND (has_prefix => is_0_at_8 |X⁷)³², for the rest

SWAP1
// stack: (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
DUP3 %shl_const(7)
NOT
// stack: (is_0_at_8|X⁷)³², (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
// stack: (is_0_at_8|X⁷)³², (~has_prefix|X⁷)³², (in_range|X⁷)³², packed_opcodes, proof_prefix_addr, ctx, jumpdest, retdest
OR
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0x00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF00FF
Expand All @@ -275,14 +277,18 @@ global write_table_if_jumpdest:
// pos 0102030405060708091011121314151617181920212223242526272829303132
PUSH 0x8080808080808080808080808080808080808080808080808080808080808080
AND
%jump_neq_const(0x8080808080808080808080808080808080808080808080808080808080808080, return)
%jump_neq_const(0x8080808080808080808080808080808080808080808080808080808080808080, return_pop_opcode)
POP
%add_const(32)

// check the remaining path
%jump(verify_path_and_write_jumpdest_table)
return_pop_opcode:
POP
return:
// stack: proof_prefix_addr, jumpdest, ctx, retdest
// stack: proof_prefix_addr, ctx, jumpdest, retdest
// or
// stack: jumpdest, ctx, proof_prefix_addr, retdest
%pop3
JUMP

Expand Down
24 changes: 10 additions & 14 deletions evm/src/cpu/kernel/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,14 @@ impl<'a> Interpreter<'a> {
.contexts
.push(MemoryContextState::default());
}
self.generation_state.memory.set(
MemoryAddress::new(
context,
Segment::ContextMetadata,
ContextMetadata::CodeSize.unscale(),
),
code.len().into(),
);
self.generation_state.memory.contexts[context].segments[Segment::Code.unscale()].content =
code.into_iter().map(U256::from).collect();
}
Expand All @@ -584,20 +592,8 @@ impl<'a> Interpreter<'a> {
.collect()
}

pub(crate) fn set_jumpdest_bits(&mut self, context: usize, jumpdest_bits: Vec<bool>) {
self.generation_state.memory.contexts[context].segments[Segment::JumpdestBits.unscale()]
.content = jumpdest_bits.iter().map(|&x| u256_from_bool(x)).collect();
self.generation_state
.set_proofs_and_jumpdests(HashMap::from([(
context,
BTreeSet::from_iter(
jumpdest_bits
.into_iter()
.enumerate()
.filter(|&(_, x)| x)
.map(|(i, _)| i),
),
)]));
pub(crate) fn set_jumpdest_analysis_inputs(&mut self, jumps: HashMap<usize, BTreeSet<usize>>) {
self.generation_state.set_jumpdest_analysis_inputs(jumps);
}

pub(crate) fn incr(&mut self, n: usize) {
Expand Down
78 changes: 77 additions & 1 deletion evm/src/cpu/kernel/tests/core/jumpdest_analysis.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::{BTreeSet, HashMap};

use anyhow::Result;
use ethereum_types::U256;

Expand Down Expand Up @@ -37,10 +39,84 @@ fn test_jumpdest_analysis() -> Result<()> {
];
let mut interpreter = Interpreter::new_with_kernel(jumpdest_analysis, initial_stack);
interpreter.set_code(CONTEXT, code);
interpreter.set_jumpdest_bits(CONTEXT, jumpdest_bits);
interpreter.set_jumpdest_analysis_inputs(HashMap::from([(
3,
BTreeSet::from_iter(
jumpdest_bits
.iter()
.enumerate()
.filter(|&(_, &x)| x)
.map(|(i, _)| i),
),
)]));

assert_eq!(
interpreter.generation_state.jumpdest_table,
// Context 3 has jumpdest 1, 5, 7. All have proof 0 and hence
// the list [proof_0, jumpdest_0, ... ] is [0, 1, 0, 5, 0, 7]
Some(HashMap::from([(3, vec![0, 1, 0, 5, 0, 7])]))
);

interpreter.run()?;
assert_eq!(interpreter.stack(), vec![]);

assert_eq!(jumpdest_bits, interpreter.get_jumpdest_bits(3));

Ok(())
}

#[test]
fn test_packed_verification() -> Result<()> {
let jumpdest_analysis = KERNEL.global_labels["jumpdest_analysis"];
const CONTEXT: usize = 3; // arbitrary

let add = get_opcode("ADD");
let jumpdest = get_opcode("JUMPDEST");

// The last push(i=0) is 0x5f which is not a valid opcode. However, this
// is still meaningful for the test and makes things easier
let mut code: Vec<u8> = std::iter::once(add)
.chain(
(0..=31)
.rev()
.map(get_push_opcode)
.chain(std::iter::once(jumpdest)),
)
.collect();

let jumpdest_bits: Vec<bool> = std::iter::repeat(false)
.take(33)
.chain(std::iter::once(true))
.collect();

// Contract creation transaction.
let initial_stack = vec![
0xDEADBEEFu32.into(),
code.len().into(),
U256::from(CONTEXT) << CONTEXT_SCALING_FACTOR,
];
let mut interpreter = Interpreter::new_with_kernel(jumpdest_analysis, initial_stack.clone());
interpreter.set_code(CONTEXT, code.clone());
interpreter.generation_state.jumpdest_table = Some(HashMap::from([(3, vec![1, 33])]));

interpreter.run()?;

assert_eq!(jumpdest_bits, interpreter.get_jumpdest_bits(CONTEXT));

// If we add 1 to each opcode the jumpdest at position 32 is never a valid jumpdest
for i in 1..=32 {
code[i] += 1;
let mut interpreter =
Interpreter::new_with_kernel(jumpdest_analysis, initial_stack.clone());
interpreter.set_code(CONTEXT, code.clone());
interpreter.generation_state.jumpdest_table = Some(HashMap::from([(3, vec![1, 33])]));

interpreter.run()?;

assert!(interpreter.get_jumpdest_bits(CONTEXT).is_empty());

code[i] -= 1;
}

Ok(())
}
2 changes: 1 addition & 1 deletion evm/src/generation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ fn simulate_cpu_between_labels_and_get_user_jumps<F: Field>(
final_label: &str,
state: &mut GenerationState<F>,
) -> Option<HashMap<usize, BTreeSet<usize>>> {
if state.jumpdest_proofs.is_some() {
if state.jumpdest_table.is_some() {
None
} else {
const JUMP_OPCODE: u8 = 0x56;
Expand Down
Loading

0 comments on commit ca2e56e

Please sign in to comment.