From 54cb1a58fd20774493e331b716dda76c06d40fe8 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 14 Jun 2023 17:52:51 +0200 Subject: [PATCH 1/2] Fix memory access. --- .../machines/double_sorted_witness_machine.rs | 17 ++++++++++++++--- riscv/runtime/src/allocator.rs | 2 +- riscv/src/compiler.rs | 2 +- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/executor/src/witgen/machines/double_sorted_witness_machine.rs b/executor/src/witgen/machines/double_sorted_witness_machine.rs index 7069a2e2f..bdfd28742 100644 --- a/executor/src/witgen/machines/double_sorted_witness_machine.rs +++ b/executor/src/witgen/machines/double_sorted_witness_machine.rs @@ -9,7 +9,7 @@ use crate::witgen::affine_expression::AffineResult; use crate::witgen::util::is_simple_poly_of_name; use crate::witgen::{EvalError, EvalResult, FixedData}; use crate::witgen::{EvalValue, IncompleteCause}; -use number::FieldElement; +use number::{DegreeType, FieldElement}; use pil_analyzer::{Expression, Identity, IdentityKind, PolynomialReference, SelectedExpressions}; @@ -17,6 +17,7 @@ use pil_analyzer::{Expression, Identity, IdentityKind, PolynomialReference, Sele #[derive(Default)] pub struct DoubleSortedWitnesses { + degree: DegreeType, //key_col: String, /// Position of the witness columns in the data. /// The key column has a position of usize::max @@ -33,7 +34,7 @@ struct Operation { impl DoubleSortedWitnesses { pub fn try_new( - _fixed_data: &FixedData, + fixed_data: &FixedData, _identities: &[&Identity], witness_cols: &HashSet<&PolynomialReference>, ) -> Option> { @@ -54,7 +55,10 @@ impl DoubleSortedWitnesses { .next() .is_none() { - Some(Box::default()) + Some(Box::new(Self { + degree: fixed_data.degree, + ..Default::default() + })) } else { None } @@ -174,6 +178,13 @@ impl DoubleSortedWitnesses { left[0], right.expressions[0] ) })?; + if addr.to_degree() >= self.degree { + return Err(format!( + "Memory access to too large address: 0x{addr:x} (must be less than 0x{:x})", + self.degree + ) + .into()); + } let step = left[1] .constant_value() .ok_or_else(|| format!("Step must be known: {} = {}", left[1], right.expressions[1]))?; diff --git a/riscv/runtime/src/allocator.rs b/riscv/runtime/src/allocator.rs index 489b2b6e4..32403c873 100644 --- a/riscv/runtime/src/allocator.rs +++ b/riscv/runtime/src/allocator.rs @@ -53,7 +53,7 @@ unsafe impl GlobalAlloc for FixedMemoryAllocator { } #[global_allocator] -static mut GLOBAL: FixedMemoryAllocator<{ 1024 * 1024 * 1024 }> = FixedMemoryAllocator::new(); +static mut GLOBAL: FixedMemoryAllocator<{ 0x2000 }> = FixedMemoryAllocator::new(); #[alloc_error_handler] fn alloc_error(layout: Layout) -> ! { diff --git a/riscv/src/compiler.rs b/riscv/src/compiler.rs index 5a90303bb..5467e14ca 100644 --- a/riscv/src/compiler.rs +++ b/riscv/src/compiler.rs @@ -13,7 +13,7 @@ pub fn compile_riscv_asm(mut assemblies: BTreeMap) -> String { // stack grows towards zero let stack_start = 0x10000; // data grows away from zero - let data_start = 0x20000; + let data_start = 0x10100; assert!(assemblies .insert("__runtime".to_string(), runtime().to_string()) From ed79e49540d11677553b3b457fbee0d792887775 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 14 Jun 2023 18:15:01 +0200 Subject: [PATCH 2/2] Move single large object to the end. --- riscv/runtime/src/allocator.rs | 7 +++++-- riscv/src/compiler.rs | 13 ++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/riscv/runtime/src/allocator.rs b/riscv/runtime/src/allocator.rs index 32403c873..7fd933c8a 100644 --- a/riscv/runtime/src/allocator.rs +++ b/riscv/runtime/src/allocator.rs @@ -8,9 +8,12 @@ use core::{ ptr::{self, addr_of}, }; +// Force C representation so that the large buffer is at the end. +// This might avoid access to memory with large gaps. +#[repr(C)] struct FixedMemoryAllocator { - mem_buffer: [u8; SIZE], next_available: Cell, + mem_buffer: [u8; SIZE], } impl FixedMemoryAllocator { @@ -53,7 +56,7 @@ unsafe impl GlobalAlloc for FixedMemoryAllocator { } #[global_allocator] -static mut GLOBAL: FixedMemoryAllocator<{ 0x2000 }> = FixedMemoryAllocator::new(); +static mut GLOBAL: FixedMemoryAllocator<{ 1024 * 1024 * 1024 }> = FixedMemoryAllocator::new(); #[alloc_error_handler] fn alloc_error(layout: Layout) -> ! { diff --git a/riscv/src/compiler.rs b/riscv/src/compiler.rs index 5467e14ca..adc8694b0 100644 --- a/riscv/src/compiler.rs +++ b/riscv/src/compiler.rs @@ -25,7 +25,7 @@ pub fn compile_riscv_asm(mut assemblies: BTreeMap) -> String { .map(|(name, contents)| (name, parser::parse_asm(&contents))) .collect(), ); - let (mut objects, object_order) = data_parser::extract_data_objects(&statements); + let (mut objects, mut object_order) = data_parser::extract_data_objects(&statements); // Reduce to the code that is actually reachable from main // (and the objects that are referred from there) @@ -35,6 +35,17 @@ pub fn compile_riscv_asm(mut assemblies: BTreeMap) -> String { replace_dynamic_label_references(&mut statements, &objects); // Sort the objects according to the order of the names in object_order. + // With the single exception: If there is large object, put that at the end. + // The idea behind this is that there might be a single gigantic object representing the heap + // and putting that at the end should keep memory addresses small. + let mut large_objects = objects + .iter() + .filter(|(_name, data)| data.iter().map(|d| d.size()).sum::() > 0x2000); + if let (Some((heap, _)), None) = (large_objects.next(), large_objects.next()) { + let heap_pos = object_order.iter().position(|o| o == heap).unwrap(); + object_order.remove(heap_pos); + object_order.push(heap.clone()); + }; let sorted_objects = object_order .into_iter() .filter_map(|n| {