From c9b9f18b327de543f30452cc68d496bdd1ab16e4 Mon Sep 17 00:00:00 2001 From: bQUARKz Date: Tue, 10 Feb 2026 00:56:15 +0000 Subject: [PATCH] fixes --- .../prometeu-compiler/src/building/linker.rs | 14 +-- crates/prometeu-vm/src/verifier.rs | 23 +++- files/Hard Reset.md | 101 ------------------ test-cartridges/canonical/golden/program.pbc | Bin 629 -> 629 bytes test-cartridges/test01/cartridge/program.pbc | Bin 3311 -> 3482 bytes 5 files changed, 29 insertions(+), 109 deletions(-) diff --git a/crates/prometeu-compiler/src/building/linker.rs b/crates/prometeu-compiler/src/building/linker.rs index 2ea6c55e..f75fa50e 100644 --- a/crates/prometeu-compiler/src/building/linker.rs +++ b/crates/prometeu-compiler/src/building/linker.rs @@ -255,14 +255,14 @@ impl Linker { patch_u32_at(&mut combined_code, imm_start, &|_| global_func_idx); } } - // Relocate intra-function control-flow immediates by module code offset to preserve absolute PCs + // Branches are strictly function-relative. Do NOT relocate. + // The emitter encodes `target_rel = label - func_start` and the verifier enforces it. OpCode::Jmp | OpCode::JmpIfFalse | OpCode::JmpIfTrue => { - // For branches, immediate must be present and represents rel PC from function start let _ = imm_u32_opt.ok_or_else(|| LinkError::IncompatibleSymbolSignature(format!( "Invalid branch immediate at pc {}", pc - code_offset )))?; - patch_u32_at(&mut combined_code, imm_start, &|cur| cur + (code_offset as u32)); + // Intentionally no patch here. } _ => {} } @@ -628,16 +628,16 @@ mod tests { // Module 2's code starts after module 1's code let module2_offset = code1.len() as u32; - // Verify that the JMP immediate equals original_target (0) + module2_offset + // Verify that the JMP immediate remains function-relative (0), no relocation applied let jmp_abs_pc = module2_offset as usize + jmp_pc as usize; let jmp_imm_off = jmp_abs_pc + 2; // skip opcode let jmp_patched = u32::from_le_bytes(result.rom[jmp_imm_off..jmp_imm_off+4].try_into().unwrap()); - assert_eq!(jmp_patched, module2_offset); + assert_eq!(jmp_patched, 0); - // Verify that the conditional JMP immediate was relocated similarly + // Verify that the conditional JMP immediate also remains function-relative (0) let cjmp_abs_pc = module2_offset as usize + cjmp_pc as usize; let cjmp_imm_off = cjmp_abs_pc + 2; let cjmp_patched = u32::from_le_bytes(result.rom[cjmp_imm_off..cjmp_imm_off+4].try_into().unwrap()); - assert_eq!(cjmp_patched, module2_offset); + assert_eq!(cjmp_patched, 0); } } diff --git a/crates/prometeu-vm/src/verifier.rs b/crates/prometeu-vm/src/verifier.rs index dc74f30c..686e7a56 100644 --- a/crates/prometeu-vm/src/verifier.rs +++ b/crates/prometeu-vm/src/verifier.rs @@ -139,7 +139,28 @@ impl Verifier { let func_len = func_end_abs - func_start; if target_rel > func_len { - return Err(VerifierError::InvalidJumpTarget { pc: func_start + pc, target: func_start + target_rel }); + // Mandatory structured diagnostic for InvalidJumpTarget + let pc_abs = func_start + pc; + let opcode = instr.opcode; + let imm_raw = instr.imm_u32().unwrap(); + let func_start_abs = func_start; + let func_end_abs_log = func_end_abs; + let target_abs_expected = func_start_abs + target_rel; + let is_boundary_target_rel = valid_pc.contains(&target_rel); + eprintln!( + "[VERIFIER] invalid jump: pc_abs={} opcode={:?} imm_raw={} func=F{} start={} end={} len={} target_rel={} target_abs_expected={} boundary_rel={}", + pc_abs, + opcode, + imm_raw, + func_idx, + func_start_abs, + func_end_abs_log, + func_len, + target_rel, + target_abs_expected, + is_boundary_target_rel + ); + return Err(VerifierError::InvalidJumpTarget { pc: pc_abs, target: target_abs_expected }); } if target_rel == func_len { diff --git a/files/Hard Reset.md b/files/Hard Reset.md index 3b4ba910..19fed897 100644 --- a/files/Hard Reset.md +++ b/files/Hard Reset.md @@ -11,107 +11,6 @@ * A **single canonical layout/decoder/spec** is used across compiler/linker/verifier/VM. * Any invalid program fails with clear diagnostics, not panics. -**Estimation scale:** - -* **1 point** = small / straightforward -* **3 points** = medium -* **5 points** = large -* Any work that feels bigger must be broken down into multiple PRs (≤ 5 points each). - ---- - -## Phase 1 — Single Source of Truth: Bytecode Spec + Decoder (Highest ROI) - -### PR-01 (3 pts) — Move OpcodeSpec to `prometeu-bytecode` and make it authoritative - -**Briefing** - -Today opcode metadata (imm sizes, stack effects, branch-ness, terminators) is duplicated and/or inconsistent across crates. This creates a perpetual maintenance nightmare. - -**Target** - -Create one authoritative opcode spec in **`prometeu-bytecode`** and delete/replace all “local” opcode knowledge. - -**Scope** - -* Create `prometeu-bytecode::opcode_spec` containing: - - * `imm_bytes` - * `pops`, `pushes` (stack effect) - * `is_branch`, `is_terminator` - * optional: `name`, `category` -* Update callers to import from `prometeu-bytecode`. - -**Requirements Checklist** - -* [ ] There is exactly one canonical `OpcodeSpec` source. -* [ ] All crates compile against that source. -* [ ] No hardcoded operand sizes remain outside the spec. - -**Completion Tests** - -* [ ] Unit test enumerating all opcodes validates: - - * every opcode has a spec - * `imm_bytes` is defined - ---- - -### PR-02 (5 pts) — Introduce canonical decoder in `prometeu-bytecode` and migrate VM to it - -**Briefing** - -The VM currently has its own decoder. The linker and other tools decode manually. This must be centralized. - -**Target** - -Add a single canonical decoder in `prometeu-bytecode` that produces typed decoded instructions. - -**Scope** - -* Add `prometeu-bytecode::decoder`: - - * `decode_next(pc, bytes) -> DecodedInstr` - * includes: opcode, pc, next_pc, raw immediate bytes slice - * helpers: `imm_u8/u16/u32/i32/i64/f64` with size validation -* Migrate VM to use `prometeu-bytecode::decoder`. - -**Requirements Checklist** - -* [ ] VM no longer has a bespoke decoder. -* [ ] No slicing-based immediate parsing in VM core paths. -* [ ] Decoder validates immediate sizes and fails deterministically. - -**Completion Tests** - -* [ ] Decoder unit tests for representative opcodes with each immediate size. -* [ ] Roundtrip test: encode→decode (table-driven; property test optional). - ---- - -### PR-03 (3 pts) — Delete/neutralize `abi::operand_size` duplication - -**Briefing** - -`prometeu-bytecode/src/abi.rs` provides partial operand sizing that can drift from the canonical spec. - -**Target** - -Make all operand sizing derived from the opcode spec. - -**Scope** - -* Replace `operand_size()` with `OpcodeSpec::imm_bytes`. -* Remove or restrict legacy APIs that leak duplication. - -**Requirements Checklist** - -* [ ] There is no second operand-size table. - -**Completion Tests** - -* [ ] Test ensuring `operand_size()` (if retained) matches spec for all opcodes. - --- ## Phase 2 — Canonical Layout + Verifier Contract (JVM-like Control Flow) diff --git a/test-cartridges/canonical/golden/program.pbc b/test-cartridges/canonical/golden/program.pbc index 3646ac8573f5be88286c9c5f6abe6e09b079a715..5a70490d7e7f3a43ff64e865774031b80c27af1b 100644 GIT binary patch delta 25 dcmey$@|9(SAJgP$CMjM9AYcXJw4%h^Q~+Fo1{weW delta 21 ccmey$@|9(S9}_1l0|P@^QDScDWG^N;07?P|2><{9 diff --git a/test-cartridges/test01/cartridge/program.pbc b/test-cartridges/test01/cartridge/program.pbc index 683f47dceabf9472c43cfbec348ac5fc319d8cb5..a59d1326e22aa7869ed77a1af048b5f3246cad34 100644 GIT binary patch literal 3482 zcmZvedx(@}6vp2f$22pYywNVCV~9~}?(FRBuu9f28{!`llU-0WV|8ZNO?RDU)Cvs3 zE{cR1NYGtS45B|&3YP?-lA;pHt#qR@qwvhE&LB0H3csO2JvOyb_6T zkHlY##QP)hW0Ck-w8p9NXpEP@EU#l;H{wi%^nCO#CA!Hn?l>J;(V_k)BRc7;XjaZu zT&(Ea-P}(F0$;)Nii=x>?iLx&9`;4EwKvh)!@g*?HZYn!?2BgWutl?neIv7jyug=Y zh9h-{U!8)tNp}@>7esQ3{uqYy>Z{a2c6}T{)egU6BmG*>qMH!P3_KIPe>^s*opfV7 z*BT}LzIFJAq2M2ng6H+I1ap-Q#+)W+KPv8Y#v0riN>yBv!xMM!dyl(3(o=DX8+B(r z6diWA;Qm^JiW@Uiex5^YFve><*JD-i`33K9BOc66c9hF8xwxB!ZE!R2)JJif_sdq? zXn*HPzTS1-GEnMhMGD@Ia@&3WI&9L9vFLZG!G-6s)91)OF|=peUmWNztS+6RW?X5Y zxk+c>Z9p@Vo(b2;^)yRA1%DdoZ6LiBt`Vh7KMQ{j%r;$vYaDuGDE|a}0Ax*n0Y3%i zoBjYk2y|+cKRfQ+O<5cG5!5Y&g_+y}A z`a}3fV3X;O;m5#c(|U8909#DI10Mi-qv?K=IHZ%oPSaE2*Mk>KC*Tc0r&0M`@NTfz zbPs$Tc++$r{9&-)^nLIu&>K?y55NzCBc|Vm9|A{B?}7INy+M_qz>Ee1rpLg?fkD&b z;S<4W(--NRM4v_Lsd}Bg*$sX*y$8M*TnNRb^+s0ycknadis>KVKY}=@$E6ddbvAwi z<4m8Yy#SJ?FVS8G(@ZNatv9~v>1^BzQl^{XX)woh3a;~^bFTc`;dcOiA*40Vzd_#g zAMk&`LepBe&%t8TU&2pB93&)}y(oH1yAN8y8DY(!6p=qVB1 zX!;lW{t0vrRPS5(UqJ7fu+}*WYn_>}Za1xGqY>yFDLx&33%D~BmtJW4U+&d}UTnG* zz5uK=y#&4j=)I(Ri{KA{O{RO{n}vwp64BctT4zr6>iu^`^gh$iQ~MQg(DVWLyP)6n z8}P&6xM@8{tw4WJs{astEl9z2>C3gD+PS)%Pd8=K-Vk;AxCQPE{#@Q$O87vjylP{2 zKATeqZ&|ZYAV(mV_m+Y16?CqruWNmNUPc|f)kyRe_5PL?0=az9=sTy#Bc0X0d`oM) zx&)2B@xzfeBJ}Z%{uDLfa$53(LZZfwX?UJ=dRgw)*Ag*sNVhM)tR~7 zo+4lCP_P}c0!@*_w?7WoPwJ@SgQ_G>q&~ns&IkW_tw(jDmt%)jgXrh{2*s)B*R+nj@R$blP V+u76C+`GQCab3AK-`5yX@IP*{Se5_) literal 3311 zcmZveZHQD=9LCSAqg!SA8H9f)?p*y9#PA1(IoCu!6!}KVT};h&#ne_; zObvC#)Jj)ue2}}CZ>y5)tR#LVCfArR7b}%wAz`JXRVC!d;j7(e@ZtCzI2I5YK}*s-hdI#GR(0mZ94I_XsC^FF9Yg znyFP+HqUo3FOTKxwo^P&X)T z^ml!ENrzvvoKF*O2))m&sg?I@8?HI9dL6W=&M4@)!>{D!<$6?kcTK2fP%_+?^A5P6 z_k^qWUTc(XJP1>a?;t<(zcP3a)l#?95a8%Z171WKy2 zxZRg4ExEydWbOG5cdbvEC|)a)^XVY>g0J68n(%YX`=`|4!uPSk*HOAA^xX7(f@dz* zg>-54ny8sVx7>#<5BnwDj1jX6|~HFC4Lnu z85i(RBAs9w?;!pow8r=o{AXyb@ge+SwBGn_{1K!RPwhvrF-M|J#-s3K&=%u3z5#7F z?!tGYUB*55UbNS^AOARd*Z6*X8R-PocsudC&>`d9_&w;T@mBm^q!U*4<0OO8DdT$l zP;}aO7=Ac9V|q(EB-zCNi@RvNAgo>jIrY4 zvBugPKcPnBv*f>_$;N+@pGQ-T6&GiWwKr}>?Z!>`6uQ&65wHDmm+>t89Y|jg&G&Eg z4_aXS2mT^jYX68{y_TdDR7>8pgw5ng5dEU_2S>Imx{Qu__WUO{?i!o0;; z`*$DqJboe4GZN3nFG6wp6hDJsi-sCk@axcMft>0q&Gj0DuGmO8(UqIOi&yMij5uR(TcVjG?Z#*7<3wk&d7cVycm$8y? zmvJ+GCeoRt`Od{JLaUAE;2%QT7mD}cpGF%ayeYz4BD^iaJ0g6*_+|RugtUh=-cI~J zq%%(ZCVoHCAC_3}T{F@jmUtQdF{H1TE`7gNlslIcv#Dez;}fF(6BiU`GRaJP)~9sg z1BK$FE4#C;zJX6!vrynR-@>O%#Q!Ju^mi@Krc=H}kZCUZng)?zkhIVArBmdI&T@Y? z9gGrW`qmFbf??7=)1QaF%EfXyn+b*qGW{xC3RG`J^V2_p{uDY31#UXs?i2kkj6yw? zN0#<GouEdywjjt5u^#CfnMUY;Fls^`gJpu5xEzF*`k#Y)Q3Qrr(R}(F4UL znQ7g{74x$#)7z5ilx14`aQH#i)Q5vi%dO7!X|I%*H1&1%^f&b_FRbh>R%ZCo6xBj{ F_z&14Id%X5