Menu

#4039 memory corruption when using --allow-undocumented-instructions through iyl and iy

closed-fixed
z80 (19)
other
5
2 days ago
4 days ago
Under4Mhz
No

The following code updates the wrong address for vdu_interrupt_state

/// GPL 2.0 or later
#include <stdio.h>
#include <stdint.h>
#include <stdbool.h>
#include <stdlib.h>

static volatile __sfr __at 0x00 VDPControlPort;
static volatile __sfr __at 0x01 VDPDataPortOut;

bool vdu_interrupt_state;
#define VDU_ADDRESS_SET_MASK 0
#define BYTE_HI(x) ((x)>>8)

#define vdu_interrupt_enable_fast() { extern bool vdu_interrupt_state; vdu_interrupt_state = true;  }
#define vdu_interrupt_disable_fast() { extern bool vdu_interrupt_state; vdu_interrupt_state = false; }
#define vdu_next_set_fast(value) VDPDataPortOut = value
#define vdu_address_set_unsafe(address) { VDPControlPort=address; VDPControlPort=BYTE_HI((address) | VDU_ADDRESS_SET_MASK ); }
#define vdu_address_set_fast(address) { vdu_interrupt_disable_fast(); vdu_address_set_unsafe(address); vdu_interrupt_enable_fast(); }

void print_hl( uint16_t hl ) { printf( "%x\n", hl ); }

inline void TileSet( uint16_t addr, uint8_t id ) {

    id *= 4;
    addr += 0x1000;

    vdu_address_set_fast( addr );
    addr += 32;

    vdu_address_set_fast( addr );
}

inline void DrawSprite( uint16_t addr, uint8_t id ) {

    TileSet( addr, id );
}

void LevelDraw() {

    uint8_t row = 24;
    uint16_t addr = 32;

    do {

        int current = 0;
        uint8_t col = 32;

        do {
            DrawSprite( addr, current );
        } while( --col );

        addr += 32;

    } while (--row);
}

void main(void) {

    printf( "Start\n" );
    LevelDraw();
    printf( "End\n" );
}

#ifdef __SDCC
__sfr __at 0xff sif;
int putchar( int c ) {

    sif = 'p';
    sif = c;

    return c;
}
#endif

// sdcc -mz80  --fverbose-asm ./iy_corrupted.c -o iy_corrupted.ihx --allow-undocumented-instructions  && ucsim_z80 -I if=outputs[0xff] iy_corrupted.ihx
// gcc ./iy_corrupted.c && ./a.out

From the assembler below, iy is set to vdu_interrupt_state, then iyl is used in the addition. iy is used again as vdu_interrupt_state, but iy has changed due to the use of iyl between

...
;./iy_corrupted.c:27: vdu_address_set_fast( addr );
    ld  iy, #_vdu_interrupt_state
    ld  0 (iy), #0x00
    out (_VDPControlPort), a
    push    af
    ld  a, h
    out (_VDPControlPort), a
    pop af
    ld  0 (iy), #0x01
;./iy_corrupted.c:28: addr += 32;
    add a, #0x20
    ld  iyl, a
    ld  a, h
    adc a, #0x00
    ld  h, a
    ld  a, iyl
;./iy_corrupted.c:30: vdu_address_set_fast( addr );
    ld  0 (iy), #0x00
    ...
SDCC : z80/sm83/ez80/z80n/mos6502/mos65c02 4.6.2 #16701 (Linux)
3 Attachments

Discussion

  • Arjan Bakker

    Arjan Bakker - 3 days ago

    Which SDCC build are you using?

    I was using SDCC 4.6.0 (release version) and I encountered a similar issue, incorrect register usage when using an inline function. I just installed the most recent snapshot (build 16701) and the problem is gone.

    Might just be a coincidence, and I'm not using undocumented instructions, but worth a try.

     
    • Under4Mhz

      Under4Mhz - 2 days ago

      I was using SDCC 4.6.2 (I usually download the latest to make sure it hasn't been fixed recently)

      I don't think it's the inline function precisely. It's getting sdcc to use all the registers without using the stack, which it will use iyl as a last resort. Which is good, but in this case it doesn't appear to be taking into account that iyl is part of iy.

      Perhaps consider submitting a report when you find an issue like that. It can take a bit of work to replicate an issue like this, but I do get a bit of a buzz when I see the developers fix an issue I've submitted. The output of SDCC is nearly perfect these days thanks to the hard work of the SDCC team, and I like to think I've contributed in some small part to that.

       
      • Philipp Klaus Krause

        The bug reports - if/when the bug gets fixed will have helped make SDCC better, both for the obvious case of users in similar situations (here retrogaming enthusiasts using -mz80 --allow-undocumented-instructions), and in different ones (through I haven't been able to reproduce the bug, this issue in principle also affects -mez80, i.e. SDCC users writing firmware for some embedded IoT devices using an eZ80 could also be affected by it).

         
    • Under4Mhz

      Under4Mhz - 2 days ago

      I'm usually able to work around the issues when I know what the problem is. In this case I moved the interrupt calls to the outer loop, which made it faster as well.

       
  • Philipp Klaus Krause

    I can reproduce the issue using SDCC built from current trunk on my Debian GNU/Linux testing system.

     
  • Philipp Klaus Krause

    • labels: --> z80
    • assigned_to: Philipp Klaus Krause
     
  • Philipp Klaus Krause

    • status: open --> closed-fixed
     
  • Philipp Klaus Krause

    Fixed in [r16708].

     

    Related

    Commit: [r16708]


Log in to post a comment.

MongoDB Logo MongoDB