Valgrind Invalid read of size 8 and Address 0x5b7e520 is 0 bytes inside a block of size 16 free'd

Question

I have read several questions with similar errors, but I can't figure out my problem. I have checked that I am not accessing anything after deleting it, or trying to delete it twice.

The class Node and List are implemented by myself, but I have used them in several exercises before this one, so I don't think the problem is with the class implementaiton. Here is the code and the error given by valgrind. By the way, the program apparently works.

==10365== Invalid read of size 8
==10365==    at 0x10992A: Node::getNext() const (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x1099F2: List::clean() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10998F: List::~List() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BF5C: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==  Address 0x5b7e520 is 0 bytes inside a block of size 16 free'd
==10365==    at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10365==    by 0x109A0A: List::clean() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10998F: List::~List() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BE24: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==  Block was alloc'd at
==10365==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10365==    by 0x109AA3: List::append(int) (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BD8E: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365== 
==10365== Invalid free() / delete / delete[] / realloc()
==10365==    at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10365==    by 0x109A0A: List::clean() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10998F: List::~List() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BF5C: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==  Address 0x5b7e520 is 0 bytes inside a block of size 16 free'd
==10365==    at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10365==    by 0x109A0A: List::clean() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10998F: List::~List() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BE24: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==  Block was alloc'd at
==10365==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10365==    by 0x109AA3: List::append(int) (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BD8E: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365== 
==10365== Invalid read of size 8
==10365==    at 0x10992A: Node::getNext() const (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x1099F2: List::clean() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10998F: List::~List() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BF6B: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==  Address 0x5b7e430 is 0 bytes inside a block of size 16 free'd
==10365==    at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10365==    by 0x109A0A: List::clean() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10998F: List::~List() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BE15: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==  Block was alloc'd at
==10365==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10365==    by 0x109AA3: List::append(int) (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BD52: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365== 
==10365== Invalid free() / delete / delete[] / realloc()
==10365==    at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10365==    by 0x109A0A: List::clean() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10998F: List::~List() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BF6B: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==  Address 0x5b7e430 is 0 bytes inside a block of size 16 free'd
==10365==    at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10365==    by 0x109A0A: List::clean() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10998F: List::~List() (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BE15: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==  Block was alloc'd at
==10365==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10365==    by 0x109AA3: List::append(int) (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365==    by 0x10BD52: main (in /mnt/HDD 1TB/TFG/Cracking the code/Chapter 2/prueba)
==10365== 
==10365== 
==10365== HEAP SUMMARY:
==10365==     in use at exit: 0 bytes in 0 blocks
==10365==   total heap usage: 31 allocs, 38 frees, 74,736 bytes allocated
==10365== 
==10365== All heap blocks were freed -- no leaks are possible
==10365== 
==10365== For counts of detected and suppressed errors, rerun with: -v
==10365== ERROR SUMMARY: 14 errors from 4 contexts (suppressed: 0 from 0)

This is the code:

int getNodeSum(Node * head1, Node * head2, List & sumList) {
  int sum = 0;
  int digit1 = 0;
  int digit2 = 0;


  digit1 = head1->getData();
  head1 = head1->getNext();
  digit2 = head2->getData();
  head2 = head2->getNext();


  int carry = 0;
  if(head1 != 0 && head2 != 0)
    carry = getNodeSum(head1, head2, sumList);


  sum = digit1 + digit2 + carry;
  carry = floor(sum / 10);
  sum = sum % 10;

  sumList.append(sum);
  return carry;
}

List sumNotReverse(List l1, List l2) {
  List mySum;

  //First we need to pad the lists
  int length1 = l1.length();
  int length2 = l2.length();
  int diff = length1-length2;

  if(diff < 0) {
    //Then we need to pad list 1
    diff = abs(diff);
    for(int i = 0; i < diff; i++) {
      Node * pad = new Node();
      pad->setNext(l1.getHead());
      l1.setHead(pad);
    }
  } else if(diff > 0 ) {
    for(int i = 0; i < diff; i++) {
      Node * pad = new Node();
      pad->setNext(l2.getHead());
      l2.setHead(pad);
    }
  }


  int carry = getNodeSum(l1.getHead(), l2.getHead(), mySum);

  if(carry == 1)
    mySum.append(carry);


  //Now we have to reverse the list. There are more/different efficient ways to implement this
  stack<int> digits;
  Node * aux = mySum.getHead();
  Node * prev = aux;

  while(aux != 0) {
    digits.push(aux->getData());
    //Doing this makes space complexity O(N) instead of O(2N)
    //(the other way is cleaning the list when we are done)
    prev = aux;
    aux = aux->getNext();
    delete prev;
  }

  //Make sure head is null

  mySum.setHead(0);


  while(digits.size() > 0) {
    mySum.append(digits.top());
    digits.pop();
  }

  return mySum;
}

I can explain what the code does, but some people find this information useless, so if you want it, please let me know.

By petition, here is the code for Node::getNext(), List::clean() and List::~List()

Node * Node::getNext() const {
  return next; 
}



List::~List() {
  clean();
}


void List::clean() {
  if(list != 0) {
    Node* aux;
    while(list != 0) {
      aux = list->getNext();
      delete list;
      list = aux;
    }
  }
}

And this is the .h to see the attributes:

class Node {
private:
  Node * next;
  int data;
public:
  Node();
  int getData() const;
  Node * getNext() const;
  void setData(int data);
  void setNext(Node * next);
};

class List {
private:
  Node * list;
public:
  List();
  ~List();
  void clean();
  int length() const;
  Node * getHead() const;
  void setHead(Node * head);
  // Insert a node at the end of the list with data given by the argument
  void append(int value);
  void print() const;
  void remove(int pos);
  //Remove node toDelete given the reference and the reference  to the previous one
  //Then, this remove is O(1) time complexity
  void remove(Node * prev, Node * toDelete);
  void removeDuplicates();
  void removeDuplicatesnoBuffer();

};

You can download the full code to compile it here. In the main(), between lines 136 and 164 is useless, you can ignore it. That's another part of the exercise.

Cannot help you as you have not posted the methods it has in the stack: Node::getNext(), List::clean() and List::~List() — Ian4264, Jun 10 '20 at 15:23
More generallly, we need a code that we can compile ourselves and run — Damien, Jun 10 '20 at 15:25
Code posted and also a link to download and compile the code. Before downvoting, ask and the user will try to fix it ASAP. If he does not do that, go ahead and downvote, but give him a chance. — southernKid33, Jun 10 '20 at 15:32

score 1 · Accepted Answer · answered Jun 10 '20 at 15:33

1

Your List class lacks a valid copy constructor and assignment operator but you are copying List objects, for instance when you call sumNotReverse.

This will result in a double free or a use after free, which is presumably the problem valgrind is complaining about.

See here for more details What is The Rule of Three?, or consult your favourite C++ book. Any decent C++ book is going to cover this topic.

answered Jun 10 '20 at 15:33

john

85,011
4
57
81

I would have never noticed that my error was because of that. Thank you so much! – southernKid33 Jun 10 '20 at 15:46

Valgrind Invalid read of size 8 and Address 0x5b7e520 is 0 bytes inside a block of size 16 free'd

1 Answers1