From 9f945f7fa78679b894c1495e89a2fc5d63176b26 Mon Sep 17 00:00:00 2001 From: Maximilien Noal Date: Mon, 9 Dec 2024 22:27:06 +0100 Subject: [PATCH] fix: No crashes, but phantom breakpoints still persist --- src/Spice86/ViewModels/BreakpointViewModel.cs | 55 ++++++++++---- .../ViewModels/BreakpointsViewModel.cs | 29 +++++--- .../ViewModels/DebugWindowViewModel.cs | 2 +- .../ViewModels/DisassemblyViewModel.cs | 73 ++++++++++--------- src/Spice86/ViewModels/MemoryViewModel.cs | 12 +-- 5 files changed, 102 insertions(+), 69 deletions(-) diff --git a/src/Spice86/ViewModels/BreakpointViewModel.cs b/src/Spice86/ViewModels/BreakpointViewModel.cs index 16c1a1d2f..67180356b 100644 --- a/src/Spice86/ViewModels/BreakpointViewModel.cs +++ b/src/Spice86/ViewModels/BreakpointViewModel.cs @@ -7,18 +7,32 @@ namespace Spice86.ViewModels; using Spice86.Models.Debugging; public partial class BreakpointViewModel : ViewModelBase { - private readonly BreakPoint _breakPoint; private readonly EmulatorBreakpointsManager _emulatorBreakpointsManager; + private readonly Action _onReached; - public BreakpointViewModel(EmulatorBreakpointsManager emulatorBreakpointsManager, AddressBreakPoint breakPoint) { - _breakPoint = breakPoint; + public BreakpointViewModel(EmulatorBreakpointsManager emulatorBreakpointsManager, + long address, + BreakPointType type, + bool isRemovedOnTrigger, + Action onReached) { _emulatorBreakpointsManager = emulatorBreakpointsManager; - IsEnabled = true; - Address = breakPoint.Address; + Address = address; + Type = type; + IsRemovedOnTrigger = isRemovedOnTrigger; + if(IsRemovedOnTrigger) { + _onReached = () => { + Reached?.Invoke(); + onReached(); + }; + } else { + _onReached = onReached; + } Enable(); } - public BreakPointType Type => _breakPoint.BreakPointType; + internal event Action? Reached; + + public BreakPointType Type { get; } //Can't get out of sync since GDB can't be used at the same time as the internal debugger private bool _isEnabled; @@ -26,17 +40,16 @@ public BreakpointViewModel(EmulatorBreakpointsManager emulatorBreakpointsManager public bool IsEnabled { get => _isEnabled; set { - if(SetProperty(ref _isEnabled, value)) { - if (value) { - Enable(); - } else { - Disable(); - } + if (value) { + Enable(); + } else { + Disable(); } + SetProperty(ref _isEnabled, value); } } - public bool IsRemovedOnTrigger => _breakPoint.IsRemovedOnTrigger; + public bool IsRemovedOnTrigger { get; } public long Address { get; } @@ -51,19 +64,29 @@ public void Toggle() { [ObservableProperty] private string? _comment; + private AddressBreakPoint GenerateBreakPoint() => new( + Type, Address, + (_) => _onReached(), IsRemovedOnTrigger); + public void Enable() { - _emulatorBreakpointsManager.ToggleBreakPoint(_breakPoint, on: true); + if (IsEnabled) { + return; + } + _emulatorBreakpointsManager.ToggleBreakPoint(GenerateBreakPoint(), on: true); _isEnabled = true; OnPropertyChanged(nameof(IsEnabled)); } public void Disable() { - _emulatorBreakpointsManager.ToggleBreakPoint(_breakPoint, on: false); + if (!IsEnabled) { + return; + } + _emulatorBreakpointsManager.ToggleBreakPoint(GenerateBreakPoint(), on: false); _isEnabled = false; OnPropertyChanged(nameof(IsEnabled)); } internal bool IsFor(CpuInstructionInfo instructionInfo) { - return _breakPoint is AddressBreakPoint addressBreakPoint && addressBreakPoint.Address == instructionInfo.Address; + return Address == instructionInfo.Address; } } \ No newline at end of file diff --git a/src/Spice86/ViewModels/BreakpointsViewModel.cs b/src/Spice86/ViewModels/BreakpointsViewModel.cs index d3cd71849..f563c71ab 100644 --- a/src/Spice86/ViewModels/BreakpointsViewModel.cs +++ b/src/Spice86/ViewModels/BreakpointsViewModel.cs @@ -42,36 +42,43 @@ private void ToggleSelectedBreakpoint() { [NotifyCanExecuteChangedFor(nameof(ToggleSelectedBreakpointCommand))] private BreakpointViewModel? _selectedBreakpoint; - internal void AddAddressBreakpoint(AddressBreakPoint addressBreakPoint) { - var breakpointViewModel = new BreakpointViewModel( _emulatorBreakpointsManager, addressBreakPoint); + internal void AddAddressBreakpoint( + long address, + BreakPointType type, + bool isRemovedOnTrigger, + Action onReached) { + var breakpointViewModel = new BreakpointViewModel( + _emulatorBreakpointsManager, + address, type, isRemovedOnTrigger, onReached); Breakpoints.Add(breakpointViewModel); + if (isRemovedOnTrigger) { + breakpointViewModel.Reached += () => DeleteBreakpoint(breakpointViewModel); + } SelectedBreakpoint = breakpointViewModel; } + internal BreakpointViewModel? GetBreakpoint(CpuInstructionInfo instructionInfo) { + return Breakpoints.FirstOrDefault(x => x.IsFor(instructionInfo)); + } + private bool RemoveBreakpointCanExecute() => SelectedBreakpoint is not null; [RelayCommand(CanExecute = nameof(RemoveBreakpointCanExecute))] private void RemoveBreakpoint() { - if (SelectedBreakpoint is not null) { - DeleteBreakpoint(SelectedBreakpoint); - BreakpointDeleted?.Invoke(); - } + DeleteBreakpoint(SelectedBreakpoint); } - internal void RemoveBreakpoint(BreakpointViewModel vm) { + internal void RemoveBreakpointInternal(BreakpointViewModel vm) { DeleteBreakpoint(vm); } - internal BreakpointViewModel? GetBreakpoint(CpuInstructionInfo instructionInfo) { - return Breakpoints.FirstOrDefault(x => x.IsFor(instructionInfo)); - } - private void DeleteBreakpoint(BreakpointViewModel? breakpoint) { if (breakpoint is null) { return; } breakpoint.Disable(); Breakpoints.Remove(breakpoint); + BreakpointDeleted?.Invoke(); } } \ No newline at end of file diff --git a/src/Spice86/ViewModels/DebugWindowViewModel.cs b/src/Spice86/ViewModels/DebugWindowViewModel.cs index 2daf4e2c0..d17b6bef8 100644 --- a/src/Spice86/ViewModels/DebugWindowViewModel.cs +++ b/src/Spice86/ViewModels/DebugWindowViewModel.cs @@ -80,7 +80,7 @@ public DebugWindowViewModel(IInstructionExecutor cpu, State cpuState, Stack stac pauseHandler.Resumed += () => uiDispatcher.Post(() => IsPaused = false); DisassemblyViewModel disassemblyVm = new( cpu, memory, cpuState, functionsInformation.ToDictionary(x => x.Key.ToPhysical(), x => x.Value), - BreakpointsViewModel, emulatorBreakpointsManager, pauseHandler, + BreakpointsViewModel, pauseHandler, uiDispatcher, messenger, textClipboard); DisassemblyViewModels.Add(disassemblyVm); PaletteViewModel = new(argbPalette, uiDispatcher); diff --git a/src/Spice86/ViewModels/DisassemblyViewModel.cs b/src/Spice86/ViewModels/DisassemblyViewModel.cs index cbda0c30b..43140f0ec 100644 --- a/src/Spice86/ViewModels/DisassemblyViewModel.cs +++ b/src/Spice86/ViewModels/DisassemblyViewModel.cs @@ -27,14 +27,13 @@ public partial class DisassemblyViewModel : ViewModelWithErrorDialog { private readonly IPauseHandler _pauseHandler; private readonly IInstructionExecutor _cpu; private readonly BreakpointsViewModel _breakpointsViewModel; - private readonly EmulatorBreakpointsManager _emulatorBreakpointsManager; private readonly IDictionary _functionsInformation; private readonly InstructionsDecoder _instructionsDecoder; public DisassemblyViewModel( IInstructionExecutor cpu, IMemory memory, State state, IDictionary functionsInformation, - BreakpointsViewModel breakpointsViewModel, EmulatorBreakpointsManager emulatorBreakpointsManager, + BreakpointsViewModel breakpointsViewModel, IPauseHandler pauseHandler, IUIDispatcher uiDispatcher, IMessenger messenger, ITextClipboard textClipboard, bool canCloseTab = false) : base(uiDispatcher, textClipboard) { @@ -44,9 +43,8 @@ public DisassemblyViewModel( .Select(x => new FunctionInfo() { Name = x.Value.Name, Address = x.Key, - }).OrderBy(x => x.Address)); + }).OrderBy(x => x.Address)); AreFunctionInformationProvided = functionsInformation.Count > 0; - _emulatorBreakpointsManager = emulatorBreakpointsManager; _breakpointsViewModel = breakpointsViewModel; _messenger = messenger; _memory = memory; @@ -55,13 +53,20 @@ public DisassemblyViewModel( _instructionsDecoder = new(memory, state, functionsInformation, breakpointsViewModel); IsPaused = pauseHandler.IsPaused; pauseHandler.Pausing += OnPausing; - pauseHandler.Resumed += () => _uiDispatcher.Post(() => IsPaused = false); + pauseHandler.Resumed += OnResumed; CanCloseTab = canCloseTab; breakpointsViewModel.BreakpointDeleted += UpdateDisassemblyInternal; breakpointsViewModel.BreakpointDisabled += UpdateDisassemblyInternal; breakpointsViewModel.BreakpointEnabled += UpdateDisassemblyInternal; } + private void OnResumed() { + _uiDispatcher.Post(() => { + IsPaused = false; + Instructions.Clear(); + }); + } + [ObservableProperty] private bool _areFunctionInformationProvided; @@ -74,7 +79,7 @@ public DisassemblyViewModel( private void OnPausing() { _uiDispatcher.Post(() => { IsPaused = true; - if(Instructions.Count == 0 && GoToCsIpCommand.CanExecute(null)) { + if (Instructions.Count == 0 && GoToCsIpCommand.CanExecute(null)) { GoToCsIpCommand.Execute(null); } }); @@ -117,12 +122,14 @@ private void ConfirmCreateExecutionBreakpoint() { CreatingExecutionBreakpoint = false; if (!string.IsNullOrWhiteSpace(BreakpointAddress) && TryParseMemoryAddress(BreakpointAddress, out ulong? breakpointAddressValue)) { - AddressBreakPoint addressBreakPoint = new(BreakPointType.EXECUTION, - (long)breakpointAddressValue.Value, (breakpoint) => { - RequestPause(breakpoint, breakpointAddressValue.Value); - UpdateDisassemblyInternal(); - }, false); - _breakpointsViewModel.AddAddressBreakpoint(addressBreakPoint); + _breakpointsViewModel.AddAddressBreakpoint( + (long)breakpointAddressValue.Value, + BreakPointType.EXECUTION, + isRemovedOnTrigger: false, + () => { + RequestPause((long)breakpointAddressValue.Value); + UpdateDisassemblyInternal(); + }); } } @@ -159,19 +166,16 @@ private void UpdateHeader(uint? address) { [RelayCommand(CanExecute = nameof(IsPaused))] private void StepOver() { - if(SelectedInstruction is null) { + if (SelectedInstruction is null) { return; } long nextInstructionAddressInListing = SelectedInstruction.Address + SelectedInstruction.Length; - AddressBreakPoint addressBreakpoint = new AddressBreakPoint( - BreakPointType.EXECUTION, - nextInstructionAddressInListing, - (breakpoint) => { - RequestPause(breakpoint, (uint)nextInstructionAddressInListing); + _breakpointsViewModel.AddAddressBreakpoint( + nextInstructionAddressInListing, + BreakPointType.EXECUTION, isRemovedOnTrigger: true, () => { + RequestPause((uint)nextInstructionAddressInListing); _uiDispatcher.Post(() => GoToCsIpCommand.Execute(null)); - }, - isRemovedOnTrigger: true); - _emulatorBreakpointsManager.ToggleBreakPoint(addressBreakpoint, on: true); + }); _pauseHandler.Resume(); } @@ -205,8 +209,8 @@ private async Task StepInto() { [RelayCommand(CanExecute = nameof(IsPaused))] private void NewDisassemblyView() { DisassemblyViewModel disassemblyViewModel = new( - _cpu, _memory, _state, _functionsInformation, - _breakpointsViewModel, _emulatorBreakpointsManager, _pauseHandler, _uiDispatcher, _messenger, + _cpu, _memory, _state, _functionsInformation, + _breakpointsViewModel, _pauseHandler, _uiDispatcher, _messenger, _textClipboard, canCloseTab: true) { IsPaused = IsPaused }; @@ -221,7 +225,7 @@ private async Task GoToCsIp() { [RelayCommand(CanExecute = nameof(IsPaused))] private async Task GoToFunction(object? parameter) { - if(parameter is FunctionInfo functionInfo) { + if (parameter is FunctionInfo functionInfo) { await GoToAddress(functionInfo.Address); } } @@ -293,8 +297,8 @@ private async Task CopyLine() { } } - private void RequestPause(BreakPoint breakPoint, ulong address) { - string message = $"{breakPoint.BreakPointType} breakpoint was reached at address {address}."; + private void RequestPause(long address) { + string message = $"Execution breakpoint was reached at address {address}."; _pauseHandler.RequestPause(message); _uiDispatcher.Post(() => { _messenger.Send(new StatusMessage(DateTime.Now, this, message)); @@ -327,27 +331,24 @@ private void RemoveExecutionBreakpointHere() { if (SelectedInstruction?.Breakpoint is null) { return; } - _breakpointsViewModel.RemoveBreakpoint(SelectedInstruction.Breakpoint); + _breakpointsViewModel.RemoveBreakpointInternal(SelectedInstruction.Breakpoint); SelectedInstruction.Breakpoint = _breakpointsViewModel.GetBreakpoint(SelectedInstruction); } private bool CreateExecutionBreakpointHereCanExecute() => SelectedInstruction?.Breakpoint is not { Type: BreakPointType.EXECUTION }; - + [RelayCommand(CanExecute = nameof(CreateExecutionBreakpointHereCanExecute))] private void CreateExecutionBreakpointHere() { if (SelectedInstruction is null) { return; } - AddressBreakPoint breakPoint = new( - BreakPointType.EXECUTION, - SelectedInstruction.Address, - (breakpoint) => { - RequestPause(breakpoint, SelectedInstruction.Address); + long address = SelectedInstruction.Address; + _breakpointsViewModel.AddAddressBreakpoint(address, + BreakPointType.EXECUTION, isRemovedOnTrigger: false, () => { + RequestPause(address); UpdateDisassemblyInternal(); - }, - isRemovedOnTrigger: false); - _breakpointsViewModel.AddAddressBreakpoint(breakPoint); + }); SelectedInstruction.Breakpoint = _breakpointsViewModel.GetBreakpoint(SelectedInstruction); } } diff --git a/src/Spice86/ViewModels/MemoryViewModel.cs b/src/Spice86/ViewModels/MemoryViewModel.cs index c135ade07..5d0bcc1f2 100644 --- a/src/Spice86/ViewModels/MemoryViewModel.cs +++ b/src/Spice86/ViewModels/MemoryViewModel.cs @@ -153,8 +153,8 @@ private void BeginCreateMemoryBreakpoint() { public BreakPointType[] BreakpointTypes => [BreakPointType.ACCESS, BreakPointType.WRITE, BreakPointType.READ]; - private void OnBreakPointReached(BreakPoint breakPoint) { - string message = $"{breakPoint.BreakPointType} breakpoint was reached."; + private void OnBreakPointReached() { + string message = $"Memory breakpoint was reached."; _pauseHandler.RequestPause(message); _uiDispatcher.Post(() => { _messenger.Send(new StatusMessage(DateTime.Now, this, message)); @@ -186,9 +186,11 @@ private void ConfirmCreateMemoryBreakpoint() { } private void CreateMemoryAddressBreakpoint(ulong breakpointAddressValue) { - AddressBreakPoint addressBreakPoint = new(SelectedBreakpointType, - (long)breakpointAddressValue, OnBreakPointReached, false); - _breakpointsViewModel.AddAddressBreakpoint(addressBreakPoint); + _breakpointsViewModel.AddAddressBreakpoint( + (long)breakpointAddressValue, + SelectedBreakpointType, + isRemovedOnTrigger: false, + OnBreakPointReached); } [RelayCommand]