CODEWITHSUNDEEP
c++memory-managementvalgrind
Nikkolasg
Nikkolasg

Reputation: 454

Valgrind Invalid read/write list

Even after a lot of research, I still don't figure out why I've got this error from valgrind.. There is a class spike (which implements a pseudo double linked list)

class spike
{
  int s_cell;
  int s_begin;
  int s_number;
  int s_type;
  spike *s_previous;
  spike *s_next;

  spike *s_origin; // pointer for original spike or itself
                   // (in the very first spike_data )

  spike *s_derive; // pointer for the spike in the derived class
                   // (note, that sometimes there are 2 or more high
                   // order spike_data in parallel; the pointer is only to one)
public:
  spike(int c, int b, int n, int typ=c_normal)
  {
    s_cell = c;
    s_begin = b;
    s_number = n;
    s_previous = NULL;
    s_next = NULL;
    s_derive = NULL;
    s_origin = this;
    s_type = typ;
  }

  ~spike()
  {
    kill();
  }

And class spike_data.

class spike_data
{
protected:
  int sd_mode;
  int sd_size;
  int sd_number;      // the whole number of spikes 
  int sd_file;        // the number of files for analysis
  int *sd_file_name;  // the names of files used in the analysis
  int sd_cells;       // the whole number of different cells

  spike **sd_array;   // array of all spikes
  spike *sd_first[c_maxcells]; // array of the first entries of spikes
  spike **sd_file_st; // spikes in array for indication of beginning of the new files

// And here is the part that's getting the error
// (it is happening when i try to release)
void spike::kill()
{
  // delete a cell from all references and t becomes "dead"
  // actual release of memory is done in renumerate
  try
  {
    if (s_previous != NULL)
      s_previous->s_next = s_next;
    if (s_next != NULL)
      s_next->s_previous = s_previous;
    if (s_origin && s_origin != this)
    {
      int tmp = 1;
    while(tmp == 1) {
        if (s_origin != NULL) {
            if (s_origin->s_derive != NULL) { // LINE 674
                if ( s_origin->s_derive != this ) { // LINE 675
                        s_origin=s_origin->s_derive;
                    }
                    else tmp = 0;
                }
            else tmp = 0;                       
                     }
             else tmp = 0;
    }

        s_origin->s_derive=NULL; // LINE 685
    }
  }
  catch (...)
  {
  }
  s_next = NULL;
  s_previous = NULL;
  s_origin = NULL;
}


spike_data::~spike_data()
{

if(sd_array!=NULL)
{
    for(int i=0;i<sd_number;i++)
        delete sd_array[i]; // LINE 697

    delete[] sd_array;
    sd_array=NULL;
}
if(sd_file_st!=NULL)
{
    delete[] sd_file_st;
    sd_file_st=NULL;
}
if(sd_file_name!=NULL)
{
    delete[] sd_file_name;
    sd_file_name=NULL;
}
}

Sorry if the code is indigest, it's not mine, but it's still ok...

Invalid read of size 4
==2079==    at 0x806B0DF: spike::kill() (spikes.cpp:674)
==2079==    by 0x8067D26: spike::~spike() (spike.h:288)
==2079==    by 0x806B174: spike_data::~spike_data() (spikes.cpp:689)
==2079==    by 0x805CB1F: spike_anal::~spike_anal() (spikea.cpp:151)
==2079==    by 0x8061C1E: statistical_analysis(spike_group*, spike_data*) (spikeg.cpp:264)
==2079==    by 0x804B37E: calculate_something(int, int) (all.cpp:422)
==2079==    by 0x8059805: real_main(int, char const*) (simple.cpp:742)
==2079==    by 0x804DC20: main (hello.cpp:66)
==2079==  Address 0x5d8f534 is 28 bytes inside a block of size 32 free'd
==2079==    at 0x4023881: operator delete(void*) (vg_replace_malloc.c:387)
==2079==    by 0x806B17C: spike_data::~spike_data() (spikes.cpp:697)
==2079==    by 0x805CB1F: spike_anal::~spike_anal() (spikea.cpp:151)
==2079==    by 0x8061C06: statistical_analysis(spike_group*, spike_data*) (spikeg.cpp:264)
==2079==    by 0x804B37E: calculate_something(int, int) (all.cpp:422)
==2079==    by 0x8059805: real_main(int, char const*) (simple.cpp:742)
==2079==    by 0x804DC20: main (hello.cpp:66)
==2079== 
==2079== Invalid read of size 4
==2079==    at 0x806B0EC: spike::kill() (spikes.cpp:675)
==2079==    by 0x8067D26: spike::~spike() (spike.h:288)
==2079==    by 0x806B174: spike_data::~spike_data() (spikes.cpp:689)
==2079==    by 0x805CB1F: spike_anal::~spike_anal() (spikea.cpp:151)
==2079==    by 0x8061C1E: statistical_analysis(spike_group*, spike_data*) (spikeg.cpp:264)
==2079==    by 0x804B37E: calculate_something(int, int) (all.cpp:422)
==2079==    by 0x8059805: real_main(int, char const*) (simple.cpp:742)
==2079==    by 0x804DC20: main (hello.cpp:66)
==2079==  Address 0x5d8f534 is 28 bytes inside a block of size 32 free'd
==2079==    at 0x4023881: operator delete(void*) (vg_replace_malloc.c:387)
==2079==    by 0x806B17C: spike_data::~spike_data() (spikes.cpp:697)
==2079==    by 0x805CB1F: spike_anal::~spike_anal() (spikea.cpp:151)
==2079==    by 0x8061C06: statistical_analysis(spike_group*, spike_data*) (spikeg.cpp:264)
==2079==    by 0x804B37E: calculate_something(int, int) (all.cpp:422)
==2079==    by 0x8059805: real_main(int, char const*) (simple.cpp:742)
==2079==    by 0x804DC20: main (hello.cpp:66)
==2079== 
==2079== Invalid write of size 4
==2079==    at 0x806B10A: spike::kill() (spikes.cpp:685)
==2079==    by 0x8067D26: spike::~spike() (spike.h:288)
==2079==    by 0x806B174: spike_data::~spike_data() (spikes.cpp:689)
==2079==    by 0x805CB1F: spike_anal::~spike_anal() (spikea.cpp:151)
==2079==    by 0x8061C1E: statistical_analysis(spike_group*, spike_data*) (spikeg.cpp:264)
==2079==    by 0x804B37E: calculate_something(int, int) (all.cpp:422)
==2079==    by 0x8059805: real_main(int, char const*) (simple.cpp:742)
==2079==    by 0x804DC20: main (hello.cpp:66)
==2079==  Address 0x5d8f534 is 28 bytes inside a block of size 32 free'd
==2079==    at 0x4023881: operator delete(void*) (vg_replace_malloc.c:387)
==2079==    by 0x806B17C: spike_data::~spike_data() (spikes.cpp:697)
==2079==    by 0x805CB1F: spike_anal::~spike_anal() (spikea.cpp:151)
==2079==    by 0x8061C06: statistical_analysis(spike_group*, spike_data*) (spikeg.cpp:264)
==2079==    by 0x804B37E: calculate_something(int, int) (all.cpp:422)
==2079==    by 0x8059805: real_main(int, char const*) (simple.cpp:742)
==2079==    by 0x804DC20: main (hello.cpp:66)

So here it is. When i want to kill spike_data,each time I call delete sd_array[i] , which calls destructor of spike => kill() The thing is this code bug at the end. It used many times during the execution and at some point,when all computations are done and need to release memory, it doesn't work. I think there is problem when i put s_origin->s_derive=NULL. but can't seize it...

If u need more code just ask :)

Thank you a lot for those who are courageous enough to get into this code !!

Good week end

Nico

Upvotes: 2

Views: 801

Answers (1)

kfmfe04
kfmfe04

Reputation: 15337

There is a good likelihood that the default copy constructor for spike has been invoked without you noticing it. If this is the case, the first destructor may run correctly, but the second call will cause valgrind to dump messages like what you have posted.

In order to see if this is the case, if you are using C++11, add:

public:
  spike( const spike& rhs ) = delete;

and your code should fail to compile indicating that the need for you to write your own copy constructor.

If you are not using C++11, you can write your own copy constructor and place a breakpoint inside to see if it stops there while debugging.

Upvotes: 1

Related Questions