Home > database >  How can I fix this? " HEAP CORRUPTION DETECTED after normal block.. CRT detected that the appli
How can I fix this? " HEAP CORRUPTION DETECTED after normal block.. CRT detected that the appli

Time:04-01

I had a program working good (without prompting errors) using references, but I decided to use a pointer-to-pointer array allocated on virtual memory, because I can use a variable as the size of the array.

The error prompts when I break the while (m_Window.isOpen()) loop, in other words when I close the game window and the game is finished. I have noticed that the program breaks when I try to erase the virtual memory in the Engine::cleanVirtualMemory() function. I have noticed that it is there because I have put two flags (cout << "running1" << endl; and cout << "running2" << endl) and I can show just the first flag.

Then it prompts a window with the following message:

HEAP CORRUPTION DETECTED after normal block.. CRT detected that the application wrote to memory after end of heap buffer

main.cpp

#include "Engine.h"
using namespace sf;

int main(){

    Engine Motor;
    Motor.run();
    Motor.cleanVirtualMemory();
}

Engine.h

#pragma once
#include <SFML/Graphics.hpp>
#include "StructureBuilder.h"

using namespace sf;
using namespace std;

class Engine{
private:

    // Lots of variables ....
    Vector2i m_ArenaSize;
    Vector2f * vectorStructureArray = new Vector2f[m_ArenaSize.y * m_ArenaSize.x * 4];
    int** logicStructureArray = new int*[m_ArenaSize.y];
    // Lots of variables ....

    //Gameloop
    void Input();
    void Update(dtAsSeconds);
    void Draw();

public:

    Engine();
    void run();
    void cleanVirtualMemory();

};

Engine.cpp

#include "Engine.h"
#include <iostream>

using namespace sf;


Engine::Engine() {

/// lots of variables and data ...

/// Making arena
m_ArenaSize = Vector2i(10, 10);
StructureBuilder(arenaStructures, vectorStructureArray, logicStructureArray,  m_ArenaSize);
}


void Engine::run() {

//Timing
Clock clock;
while (m_Window.isOpen()) {

    // Each time clock restarted, dt = time elapse (from 0 to now, then clock = 0)
    Time dt = clock.restart();

    // Convert time elapse to seconds
    double dtAsSeconds = dt.asSeconds();
   
    //  Call each part of the game in turn
    Input();
    Update(dtAsSeconds);
    Draw();
    }
}

void Engine::cleanVirtualMemory() {
    // Deallocate Virtual Memory
    
    // first flag
    cout << "running1" << endl;

    for (int i = m_ArenaSize.x - 1; i > -1; i--) {
        delete[] logicStructureArray[i];
    }
    delete[] logicStructureArray;
    logicStructureArray = NULL;
    delete[] vectorStructureArray;
    vectorStructureArray = NULL;

   // second flag
   cout << "running2" << endl;
}

StructureBuilder.h

#pragma once
#include <SFML/Graphics.hpp>
#include <iostream>
#include <string.h>

using namespace sf;
using namespace std;

Vector2i StructureBuilder(VertexArray& rVA, Vector2f* rA, int** rLA, Vector2i ArenaSize);

In the following code, you can see where I use pointers, I have erased code just to simplify.

StructureBuilder.cpp

#include "StructureBuilder.h"

Vector2i StructureBuilder(VertexArray& rVA, Vector2f* rA, int** rLA, Vector2i ArenaSize) {

    //Set map properties and VertexArrayType
    double tileSize = 100;
    double Height = ArenaSize.y * tileSize;
    double angle = 30;
    int offset = 0;
    int Primitive = 4;
    int currentVertex = 0;

    rVA.setPrimitiveType(Quads);
    rVA.resize(ArenaSize.x * ArenaSize.y * 4);

    // First build graphically our map structures's using char strings 
    string stringArray[10]; 
    stringArray[0] = "1000000000";
    stringArray[1] = "0000000000";
    stringArray[2] = "0000010000";
    stringArray[3] = "0000000000";
    stringArray[4] = "0000000000";
    stringArray[5] = "0000000000";
    stringArray[6] = "0000000000";
    stringArray[7] = "0000000000";
    stringArray[8] = "0000000000";
    stringArray[9] = "0000000000";

    // Convert stringArray to charArray, finally charArray to intArray
    char** charArray = new char*[ArenaSize.y]; 

    Vector2f Vector1;
    Vector2f Vector2;
    Vector2f Vector3;
    Vector2f Vector4;

    for (int i = 0; i < ArenaSize.x; i  ) {
       charArray[i] = new char[ArenaSize.x];
       rLA[i] = new int[ArenaSize.x];
        
    }

    for (int i = 0; i < ArenaSize.x; i  ) {
        for (int j = 0; j < ArenaSize.y; j  ) {
            charArray[j][i] = stringArray[j][i];
            rLA[j][i] = charArray[j][i] - 48;

            // Check when we have a value greater or equal to 1, if yes build a structure.
            if (rLA[j][i] == 1) {

                
                Vector1 = Vector2f(Value..., Value...);// in order to understand I dont put the whole calculation here
                Vector2 = Vector2f(Value..., Value ...); // is just trigonometry to find vertex
                Vector3 = Vector2f(Value..., Value ...);
                Vector4 = Vector2f(Value..., Value ...);


                rVA[currentVertex   0].position = Vector1;
                rVA[currentVertex   1].position = Vector2;
                rVA[currentVertex   2].position = Vector3;
                rVA[currentVertex   3].position = Vector4;

                rVA[currentVertex   0].texCoords = Vector2f(42, 0); // coords on my spritesheet
                rVA[currentVertex   1].texCoords = Vector2f(86, 24);
                rVA[currentVertex   2].texCoords = Vector2f(42, 49);
                rVA[currentVertex   3].texCoords = Vector2f(0, 24);

                rA[currentVertex   0] = Vector1; //Later I use this to tell the program where to construct restrictions (where the player can't move)
                rA[currentVertex   1] = Vector2;
                rA[currentVertex   2] = Vector3;
                rA[currentVertex   3] = Vector4;
            }
           
            currentVertex = currentVertex   Primitive;
        }
    }

    // Deallocate Virtual Memory
    for (int i = ArenaSize.x - 1; i > -1; i--) {
        delete[] charArray[i];
    }
    delete[] charArray;
    charArray = NULL;


    return ArenaSize;
}

CodePudding user response:

Let's take a look at the declaration of the Engine class.

class Engine{
private:

    // Lots of variables ....
    Vector2i m_ArenaSize;
    Vector2f * vectorStructureArray = new Vector2f[m_ArenaSize.y * m_ArenaSize.x * 4];

The first member of your Engine class is called m_ArenaSize. This is the first class member that will get constructed when an Engine gets constructed. That's how object construction works in C : when a new object gets constructed, all of the new object's members get constructed in declaration order.

The second member of your Engine class is this vectorStructureArray pointer. It will be allocated to point to an array, with dynamic size, that gets newed using two of the constructed m_ArenaSize's own members, x and y.

And now, let's take a look at Engine's constructor:

Engine::Engine() {

All right. That's your constructor. So, according to our plan, m_ArenaSize is going to get default-constructed since it is not explicitly constructed, in Engine's constructor's initialization section (there is none, here). If you investigate what m_ArenaSize's default constructor does you will discover that it defaults x and y to 0.

And that's what's going to construct the vectorStructureArray pointer, as the 2nd order of business of Engine's default initialization. And, because its x and y are 0, the pointer will point to a grand total of 0 values.

m_ArenaSize = Vector2i(10, 10);

And only now the default-constructed m_ArenaSize gets replaced with a different subject, with different x and y values. The shown code clearly expects the pointer to get reallocated, to reflect m_ArenaSize's new x and y values,. However C does not work this way. vectorStructureArray has already been constructed. It's not going to get constructed again simply because a different class member gets replaced. Subsequent code's assumption is the new x and y values, and this results in fairly obvious memory corruption.

The same exact bug occurs with logicStructureArray, too.

These are just the first two major bugs in the shown code regarding memory allocation. There are several ways to fix them, but the easiest way to fix bugs is to make it logically impossible for them to happen in the first place. It is logically impossible for new and delete-related bugs to occur if they are never used. Modern C code rarely uses new and delete, but rather employs the services of the C library's many containers.

Here, both pointers can be simply replaced by std::vector, with its resize() member taking care of allocating both vectors' sizes. And, as extra bonus, all allocated memory gets automatically freed, automatically preventing all memory leaks.

Replacing all the error-prone new and delete logic with std::vector will fix all memory-related issues in the shown code, the only thing you will have to make sure is that the vectors are correctly resize()d.

  • Related