Valgrind: Invaild read of size 8

Question

I have been working on an open source project for awhile, http://gtkworkbook.sourceforge.net/, and recently ran into an issue that just seems like I am going in circles. I'm pretty sure there is a heap problem but I have been looking at this code too long to figure out exactly what it is.

So, in brief, what I am doing is reallocating a block of memory from N pointers to M pointers while working with a libcsv parser. If there are additional columns I want to increase the maximum size of the array to 2 times the current size. Here's the code currently:

 struct csv_column {
    Sheet * sheet;
    Cell ** array;
    int & array_max;
    int & array_size;
    int row;
    int field;
    char * value;
  };

  static void 
  cb1 (void * s, size_t length, void * data) {
    struct csv_column * column = (struct csv_column *) data;
    int & array_max = column->array_max;

    // Resize the cell array here.
    if (column->field >= array_max) {
      int max = (2 * array_max);
      (column->array) = (Cell **) g_realloc ((column->array), max * sizeof (Cell*));

      for (int ii = array_max; ii array)[column->field] == NULL)
      (column->array)[column->field] = cell_new();

    Cell * cell = (column->array)[column->field];
    cell->set_row (cell, column->row);
    cell->set_column (cell, column->field++);
    cell->set_value_length (cell, s, length);
  }

  CsvParser::CsvParser (Workbook * wb,
            FILE * log,
            int verbosity,
            int maxOfFields) {
    this->wb = wb;
    this->log = log;
    this->verbosity = verbosity;
    this->sizeOfFields = 0;
    this->maxOfFields = maxOfFields;
    this->fields = (Cell **) g_malloc (maxOfFields * sizeof (Cell*));

    for (int ii = 0; ii maxOfFields; ii++)
      this->fields[ii] = NULL;
  }

  CsvParser::~CsvParser (void) {
    for (int ii = 0; ii maxOfFields; ii++) {
      if (this->fields[ii])
        this->fields[ii]->destroy (this->fields[ii]);
    }

    g_free (this->fields);
  }

Here is the valgrind output:

==28476== Thread 9:
==28476== Invalid read of size 8
==28476==    at 0x771AF4F: sheet_method_apply_cellarray (sheet.c:351)
==28476==    by 0xD930DB7: largefile::CsvParser::run(void*) (CsvParser.cpp:147)
==28476==    by 0xDD624C8: concurrent::thread_run(void*) (Thread.cpp:28)
==28476==    by 0xA7B73B9: start_thread (in /lib/libpthread-2.9.so)
==28476==    by 0x80DBFCC: clone (in /lib/libc-2.9.so)
==28476==  Address 0xbc5d4a8 is 0 bytes inside the accessing pointer's
==28476==  once-legitimate range, a block of size 160 free'd
==28476==    at 0x4C25D4F: free (vg_replace_malloc.c:323)
==28476==    by 0xD9314CA: largefile::cb1(void*, unsigned long, void*) (CsvParser.cpp:57)
==28476==    by 0xDB42681: csv_parse (in /home/jbellone/work/gtkworkbook/lib/libcsv.so)
==28476==    by 0xD930D00: largefile::CsvParser::run(void*) (CsvParser.cpp:136)
==28476==    by 0xDD624C8: concurrent::thread_run(void*) (Thread.cpp:28)
==28476==    by 0xA7B73B9: start_thread (in /lib/libpthread-2.9.so)
==28476==    by 0x80DBFCC: clone (in /lib/libc-2.9.so)
==28476== 
==28476== Invalid read of size 8
==28476==    at 0x771AF66: sheet_method_apply_cellarray (sheet.c:351)
==28476==    by 0xD930DB7: largefile::CsvParser::run(void*) (CsvParser.cpp:147)
==28476==    by 0xDD624C8: concurrent::thread_run(void*) (Thread.cpp:28)
==28476==    by 0xA7B73B9: start_thread (in /lib/libpthread-2.9.so)
==28476==    by 0x80DBFCC: clone (in /lib/libc-2.9.so)
==28476==  Address 0xbc5d4a8 is 0 bytes inside the accessing pointer's
==28476==  once-legitimate range, a block of size 160 free'd
==28476==    at 0x4C25D4F: free (vg_replace_malloc.c:323)
==28476==    by 0xD9314CA: largefile::cb1(void*, unsigned long, void*) (CsvParser.cpp:57)
==28476==    by 0xDB42681: csv_parse (in /home/jbellone/work/gtkworkbook/lib/libcsv.so)
==28476==    by 0xD930D00: largefile::CsvParser::run(void*) (CsvParser.cpp:136)
==28476==    by 0xDD624C8: concurrent::thread_run(void*) (Thread.cpp:28)
==28476==    by 0xA7B73B9: start_thread (in /lib/libpthread-2.9.so)
==28476==    by 0x80DBFCC: clone (in /lib/libc-2.9.so)

sheet.c line 351

   gtk_sheet_set_cell_text (GTK_SHEET (sheet->gtk_sheet),
                 array[ii]->row,
                 array[ii]->column,
                 array[ii]->value->str);

The whole function from sheet.c:

static void
sheet_method_apply_cellarray (Sheet * sheet, 
                  Cell ** array,
                  gint size)
{
  ASSERT (sheet != NULL);
  g_return_if_fail (array != NULL);

  gdk_threads_enter ();

  /* We'll see how this performs for now. In the future we may want to go
     directly into the GtkSheet structures to get a little more performance
     boost (mainly because we should not have to check all the bounds each
     time we want to update). */
  for (gint ii = 0; ii gtk_sheet),
                 array[ii]->row,
                 array[ii]->column,
                 array[ii]->value->str);

    if (!IS_NULLSTR (array[ii]->attributes.bgcolor->str))
      sheet->range_set_background (sheet, 
                   &array[ii]->range, 
                   array[ii]->attributes.bgcolor->str);

    if (!IS_NULLSTR (array[ii]->attributes.fgcolor->str))
      sheet->range_set_foreground (sheet, 
                   &array[ii]->range, 
                   array[ii]->attributes.fgcolor->str);

    /* Clear all of the strings */
    g_string_assign (array[ii]->value, "");
    g_string_assign (array[ii]->attributes.bgcolor, "");
    g_string_assign (array[ii]->attributes.fgcolor, "");
  }

  gdk_threads_leave ();
}

CSV parser thread

  void *
  CsvParser::run (void * null) {
    this->running = true;
    std::queue queue;
    struct csv_parser csv;
    struct csv_column column = {this->wb->sheet_first,
                this->fields,
                this->maxOfFields,
                this->sizeOfFields,
                0,
                0,
                new char [1024]};

    if (csv_init (&csv, CSV_STRICT) != 0) {
      std::cerr running == true) {
      if (this->inputQueue.size() > 0) {

    // Lock, copy, clear, unlock. - Free this up.
    this->inputQueue.lock();
    this->inputQueue.copy (queue);
    this->inputQueue.clear();
    this->inputQueue.unlock();

    while (queue.size() > 0) {
      std::string buf = queue.front(); queue.pop();
      size_t bytes = buf.length();

      if (this->running == false)
        break;

      if ((bytes = csv_parse (&csv, buf.c_str(), bytes, cb1, cb2, &column)) == bytes) {
        if (csv_error (&csv) == CSV_EPARSE) {
          std::cerr wb->sheet_first->apply_array (this->wb->sheet_first,
                                          this->fields,
                                          this->sizeOfFields);

      if (column.row >= (column.sheet)->max_rows)
        column.row = 0;
    }
      } 
      concurrent::Thread::sleep(1);
    }

    return NULL;
  }

Answer 1

My guess is that you are running into a race condition with threading. Cb1 is reallocating the array while your parser is trying to use it. After the realloc the old address is no longer valid and that is where your invalid read is coming from. You'll need to put a lock (perhaps a reader/writer lock) around the array to keep from hitting this problem. If it's possible, try running this code single-threaded to see if the problem reproduces. If it doesn't then it's a threading problem, otherwise it's something else entirely.