From 8aaa6c0af2b0c240a11ed22d8539334b31410091 Mon Sep 17 00:00:00 2001 From: jdgleaver Date: Tue, 18 May 2021 17:16:11 +0100 Subject: [PATCH] Prevent segfaults due to audio buffer overflows --- libgambatte/include/gambatte.h | 5 ++++- libgambatte/libretro/libretro.cpp | 29 ++++++++++++++++++++++------- libgambatte/src/cpu.h | 2 +- libgambatte/src/gambatte-memory.h | 2 +- libgambatte/src/gambatte.cpp | 4 ++-- libgambatte/src/sound.cpp | 8 ++++++-- libgambatte/src/sound.h | 3 ++- 7 files changed, 38 insertions(+), 15 deletions(-) diff --git a/libgambatte/include/gambatte.h b/libgambatte/include/gambatte.h index b745aa6..5f67921 100644 --- a/libgambatte/include/gambatte.h +++ b/libgambatte/include/gambatte.h @@ -54,6 +54,8 @@ public: * * There are 35112 stereo sound samples in a video frame. * May run for uptil 2064 stereo samples too long. + * EDIT: Due to internal emulator bugs, may in fact run for + * an arbitrary number of samples... * A stereo sample consists of two native endian 2s complement 16-bit PCM samples, * with the left sample preceding the right one. Usually casting soundBuf to/from * short* is OK and recommended. The reason for not using a short* in the interface @@ -67,11 +69,12 @@ public: * @param videoBuf 160x144 RGB32 (native endian) video frame buffer or 0 * @param pitch distance in number of pixels (not bytes) from the start of one line to the next in videoBuf. * @param soundBuf buffer with space >= samples + 2064 + * @param soundBufSize actual size of soundBuf buffer * @param samples in: number of stereo samples to produce, out: actual number of samples produced * @return sample number at which the video frame was produced. -1 means no frame was produced. */ long runFor(gambatte::video_pixel_t *videoBuf, int pitch, - gambatte::uint_least32_t *soundBuf, unsigned &samples); + gambatte::uint_least32_t *soundBuf, std::size_t soundBufSize, unsigned &samples); /** Reset to initial state. * Equivalent to reloading a ROM image, or turning a Game Boy Color off and on again. diff --git a/libgambatte/libretro/libretro.cpp b/libgambatte/libretro/libretro.cpp index 7e86f85..167774e 100644 --- a/libgambatte/libretro/libretro.cpp +++ b/libgambatte/libretro/libretro.cpp @@ -90,6 +90,21 @@ bool use_official_bootloader = false; #define VIDEO_BUFF_SIZE (256 * NUM_GAMEBOYS * VIDEO_HEIGHT * sizeof(gambatte::video_pixel_t)) #define VIDEO_PITCH (256 * NUM_GAMEBOYS) +/* There are 35112 stereo sound samples in a video frame */ +#define SOUND_SAMPLES_PER_FRAME 35112 +/* We request 2064 samples from each call of GB::runFor() */ +#define SOUND_SAMPLES_PER_RUN 2064 +/* GB::runFor() nominally generates up to + * (SOUND_SAMPLES_PER_RUN + 2064) samples, which + * defines our sound buffer size + * NOTE: This is in fact a lie - in the upstream code, + * GB::runFor() can generate more than + * (SOUND_SAMPLES_PER_RUN + 2064) samples, causing a + * buffer overflow. It has been necessary to add an + * internal hard cap/bail out in the event that + * excess samples are detected... */ +#define SOUND_BUFF_SIZE (SOUND_SAMPLES_PER_RUN + 2064) + /*****************************/ /* Interframe blending START */ /*****************************/ @@ -903,7 +918,7 @@ void retro_init(void) #endif double fps = 4194304.0 / 70224.0; - double sample_rate = fps * 35112; + double sample_rate = fps * SOUND_SAMPLES_PER_FRAME; #ifdef CC_RESAMPLER CC_init(); @@ -1825,7 +1840,7 @@ void retro_run() input_poll_cb(); update_input_state(); - uint64_t expected_frames = samples_count / 35112; + uint64_t expected_frames = samples_count / SOUND_SAMPLES_PER_FRAME; if (frames_count < expected_frames) // Detect frame dupes. { video_cb(NULL, VIDEO_WIDTH, VIDEO_HEIGHT, VIDEO_PITCH * sizeof(gambatte::video_pixel_t)); @@ -1835,12 +1850,12 @@ void retro_run() union { - gambatte::uint_least32_t u32[2064 + 2064]; - int16_t i16[2 * (2064 + 2064)]; + gambatte::uint_least32_t u32[SOUND_BUFF_SIZE]; + int16_t i16[2 * SOUND_BUFF_SIZE]; } static sound_buf; - unsigned samples = 2064; + unsigned samples = SOUND_SAMPLES_PER_RUN; - while (gb.runFor(video_buf, VIDEO_PITCH, sound_buf.u32, samples) == -1) + while (gb.runFor(video_buf, VIDEO_PITCH, sound_buf.u32, SOUND_BUFF_SIZE, samples) == -1) { #ifdef CC_RESAMPLER CC_renderaudio((audio_frame_t*)sound_buf.u32, samples); @@ -1857,7 +1872,7 @@ void retro_run() #endif samples_count += samples; - samples = 2064; + samples = SOUND_SAMPLES_PER_RUN; } #ifdef DUAL_MODE while (gb2.runFor(video_buf + GB_SCREEN_WIDTH, VIDEO_PITCH, sound_buf.u32, samples) == -1) {} diff --git a/libgambatte/src/cpu.h b/libgambatte/src/cpu.h index 50c16d7..76c29de 100644 --- a/libgambatte/src/cpu.h +++ b/libgambatte/src/cpu.h @@ -86,7 +86,7 @@ public: #if 0 bool loaded() const { return mem_.loaded(); } #endif - void setSoundBuffer(uint_least32_t *buf) { mem_.setSoundBuffer(buf); } + void setSoundBuffer(uint_least32_t *buf, std::size_t size) { mem_.setSoundBuffer(buf, size); } std::size_t fillSoundBuffer() { return mem_.fillSoundBuffer(cycleCounter_); } bool isCgb() const { return mem_.isCgb(); } diff --git a/libgambatte/src/gambatte-memory.h b/libgambatte/src/gambatte-memory.h index 3daa89b..b925718 100644 --- a/libgambatte/src/gambatte-memory.h +++ b/libgambatte/src/gambatte-memory.h @@ -120,7 +120,7 @@ public: void setSerialIO(SerialIO* serial_io) { serial_io_ = serial_io; } #endif void setEndtime(unsigned long cc, unsigned long inc); - void setSoundBuffer(uint_least32_t *buf) { psg_.setBuffer(buf); } + void setSoundBuffer(uint_least32_t *buf, std::size_t size) { psg_.setBuffer(buf, size); } std::size_t fillSoundBuffer(unsigned long cc); void setVideoBuffer(video_pixel_t *videoBuf, std::ptrdiff_t pitch) { diff --git a/libgambatte/src/gambatte.cpp b/libgambatte/src/gambatte.cpp index 8208b5f..bef0bf2 100644 --- a/libgambatte/src/gambatte.cpp +++ b/libgambatte/src/gambatte.cpp @@ -43,10 +43,10 @@ GB::~GB() { } long GB::runFor(gambatte::video_pixel_t *const videoBuf, const int pitch, - gambatte::uint_least32_t *const soundBuf, unsigned &samples) { + gambatte::uint_least32_t *const soundBuf, std::size_t soundBufSize, unsigned &samples) { p_->cpu.setVideoBuffer(videoBuf, pitch); - p_->cpu.setSoundBuffer(soundBuf); + p_->cpu.setSoundBuffer(soundBuf, soundBufSize); const long cyclesSinceBlit = p_->cpu.runFor(samples * 2); samples = p_->cpu.fillSoundBuffer(); diff --git a/libgambatte/src/sound.cpp b/libgambatte/src/sound.cpp index 5533e6f..ecd6832 100644 --- a/libgambatte/src/sound.cpp +++ b/libgambatte/src/sound.cpp @@ -45,6 +45,7 @@ namespace gambatte PSG::PSG() : buffer_(0) + , bufferSize_(0) , bufferPos_(0) , lastUpdate_(0) , soVol_(0) @@ -96,7 +97,6 @@ namespace gambatte void PSG::accumulateChannels(const unsigned long cycles) { uint_least32_t *const buf = buffer_ + bufferPos_; - std::memset(buf, 0, cycles * sizeof(uint_least32_t)); ch1_.update(buf, soVol_, cycles); ch2_.update(buf, soVol_, cycles); @@ -106,7 +106,11 @@ namespace gambatte void PSG::generateSamples(unsigned long const cycleCounter, bool const doubleSpeed) { - unsigned long const cycles = (cycleCounter - lastUpdate_) >> (1 + doubleSpeed); + unsigned long cycles = (cycleCounter - lastUpdate_) >> (1 + doubleSpeed); + + if (cycles + bufferPos_ > bufferSize_) + cycles = (bufferSize_ > bufferPos_) ? (bufferSize_ - bufferPos_) : 0; + lastUpdate_ += cycles << (1 + doubleSpeed); if (cycles) diff --git a/libgambatte/src/sound.h b/libgambatte/src/sound.h index 5e8d15b..1046011 100644 --- a/libgambatte/src/sound.h +++ b/libgambatte/src/sound.h @@ -41,7 +41,7 @@ public: void generateSamples(unsigned long cycleCounter, bool doubleSpeed); void resetCounter(unsigned long newCc, unsigned long oldCc, bool doubleSpeed); std::size_t fillBuffer(); - void setBuffer(uint_least32_t *buf) { buffer_ = buf; bufferPos_ = 0; } + void setBuffer(uint_least32_t *buf, std::size_t size) { buffer_ = buf; bufferSize_ = size; bufferPos_ = 0; } bool isEnabled() const { return enabled_; } void setEnabled(bool value) { enabled_ = value; } @@ -80,6 +80,7 @@ private: Channel3 ch3_; Channel4 ch4_; uint_least32_t *buffer_; + std::size_t bufferSize_; std::size_t bufferPos_; unsigned long lastUpdate_; unsigned long soVol_;