openspec: mark Phase 3 done; record Phase 2/3 review deviations

Phase 3 (managed assembler) is implemented and tested — check 3.1-3.9.
Record the review-driven deviations: x64 Microsoft-ABI stub (shadow space,
16-byte alignment, imm64 loads, nuint[] signature, MaxArguments guard) with the
outstanding live-execution test noted; MarshalCache Size/MarshalSize split; and
the RpmHelper reader dedupe. Task 2.10 (ReadString code-unit-aligned scan)
stays open.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
kbe
2026-07-21 22:30:11 +02:00
co-authored by Claude Opus 4.8
parent 75ff4a2320
commit a0ca7050a2
@@ -9,9 +9,9 @@
## 2. Core Memory Access (spec: memory-access)
- [x] 2.1 Add tests for `MarshalCache<T>`: blittable size, marshal-required flag, IsIntPtr, computed-once behavior
- [x] 2.2 Implement `WhiteMagic/MarshalCache.cs` to pass 2.1
- [x] 2.2 Implement `WhiteMagic/MarshalCache.cs` to pass 2.1. **Deviation (review):** split `Size` (managed `Unsafe.SizeOf<T>`, blittable/`MemoryMarshal` path) from `MarshalSize` (`Marshal.SizeOf<T>`, marshal path). A single size mis-sized structs whose unmanaged width differs — a `bool` field (managed 1 / unmanaged 4) over-read the blittable path; an inline `ByValTStr`/`ByValArray` under-sized the marshal path and overran the pinned buffer (heap corruption on write). `MemoryBase` now picks per `TypeRequiresMarshal` at all four IO sites. Unused fields (`SizeU`, `IsIntPtr`, `TypeCode`, `RealType`) dropped.
- [x] 2.3 Add tests for `MemoryBase` abstract contract + `ExternalReader` round-trip (`Read<T>`/`Write<T>`, arrays) using the current process as target
- [x] 2.4 Implement `WhiteMagic/MemoryBase.cs` (abstract) and `WhiteMagic/ExternalReader.cs` to pass 2.3
- [x] 2.4 Implement `WhiteMagic/MemoryBase.cs` (abstract) and `WhiteMagic/ExternalReader.cs` to pass 2.3. **Deviation (review):** shared RPM/WPM extracted to `WhiteMagic/RpmHelper.cs` so `ExternalReader` and `InProcessReader` stay byte-consistent — partial reads honored (returns exactly `bytesRead`), write returns actual bytes / 0 on total failure; `InProcessReader` guards `MainModule` like `ExternalReader`.
- [x] 2.5 Add tests for string read/write with encoding, null-terminator stop, and max length
- [x] 2.6 Implement `ReadString`/`WriteString` on `MemoryBase` to pass 2.5
- [x] 2.7 Add tests for relative/absolute addressing (`GetAbsolute`/`GetRelative`, `isRelative` flag)
@@ -27,8 +27,8 @@
- [x] 3.4 Implement x86 cdecl stub to pass 3.3
- [x] 3.5 Add tests for stdcall (no caller cleanup), thiscall (ecx = this), fastcall (ecx/edx) x86 stubs
- [x] 3.6 Implement x86 stdcall/thiscall/fastcall stubs to pass 3.5
- [x] 3.7 Add tests for x64 stub argument-register placement and call. Now also asserts Microsoft x64 ABI compliance: `sub rsp, 0x20` shadow-space allocation (stack args land at `[rsp + 0x20 + 8*(i-4)]`), 16-byte stack alignment at the inner `call target`, `mov r64, imm64` loads for RCX/RDX/R8/R9 with the full 64-bit immediate (regression test for the old `mov r32d, imm32` truncation bug).
- [x] 3.8 Implement x64 stub to pass 3.7. Builds a compliant Microsoft x64 ABI frame: `sub rsp, 0x20`, 64-bit register loads, stack-argv above the shadow window, `call rel32`, `add rsp, 0x20`, `ret`. Accepts `nuint[]` so callers can pass full 64-bit pointers unchanged (x86 path truncates `nuint``uint` with a range check).
- [x] 3.7 Add tests for x64 stub argument-register placement and call
- [x] 3.8 Implement x64 stub to pass 3.7. **Deviation (review):** `BuildCallStub` takes `nuint[]` (was `uint[]`). x64 stub is Microsoft-x64-ABI compliant: allocates 32-byte shadow space, keeps 16-byte stack alignment at the inner `call` (frame `K ≡ 8 (mod 16)`, `K ≥ 0x20 + 8·stackArgs`), loads RCX/RDX/R8/R9 with full 64-bit `imm64` (no >4 GiB pointer truncation), and writes stack args above the shadow window (no return-address clobber). x86 rejects args > `uint.MaxValue`. Argument count bounded by `MaxArguments` (256) to keep frame arithmetic overflow-free. **Byte-level tests only — a live-execution test (5-arg + SSE callee via `CreateRemoteThread`) is still needed to prove the ABI at runtime.**
- [x] 3.9 Confirm no FASM/`ManagedFasm` reference exists in `WhiteMagic` output (assert via a test that scans loaded references)
## 4. Crash-Safe Execution Slice (spec: remote-execution, function-hooking)