c++ double free or corruption (out) error

Question

I am getting error of "Double free or corruption(out)" after I print my output. But this error is only coming for small inputs. For bigger inputs program doesn't throw that error. When I create the multidimensional arrays inside the main and delete them, I do not get the error. I have only posted the part of the code which is relevant to this issue here. Please kindly explain how to resolve the issue.

#include<iostream>
#include<vector>
using namespace std;

class Knapsack{
  public:
    int noItems, capacity, value, weight;
    int *weightArray, *valueArray;
    int **ValueMatrix, **BacktrackMatrix;
    vector<int> itemsChosen;
    ~Knapsack();
    void getInputs();              // reads in data
    void findItems();         // calculates best value of items
    void backTrack(int row, int col); // backtracks items selected
    void print();                       //prints out data
};

Knapsack::~Knapsack()
{
  delete[] weightArray;
  delete[] valueArray;
  for(int i=1;i<=noItems;i++)
  {
    delete[] ValueMatrix[i];
  }
  delete[] ValueMatrix;
  for(int i=1;i<=noItems;i++)
  {
    delete[] BacktrackMatrix[i];
  }
  delete[] BacktrackMatrix;
}

void Knapsack::getInputs()
{
  cin>>noItems;
  cin>>capacity;
  weightArray=new int[noItems];
  valueArray=new int[value];
  for(int i=1;i<=noItems;i++)
  {
    cin>>value;
    valueArray[i]=value;
  }
  for(int i=1;i<=noItems;i++)
  {
    cin>>weight;
    weightArray[i]=weight;
  }
  ValueMatrix=new int*[noItems];
  for(int i=1;i<=noItems;i++)
  {
    ValueMatrix[i]=new int[capacity+1];
  }
  BacktrackMatrix=new int*[noItems];
  for(int i=1;i<=noItems;i++)
  {
    BacktrackMatrix[i]=new int[capacity+1];
  }
}

int main()
{
  Knapsack *knap=new Knapsack();
  knap->getInputs();
  knap->findItems();
  knap->print();
  delete knap;
  return 0;
}

Answer 1

I believe the root of your issue is caused by the allocation of valueArray, and the fact that you are iterating out of bounds.

The line valueArray=new int[value]; initializes valueArray with an array of size value which is an uninitialized variable. Perhaps you meant to use noItems?

Also, as songyuanyao pointed out in the comments, your for loops look like for(int i=1;i<=noItems;i++) which starts the counter at 1 and finishes with the counter at noItems, which is erroneous. In a lot of languages, C++ included, arrays start at index 0 (meaning the first item is array[0], not array[1]) and the last item is one minus the size of the array (so the last item of an array with 5 elements is array[4]).

If you change your for loop to start at 0 and end one element before noItems you should be golden. That would be for(int i = 0; i < noItems; i++ )

What's probably happening with smaller allocations is the different chunks of memory are arranged sequentially in the same area of the memory heap, so when you overrun the buffer with data, you're smashing new's bookkeeping data.

When you have larger allocations, the new memory can't fit as cleanly into the free space of the heap, so the allocator ends up leaving some slack space between the allocations. Thus, a small overrun doesn't destroy heap information.