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

Breakpoint incorrectly triggers on CALL. #39

Open
mooskagh opened this issue Oct 17, 2021 · 3 comments
Open

Breakpoint incorrectly triggers on CALL. #39

mooskagh opened this issue Oct 17, 2021 · 3 comments
Labels

Comments

@mooskagh
Copy link

Possibly that's bug on my side, I'll look into it from my side further.

So, I'm trying to implement breakpoints, in a way similar to one in z80.h.

What I do:

void Machine::run(uint64_t max_ops) {
  for (uint64_t op = 0; op < max_ops; ++op) {
    events_ = 0;
    on_step();
    if (events_ & kEventInterrupt) {
      ++interrupts;
      on_handle_active_int();
    }
    if (event_mask_ & events_) break;
  }
}
void Machine::on_set_pc(z80::fast_u16 addr) {
  if (breakpoints_[addr]) events_ |= kEventBreakpoint;
  base::on_set_pc(addr);
}

Now I have the following code:

...
40147 | CALL 41055
40150 | CALL 40433
40153 | CALL 41602
...
...
41055 | PUSH HL  
41056 | LD A,121
...

When I do:

my_machine.breakpoints_[40150] = true;
my_machine.run(1000000);
std::cout << my_machine.get_pc();

I expect it to stop at 40150, but instead 41055 is output.

@kosarev
Copy link
Owner

kosarev commented Oct 24, 2021

Right, on_set_pc() does not look to be the best handler to catch breakpoints. And I have doubts about on_fetch_cycle() and even on_m1_fetch_cycle(). on_step() maybe?

But it won't stop at 40150 anyway, if you set a breakpoint at that address. The general approach is that for a breakpoint or watchpoint to be hit, the instruction has to be actually executed. It's the debugger's job to then figure out which breakpoint it is.

@kosarev kosarev added the bug label Oct 24, 2021
@mooskagh
Copy link
Author

In the end I did it in run(), in other places it had issues (especially given that I need to breakpoint before the instruction, like other libs do):

void Machine::run(uint64_t max_ops) {
  for (uint64_t op = 0; op < max_ops; ++op) {
    events_ = 0;
    on_step();
    if (events_ & kEventInterrupt) {
      ++interrupts;
      on_handle_active_int();
    }
    if (breakpoints_[get_pc()]) events_ |= kEventBreakpoint;
    if (event_mask_ & events_) break;
  }
}

@kosarev
Copy link
Owner

kosarev commented Dec 26, 2021

Yep, this should do. We couldn't do the same for watchpoints and other kinds of breakpoints, however, so we still might want to give this all a proper thought...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants