Stack around array was corrupted

[johntommy]

I've nearly finished with my classes knight tour program but I keep getting an odd error saying the stack around my chessboard 2d array was corrupted. I've tried searching for some answers and I've changed around the amount of times my code can loop but I have not been able to get that error to go away. It does a full tour so I don't really need help there just want the error to go away

  // John Ford
// CSC 102
// Feb 2022
// Knights Tour

#include <iostream>
#include <iomanip>

using namespace std;

void resetChessboard(int chessBoard[8][8]);
void makeMove(int chessBoard[8][8]);

int main()
{
    int chessBoard[8][8];
    resetChessboard(chessBoard);
    makeMove(chessBoard);
}

void resetChessboard(int chessBoard[8][8])
{
    for (int i = 0; i < 8; i++) {
        for (int j = 0; j < 8; j++) {
            chessBoard[i][j] = 0;
        }
    }
}
void makeMove(int chessBoard[8][8])
{
    int counter = 2;
    chessBoard[0][0] = 1;
    int currentRow = 0;
    int currentColumn = 0;
    int horizontal[8] = { 2, 1, -1, -2, -2, -1, 1, 2 };
    int vertical[8] = { -1, -2, -2, -1, 1, 2, 2, 1 };
    int heuristicChessboard[8][8] = { 2,3,4,4,4,4,3,2,
                                      3,4,6,6,6,6,4,3,
                                      4,6,8,8,8,8,6,4,
                                      4,6,8,8,8,8,6,4,
                                      4,6,8,8,8,8,6,4,
                                      4,6,8,8,8,8,6,4,
                                      3,4,6,6,6,6,4,3,
                                      2,3,4,4,4,4,3,2 };
    int arrayHolder[8] = { 10,10,10,10,10,10,10,10 };
    int bestHeuristic = 10;
    int bestIndex = 10;
    int bestHorizontal = 10;
    int bestVertical = 10;
    int tester = 0;
    int resetCounter1 = 0;
    int resetCounter2 = 0;
    int totalResets = 0;
    // I need to take all the moves the knight can make and grade them based on the heuristic chessboard array
    // I then need to make the lowest ranked move on the board 
    // instead of making the move right away can store the for loop j value into an array that resets after the loop is done
    // then pick the best of those moves and preform that move
    // possibleMoves[8] is to acceept all the j values that actually work so we can compare them later

    int chessOutlineHorizontal[8] = { 1, 2, 3, 4, 5, 6, 7, 8 };
    char chessOutlineVertical[8] = { 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H' };
    for (int i = 0; i < 66; i++) {
        bestHeuristic = 10;
        bestIndex = 10;
        bestHorizontal = 10;
        bestVertical = 10;
        for (int h = 0; h < 8; h++) {
            arrayHolder[h] = 10;
        }
        for (int j = 0; j < 8; j++) {
            if (currentRow + vertical[j] >= 0 && currentRow + vertical[j] <= 7) {
                if (currentColumn + horizontal[j] >= 0 && currentColumn + horizontal[j] <= 7) {
                    if (chessBoard[currentRow + vertical[j]][currentColumn + horizontal[j]] == 0) {
                        arrayHolder[j] = j;
                    }
                }
            }

        }
        for (int h = 0; h < 8; h++) {
            if (arrayHolder[h] >= 0 && arrayHolder[h] <= 8) {
                if (heuristicChessboard[currentRow + vertical[h]][currentColumn + horizontal[h]] < bestHeuristic) {
                    bestIndex = h;
                    bestVertical = vertical[h];
                    bestHorizontal = horizontal[h];
                    bestHeuristic = heuristicChessboard[currentRow + vertical[h]][currentColumn + horizontal[h]];
                }
            }
            if (h == 7 && counter != 65) {
                currentRow += bestVertical;
                currentColumn += bestHorizontal;
                chessBoard[currentRow][currentColumn] = counter;
                counter++;
                if (bestHorizontal == 10 && counter != 64) {
                    i = 0;
                    counter = 0;
                    resetCounter1++;
                    totalResets++;
                    if (resetCounter1 > 8) {
                        resetCounter1 = 0;
                        resetCounter2++;
                    }
                    chessBoard[resetCounter1][resetCounter2] = 1;
                    currentColumn = resetCounter1;
                    currentRow = resetCounter2;
                    for (int z = 0; z < 8; z++) {
                        for (int y = 0; y < 8; y++) {
                            chessBoard[z][y] = 0;
                        }
                    }
                }

            }
        }
        if (i == 65) {
            cout << "After " << totalResets << " tries the knight made a full tour and ended on square " << chessOutlineVertical[currentColumn] << chessOutlineHorizontal[currentRow] << endl << endl;
            cout << setw(5) << "     A    " << "B    " << "C    " << "D    " << "E    " << "F    " << "G    " << "H    " << endl << endl;
            for (int k = 0; k < 8; k++) {
                cout << chessOutlineHorizontal[k];
                for (int l = 0; l < 8; l++) {
                    cout << setw(5) << chessBoard[k][l];
                }
                cout << endl << endl;
            }
            cout << endl;
        }
    }


}

[jonnin]

that error means you accessed memory you do not own, so one of your arrays went out of bounds along the way. You know which variable even -- it told you -- so look deeply at that logic and variable.

it may be possible to get this on the variables next to as well, in the code, eg
int foo[]
int middle[]
int bar[]
if you mess up foo(go too far) you end up in middle. C++ allows negative indexing, so you can also go backwards in bar and mess up middle. It could warn on any of the three, but you didn't do negative indexing so focus the hunt on the one before it and the one it listed.

[johntommy]

After looking back over my code I cant find where the chessboard could be spilling over into one of the other arrays and I cant find where one of the arrays goes over what it should. Could you give me another hint?

[DizzyDon]

My eye goes to

            if (arrayHolder[h] >= 0 && arrayHolder[h] <= 8

since all other comparisons are either < 8 or <= 7

Also, right after that you access an element that could be out of bounds:

                && heuristics[row + vertical[h]][col + horizontal[h]] < bestHeuristic) {

(I've shortened some of the variable names for clarity.)

So maybe that part should be:

            int rv = row + vertical[h], ch = col + horizontal[h];
            if (arrayHolder[h] >= 0 && arrayHolder[h] <= 7  // or maybe it really should be 8?!
                && rv >= 0 && rv <= 7 && ch >= 0 && ch <= 7
                && heuristics[rv][ch] < bestHeuristic) {
                bestIndex = h;
                bestVertical = vertical[h];
                bestHorizontal = horizontal[h];
                bestHeuristic = heuristics[rv][ch];
            }

Also note a "set but not used" warning for 'bestIndex'
and an "unused variable" warning for 'tester'.

[johntommy]

After testing that it did not change anything but I changed the declaration in the main to "int chessBoard[9][8]; and that got rid of the problem so I will keep looking through the code and try to figure out where in the code its going wrong. Also if I leave it at [8][8] and I stop the program one short so it only moves 63 times it works fine

[jonnin]

two major clues... almost there I would say...
You would probably be astonished at how much software has been debugged and 'fixed' this way (treating the symptoms) vs proper repair. Its easier, and faster, and it works if you can tolerate the side effects of the main bug. Its not right, its the software equivalent of duct tape, but sometimes that is where things land out in the world.

[DizzyDon]

It looks like the test for bestHorizontal == 10 needs to be moved back a bit since you shouldn't be using bestHorizontal if it's 10. Probably it's the line that writes to chessBoard here that causes the out-of-bounds writes.

                currentRow += bestVertical;
                currentColumn += bestHorizontal;
                chessBoard[currentRow][currentColumn] = counter;
                counter++;
                if (bestHorizontal == 10 && counter != 64) {

The code generally looks like it could be cleaned up quite a bit. In particular, the variables i and counter are virtually the same. I doubt both are needed.
And the loop structure is a little odd, with a limit of 66 and special checks for i (or counter) being 65. It suggests some restructuring like bringing the bits where i should be 65 out of and after the loop.