Skip to content

Commit ca43482

Browse files
0xrinegadeclaude
andcommitted
fix(ovsm): Fix account field access and register clobbering bug
- Fix Solana serialization offsets: lamports at +72, data_len at +80, data at +88 - Fix critical register clobbering bug: next_reg now starts at 8 to skip reserved R6/R7 - Bump osvm to 0.9.7, ovsm to 1.0.5 The register clobbering bug caused the saved accounts pointer (R6) to be overwritten with 0 when allocating temp registers for constants. This fix ensures temp registers never conflict with reserved registers 6 and 7. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent e540aec commit ca43482

File tree

1,174 files changed

+32831
-68
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

1,174 files changed

+32831
-68
lines changed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ resolver = "2"
44

55
[package]
66
name = "osvm"
7-
version = "0.9.6"
7+
version = "0.9.7"
88
edition = "2021"
99
license = "MIT"
1010
description = "OpenSVM CLI tool for managing SVM nodes and deployments"

crates/ovsm/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "ovsm"
3-
version = "1.0.4"
3+
version = "1.0.5"
44
edition = "2021"
55
authors = ["OSVM Team <[email protected]>"]
66
description = "OVSM (Open Versatile Seeker Mind) language interpreter for blockchain automation and scripting"

crates/ovsm/src/compiler/ir.rs

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,13 @@ impl IrGenerator {
197197
self.var_map.insert("accounts".to_string(), saved_accounts);
198198
self.var_map.insert("instruction-data".to_string(), saved_instr_data);
199199

200+
// CRITICAL: Ensure next_reg skips past the reserved registers (6 and 7)
201+
// Otherwise alloc_reg() will return IrReg(6) or IrReg(7) for temporaries,
202+
// which will clobber the saved accounts/instruction-data pointers!
203+
if self.next_reg <= 7 {
204+
self.next_reg = 8;
205+
}
206+
200207
eprintln!("🔍 IR DEBUG: Generating IR for {} statements", program.statements.len());
201208

202209
// Generate IR for each statement, tracking last result
@@ -518,9 +525,26 @@ impl IrGenerator {
518525
}
519526

520527
// Handle (account-lamports idx) - get lamports for account at index
521-
// Solana format: after num_accounts (8 bytes), each account has:
522-
// [u8 dup][u8 signer][u8 writable][u8 exec][4 pad][32 pubkey][32 owner][u64 lamports]...
523-
// lamports offset = 8 + 1 + 1 + 1 + 1 + 4 + 32 + 32 = 80 from account start
528+
// Solana serialized format (from sol_deserialize in deserialize.h):
529+
// After num_accounts (8 bytes), each account entry:
530+
// +0: u8 dup_info (0xFF = new, else = index)
531+
// +1: u8 is_signer
532+
// +2: u8 is_writable
533+
// +3: u8 executable
534+
// +4: 4 bytes padding
535+
// +8: 32 bytes pubkey
536+
// +40: 32 bytes owner
537+
// +72: u64 lamports (THE VALUE, not a pointer!)
538+
// +80: u64 data_len
539+
// +88: data_len bytes of data
540+
// +88+data_len: 10240 bytes MAX_PERMITTED_DATA_INCREASE
541+
// +aligned: u64 rent_epoch
542+
//
543+
// IMPORTANT: Account size is VARIABLE due to data_len!
544+
// For idx=0, lamports is at offset 8 + 72 = 80 from input start
545+
// For subsequent accounts, we'd need to iterate and sum data_lens
546+
//
547+
// For now: only support account 0 correctly
524548
if name == "account-lamports" && args.len() == 1 {
525549
let idx_reg = self.generate_expr(&args[0].value)?
526550
.ok_or_else(|| Error::runtime("account-lamports index has no result"))?;
@@ -529,26 +553,34 @@ impl IrGenerator {
529553
let accounts_ptr = *self.var_map.get("accounts")
530554
.ok_or_else(|| Error::runtime("accounts not available"))?;
531555

532-
// Calculate account offset: 8 (for num_accounts) + idx * account_size
533-
// For simplicity, assume fixed account size of 165 bytes (minimum without data)
534-
// Real implementation would need to iterate
556+
// For account 0: skip num_accounts (8 bytes) + offset within account (72 bytes)
557+
// lamports offset from account start = 1+1+1+1+4+32+32 = 72
558+
// Total for account 0: 8 + 72 = 80
559+
//
560+
// For other accounts, we'd need to scan forward through variable-length entries
561+
// For now, approximate: each account has ~10334 bytes minimum (with MAX_PERMITTED_DATA_INCREASE)
562+
// But this is wrong for accounts with non-zero data_len
563+
//
564+
// Better approach: For account 0, use hardcoded offset 80
565+
// TODO: Implement proper iteration for account > 0
566+
535567
let eight_reg = self.alloc_reg();
536568
self.emit(IrInstruction::ConstI64(eight_reg, 8));
537569

538-
// For account 0: offset = 8, lamports at offset 80 from account start
539-
// Total: 8 + 80 = 88
570+
// For accounts with no data: 1+1+1+1+4+32+32+8+8+0+10240+padding(~8)+8 ≈ 10344
571+
// Use a large approximate size for subsequent accounts
540572
let account_size = self.alloc_reg();
541-
self.emit(IrInstruction::ConstI64(account_size, 165)); // Approximate
573+
self.emit(IrInstruction::ConstI64(account_size, 10344));
542574

543575
let account_offset = self.alloc_reg();
544576
self.emit(IrInstruction::Mul(account_offset, idx_reg, account_size));
545577

546578
let base_offset = self.alloc_reg();
547579
self.emit(IrInstruction::Add(base_offset, eight_reg, account_offset));
548580

549-
// Add lamports offset within account (80 bytes)
581+
// Add lamports offset within account (72 bytes, not 80!)
550582
let lamports_offset = self.alloc_reg();
551-
self.emit(IrInstruction::ConstI64(lamports_offset, 80));
583+
self.emit(IrInstruction::ConstI64(lamports_offset, 72));
552584

553585
let total_offset = self.alloc_reg();
554586
self.emit(IrInstruction::Add(total_offset, base_offset, lamports_offset));
@@ -561,8 +593,9 @@ impl IrGenerator {
561593
return Ok(Some(dst));
562594
}
563595

564-
// Handle (account-data-ptr idx) - get data pointer for account
565-
// data_len is at offset 88, data starts at offset 96
596+
// Handle (account-data-ptr idx) - get pointer to account data
597+
// Data starts at offset 88 from account start (after data_len at 80)
598+
// For account 0: 8 + 88 = 96 from input start
566599
if name == "account-data-ptr" && args.len() == 1 {
567600
let idx_reg = self.generate_expr(&args[0].value)?
568601
.ok_or_else(|| Error::runtime("account-data-ptr index has no result"))?;
@@ -573,18 +606,20 @@ impl IrGenerator {
573606
let eight_reg = self.alloc_reg();
574607
self.emit(IrInstruction::ConstI64(eight_reg, 8));
575608

609+
// Same approximate account size as lamports
576610
let account_size = self.alloc_reg();
577-
self.emit(IrInstruction::ConstI64(account_size, 165));
611+
self.emit(IrInstruction::ConstI64(account_size, 10344));
578612

579613
let account_offset = self.alloc_reg();
580614
self.emit(IrInstruction::Mul(account_offset, idx_reg, account_size));
581615

582616
let base_offset = self.alloc_reg();
583617
self.emit(IrInstruction::Add(base_offset, eight_reg, account_offset));
584618

585-
// Data starts at offset 96 from account start
619+
// Data starts at offset 88 from account start
620+
// = 1+1+1+1+4+32+32+8+8 = 88
586621
let data_offset = self.alloc_reg();
587-
self.emit(IrInstruction::ConstI64(data_offset, 96));
622+
self.emit(IrInstruction::ConstI64(data_offset, 88));
588623

589624
let total_offset = self.alloc_reg();
590625
self.emit(IrInstruction::Add(total_offset, base_offset, data_offset));
@@ -595,6 +630,8 @@ impl IrGenerator {
595630
}
596631

597632
// Handle (account-data-len idx) - get data length for account
633+
// data_len is at offset 80 from account start
634+
// For account 0: 8 + 80 = 88 from input start
598635
if name == "account-data-len" && args.len() == 1 {
599636
let idx_reg = self.generate_expr(&args[0].value)?
600637
.ok_or_else(|| Error::runtime("account-data-len index has no result"))?;
@@ -606,17 +643,18 @@ impl IrGenerator {
606643
self.emit(IrInstruction::ConstI64(eight_reg, 8));
607644

608645
let account_size = self.alloc_reg();
609-
self.emit(IrInstruction::ConstI64(account_size, 165));
646+
self.emit(IrInstruction::ConstI64(account_size, 10344));
610647

611648
let account_offset = self.alloc_reg();
612649
self.emit(IrInstruction::Mul(account_offset, idx_reg, account_size));
613650

614651
let base_offset = self.alloc_reg();
615652
self.emit(IrInstruction::Add(base_offset, eight_reg, account_offset));
616653

617-
// data_len is at offset 88 from account start
654+
// data_len is at offset 80 from account start (right after lamports)
655+
// = 1+1+1+1+4+32+32+8 = 80
618656
let len_offset = self.alloc_reg();
619-
self.emit(IrInstruction::ConstI64(len_offset, 88));
657+
self.emit(IrInstruction::ConstI64(len_offset, 80));
620658

621659
let total_offset = self.alloc_reg();
622660
self.emit(IrInstruction::Add(total_offset, base_offset, len_offset));

crates/ovsm/src/compiler/sbpf_codegen.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -588,13 +588,17 @@ impl RegisterAllocator {
588588
// Pre-map IR virtual regs 1,2 to physical R1,R2 (Solana ABI: accounts, instruction-data)
589589
alloc.insert(IrReg(1), SbpfReg::R1);
590590
alloc.insert(IrReg(2), SbpfReg::R2);
591+
// Pre-map IR virtual regs 6,7 to physical R6,R7 (callee-saved, used for saved accounts/instr-data)
592+
alloc.insert(IrReg(6), SbpfReg::R6);
593+
alloc.insert(IrReg(7), SbpfReg::R7);
591594

592595
Self {
593596
allocation: alloc,
594-
// Use R3-R9 for allocation (R0=return, R1-R2=reserved, R10=FP)
597+
// Use R3-R5, R8-R9 for allocation (R0=return, R1-R2=ABI, R6-R7=saved builtins, R10=FP)
598+
// IMPORTANT: Callee-saved (R8-R9) first to avoid clobbering by syscalls
595599
available: vec![
596-
SbpfReg::R9, SbpfReg::R8, SbpfReg::R7, SbpfReg::R6,
597-
SbpfReg::R5, SbpfReg::R4, SbpfReg::R3,
600+
SbpfReg::R9, SbpfReg::R8, // Callee-saved: safe across syscalls
601+
SbpfReg::R5, SbpfReg::R4, SbpfReg::R3, // Caller-saved: clobbered by syscalls
598602
],
599603
spills: HashMap::new(),
600604
next_spill: -8, // Grow downward from frame pointer

0 commit comments

Comments
 (0)