From 340c8294f4702ca6e2fc7399d00fe4ce91f82fd8 Mon Sep 17 00:00:00 2001 From: "Christopher A. Mosher" Date: Sun, 11 Dec 2022 02:07:28 -0500 Subject: [PATCH] fix uninitialized vars (per valgrind); add debugging for LSS13; add some std lib bounds checking --- src/E2wxApp.cpp | 2 + src/addressbus.cpp | 23 ++++++- src/addressbus.h | 3 + src/analogtv.cpp | 9 +-- src/cpu.cpp | 9 ++- src/diskcontroller.cpp | 11 +++- src/diskcontroller.h | 4 ++ src/keyboard.cpp | 1 + src/lss.cpp | 145 +++++++++++++++++++++++++++++++---------- src/lss.h | 16 ++--- src/textcharacters.h | 2 +- src/video.cpp | 4 +- src/video.h | 4 +- src/wozfile.cpp | 20 +++++- 14 files changed, 192 insertions(+), 61 deletions(-) diff --git a/src/E2wxApp.cpp b/src/E2wxApp.cpp index e208f23..eedf628 100644 --- a/src/E2wxApp.cpp +++ b/src/E2wxApp.cpp @@ -136,6 +136,8 @@ bool E2wxApp::OnInit() { InitBoostLog(); + BOOST_LOG_TRIVIAL(info) << "Application ID: " << this->GetID(); + BOOST_LOG_TRIVIAL(info) << "Application version: " << this->GetVersion(); this->confdir = dirConfig() / path_from_string(GetID()+".d"); diff --git a/src/addressbus.cpp b/src/addressbus.cpp index e8e6d4f..d3cdd96 100644 --- a/src/addressbus.cpp +++ b/src/addressbus.cpp @@ -25,10 +25,11 @@ #include "speakerclicker.h" #include "cassettein.h" #include "cassetteout.h" +#include "diskcontroller.h" #include "slots.h" AddressBus::AddressBus(ScreenImage& gui, int& revision, MemoryRandomAccess& ram, Memory& rom, Keyboard& kbd, VideoMode& vid, Paddles& paddles, PaddleButtonStates& paddleButtonStates, SpeakerClicker& speaker, CassetteIn& cassetteIn, CassetteOut& cassetteOut, Slots& slts): - gui(gui), revision(revision), ram(ram), rom(rom), kbd(kbd), vid(vid), paddles(paddles), paddleButtonStates(paddleButtonStates), speaker(speaker), cassetteIn(cassetteIn), cassetteOut(cassetteOut), slts(slts) { + gui(gui), revision(revision), ram(ram), rom(rom), kbd(kbd), vid(vid), paddles(paddles), paddleButtonStates(paddleButtonStates), speaker(speaker), cassetteIn(cassetteIn), cassetteOut(cassetteOut), slts(slts), debugoutpos(0), debugfirst(true) { } @@ -153,6 +154,26 @@ void AddressBus::readSwitch(unsigned short address) { const int islot = (address >> 4) & 0xF; const int iswch = (address & 0xF); this->data = this->slts.io(islot, iswch, this->data, false); + /////////////////////////////////////////////// + // debug raw nibble disk reads + if (islot==6 && ((this->data & 0x80u) != 0u)) { + if (debugfirst) { + debugfirst = false; + for (int i = 0; i < 128; ++i) { + printf("%02X", i); + } + printf("\n"); + } + printf("%02X", this->data); + ++debugoutpos; + if (128 <= debugoutpos) { + debugoutpos = 0; + printf("\n"); +// DiskController* dsk = (DiskController*)(this->slts.get(6)); +// dsk->dumpLss(); + } + } + /////////////////////////////////////////////// } } diff --git a/src/addressbus.h b/src/addressbus.h index 08d2c50..1c5f464 100644 --- a/src/addressbus.h +++ b/src/addressbus.h @@ -47,6 +47,9 @@ class AddressBus { unsigned char data; // this emulates the (floating) data bus + int debugoutpos; + bool debugfirst; + void setD7(const bool set); void readSwitch(unsigned short address); void writeSwitch(unsigned short address); diff --git a/src/analogtv.cpp b/src/analogtv.cpp index f3b7d47..c51ceb1 100644 --- a/src/analogtv.cpp +++ b/src/analogtv.cpp @@ -44,6 +44,7 @@ AnalogTV::AnalogTV(ScreenImage& image): image(image), on(false), noise(false), + type(TV_OLD_COLOR), bleed_down(true) { hirescolor.push_back(colors.c()[A2ColorsObserved::HIRES_GREEN]); @@ -115,7 +116,7 @@ void AnalogTV::toggleBleedDown() // int i = ((yiqv>>8)&0xFF)-IQINTOFF; // int q = ((yiqv>>16)&0xFF)-IQINTOFF; // System.out.printf("(%+04d,%+04d,%+04d)",y,i,q); -// +// // const int rgb = yiq2rgb(yiqv); // const int r = (rgb >> 16) & 0xff; // const int g = (rgb >> 8) & 0xff; @@ -124,7 +125,7 @@ void AnalogTV::toggleBleedDown() // } // System.out.println(); // } -// +// // } @@ -461,8 +462,8 @@ int inline AnalogTV::yiq2rgb(const int yiq) double b = (((yiq)&0xFF)-IQINTOFF) - 1.105 * (((yiq>>8)&0xFF)-IQINTOFF) + 1.702 * (((yiq>>16)&0xFF)-IQINTOFF); const int rgb = - (calc_color(r) << 16)| - (calc_color(g) << 8)| + (calc_color(r) << 16)| + (calc_color(g) << 8)| (calc_color(b) << 0); return rgb; diff --git a/src/cpu.cpp b/src/cpu.cpp index 5387450..5181dfe 100644 --- a/src/cpu.cpp +++ b/src/cpu.cpp @@ -24,7 +24,14 @@ CPU::CPU(AddressBus& addressBus): - addressBus(addressBus) + addressBus(addressBus), + started(false), + pendingReset(false), + pendingIRQ(false), + pendingNMI(false), + p(PMASK_M), + t(0), + data(0) { } diff --git a/src/diskcontroller.cpp b/src/diskcontroller.cpp index eb6b694..05dd2e9 100644 --- a/src/diskcontroller.cpp +++ b/src/diskcontroller.cpp @@ -26,8 +26,11 @@ DiskController::DiskController(ScreenImage& gui, int slot, bool lss13, double ra load(false), write(false), ioStepped(false), + dataBusReadOnlyCopy(0), lssp6rom(lss13), + dataRegister(0), seq(0x20), // gotta start somewhere + prev_seq(seq), t(0) { } @@ -125,6 +128,8 @@ void DiskController::rotateCurrentDisk() { } } +enum lss_cmd { NOP, SL, SR, LD }; + void DiskController::stepLss() { std::uint8_t adr = this->write<<3 | this->load<<2 | (this->dataRegister>>7)<<1 | this->currentDrive->readPulse(); std::uint8_t cmd = this->lssp6rom.read(this->seq|adr); @@ -140,14 +145,14 @@ void DiskController::stepLss() { } } switch (cmd & 3u) { - case 3: + case LD: this->dataRegister = this->dataBusReadOnlyCopy; break; - case 2: + case SR: this->dataRegister >>= 1; this->dataRegister |= (isWriteProtected() << 7); break; - case 1: + case SL: this->dataRegister <<= 1; this->dataRegister |= ((cmd & 4u) >> 2); break; diff --git a/src/diskcontroller.h b/src/diskcontroller.h index 26b86ac..28f4f24 100644 --- a/src/diskcontroller.h +++ b/src/diskcontroller.h @@ -152,4 +152,8 @@ public: virtual std::string getName() { return "disk][ drive 1 drive 2 "; } + + void dumpLss() { + this->lssp6rom.dump(); + } }; diff --git a/src/keyboard.cpp b/src/keyboard.cpp index 1aa2e1f..c953b1b 100644 --- a/src/keyboard.cpp +++ b/src/keyboard.cpp @@ -23,6 +23,7 @@ Keyboard::Keyboard(KeypressQueue& q, HyperMode& fhyper, KeyboardBufferMode& buff keys(q), fhyper(fhyper), buffered(buffered), + latch(0xC1u), // TODO: randomize this a little cGet(0) { } diff --git a/src/lss.cpp b/src/lss.cpp index f859fa8..ce83053 100644 --- a/src/lss.cpp +++ b/src/lss.cpp @@ -22,16 +22,16 @@ #include #include -static void setcmd(std::uint8_t lssrom[], std::uint8_t x, std::uint8_t cmd) { - lssrom[x] = (lssrom[x] & 0xF0u) | cmd; +static void setcmd(std::array &lssrom, std::uint8_t x, std::uint8_t cmd) { + lssrom.at(x) = (lssrom.at(x) & 0xF0u) | cmd; } -static void setseq(std::uint8_t lssrom[], std::uint8_t x, std::uint8_t seq) { - lssrom[x] = (lssrom[x] & 0x0Fu) | seq; +static void setseq(std::array &lssrom, std::uint8_t x, std::uint8_t seq) { + lssrom.at(x) = (lssrom.at(x) & 0x0Fu) | seq; } -static void setbth(std::uint8_t lssrom[], std::uint8_t x, std::uint8_t both) { - lssrom[x] = both; +static void setbth(std::array &lssrom, std::uint8_t x, std::uint8_t both) { + lssrom.at(x) = both; } static void inst(std::uint8_t seq, std::uint8_t inst) { @@ -71,28 +71,28 @@ static void inst(std::uint8_t seq, std::uint8_t inst) { printf(" "); } -static void showua2seq(std::uint8_t lssrom[], std::uint8_t seq) { +static void showua2seq(const std::array &lssrom, std::uint8_t seq) { const std::uint8_t s(seq >> 4); printf("%1X: | ", s); - inst(s,lssrom[seq|0x9]); - inst(s,lssrom[seq|0xB]); - inst(s,lssrom[seq|0xD]); - inst(s,lssrom[seq|0xF]); + inst(s,lssrom.at(seq|0x9)); + inst(s,lssrom.at(seq|0xB)); + inst(s,lssrom.at(seq|0xD)); + inst(s,lssrom.at(seq|0xF)); printf("| "); - inst(s,lssrom[seq|0x8]); - inst(s,lssrom[seq|0xA]); - inst(s,lssrom[seq|0xC]); - inst(s,lssrom[seq|0xE]); + inst(s,lssrom.at(seq|0x8)); + inst(s,lssrom.at(seq|0xA)); + inst(s,lssrom.at(seq|0xC)); + inst(s,lssrom.at(seq|0xE)); printf("| "); - inst(s,lssrom[seq|0x1]); - inst(s,lssrom[seq|0x0]); - inst(s,lssrom[seq|0x3]); - inst(s,lssrom[seq|0x2]); + inst(s,lssrom.at(seq|0x1)); + inst(s,lssrom.at(seq|0x0)); + inst(s,lssrom.at(seq|0x3)); + inst(s,lssrom.at(seq|0x2)); printf("| "); - inst(s,lssrom[seq|0x5]); - inst(s,lssrom[seq|0x4]); - inst(s,lssrom[seq|0x7]); - inst(s,lssrom[seq|0x6]); + inst(s,lssrom.at(seq|0x5)); + inst(s,lssrom.at(seq|0x4)); + inst(s,lssrom.at(seq|0x7)); + inst(s,lssrom.at(seq|0x6)); printf("\n"); if (s == 7) { printf(" +-------------------------+-------------------------+-------------------------+------------------------\n"); @@ -101,6 +101,13 @@ static void showua2seq(std::uint8_t lssrom[], std::uint8_t seq) { LSS::LSS(bool use13SectorDos32LSS): use13Sector(use13SectorDos32LSS) { + + // fill all with invalid value, to aid in error detection + for (int i = 0; i < 0x100; ++i) { + this->lssrom.at(i) = 0x44u; + this->lss13rom.at(i) = 0x44u; + } + /* * LSS P6 ROM is stored here with different addressing bits * than in the original hardware, just for ease of understanding. @@ -121,12 +128,12 @@ LSS::LSS(bool use13SectorDos32LSS): for (std::uint8_t s(0u); s < 0x10u; ++s) { std::uint8_t seq(s << 4u); for (std::uint8_t adr(0u); adr < 0x10u; ++adr) { - lssrom[seq|adr] = seq+0x18u; + lssrom.at(seq|adr) = seq+0x18u; if ((adr & 0xCu) == 4u) { - lssrom[seq|adr] = 0x0Au; + lssrom.at(seq|adr) = 0x0Au; } if (adr == 1u || adr == 3u) { - lssrom[seq|adr] = 0xD8u; + lssrom.at(seq|adr) = 0xD8u; } } } @@ -217,17 +224,83 @@ LSS::LSS(bool use13SectorDos32LSS): setseq(lss13rom,0x23u,0x30u); setseq(lss13rom,0x33u,0xD0u); - printf("Disk ][ Controller Logic State Sequencer ROM:\n"); - if (use13Sector) { - for (unsigned int seq = 0; seq < 0x100u; seq += 0x10u) { - showua2seq(lss13rom,seq); - } - } else { - for (unsigned int seq = 0; seq < 0x100u; seq += 0x10u) { - showua2seq(lssrom,seq); - } - } + dump(); } LSS::~LSS() { } + +void LSS::dump() const { + printf("%s\n", "Disk ][ Controller Logic State Sequencer ROM:"); + if (this->use13Sector) { + printf("%s\n", "for 13-sector disks"); + for (unsigned int seq = 0; seq < 0x100u; seq += 0x10u) { + showua2seq(this->lss13rom,seq); + } +// printf("%s\n", "==================================================="); +// for (int r = 0; r < 0x10; ++r) { +// for (int c = 0; c < 0x10; ++c) { +// printf("%02X ", this->lss13rom.at(r*0x10+c)); +// } +// printf("\n"); +// } + } else { + printf("%s\n", "for 16-sector disks"); + for (unsigned int seq = 0; seq < 0x100u; seq += 0x10u) { + showua2seq(this->lssrom,seq); + } +// printf("%s\n", "==================================================="); +// for (int r = 0; r < 0x10; ++r) { +// for (int c = 0; c < 0x10; ++c) { +// printf("%02X ", this->lssrom.at(r*0x10+c)); +// } +// printf("\n"); +// } + } +} + +/* + * AA 10101010 + * AE 10101110 + + * B6 10110110 + * BA 10111010 + * BE 10111110 + + * D6 11010110 + * DA 11011010 + * DE 11011110 + + * EA 11101010 + * EE 11101110 + + * F6 11110110 + * FA 11111010 + * FE 11111110 + */ + +std::uint8_t LSS::read(const std::uint8_t addr) { + int i = addr; + i &= 0x000000FF; + if (0x100 <= i) { + throw std::runtime_error("bad lss ROM read"); + } + + //return use13Sector ? lss13rom[addr] : lssrom[addr]; + std::uint8_t r = 0; + if (this->use13Sector) { + const std::uint8_t candidate = this->lss13rom.at(i); + if (candidate == 0x44u) { + throw std::runtime_error("attempt to read uninitialized LSS ROM"); + } + r = candidate; + } else { + const std::uint8_t candidate = this->lssrom.at(i); + if (candidate == 0x44u) { + throw std::runtime_error("attempt to read uninitialized LSS ROM"); + } + r = candidate; + } + + return r; +} diff --git a/src/lss.h b/src/lss.h index d528e1d..6835ef6 100644 --- a/src/lss.h +++ b/src/lss.h @@ -20,22 +20,22 @@ #ifndef LSS_H #define LSS_H +#include +#include #include -class LSS -{ +class LSS { private: - bool use13Sector; - std::uint8_t lssrom[0x100]; - std::uint8_t lss13rom[0x100]; + const bool use13Sector; + std::array lss13rom; + std::array lssrom; public: LSS(bool use13SectorDos32LSS); ~LSS(); - std::uint8_t read(const std::uint8_t addr) { - return use13Sector ? lss13rom[addr] : lssrom[addr]; - } + std::uint8_t read(const std::uint8_t addr); + void dump() const; }; #endif diff --git a/src/textcharacters.h b/src/textcharacters.h index 6930aae..378930c 100644 --- a/src/textcharacters.h +++ b/src/textcharacters.h @@ -30,7 +30,7 @@ public: ~TextCharacters() {} unsigned char get(unsigned int iRow) { - return this->rows[iRow]; + return this->rows.at(iRow); } }; diff --git a/src/video.cpp b/src/video.cpp index a0fced4..1deff66 100644 --- a/src/video.cpp +++ b/src/video.cpp @@ -78,7 +78,7 @@ unsigned char Video::getDataByte() { // TODO should fix the mixed-mode scanning during VBL (see U.A.][, p. 5-13) - std::vector& lut = + std::vector& lut = (this->mode.isDisplayingText(this->t) || !this->mode.isHiRes()) ? this->lutTEXT[this->mode.getPage()] @@ -107,7 +107,7 @@ bool Video::inverseChar(const unsigned char d) { bool inverse; - const int cs = (d >> 6) & 3; + const unsigned char cs = (d >> 6) & 3; if (cs == 0) { inverse = true; diff --git a/src/video.h b/src/video.h index 17894e9..5533549 100644 --- a/src/video.h +++ b/src/video.h @@ -35,8 +35,8 @@ private: enum { HRES_BASE_1 = 0x2000 }; enum { HRES_BASE_2 = 0x4000 }; enum { HRES_LEN = 0x2000 }; - - std::vector lutTEXT[2]; // [0] is page 1, [1] is page 2 + + std::vector lutTEXT[2]; // [0] is page 1, [1] is page 2 std::vector lutHRES[2]; // [0] is page 1, [1] is page 2 VideoMode& mode; diff --git a/src/wozfile.cpp b/src/wozfile.cpp index 7fa8857..c9c9e70 100644 --- a/src/wozfile.cpp +++ b/src/wozfile.cpp @@ -45,7 +45,7 @@ struct trk_t { std::uint32_t bitCount; }; -WozFile::WozFile() : tmap(0) { +WozFile::WozFile() : tmap(0), lastQuarterTrack(C_QTRACK) { for (int i(0); i < C_QTRACK; ++i) { this->trk[i] = 0; } @@ -162,6 +162,11 @@ bool WozFile::load(const std::filesystem::path& orig_file) { case 0x4F464E49: { // INFO std::uint8_t* buf = new std::uint8_t[chunk_size]; in->read((char*)buf, chunk_size); + const std::streamsize n_actual = in->gcount(); + printf("read INFO chuck of size: %ld\n", n_actual); + if (n_actual < chunk_size) { + printf("WARNING: read less than expected bytes from woz file: %ld < %u\n", n_actual, chunk_size); + } printf("INFO version %d\n", *buf); if (*buf != 2) { printf("File is not WOZ2 version.\n"); @@ -185,7 +190,7 @@ bool WozFile::load(const std::filesystem::path& orig_file) { printf("Creator: \"%.32s\"\n", buf+5); this->timing = buf[39]; printf("Timing: %d/8 microseconds per bit\n", this->timing); - std::uint16_t compat = *((std::uint16_t*)buf+40); + std::uint16_t compat = *((std::uint16_t*)(buf+40)); printf("Compatible hardware: "); if (!compat) { printf("unknown\n"); @@ -527,20 +532,29 @@ void WozFile::rotateOneBit(std::uint8_t currentQuarterTrack) { return; } + // this is the case where we are here for the first time, + // and lastQuarterTrack has not been set to any valid prior track + if (this->lastQuarterTrack == C_QTRACK) { + this->lastQuarterTrack = currentQuarterTrack; + } + // If we changed tracks since the last time we were called, // we may need to adjust the rotational position. The new // position will be at the same relative position as the // previous, based on each track's length (tracks can be of // different lengths in the WOZ image). if (currentQuarterTrack != this->lastQuarterTrack) { -// printf("switching from tmap[%02x] --> [%02x]\n", this->lastQuarterTrack, currentQuarterTrack); + printf("\nswitching from tmap[%02x] --> [%02x]\n", this->lastQuarterTrack, currentQuarterTrack); const double oldLen = this->trk_bits[this->tmap[this->lastQuarterTrack]]; const double newLen = this->trk_bits[this->tmap[currentQuarterTrack]]; const double ratio = newLen/oldLen; if (!(fabs(1-ratio) < 0.0001)) { const std::uint16_t newBit = static_cast(round((this->byt*8+bc(this->bit)) * ratio)); + printf("... detected non 1:1 ratio: %f\n", ratio); + printf("... old byt/bit: %hu/%hu\n", this->byt, this->bit); this->byt = newBit / 8; this->bit = cb(newBit % 8); + printf("... new byt/bit: %hu/%hu\n", this->byt, this->bit); } this->lastQuarterTrack = currentQuarterTrack; }