Buffer Overrun while writing.... Please help!

[DaveGold]

Hi there all. I am writing a little Hangman console-app in c++ to try and learn the language.

It is mostly functional and can be played, but I have this Warning in IDE (vs2019 Community) saying:

"Warning C6386 Buffer overrun while writing to 'IsLetterFound': the writable size is 'GameWord.public: unsigned int __thiscall std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >::length(void)const ()*1' bytes, but '2' bytes might be written. Hangman C:\SomeFolder\RepoFolder\HANGMAN\HANGMAN\GAME.CPP 18 "

Now, this warning has been in my code for a while, but now after closing the game down I SOMETIMEs get a Windows 10 Error box pop-up saying something like "Overflow on stack-based system, this can be malicious/security risk"

I believe the problem is one of two things. Either, it is my array for checking if IsLetterFound (IMPORTANT NOTE: This array started life as a bool array, i then made it int and finally char to see if that fixed the problem. BUT I struggled in initializing this array as it was the first ever array i initialized with a value provided at runtime - this is my top candidate for the problem, but i have no cleu how to fix it!)

Failing that, could it possibly be my use of 'exit(EXIT_SUCCESS)' which i used to force the console to close when the user puts 'no' for PlayingAgain?

The actual Error Popup message only happens sometimes, and happens after closing VS2019, not my app.

I post my game.cpp and game.h in the entirety now. (main.cpp is so basic i left it out):


GAME.H:

  #include <iostream>
#include <string>
#include <random>

using String = std::string;
using RandGenerator = std::default_random_engine;
using RandDistribution = std::uniform_int_distribution<int>;

class Game
{
public:
	Game();
	String GetGameWord() const;
	void StartNewGame();
	String GetRandomGameWord() const;
	bool CheckLetterFound(int);
	bool SubmitGuess(char);
	void DoNextTurn();
	void AddHangmanPiece();
	void PrintHangmanVisuals() const;
	bool AskToPlayAgain();
	bool CheckForGameCompletion();
private:
	String GameWord;
	String Words[4] = { "ALIEN", "BOOK", "PRICKLE", "ICE" };
	char* IsLetterFound;
	int CurrentHangmanPieces = 0;
	static constexpr int HANGMAN_PIECES_MAX = 10;
};

GAME.CPP:

    #include "Game.h"
#include <stdlib.h>

Game::Game()
{
	StartNewGame();
}

void Game::StartNewGame() {
	CurrentHangmanPieces = 0;

	std::cout << "Let's Play Hangman!\n\n\n";
	GameWord = GetRandomGameWord();
	IsLetterFound = new char[GameWord.length()];
	for (char i = 0; i < GameWord.length(); i++) {
		IsLetterFound[i] = 'n'; // I HAVE THE WARNING, AND GREEN SQUIGGLE HERE!
	}

	DoNextTurn();
}

String Game::GetGameWord() const
{
	return GameWord;
}

String Game::GetRandomGameWord() const
{
	RandDistribution randDistribution(0, Words->length() - 1);
	RandGenerator randGenerator;
	return Words[randDistribution(randGenerator)];
}

bool Game::CheckLetterFound(int Index)
{
	if (IsLetterFound[Index] == 'y')
		return true;

	return false;
}

bool Game::SubmitGuess(char Letter)
{
	char Letter_Uppercase = toupper(Letter);
	bool Found = false;
	for (int i = 0; i < GameWord.length(); i++) {
		if (GameWord[i] == Letter_Uppercase) {
			Found = true;
			IsLetterFound[i] = 'y';
		}
	}
	return Found;
}

void Game::DoNextTurn() {
	String DisplayString = "";
	for (int i = 0; i < GetGameWord().length(); i++) {
		if (CheckLetterFound(i)) {
			DisplayString += GetGameWord()[i];
		}
		else {
			DisplayString += "_";
		}
		DisplayString += " ";
	}
	char Letter;
	std::cout << "***********************************************************" << "\n\n";
	std::cout << DisplayString << "\n\n";
	std::cout << "Enter a letter: ";
	std::cin >> Letter;

	bool Result = SubmitGuess(Letter);
	if (!Result) {
		AddHangmanPiece();
	}
	else {
		if (CheckForGameCompletion()) {
			std::cout << "You clevercloggs!\n";
			if (AskToPlayAgain()) {
				StartNewGame();
			}
			else {
				exit(EXIT_SUCCESS);
			}
		}
	}
	PrintHangmanVisuals();

	if (CurrentHangmanPieces < HANGMAN_PIECES_MAX) {
		DoNextTurn();
	}
	else {
		if (AskToPlayAgain()) {
			StartNewGame();
		}
		else {
			exit(EXIT_SUCCESS);
		}
	}

}

void Game::AddHangmanPiece() {
	CurrentHangmanPieces++;
}

void HackyClearScreen() {
	for (char i = 0; i < 10; i++) {
		std::cout << "\n\n\n\n\n\n\n\n\n\n";
	}
}

void Game::PrintHangmanVisuals() const {	
	// Left method out to save space
}

bool Game::AskToPlayAgain() {
	char Response;
	std::cout << "\nDo you want to play again? (Y/N) " << "\n";
	std::cin >> Response;
	char Response_ToLower = tolower(Response);
	if (Response_ToLower == 'y') {
		return true;
	}
	else if (Response_ToLower == 'n') {
		return false;
	}
	else {
		std::cout << "Invalid response\n";
		AskToPlayAgain();
	}
}

bool Game::CheckForGameCompletion()
{
	for (int i = 0; i < GameWord.length(); i++) {
		if (IsLetterFound[i] != 'y') {
			return false;
		}
	}
	return true;
}

Image to show Visual studio error: https://pasteboard.co/IpCCZn7.png

If anyone can see my problem and help me I would be massively grateful. Also I realise my Random Number generator code does not work properly also :D

[coder777]

So why do you use char on line 15 in game.cpp? Why not int or size_t?

Line 29 in game.cpp: Words->length() - 1 takes the length of the first word.
I guess you want something like this: std::size(Words) - 1;

[jonnin]

do not use the char*
use a string for that.
you never delete your pointer, so you are leaking memory on each new game, at the very least; a string would fix that.

I don't see the bug, or if there really is one (the loop you marked is fine, its 0-n, and the array is sized n).

[DaveGold]

thanks for the useful info guys. Its strange , I changed my for loop code from < to != , and also gave the GameWord.length() into it's own private variable and the error seemed to go.

I will certainly look into deleteing the pointer next.

The 'char' thing inside the loop was my stupid attempt at removing the error about 1 bytes and 2 bytes being used lol.... I'm still very new/dumb but getting a bit smarter by the day :P


thanks again friends

[DaveGold]

as a side-question. From failing to delete my pointer(s) , will the memory leak continue after closing down the console (assuming I was a general user running an .exe of this game) ? thanks

[Repeater]

No. On a modern, standard OS (such as Windows or a Linux), when a process ends, all the memory that was given to that process is recovered by the OS.

[DaveGold]

thank you everyone. And also I have corrected my IsLetterFound array back to being bool. And used a private variable of type 'size_t' to store the length of the gameword. It all compiles without any warnings now :D love you all!

Next I am having to find why my Random Generator isn't working, as I thought I had followed the code examples exactly, but I always seem to get the 3rd word in my array every time. I'm guessing its something to do with seed but I will have to investigate further. Have to go work in 20 mins :[ boo

[dutch]

You never seed your random engine. Here's a demo on how to do it:

#include <iostream>
#include <string>
#include <random>
#include <chrono>

using String = std::string;
using RandGenerator = std::default_random_engine;
using RandDistribution = std::uniform_int_distribution<int>;

template<typename T, size_t Size>
size_t ArraySize(T(&)[Size]) { return Size; }

String Words[] { "ALIEN", "BOOK", "PRICKLE", "ICE" };

String GetRandomGameWord() {
    static RandDistribution randDistribution(0, ArraySize(Words) - 1);
    static RandGenerator randGenerator(
                std::chrono::high_resolution_clock{}.now()
                            .time_since_epoch().count());
    return Words[randDistribution(randGenerator)];
}

int main() {
    for (int i = 0; i < 10; ++i)
        std::cout << GetRandomGameWord() << '\n';
}

[DaveGold]

Thanks, I'm taking a look now. I seem to have it working now with Random (done it before seeing your message). I did it like this (it's probably wrong because I just did what I guessed was right after following the C++ API docs on this website.)

RandGenerator randGenerator;

String Game::GetRandomGameWord()
{
	RandDistribution randDistribution(0, Words->length() - 1);
	RandGenerator::result_type seedval = (unsigned int)time(NULL);
	randGenerator.seed(seedval);
	unsigned int Index = randDistribution(randGenerator);
	if (Index > Words->length() - 1) {
		Index = Words->length() - 1;
	}
	
	String Word = Words[Index];
	return Word;
}

I also found the amazing (and somewhat weird IMO) technique of using <vector> as the dynamically sized arrays. Which has saved me a lot of headache, and is much more like what I am used to using with previous languages i have used.

[dutch]

A random number generator should be seeded only once before it's first use.
That's why I made the generator static, so it would only be initialized once.
The distrubution may as well be static, too.
And using <chrono> we can get a much finer clock resolution than the seconds that the ancient time() function returns. So it makes a much better seed.

I don't understand why you are testing whether the returned value is out of the requested range. It won't be.

What is amazing (and weird) about std::vector?
What other languages do you know well?

[DaveGold]

What is amazing (and weird) about std::vector?
What other languages do you know well?

well i have quite a bit of experience with C# and Java. But not enough to do anything competently :P

I always thought of a 'vector' as a way of storing positional values, and comparing those values to another objects vector (or position), as to calculate distance, direction, velocity etc over a 2d or 3d space.

It hadn't occurred to me that vector could hold any datatype, and that it would have methods such as size() and push_back() etc. These I thought were traits of 'arrays'. When i think about it, in C++ (my one week of experience of it lol), I can't see any reason to ever use 'array' over 'vector' (unless perhaps I explicitly want to fix the capacity in hard-stone AND i was working on a project team of 2 or more --ie. probably never happen, i will always work alone!! haha)

[DaveGold]

By the way... If anyone ever reads this thread to do with the original title. The 'overrun' problem was only present in vs2019 (v142 platform toolset). When I redownloaded vs2017, and changed each of my projects back to v141 toolset in the options, they all opened fine, and this Stack Overrun thing stopped happening

[Albatross]

IMO std::vector is definitely a bit confusingly named, depending on your background. It's not meant for storing mathematical vectors, but is, as you said, very much like a resizable array (it's a bit more technical than that, but we'll skip the fine print).

-Albatross

[Repeater]

I remember this conversation happened before many years ago. Maybe on the IRC.

Someone said they should have been called "bunch" rather than vector; bunch makes it clear it's collection of objects of unspecified type, and also carries the information that they in a group together.

[Ganado]

"Just make a bunch of objects!" I think would be even more confusing when typing or speaking out loud due to its ambiguity.

Personally I'd prefer "pack". You make a pack of dog objects. A pack of cards, etc, etc.

[Niccolo]

@DaveGold,

By the way... The 'overrun' problem was only present in vs2019 (v142 platform toolset)..changed each of my projects back to v141...this Stack Overrun thing stopped happening

That may not be good news, actually.

A stack overrun is not (yet) known to be an issue in VS2019. There are several issue, but I've yet to find that one. You may do well to use a memory checking feature under a debug mode, something a bit more powerful than the built in tools. I've not tried Visual Leak detector in quite a while, but it was a good one for Visual Studio. Valgrind is known for Linux with GCC.

@TheDaemoness, @Ganado, @Repeater...I think the name vector implies the notion of the size and index of an array. The "starting at zero" known of arrays meant that the measurement is that of a single dimension vector from the beginning of storage.

It is quite confusing to anyone in graphics or math.

DynArray was a container is some library I used in the 90's, before the STL was widely used.