Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ is easiest to understand by reading the commit.
diagnostic counters were moved off the hot path behind the new default-on
`rexdiag` feature, so a last-drops build can drop them with
`--no-default-features` (`f1d0fcb`).
- **Crash fix: `rex-jit` CIDMATCH probe on aux-plane draws.** A draw into an
overlay plane (OLAY/PUP/CID) with CID checking live re-derived the CID probe's
offset against `fb_rgb` while the pixel pointer was already `fb_aux`-based,
reading `fb_aux + (fb_aux - fb_rgb) + off` — a wild pointer that segfaulted the
REX3 thread under X11. `Dm1::use_aux()` now picks the base in both shader
emitters (`rules/rex3/cidmatch-aux-plane-base.md`).
- The GFIFO push is retryable, and a shader rejected by a full compile queue can
be requested again (`rules/testing/rex-jit-queue-retry.md`).
- `CIDMATCH` is a mask of permitted CIDs, not an equality value
Expand Down
96 changes: 96 additions & 0 deletions rules/rex3/cidmatch-aux-plane-base.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# CIDMATCH probe used the wrong framebuffer base on aux-plane draws — 2026-09-18

**Status: FIXED.** `Dm1::use_aux()` now picks the base for the CID probe in both shader
emitters; regression test `jit_cidmatch_aux_plane_matches_interp`, verified to fail
without the fix.

A SIGSEGV on the `REX3-Processor` thread after ~5 hours of IRIX 6.5 with X11 running.
Worth keeping for two reasons: the fix is one line per site, and the *diagnosis* was done
entirely from a macOS crash report with no symbols — the faulting frame is JIT code, so
there is nothing to symbolicate. The method below generalises to any rex-jit or jitv2
crash report.

## The bug

`emit_calculate_fb_address` picks its base by plane:

```rust
let fb_ptr = if use_aux { pctx.fb_aux } else { pctx.fb_rgb }; // OLAY/PUP/CID → fb_aux
let px_ptr = b.ins().iadd(fb_ptr, byte_off64);
```

The CIDMATCH probe then re-derived the byte offset — unconditionally against `fb_rgb`:

```rust
let byte_off64 = b.ins().isub(px_ptr, pctx.fb_rgb); // wrong base when px_ptr is aux
b.ins().iadd(pctx.fb_aux, byte_off64)
```

For an RGB/RGBA draw the two cancel and the probe is right, which is why every existing
CIDMATCH test passed: `check_cid_write_masks` sweeps all 16 masks, 4 CIDs and 3 adrmodes,
but only ever with `DM1_RGB24_SRC`. For an OLAY/PUP/CID draw `px_ptr` is *already*
`fb_aux + off`, so the probe read

fb_aux + (fb_aux - fb_rgb) + off

`fb_rgb` and `fb_aux` are two independent `Box<[u32]>` allocations, so `fb_aux - fb_rgb`
is whatever the allocator chose — 0x29C0_0000 (700 MB) in the crashed process. The
consequence therefore ranges from reading a stray mapped word to SIGSEGV, with nothing in
the shader to distinguish the cases.

X11 draws menus, popups and the cursor into the overlay planes with CID checking live, so
the shape is common; it needs the *pair* (aux plane, `cidmatch != 0xF`) to fire, which is
why it survived this long. Both emitters carried the probe — `emit_shader` and the line
emitter — so both had to be fixed.

## Reading the crash report

The report gives `pc` in no mapped image and a thread name, and that is the whole of it:

```
Thread 33 Crashed:: REX3-Processor
0 ??? 0x117b141fc ???
1 iris-gui ..._4iris4rex3..Rex3..Device5start + 1980
KERN_INVALID_ADDRESS at 0x0000007d3188c028 esr 0x92000006 (translation fault)
x1 0x7cde000000 x2 0x7d07c00000 x3 0x29c8c028 x24 0x800 x26 0x8c028 x27 0x46
```

The lever is `instructionByteStream` in the JSON tail: `beforePC` and `atPC`, 40 bytes
each, base64. Decode them (`base64 -d`, then read 4-byte little-endian words) and
disassemble by hand or with `llvm-objdump -d --triple=arm64`:

```
-12 8b3a4043 add x3, x2, w26, uxtw ; px_ptr = fb_ptr + byte_off
-8 f84003e1 ldur x1, [sp] ; reload the spilled fb_rgb
-4 cb010063 sub x3, x3, x1 ; "byte offset" = px_ptr - fb_rgb
+0 b8636843 ldr w3, [x2, x3] ; ← fault: base is fb_ptr, not fb_rgb
+4 12000463 and w3, w3, #3 ; cid = aux_raw & 3
+8 5280009b mov w27, #4 ; cidmatch nibble, folded in as a constant
+12 1ac32763 lsrv w3, w27, w3 ; cidmatch >> cid
+16 360000a3 tbz w3, #0, ... ; bit clear → skip the pixel
```

`and #3` / `lsrv` / `tbz` is the CIDMATCH block and nothing else in the shader, so the
site is identified without symbols. The folded `mov w27, #4` even gives the register
state: `CIDMATCH=0b0100`, i.e. only CID 2 permitted.

Then the arithmetic confirms it rather than merely suggesting it:

- `x2 - x1 = 0x29C0_0000` — two allocation bases, 700 MB apart.
- `x26 = 0x8C028` → `/4 = 143370` → `y = 143370 / 2048 = 70`, `x = 10`. A pixel comfortably
inside a 1280x1024 screen: **the coordinates were never out of bounds**, which rules out
a clipping or DDA bug and points at the addressing itself.
- `x2 + (x2 - x1 + x26) = 0x7D31_88C0_28` = `far`, exactly. The faulting address is
reproduced from the registers, so the mechanism is not a guess.

Two things made this fast, and both are worth repeating: **the shader emitters are
straight-line IR, so a short instruction window is enough to name the block**, and
**reproducing `far` arithmetically from the register file turns a hypothesis into a
proof.** If the recomputation had not matched, the story would have been wrong.

## Guard against the general shape

Any pointer formed in the shader has to pick its base with the *same* predicate
`emit_calculate_fb_address` used. `Dm1::use_aux()` now exists so the choice is made in one
place; the inlined `matches!(dm1.planes(), OLAY | PUP | CID)` copies (there were three)
are gone. A second base derived from a pointer is the smell — prefer carrying the offset.
46 changes: 29 additions & 17 deletions src/rex3_jit/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,16 @@ impl Dm1 {
fn blendalpha(&self) -> bool { self.val & (1 << 27) != 0 }
fn logicop(&self) -> u32 { (self.val >> 28) & 0xF }

/// OLAY/PUP/CID are packed into the auxiliary framebuffer; RGB/RGBA live in
/// the main one. Every framebuffer pointer the shader forms — the pixel
/// address, the scr2scr source, the CIDMATCH probe — has to pick its base
/// with this, and mixing two bases is what made a CID-plane draw under a
/// live CIDMATCH read `fb_aux + (fb_aux - fb_rgb) + off`.
fn use_aux(&self) -> bool {
matches!(self.planes(),
p if p == DRAWMODE1_PLANES_OLAY || p == DRAWMODE1_PLANES_PUP || p == DRAWMODE1_PLANES_CID)
}

/// Compute compile-time host pixel count (pixels per host word) from dm1 fields.
/// Mirrors Rex3::host_setup() count logic.
fn host_count(&self) -> u32 {
Expand Down Expand Up @@ -496,8 +506,7 @@ fn emit_calculate_fb_address(
let c4 = b.ins().iconst(types::I32, 4);
let byte_off = b.ins().imul(addr_i32, c4);
let byte_off64 = b.ins().uextend(types::I64, byte_off);
let use_aux = matches!(dm1.planes(),
p if p == DRAWMODE1_PLANES_OLAY || p == DRAWMODE1_PLANES_PUP || p == DRAWMODE1_PLANES_CID);
let use_aux = dm1.use_aux();
let fb_ptr = if use_aux { pctx.fb_aux } else { pctx.fb_rgb };
let px_ptr = b.ins().iadd(fb_ptr, byte_off64);
let _ptr_type = ptr_type; // consumed by caller if needed
Expand All @@ -523,8 +532,7 @@ fn emit_pixel_write(
dm1: &Dm1,
is_hostw: bool,
) {
let use_aux = matches!(dm1.planes(),
p if p == DRAWMODE1_PLANES_OLAY || p == DRAWMODE1_PLANES_PUP || p == DRAWMODE1_PLANES_CID);
let use_aux = dm1.use_aux();
let depth_mask: i64 = match dm1.drawdepth() { 0 => 0xF, 1 => 0xFF, 2 => 0xFFF, _ => 0xFFFFFF };
let dblsrc_shift: i64 = match dm1.drawdepth() { 0 => 4, 1 => 8, 2 => 12, _ => 0 };
let (aux_read_shift0, aux_read_shift1, aux_read_mask): (i64, i64, i64) = match dm1.planes() {
Expand Down Expand Up @@ -640,9 +648,9 @@ fn emit_pixel_write(
//
// Reuse the destination load above when there was one. `fb_px_raw` is the
// same address, loaded with no intervening store, so a second load fetches
// a value we already have. (Note this is only the *plane* pixel: the CID
// match test reads fb_aux at a different pointer and is a genuinely
// separate access, not part of this pair.) Cranelift's redundant-load
// a value we already have. (Note this is only the *plane* pixel: for an
// RGB-plane draw the CID match test reads fb_aux at a different pointer and
// is a genuinely separate access, not part of this pair.) Cranelift's redundant-load
// elimination will not merge them itself — the loads use `memv`, the
// possibly-aliased flag, so it cannot prove nothing wrote in between.
let old_val = if needs_dst {
Expand Down Expand Up @@ -1188,12 +1196,16 @@ fn emit_shader(
// Only emitted when cidmatch != 0xF (0xF = disabled).
// Does not apply to HOSTR (READ) since that reads from fb, not writes to it.
if cidmatch != 0xF && !is_hostr {
let aux_ptr = {
// Re-derive the fb_aux byte offset from px_ptr (which is already byte-offset into fb_rgb).
let aux_ptr = if dm1.use_aux() {
// px_ptr is already an fb_aux pointer for this pixel.
px_ptr
} else {
// Re-derive the fb_aux byte offset from px_ptr (an fb_rgb pointer here).
// fb_rgb and fb_aux share the same stride/layout (2048 u32 entries per row),
// so the byte offset into fb_aux is the same as into fb_rgb.
let fb_rgb_base = pctx.fb_rgb;
let byte_off64 = b.ins().isub(px_ptr, fb_rgb_base);
// so the byte offset into fb_aux is the same as into fb_rgb. Subtracting
// the wrong base is a wild pointer, not a wrong pixel — see
// rules/rex3/cidmatch-aux-plane-base.md.
let byte_off64 = b.ins().isub(px_ptr, pctx.fb_rgb);
b.ins().iadd(pctx.fb_aux, byte_off64)
};
let aux_raw = b.ins().load(types::I32, memv, aux_ptr, ir::immediates::Offset32::new(0));
Expand Down Expand Up @@ -1230,8 +1242,7 @@ fn emit_shader(
// to decide whether we consumed a host pixel (false) or drew colorback (true).
let mut draw_use_bg_bool: Value = b.ins().iconst(types::I8, 0); // default: drew from host/DDA
let src_color = if is_scr2scr {
let use_aux = matches!(dm1.planes(),
p if p == DRAWMODE1_PLANES_OLAY || p == DRAWMODE1_PLANES_PUP || p == DRAWMODE1_PLANES_CID);
let use_aux = dm1.use_aux();
let (aux_read_shift0, aux_read_shift1, aux_read_mask): (i64, i64, i64) = match dm1.planes() {
p if p == DRAWMODE1_PLANES_OLAY => (8, 16, 0xFF),
p if p == DRAWMODE1_PLANES_CID => (0, 4, 0x3),
Expand Down Expand Up @@ -1996,9 +2007,10 @@ fn emit_draw_iline(

// CID mask check for lines (same logic as emit_shader).
if cidmatch != 0xF {
let aux_ptr = {
let fb_rgb_base = pctx.fb_rgb;
let byte_off64 = b.ins().isub(px_ptr, fb_rgb_base);
let aux_ptr = if dm1.use_aux() {
px_ptr // already an fb_aux pointer for this pixel
} else {
let byte_off64 = b.ins().isub(px_ptr, pctx.fb_rgb);
b.ins().iadd(pctx.fb_aux, byte_off64)
};
let aux_raw = b.ins().load(types::I32, memv, aux_ptr, ir::immediates::Offset32::new(0));
Expand Down
79 changes: 79 additions & 0 deletions src/rex3_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4807,6 +4807,85 @@ mod jit_tests {
dm0, dm1,
);
}

/// CIDMATCH on a draw whose target plane lives in fb_aux (OLAY/PUP/CID).
///
/// The CID probe re-derives its fb_aux offset from `px_ptr`, which is only
/// an fb_rgb pointer when the target plane is RGB/RGBA. For an aux-plane
/// draw `px_ptr` is already fb_aux-based, and subtracting fb_rgb from it
/// produced `fb_aux + (fb_aux - fb_rgb) + off` — a pointer hundreds of MB
/// outside either framebuffer. It segfaulted the REX3 thread the moment
/// IRIX drew into an overlay plane with CID checking live (X11 menus and
/// the cursor do exactly that). `fb_rgb` and `fb_aux` are two independent
/// `Box<[u32]>` allocations, so what the wild read hits is a property of
/// the heap layout, not of the shader: in the emulator (bases 700 MB
/// apart) SIGSEGV; in this test process (bases adjacent) a stray but
/// mapped word, which shows up as the CID test answering differently from
/// the interpreter. Either outcome fails the test.
///
/// Both shader emitters carry the probe, so the line adrmode is covered
/// alongside the block one. cid_write_masks_jit already covers the
/// RGB-plane half of the same test.
#[test]
fn jit_cidmatch_aux_plane_matches_interp() {
fn aux_pixel(rex: &Rex3, x: i32, y: i32) -> u32 {
unsafe { (*rex.fb_aux.get())[(y as u32 * 2048 + x as u32) as usize] }
}

let rex_interp = make_rex3();
let rex_jit = make_rex3_jit();
let dst = 20 * 2048 + 10;
let mut drew_something = false;

for planes in [DRAWMODE1_PLANES_OLAY, DRAWMODE1_PLANES_PUP, DRAWMODE1_PLANES_CID] {
let dm1 = planes | DRAWMODE1_COMPARE_DISABLE_SH | DRAWMODE1_LOGICOP_SRC_SH;
for dm0 in [DM0_DRAW_BLOCK,
DRAWMODE0_OPCODE_DRAW | DRAWMODE0_ADRMODE_I_LINE_SH | DM0_DOSETUP | DM0_STOPONXY] {
for mask in [0b0001u32, 0b0100, 0b1010] {
let cm = mask << CLIPMODE_CIDMATCH_SHIFT;
let jit = rex_jit.rex_jit.as_ref().unwrap();
jit.request_compile(dm0, dm1, cm);
assert!(jit.wait_compiled(dm0, dm1, cm),
"JIT compile failed for dm0={dm0:#010x} dm1={dm1:#010x} cm={cm:#010x}");

for cid in 0..4_u32 {
let seed = 0x80000000 | (cid << 4) | cid;
let run = |rex: &Rex3| -> u32 {
rex3init(rex);
wait(rex);
unsafe { (*rex.fb_aux.get())[dst] = seed; }
reg(rex, REX3_DRAWMODE1, dm1);
reg(rex, REX3_COLORI, 0xFF);
reg(rex, REX3_CLIPMODE, cm);
reg(rex, REX3_XYSTARTI, xy(10, 20));
reg(rex, REX3_XYENDI, xy(10, 20));
reg_go(rex, REX3_DRAWMODE0, dm0);
aux_pixel(rex, 10, 20)
};

let before = rex_jit.jit_go_count.load(std::sync::atomic::Ordering::Relaxed);
let got_jit = run(rex_jit);
assert_eq!(rex_jit.jit_go_count.load(std::sync::atomic::Ordering::Relaxed) - before, 1,
"GO did not dispatch to the compiled shader: \
dm0={dm0:#010x} dm1={dm1:#010x} cm={cm:#010x}");

let got_interp = run(rex_interp);
assert_eq!(got_jit, got_interp,
"JIT/interp mismatch: planes={planes} dm0={dm0:#010x} \
CIDMATCH={mask:04b} cid={cid}");
drew_something |= got_interp != seed;
}
}
}
}

// A permitted CID has to actually write, or every comparison above is
// a comparison of two untouched seed values.
assert!(drew_something, "no aux-plane write landed — the comparison was vacuous");

rex_jit.stop();
rex_interp.stop();
}
}

// ---------------------------------------------------------------------------
Expand Down
Loading