Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Internal Debugger: All breakpoints can be created (including I/O ports breakpoints) + QoL improvements #998

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ public DosInt21Handler(IMemory memory, Cpu cpu,
FillDispatchTable();
}

public override void Run(int index) {
base.Run(index);
}

Comment on lines +70 to +73
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

private void FillDispatchTable() {
AddAction(0x00, QuitWithExitCode);
AddAction(0x02, DisplayOutput);
Expand Down
3 changes: 3 additions & 0 deletions src/Spice86/MemoryWrappers/InstructionsDecoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public List<CpuInstructionInfo> DecodeInstructions(uint startAddress,
var instructions = new List<CpuInstructionInfo>();
while (instructions.Count < numberOfInstructionsShown) {
long instructionAddress = codeMemoryStream.Position;
if(instructionAddress >= emulatedMemoryStream.Length) {
break;
}
decoder.Decode(out Instruction instruction);
CpuInstructionInfo instructionInfo = new() {
Instruction = instruction,
Expand Down
93 changes: 57 additions & 36 deletions src/Spice86/ViewModels/BreakpointViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,50 +2,59 @@

using CommunityToolkit.Mvvm.ComponentModel;

using Spice86.Core.Emulator.VM;
using Spice86.Core.Emulator.VM.Breakpoint;
using Spice86.Models.Debugging;

public partial class BreakpointViewModel : ViewModelBase {
public partial class BreakpointViewModel : ViewModelBase
{
private readonly Action _onReached;
private readonly EmulatorBreakpointsManager _emulatorBreakpointsManager;
private AddressBreakPoint? _breakPoint;
private BreakPoint? _breakPoint;

public BreakpointViewModel(
BreakpointsViewModel breakpointsViewModel,
EmulatorBreakpointsManager emulatorBreakpointsManager,
uint address,
BreakPointType type,
bool isRemovedOnTrigger,
Action onReached,
string comment = "") {
long addressOrIoPortOrCyclesOrInterruptNumber,
BreakPointType type,
bool isRemovedOnTrigger,
Action onReached,
string comment = "")
{
_emulatorBreakpointsManager = emulatorBreakpointsManager;
Address = address;
Address = addressOrIoPortOrCyclesOrInterruptNumber;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe trigger is a friendlier name?

Type = type;
IsRemovedOnTrigger = isRemovedOnTrigger;
if(IsRemovedOnTrigger) {
_onReached = () => {
if (IsRemovedOnTrigger)
{
_onReached = () =>
{
breakpointsViewModel.RemoveBreakpointInternal(this);
onReached();
};
} else {
}
else
{
_onReached = onReached;
}

Check notice

Code scanning / CodeQL

Missed ternary opportunity Note

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Comment = comment;
Enable();
}

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;

public bool IsEnabled {
public bool IsEnabled
{
get => _isEnabled;
set {
if (value) {
set
{
if (value)
{
Enable();
} else {
}
else
{
Disable();
}
SetProperty(ref _isEnabled, value);
Expand All @@ -54,48 +63,60 @@

public bool IsRemovedOnTrigger { get; }

public uint Address { get; }
public long Address { get; }

public void Toggle() {
if (IsEnabled) {
public void Toggle()
{
if (IsEnabled)
{
Disable();
} else {
}
else
{
Enable();
}
}

[ObservableProperty]
private string? _comment;

private AddressBreakPoint GetOrCreateBreakpoint() {
_breakPoint ??=
new AddressBreakPoint(
Type,
Address,
(_) => _onReached(),
private BreakPoint GetOrCreateBreakpoint() {
_breakPoint ??= new AddressBreakPoint(Type,
Address, (_) => _onReached(),
IsRemovedOnTrigger);
return _breakPoint;
}

public void Enable() {
if (IsEnabled) {
public void Enable()
{
if (IsEnabled)
{
return;
}
_emulatorBreakpointsManager.ToggleBreakPoint(GetOrCreateBreakpoint(), on: true);
_emulatorBreakpointsManager.ToggleBreakPoint(GetOrCreateBreakpoint(),
on: true);
_isEnabled = true;
OnPropertyChanged(nameof(IsEnabled));
}

public void Disable() {
if (!IsEnabled) {
public void Disable()
{
if (!IsEnabled)
{
return;
}
_emulatorBreakpointsManager.ToggleBreakPoint(GetOrCreateBreakpoint(), on: false);
_emulatorBreakpointsManager.ToggleBreakPoint(GetOrCreateBreakpoint(),
on: false);
_isEnabled = false;
OnPropertyChanged(nameof(IsEnabled));
}

internal bool IsFor(CpuInstructionInfo instructionInfo) {
return Address == instructionInfo.Address;
internal bool IsFor(CpuInstructionInfo instructionInfo)
{
return Address == instructionInfo.Address &&
(Type == BreakPointType.CPU_EXECUTION_ADDRESS ||
Type == BreakPointType.MEMORY_READ ||
Type == BreakPointType.MEMORY_WRITE ||
Type == BreakPointType.MEMORY_ACCESS);
}
}
}
Loading
Loading