#174 Monitor trace/breakpoints should optionally ignore dummy accesses

general
open
nobody
None
new feature (Monitor)
2013-08-11
2013-07-22
Uffe Jakobsen
No

Monitor trace reports errorneus load on indirect zp store instructions

How to reproduce:

.C:1000 A9 00 LDA #$00
.C:1002 85 FC STA $FC
.C:1004 A9 04 LDA #$04
.C:1006 85 FD STA $FD
.C:1008 A0 23 LDY #$23
.C:100a 91 FC STA ($FC),Y
.C:100c 60 RTS
.C:100d 00 BRK
.C:100e 00 BRK

(C:$e5cf) del
Deleting all checkpoints
(C:$e5cf) tr 0400
TRACE: 1 C:$0400 (Trace load store)
(C:$e5cf) x

Inside BASIC execute: SYS4096
No trace occours

Now try this:

(C:$e5d1) del
(C:$e5d1) Deleting all checkpoints
(C:$e5d1) tr 0423
TRACE: 2 C:$0423 (Trace load store)
(C:$e5d1) x

Inside BASIC execute: SYS4096
Trace occours:

2 (Trace load 0423) 095 021

.C:100a 91 FC STA ($FC),Y - A:04 X:00 Y:23 SP:f7 ..-B.... 120104166

2 (Trace store 0423) 095 021

.C:100a 91 FC STA ($FC),Y - A:04 X:00 Y:23 SP:f7 ..-B.... 120104166

The first trace report is a bug - if trace was to report intermediate steps of instructions this should have been reported as (Trace load 0400) is something...

But to be honest I do not believe that the first trace (load) should be displayed at all.

This problem is seen on latest trunk 27609 using both x64 and x64sc (Linux) (but I have a feeling that this problem is not a new one)

/Uffe

Discussion

  • Uffe Jakobsen
    Uffe Jakobsen
    2013-07-22

    UPDATE:

    Problem seems that some of the sub-instruction macro-primitives generates a separate trace "event".

    This change seems to have a positive effect on the "STA ($ZP),Y" double-trace output - but I havent tested it for side effects nor timing correctness.

    $ svn diff src/6510core.c
    Index: src/6510core.c
    ===================================================================
    --- src/6510core.c  (revision 27609)
    +++ src/6510core.c  (working copy)
    @@ -2645,7 +2645,8 @@
                     break;
    
                 case 0x91:          /* STA ($nn),Y */
    -                STA_IND_Y(p1);
    +                //STA_IND_Y(p1);
    +                STA((LOAD_ZERO_ADDR(p1) + reg_y_read), 2, 2, 2, STORE_ABS);
                     break;
    
                 case 0x93:          /* SHA ($nn),Y */
    

    BTW; it seems the the instruction "LDA ($ZP),Y" suffers from same problem with double-trace output as "STA ($ZP),Y"

    /Uffe

     
  • gpz
    gpz
    2013-07-23

    "I do not believe that the first trace (load) should be displayed at all."
    it should, all dummy accesses should be shown too.

     
    • Uffe Jakobsen
      Uffe Jakobsen
      2013-07-23

      Hi,

      gpz: "it should, all dummy accesses should be shown too."

      What qualifies a dummy access ? Is it the zp access (I guess) ?

      If I specify a tracepoint for $0423 - why would I like to see a dummy access for something unrelated to $0423 ?
      I mean the zp's in my example point to $0400 and along with the y reg ($23) the target address there is nothing in the zp access that qualifies for my tracepoint ($0423)

      Also "STA ($ZP,X)" does not show its dummy access

      /Uffe

       
  • gpz
    gpz
    2013-07-23

    sorry, what you see is wrong ofcourse....

    by dummy access i ment eg the additional write done by RMW instructions, or the additional fetches done by eg RTI. (that said, i dont recall that zp-ind-y does anything like that)

     
  • Uffe Jakobsen
    Uffe Jakobsen
    2013-07-24

    Hi,

    Thanks for your reply and thanks for confirming that I'm not totally wrong :-)

    I investigated further and it seems that the "sta ($zp),y" instruktion is the only of the indirect instruction that have this mis-behaviour. I did not check the RMW instructions - but in their case multiple traces would be alright as they relate to the actual address traced for.

    I found the problem to be related to the actual instruction build-up.

    According til 64doc.txt:

         Write instructions (STA, SHA)
    
            #    address   R/W description
           --- ----------- --- ------------------------------------------
            1      PC       R  fetch opcode, increment PC
            2      PC       R  fetch pointer address, increment PC
            3    pointer    R  fetch effective address low
            4   pointer+1   R  fetch effective address high,
                               add Y to low byte of effective address
            5   address+Y*  R  read from effective address,
                               fix high byte of effective address
            6   address+Y   W  write to effective address
    
           Notes: The effective address is always fetched from zero page,
                  i.e. the zero page boundary crossing is not handled.
    
                  * The high byte of the effective address may be invalid
                    at this time, i.e. it may be smaller by $100.
    

    The "sta ($zp),y" instruction (opcode $91) is constructed from these macros

    File: src/6510core.c - line 2662:
    case 0x91: / STA ($nn),Y /
    STA_IND_Y(p1);
    break;

    File: src/6510core.c - line 2662:
    #define STA_IND_Y(addr) \ do { \ unsigned int tmp; \ \ CLK_ADD(CLK, 2); \ tmp = LOAD_ZERO_ADDR(addr); \ LOAD_IND((tmp & 0xff00) | ((tmp + reg_y_read) & 0xff)); \ CLK_ADD(CLK, CLK_IND_Y_W); \ INC_PC(2); \ STORE_IND(tmp + reg_y_read, reg_a_read); \ } while (0)

    Looking into the STA_IND_Y() macro - the contained LOAD_IND() macro seems totally useless and unrelated to the "sta ($zp),y" instauction:

            LOAD_IND((tmp & 0xff00) | ((tmp + reg_y_read) & 0xff)); \
    

    It loads - only to throw the result away ???

    The LOAD_IND() is actually also the macro that is responsible for the bogus trace - and I've now seen any side effects from removing the LOAD_IND()

    Currently the corrected STA_IND_Y() looks like this:

    #define STA_IND_Y(addr)                                         \
        do {                                                        \
            unsigned int tmp;                                       \
                                                                    \
            CLK_ADD(CLK, 2);                                        \
            tmp = LOAD_ZERO_ADDR(addr);                             \
            CLK_ADD(CLK, CLK_IND_Y_W);                              \
            INC_PC(2);                                              \
            STORE_IND(tmp + reg_y_read, reg_a_read);                \
        } while (0)
    

    So far every thing have worked with this change - but I still need to verify the timing. And I need to figure out if VICE have some sort of test ssuite that I can run. Mayby you can give me a hint ?

    If everything turns out as I hope - Iøll submit it as a patch report/request.

    Please let me know if I've overlooked something.

    /Uffe

     
    • Ingo Korb
      Ingo Korb
      2013-07-24

      The LOAD_IND invocation you removed from the STA_IND_Y macro exactly corresponds to the behaviour described in step 5 of your quote from 64doc.txt: The CPU does a dummy read access to an intermediate address where the low byte of the address has already been adjusted by Y and the high byte may still be incorrect if the result of the addition needs to overflow from the low to the high byte.

      Unlike LDA (zp),Y the dummy read access always happens for STA, even if there is no need for an additional cycle to calculate the overflow.

       
      • Uffe Jakobsen
        Uffe Jakobsen
        2013-07-24

        Hi,

        Thanks for the answer - I missed that point - hmm back to zero :-)

         
  • Uffe Jakobsen
    Uffe Jakobsen
    2013-07-24

    I was looking for a way of following the results of the execution - just like trace (reporting without breaking into the monitor) - but also without the dummy accesses instruction fetches etc - I was only interested in the "effective" results from the instructions.
    But at far as I can see this is not possible with any of the current break, watch, trace functionality.

    Ideas are welcome.

    Thanks :-)

     
  • gpz
    gpz
    2013-07-26

    that would require some major rewriting of the break/watchpoint stuff, i guess

     
  • gpz
    gpz
    2013-08-11

    a) i am moving this to feature requests, as the original issue was no bug
    b) here is an idea on how to implement what you want to do: for the macros used for the "dummy accesses" create duplicates (like LOAD_IND_DUMMY) and then make the respective break/watchpoint stuff within those macros optional so that it can be selected at runtime whether you want to include them or not... should do the trick

     
    • Uffe Jakobsen
      Uffe Jakobsen
      2013-08-12

      Thanks for your suggestion - I'll look into that - thx

       
  • gpz
    gpz
    2013-08-11

    Ticket moved from /p/vice-emu/bugs/458/

    Can't be converted:

    • _milestone: v2.4.x
    • _port: Linux
     
  • gpz
    gpz
    2013-08-11

    • summary: Monitor trace reports errorneus load on indirect zp store instructions. --> Monitor trace/breakpoints should optionally ignore dummy accesses
    • Group: v2.4.x --> general
    • Category: Monitor --> new feature (Monitor)