Stack smashing detected after closing the sf::RenderWindow

Question

I'm writing both my first chip8 interpreter and multi-threaded program and ran into this problem: as soon as I close the window I get stack smashing detected. I've been inspecting the code for quite some time but I can't find the problem. Would really like some help or advice.

emulator.hpp

#ifndef CHEAP8_SRC_EMULATOR_HPP
#define CHEAP8_SRC_EMULATOR_HPP

#include "chip8.hpp"
#include "ui-settings.hpp"

#include <SFML/Graphics.hpp>

#include <atomic>
#include <memory>
#include <mutex>
#include <string>
#include <thread>

#ifdef _WIN32
#define OS_PATH_SEP '\\'
#else
#define OS_PATH_SEP '/'
#endif

class Emulator
{
    //misc
    const std::string gamePath;
    const std::string gameTitle;

    //emulator
    unsigned char memory[chip8::kMemSize] = {0};

    //flags
    bool gameLoaded = false;
    std::atomic<bool> gameRunning{false};

    //ui
    const double kPixelSize = 12.5;
    std::shared_ptr<sf::RenderWindow> window;
    const UiSettings uiSettings;

    //threading
    std::thread uiThread;
    std::mutex mutex;

    void loadGame();
    void loadFonts();
public:
    explicit Emulator(char*);
    ~Emulator();
    void run();
};

#endif //CHEAP8_SRC_EMULATOR_HPP

emulator.cpp

#include "emulator.hpp"

#include <chrono> //================
#include <fstream>
#include <iostream>
#include <iterator>
#include <vector>

void launchUi(std::shared_ptr<sf::RenderWindow> window,
              const UiSettings& uiSettings,
              std::atomic<bool>& gameRunning)
{
    window = std::make_shared<sf::RenderWindow>();
    window->create(sf::VideoMode(uiSettings.width, uiSettings.height),
                   uiSettings.title,
                   sf::Style::Titlebar | sf::Style::Close);
    window->setVerticalSyncEnabled(true);
    window->setKeyRepeatEnabled(false);
    const auto screenParams = sf::VideoMode::getDesktopMode();
    window->setPosition(sf::Vector2i(screenParams.width / 2 - uiSettings.width / 2,
                                     screenParams.height / 2 - uiSettings.height / 2));

    sf::Event event;
    while (window->isOpen()) {
        while (window->pollEvent(event)) {
            switch (event.type) {
                case sf::Event::Closed:
                    window->close();
                    gameRunning.store(false);
                break;

                default:
                break;
            }
        }
    }
}

Emulator::Emulator(char* path)
    : gamePath(path),
      gameTitle(gamePath.substr(gamePath.find_last_of(OS_PATH_SEP) + 1)),
      uiSettings(kPixelSize, chip8::kScreenWidth, chip8::kScreenHeight, gameTitle),
      uiThread(launchUi, window, std::ref(uiSettings), std::ref(gameRunning))
{
    loadGame();
    loadFonts();
}

Emulator::~Emulator()
{
    uiThread.join();
}

void Emulator::loadGame()
{
    std::ifstream file(gamePath, std::ios::binary);
    if (!file.is_open()) {
        std::cerr << "Couldn't open the game." << std::endl;
        return;
    }

    std::vector<unsigned char> gameData;
    gameData.insert(gameData.begin(),
                std::istream_iterator<unsigned char>(file),
                std::istream_iterator<unsigned char>());

    if (gameData.size() > chip8::kAllowedGameSize) {
        std::cerr << "The game is too big to fit into memory." << std::endl;
        return;
    }

    for (size_t i = 0; i < gameData.size(); ++i) {
        memory[i + chip8::kMemStart] = gameData[i];
    }

    gameLoaded = true;
    std::cout << "Game loaded. Size: " << gameData.size() << '\n';
}

void Emulator::loadFonts()
{
    for (int i = 0; i < chip8::kFontsetSize; ++i) {
        memory[i] = chip8::kFontset[i];
    }
    std::cout << "Fontset loaded." << '\n';
}

void Emulator::run()
{
    if (!gameLoaded) {
        return;
    }

    gameRunning.store(true);
    while (gameRunning.load()) {
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
    }
}

EDIT:

#include "emulator.hpp"

#include <iostream>

int main(int argc, char *argv[])
{
    if (argc != 2) {
        std::cerr << "Usage: ./" << OS_PATH_SEP << "cheap8 game." << std::endl;
        return 1;
    }

    Emulator emu(argv[1]);
    emu.run();

    return 0;
}

Answer 1

Your instance of Emulator is being created on the stack, so it will use stack space sufficient to store all its member variables.

This one is pretty big:

unsigned char memory[chip8::kMemSize] = {0};

You might change that to a std::vector so that it stores the data on the heap and the class instance only contains metadata (size, pointer to data).

It will also change the effect of out-of-bounds access to your memory array -- instead of messing with the stack and changing things like return addresses, any corruption will be done to pure data objects.

To ensure there are no out-of-bounds accesses, you could use vector's at() member function.