diff --git a/iris-gui/src/config_ui.rs b/iris-gui/src/config_ui.rs index c6b3390..397429f 100644 --- a/iris-gui/src/config_ui.rs +++ b/iris-gui/src/config_ui.rs @@ -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", &["*"])]; diff --git a/rules/gui/cycles-ptr-must-be-cleared-before-dropping-the-machine.md b/rules/gui/cycles-ptr-must-be-cleared-before-dropping-the-machine.md new file mode 100644 index 0000000..32106af --- /dev/null +++ b/rules/gui/cycles-ptr-must-be-cleared-before-dropping-the-machine.md @@ -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` — cloning it into the GUI worker +kept the counter alive on its own. It is now +`Cell`: 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>>, found Option>`; 2be036c ported the GUI +to `r.cycles.get()` into an `Option`. The drop-order invariant above +is the part that is easy to miss — that borrow is invisible to the compiler. diff --git a/src/coffdump.rs b/src/coffdump.rs index d069202..395bab9 100644 --- a/src/coffdump.rs +++ b/src/coffdump.rs @@ -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, @@ -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, diff --git a/src/headless_gl.rs b/src/headless_gl.rs index 1ac28b3..962cdd0 100644 --- a/src/headless_gl.rs +++ b/src/headless_gl.rs @@ -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; @@ -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)); diff --git a/src/main.rs b/src/main.rs index f2e0744..ce35213 100644 --- a/src/main.rs +++ b/src/main.rs @@ -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 diff --git a/src/mips_cache_v2.rs b/src/mips_cache_v2.rs index 490bb5b..3c04214 100644 --- a/src/mips_cache_v2.rs +++ b/src/mips_cache_v2.rs @@ -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 = None; for i in 0..L2Cache::CHUNKS_PER_LINE { let val = unsafe { (*src.add(i)).rotate_left(32) }; @@ -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 = None; for i in 0..L2Cache::CHUNKS_PER_LINE { let val = dest[i]; diff --git a/src/mips_exec.rs b/src/mips_exec.rs index 70cea5e..bf4a613 100644 --- a/src/mips_exec.rs +++ b/src/mips_exec.rs @@ -744,7 +744,9 @@ impl MipsExecutor { // 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; @@ -5846,6 +5848,8 @@ impl 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)] diff --git a/src/ui.rs b/src/ui.rs index 1f53dfe..56a21f1 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -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; @@ -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; @@ -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 @@ -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) }; @@ -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, require_motion: bool) { @@ -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, + rex3: Arc, + scsi: Arc, + window: Arc, + window_size: Arc>>, + scale_snap: Arc>>, + display_res: Arc>, + mouse_delta: Arc>, + 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::*;