conditional traps

[ale-79]

From the stella 5.0 thread on AA:

It could be useful to have conditional trap commands in the debugger, similar to conditional breaks, and using the same functions and pseudo-registers:

trapif 		<condition> 	<address or range>
trapwriteif 	<condition> 	<address or range>
trapreadif 	<condition> 	<address or range>

For example:
trapreadif _bank==1 BE00 BEFF

would trap any read from addresses $BE00 to $BEFF and mirrors if bank 1 is active

[thrust26]

...and mirrors! 😆

[ale-79]

Corrected!

[ale-79]

I think the condition should be evaluated at the start of the read/write operation.

So in the previous example, in case the address is an hotspot, the condition is true if the starting bank is 1, that is before the bankswitching caused by accessig the hotspot.

[thrust26]

But the trap is evaluated afterwards.

[ale-79]

Would it be possible to evaluate the condition, then perform the memory access, and then, only if the condition was true, evaluate the trap?

I thought it would be preferable in the case of hotspot access, but no big issue if it's not feasible.

And, thinking more about it, there are probably situations (when storing values in ram or registers, for example) where it might be preferable to evaluate the condition after the memory access.

[thrust26]

Actually I would prefer the trap to stop before instruction execution (like a break). Then you can move one step forward if you want.

Not sure if that's feasible.

[thrust26]

I scratched my head about interrupting before a trap is hit.

The problem is, that a trap can trigger somewhere in the middle of an instruction. And if you want to break there, you have to be able to continue from exactly there too. Unless someone has a better idea, it seems like that would require a major rewrite of the 6502 emulation core.
We would have to:

1. know the exact spot where we interrupted the instruction
2. avoid changing e.g. the program counter when doing a read/or write

So a current:

  A = peek(PC++);

would have to be changed into something like:

label_instruction_step_n:
  continueLabel = label_instruction_step_n;
  A = peek(PC);
  PC++

And that for all 240 or so opcodes.

[thrust26]

@ale-79 I am currently working on trapifs. This turned out to be quite adventurous. 😄

E.g. I found that two traps with overlapping address ranges eliminate themselves where they overlap. I don't think this is a good idea for trapifs, because usually their conditions will differ. Only in the special case where the conditions are identical, this might make sense and would be consistent with trap (which would be just a special case of trapif with a condition always being true).

Is that what you would expect from trapif? And would you be prepared to test this extensively?

[ale-79]

Is that what you would expect from trapif?

Yes. I think that it could behave the same as break and breakif.
Defining the same (unconditional) trap twice, as in the case of overlapping ranges, would clear it, while conditional traps would be identified by a number (like breakif) and only cleared by specifying that number (e.g. cleartrapif number). The ability to specify a range of numbers could also be useful, as using a range of addresses will create a bunch of traps with sequential numbers.
Two conditional traps at the same address but with different condition would be separate entities and you could clear just one of them leaving the other one untouched.
Clearing trapifs when they are defined twice with the same condition would be a nice feature, but I think it's not strictly required.

And would you be prepared to test this extensively?

I sure will, but I also suggest to open a thread on AA in the emulation or 2600 programming forum so that you can get more authoritative opinions and feedback from programmers.

[thrust26]

Thanks for the feedback. I have discussed the issue and some technical implications with Stephen today. (BTW: we were also wondering about how to delete the traps)

An alternative Idee was to extend breakif so that you could define read/write access ranges as additional conditions.
E.g. breakif _scan>50 && readaccess(f000 f100)

[ale-79]

That sounds like a good solution to me.

[ale-79]

Is it feasible, when constinuous savestates is enabled with ALT-R, to reserve a slot for a savestate of the situation before executing each instruction, in addition to those saved every frame so that if a break/trap is triggered, you can go back before the memory access occurs?

The first rewind would be of 1 instruction, the following ones of 1 frame as per #71.

[thrust26]

I am sure that would be way too slow.

[sa666666]

To be clear, with breakif {_scan>50 && readaccess(f000 f100)} we only need to create a function for readaccess; the rest of the command can already be parsed, including the '&&'.

[thrust26]

I noticed that.

Probably adding brackets around all conditions before the && access part might become the most problematic part. But I have to look into the code.

[thrust26]

@ale-79 I have implemented 'trapif' in a new branch called 'trapif_attempt_1'. Please give it a try.

Things to look for:

1. Is the old 'trap' still working as before? (Note: overlapping traps do not eliminate each other anymore)
2. Do listing, deleting and clearing traps work correctly?
3. Are the mirrors handled correctly? (I think there was before and still is a bug with TIA and zeropage RAM mirrors)
4. Any performance issues you notice, especially when stressing with lots of 'traps' and 'trapifs'?
5. Anything you else notice. 😄

Of course I have made developer tests for everything already, but not that thoroughly. Thanks in advance!

[ale-79]

I won't be able to test it until tomorrow as I won't be at home today after lunch.
Anyway, I cloned the branch in a new directory but couldn't compile it.

This is the error I get (I figured out how to have "make" print out errors in English, instead of the Italian/English mix I usually get...):

In file included from src/debugger/Debugger.cxx:36:0:
src/emucore/M6502.hxx:27:12: fatal error: TrapArray.hxx: No such file or directory
   #include "TrapArray.hxx"
            ^~~~~~~~~~~~~~~
compilation terminated.
make: *** [Makefile:157: src/debugger/Debugger.o] Error 1

[thrust26]

My bad. Fixed (I hope).

[ale-79]

Yup, it works now.

[thrust26]

@ale-79 Any updates from your test?

[ale-79]

Not much to report so far, I need to do more tests.

Old traps seem to work like before. I like the new method of displaying address ranges in a single line, it makes the 'listtraps' output more readable.
The same for using labels for registers and ram.

Conditional traps seem to work fine too.

'listtraps', 'deltrap' and 'cleartraps' work as expected.

It seems that mirrors work correctly now, (e.g. 'trap 40 7f')

There's a bug in the status line: addresses/labels are not shown if they're in cart space (this is also true for breakpoints, not only traps).

[thrust26]

Thanks. I have committed trapif (and more) with 3fddc03 to the master now. (the last point is fixed too)

[thrust26]

BTW: You can also delete trap(if)s by adding the same one again

[ale-79]

I just compiled the master with 3fddc03, but the status line is still missing the address if in cart space.
e.g. in "Combat" if I set "trapread f070", the status line just contains "RTrap( ):"

[thrust26]

Sorry, my old fix only worked for user labels, not bare addresses. Please try again.

[ale-79]

Works!