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
1 change: 0 additions & 1 deletion iris-gui/src/config_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1668,5 +1668,4 @@ const DISK_FILTERS: &[(&str, &[&str])] = &[
("All", &["*"]),
];
const ANY_FILTERS: &[(&str, &[&str])] = &[("All", &["*"])];
const SOCKET_FILTERS: &[(&str, &[&str])] = &[("Unix socket", &["sock"]), ("All", &["*"])];

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# iris-gui: clear the cached `CyclesPtr` before dropping the Machine

**Keywords:** cycles,CyclesPtr,mips,mips_core,rex3,mips estimate,status,handle.rs,worker_loop,dangling,raw pointer,AtomicU64,stop,sync,quit
**Category:** gui

## The counter behind the MIPS estimate is a raw pointer now, not an `Arc`

`Rex3::cycles` used to be an `Arc<AtomicU64>` — cloning it into the GUI worker
kept the counter alive on its own. It is now
`Cell<crate::mips_core::CyclesPtr>`: a bare `*const u64` pointing into
`MipsCore.hot.cycles`, which lives inside the `Machine`'s executor. Read it with
`CyclesPtr::get()` (a volatile read; returns 0 while unwired).

`iris-gui`'s `worker_loop` (`iris-gui/src/handle.rs`) latches that pointer on
`Cmd::Start` and polls it every status tick to compute the live MIPS number. The
pointer is only valid while the `Machine` it came from is alive, so **every path
that drops the machine must set `cycles = None` first** — `Cmd::Stop`,
`Cmd::SyncDisks`, and `Cmd::Quit` all do (`Quit` returns immediately, so it
cannot poll afterwards). Reading a stale pointer is a use-after-free, not a
stale number.

## Why this bites

926d56f ("convert cycles atomic into regular volatile variable") changed the
field type without touching `iris-gui`, so every release job failed on
`cycles = m.get_rex3().map(|r| r.cycles.clone())` with `expected
Option<Arc<Atomic<u64>>>, found Option<Cell<CyclesPtr>>`; 2be036c ported the GUI
to `r.cycles.get()` into an `Option<CyclesPtr>`. The drop-order invariant above
is the part that is easy to miss — that borrow is invisible to the compiler.
4 changes: 4 additions & 0 deletions src/coffdump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ struct Hdrr {
}

// File Descriptor Record (FDR)
// Mirrors the on-disk record: every field is parsed, only some are printed.
#[allow(dead_code)]
#[derive(Debug, Default, Clone)]
struct Fdr {
adr: u32,
Expand All @@ -136,6 +138,8 @@ struct Fdr {
}

// Procedure Descriptor Record (PDR)
// Mirrors the on-disk record: every field is parsed, only some are printed.
#[allow(dead_code)]
#[derive(Debug, Default, Clone)]
struct Pdr {
adr: u32,
Expand Down
4 changes: 2 additions & 2 deletions src/headless_gl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use glutin::display::GetGlDisplay;
use glutin::prelude::*;
use glutin::surface::{GlSurface, Surface, SwapInterval, WindowSurface};
use glutin_winit::{DisplayBuilder, GlWindow};
use raw_window_handle::HasRawWindowHandle;
use raw_window_handle::HasWindowHandle;
use winit::event_loop::EventLoop;
use winit::window::WindowAttributes;

Expand Down Expand Up @@ -47,7 +47,7 @@ impl HeadlessGl {
.ok()?;

let window = window?;
let raw_window_handle = window.raw_window_handle().ok()?;
let raw_window_handle = window.window_handle().ok()?.as_raw();
let gl_display = gl_config.display();

let context_attributes = ContextAttributesBuilder::new().build(Some(raw_window_handle));
Expand Down
2 changes: 2 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use iris::machine::Machine;
fn main() {
print_build_features();

// `mut` is only used by the pcap interface prompt below.
#[cfg_attr(not(feature = "pcap"), allow(unused_mut))]
let (mut cfg, scale) = load_config();

// If PCAP networking is selected but no interface was configured, prompt the
Expand Down
3 changes: 3 additions & 0 deletions src/mips_cache_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1583,7 +1583,9 @@ impl R4000Cache {
// Fast path: single pass over source — rotate into l2.data and fill l2.instrs.
#[cfg(not(feature = "r5k"))]
let l2_instrs = self.l2.instrs.get_mut();
// Only fuse_pair! mutates prev_s1, and it is a no-op without opcodefusion.
#[cfg(not(feature = "r5k"))]
#[cfg_attr(not(feature = "opcodefusion"), allow(unused_mut))]
let mut prev_s1: Option<usize> = None;
for i in 0..L2Cache::CHUNKS_PER_LINE {
let val = unsafe { (*src.add(i)).rotate_left(32) };
Expand All @@ -1610,6 +1612,7 @@ impl R4000Cache {
#[cfg(not(feature = "r5k"))]
{
let l2_instrs = self.l2.instrs.get_mut();
#[cfg_attr(not(feature = "opcodefusion"), allow(unused_mut))]
let mut prev_s1: Option<usize> = None;
for i in 0..L2Cache::CHUNKS_PER_LINE {
let val = dest[i];
Expand Down
6 changes: 5 additions & 1 deletion src/mips_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,9 @@ impl<T: Tlb, C: MipsCache> MipsExecutor<T, C> {

// Build unified cache hierarchy. Cache geometry is fixed at compile time;
// IC_SIZE/IC_LINE/DC_SIZE/DC_LINE/L2_SIZE/L2_LINE are consts from mips_cache_v2.
let cache = C::from(sysad.clone());
// `mut` is only used by the Triton L2-enable sync below.
#[cfg_attr(not(feature = "r5ksc_triton"), allow(unused_mut))]
let mut cache = C::from(sysad.clone());

// Build CP0 Config register from architecture constants.
let mut config = 0u32;
Expand Down Expand Up @@ -5846,6 +5848,8 @@ impl<T: Tlb + Send + 'static, C: MipsCache + Send + 'static> Device for MipsCpu<
// iteration, so its state never repeats — we must NOT park it or
// boot stalls. The state-repeat test distinguishes the two.
#[cfg(feature = "idle-pause")]
// Unreachable when the JIT dispatch above returns; harmless.
#[allow(unreachable_code)]
let mut idle_state = crate::idle_park::IdleParkState::default();

#[allow(unreachable_code)]
Expand Down
231 changes: 130 additions & 101 deletions src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ use std::sync::Arc;
use std::time::{Duration, Instant};
use parking_lot::Mutex;
use winit::{
event::{ElementState, Event, KeyEvent, WindowEvent, MouseButton},
event_loop::{ControlFlow, EventLoop},
application::ApplicationHandler,
event::{DeviceEvent, DeviceId, ElementState, KeyEvent, WindowEvent, MouseButton},
event_loop::{ActiveEventLoop, ControlFlow, EventLoop},
keyboard::{KeyCode, PhysicalKey},
window::{Window, WindowAttributes},
window::{Window, WindowAttributes, WindowId},
};
use glow::HasContext;
use crate::ps2::Ps2Controller;
Expand All @@ -22,7 +23,7 @@ use glutin::display::GetGlDisplay;
use glutin::prelude::*;
use glutin::surface::{GlSurface, SurfaceAttributesBuilder, SwapInterval, WindowSurface, Surface};
use glutin_winit::DisplayBuilder;
use raw_window_handle::{HasRawWindowHandle, RawWindowHandle};
use raw_window_handle::{HasWindowHandle, RawWindowHandle};
use std::num::NonZeroU32;
use std::ffi::CString;

Expand Down Expand Up @@ -711,7 +712,7 @@ impl Ui {
// that created the window/display above. The refresh thread only
// makes it current later (in GlRenderer::ensure_init); it never calls
// create_context itself. See the not_current_context field comment.
let raw_window_handle = window.raw_window_handle().expect("no raw window handle");
let raw_window_handle = window.window_handle().expect("no raw window handle").as_raw();
let gl_display = gl_config.display();

// Try, in order: (1) explicit GL 3.2 core — what GlCompositor and the
Expand Down Expand Up @@ -786,11 +787,7 @@ impl Ui {
let Ui { ps2, rex3, scsi, window, window_size, scale_snap, display_res, timer_manager, initial_scale, scroll_pixels_per_line, lock_aspect_ratio } = self;
let scale = initial_scale;

let mut mouse_grabbed = false;
let mut rctrl_held = false;
// Last window size we accepted, used to tell which edge the user is
// dragging when locking the aspect ratio.
let mut last_win_size = {
let last_win_size = {
let s = window.inner_size();
(s.width, s.height)
};
Expand All @@ -811,97 +808,12 @@ impl Ui {
}

event_loop.set_control_flow(ControlFlow::Wait);
event_loop.run(move |event, elwt| {
match event {
Event::WindowEvent { event, .. } => match event {
WindowEvent::CloseRequested => { elwt.exit() },
WindowEvent::Resized(size) => {
if size.width != 0 && size.height != 0 {
let mut new_size = (size.width, size.height);
// Lock the window to the display's aspect ratio so the
// picture fills it without letterbox bars. Skipped when
// fullscreen or maximized (aspect can't be honoured there)
// and when disabled by config.
if lock_aspect_ratio
&& window.fullscreen().is_none()
&& !window.is_maximized()
{
let (dw, dh) = *display_res.lock();
if let Some(fixed) = Self::aspect_fit(
size.width, size.height, last_win_size, dw, dh)
{
new_size = match window.request_inner_size(
winit::dpi::PhysicalSize::new(fixed.0, fixed.1))
{
// Some => applied synchronously, no further
// Resized event; use the actual granted size.
Some(actual) => (actual.width, actual.height),
None => fixed,
};
}
}
last_win_size = new_size;
*window_size.lock() = Some(new_size);
}
}
WindowEvent::KeyboardInput { event, .. } => {
Self::handle_keyboard(&ps2, &rex3, &scsi, &scale_snap, event, &mut mouse_grabbed, &mut rctrl_held, &window);
}
WindowEvent::MouseInput { state, button, .. } => {
if mouse_grabbed {
let pressed = state == ElementState::Pressed;
let mask = match button {
MouseButton::Left => 1,
MouseButton::Right => 2,
MouseButton::Middle => 4,
_ => 0,
};
if mask != 0 {
let mut md = mouse_delta.lock();
if pressed { md.buttons |= mask; } else { md.buttons &= !mask; }
drop(md);
Self::flush_mouse_delta(&ps2, &mouse_delta, false);
}
} else if state == ElementState::Pressed && button == MouseButton::Left {
mouse_grabbed = true;
if window.set_cursor_grab(winit::window::CursorGrabMode::Locked).is_err() {
let _ = window.set_cursor_grab(winit::window::CursorGrabMode::Confined);
}
window.set_cursor_visible(false);
mouse_delta.lock().accum = (0.0, 0.0);
}
}
WindowEvent::MouseWheel { delta, .. } => {
if mouse_grabbed {
let lines = match delta {
winit::event::MouseScrollDelta::LineDelta(_, y) => y as f64,
winit::event::MouseScrollDelta::PixelDelta(p) => p.y / scroll_pixels_per_line,
};
mouse_delta.lock().wheel += lines;
}
}
WindowEvent::Focused(false) => {
if mouse_grabbed {
mouse_grabbed = false;
let _ = window.set_cursor_grab(winit::window::CursorGrabMode::None);
window.set_cursor_visible(true);
}
}
WindowEvent::RedrawRequested => {
// Rendering is driven by the Rex3 refresh thread
}
_ => (),
},
Event::DeviceEvent { event: winit::event::DeviceEvent::MouseMotion { delta }, .. } => {
if mouse_grabbed {
let mut md = mouse_delta.lock();
md.accum.0 += delta.0 / scale as f64;
md.accum.1 += delta.1 / scale as f64;
}
},
_ => (),
}
}).unwrap();
let mut app = UiApp {
ps2, rex3, scsi, window, window_size, scale_snap, display_res, mouse_delta,
scale, scroll_pixels_per_line, lock_aspect_ratio,
mouse_grabbed: false, rctrl_held: false, last_win_size,
};
event_loop.run_app(&mut app).unwrap();
}

fn flush_mouse_delta(ps2: &Ps2Controller, mouse_delta: &Mutex<MouseDelta>, require_motion: bool) {
Expand Down Expand Up @@ -1027,6 +939,123 @@ impl Ui {
}
}

/// `Ui::run`'s event-loop state: `run_app` takes the old closure's captures as a struct.
struct UiApp {
ps2: Arc<Ps2Controller>,
rex3: Arc<Rex3>,
scsi: Arc<Wd33c93a>,
window: Arc<Window>,
window_size: Arc<Mutex<Option<(u32, u32)>>>,
scale_snap: Arc<Mutex<Option<ScaleSnap>>>,
display_res: Arc<Mutex<(u32, u32)>>,
mouse_delta: Arc<Mutex<MouseDelta>>,
scale: u32,
scroll_pixels_per_line: f64,
lock_aspect_ratio: bool,
mouse_grabbed: bool,
rctrl_held: bool,
// Last accepted window size: tells which edge is being dragged when locking the aspect ratio.
last_win_size: (u32, u32),
}

impl ApplicationHandler for UiApp {
// Window and GL context are created up front in Ui::new — nothing to (re)create here.
fn resumed(&mut self, _event_loop: &ActiveEventLoop) {}

fn window_event(&mut self, event_loop: &ActiveEventLoop, _id: WindowId, event: WindowEvent) {
match event {
WindowEvent::CloseRequested => { event_loop.exit() },
WindowEvent::Resized(size) => {
if size.width != 0 && size.height != 0 {
let mut new_size = (size.width, size.height);
// Lock the window to the display's aspect ratio so the
// picture fills it without letterbox bars. Skipped when
// fullscreen or maximized (aspect can't be honoured there)
// and when disabled by config.
if self.lock_aspect_ratio
&& self.window.fullscreen().is_none()
&& !self.window.is_maximized()
{
let (dw, dh) = *self.display_res.lock();
if let Some(fixed) = Ui::aspect_fit(
size.width, size.height, self.last_win_size, dw, dh)
{
new_size = match self.window.request_inner_size(
winit::dpi::PhysicalSize::new(fixed.0, fixed.1))
{
// Some => applied synchronously, no further
// Resized event; use the actual granted size.
Some(actual) => (actual.width, actual.height),
None => fixed,
};
}
}
self.last_win_size = new_size;
*self.window_size.lock() = Some(new_size);
}
}
WindowEvent::KeyboardInput { event, .. } => {
Ui::handle_keyboard(&self.ps2, &self.rex3, &self.scsi, &self.scale_snap, event,
&mut self.mouse_grabbed, &mut self.rctrl_held, &self.window);
}
WindowEvent::MouseInput { state, button, .. } => {
if self.mouse_grabbed {
let pressed = state == ElementState::Pressed;
let mask = match button {
MouseButton::Left => 1,
MouseButton::Right => 2,
MouseButton::Middle => 4,
_ => 0,
};
if mask != 0 {
let mut md = self.mouse_delta.lock();
if pressed { md.buttons |= mask; } else { md.buttons &= !mask; }
drop(md);
Ui::flush_mouse_delta(&self.ps2, &self.mouse_delta, false);
}
} else if state == ElementState::Pressed && button == MouseButton::Left {
self.mouse_grabbed = true;
if self.window.set_cursor_grab(winit::window::CursorGrabMode::Locked).is_err() {
let _ = self.window.set_cursor_grab(winit::window::CursorGrabMode::Confined);
}
self.window.set_cursor_visible(false);
self.mouse_delta.lock().accum = (0.0, 0.0);
}
}
WindowEvent::MouseWheel { delta, .. } => {
if self.mouse_grabbed {
let lines = match delta {
winit::event::MouseScrollDelta::LineDelta(_, y) => y as f64,
winit::event::MouseScrollDelta::PixelDelta(p) => p.y / self.scroll_pixels_per_line,
};
self.mouse_delta.lock().wheel += lines;
}
}
WindowEvent::Focused(false) => {
if self.mouse_grabbed {
self.mouse_grabbed = false;
let _ = self.window.set_cursor_grab(winit::window::CursorGrabMode::None);
self.window.set_cursor_visible(true);
}
}
WindowEvent::RedrawRequested => {
// Rendering is driven by the Rex3 refresh thread
}
_ => (),
}
}

fn device_event(&mut self, _event_loop: &ActiveEventLoop, _id: DeviceId, event: DeviceEvent) {
if let DeviceEvent::MouseMotion { delta } = event {
if self.mouse_grabbed {
let mut md = self.mouse_delta.lock();
md.accum.0 += delta.0 / self.scale as f64;
md.accum.1 += delta.1 / self.scale as f64;
}
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
Loading