Malloc Error: incorrect checksum for freed object

Question

I am working on implementing tail for an assignment. I have it working correctly however I seem to be getting an error from free at random times.

I can't see, to track it down to a pattern or anything besides it is consistent.

For example if I call my program as "tail -24 test.in" I would get the the the incorrect checksum error at the same line on multiple runs. However, with different files and even different numbers of lines to print back I will come back without errors.

Any idea on how to track down the issue, I've been trying to debug it for hours to no avail.

Here is the offending code:

lines is defined as a char** and was malloc as:

lines = (char**) malloc(nlines * sizeof(char *));

void insert_line(char *s, int len){

  printf("\t\tLine Number: %d Putting a %d line into slot: %d\n",processed,len,slot);
  if(processed > numlines -1){//clean up
    free(*(lines+slot));
    *(lines + slot) = NULL;
  }
  *(lines + slot) = (char *) malloc(len * sizeof(char));
  if(*(lines + slot) == NULL) exit(EXIT_FAILURE);
  strcpy(*(lines+slot),s);
  slot = ++processed % numlines;
}

Answer 1

Do nlines and numlines have the same value?

Does the caller of insert_line allow room for the trailing NUL when passing the length in the second parameter?

Answer 2

I'm not sure it's related but these two lines seems suspicious to me:

  *(lines + slot) = (char *) malloc(len * sizeof(char));
  if((lines + slot) == NULL) exit(EXIT_FAILURE);

You first assign the return of malloc to lines[slot] and then you check (lines+slot), if the latter was NULL, you had dereference a NULL pointer!

Also if lines[slot] (your *(lines+slot)) is not null, you will leak memory when you will assign the result of malloc() to it.

I assume lines is a char *lines[]` and slot is within the allowed boundary!

Answer 3

I agree with remo's suspicion about those two lines, but not the tangent that remo went off on. We should share credit for finding this bug.

*(lines + slot) = some value
if((lines + slot) == NULL) then die
should be
if(*(lines + slot) == NULL) then die

Answer 4

If you can consistently reproduce the problem with specific input parameters, you should debug like this:

• First debug to the precise free that causes the problem.
• Then figure out when the memory that is about to be free'd was malloc'ed.
• Next, debug to the place where the memory is malloc'ed.
• Locate in the memory viewer the allocated block of memory. Note both the start and end of the block. There is probably a special value called a guard block just before and just after the block.
• Now step through the code until the memory is free'd. At some point your code should mistakenly overwrite the guard block. That is the offending statement.

Note that the problem might very well be in a completely different part of your program. Even though it is this free that is reporting the error, the code that overwrites the guard block can be anywhere.

Answer 5

My first question is how do you calculate len? Is it just strlen or does it include room for the \0 terminator? I think you may be overshooting your allocation in your strcpy. Bad behavior will tend to happen on word boundaries and appear random. Also, check to make sure that your source strings are null terminated. If you made a mistake on the read side and didn't terminate them. Then strcpy may be randomly overwriting things.

  *(lines + slot) = (char *) malloc(len * sizeof(char));
  if(*(lines + slot) == NULL) exit(EXIT_FAILURE);
  strcpy(*(lines+slot),s);

Perhaps try:

  lines[slot] = (char *) malloc((len + 1) * sizeof(char));
  if(lines[slot] == NULL) exit(EXIT_FAILURE);
  if(strlen(s) <= len){
    strcpy(lines[slot],s);
  }
  else{
    /* do something else... */
  }

In terms of general form, I'd also encourage you to make a few stylistic changes to make the whole thing a bit more readable, easier to follow and resistant to errors.

Pointer arithmetic is valid and fun, but I think your intent is a little more clear if you use the array form like:

free(lines[slot]);
lines[slot] = NULL;

instead of

free(*(lines+slot));
*(lines + slot) = NULL;

I'd also encourage you to use fewer statics. It's easy enough to through them in a data structure and pass them around into your accessors and mutators. It becomes much more clear where the action is happening prevents you from doing things like:

static int numlines = 0;
void insert_line(char *s, int len){
    int numlines = 5;

where you can introduce scoping issues that are just miserable to debug.

Answer 6

Your routine is writing beyond the allocated line buffer.

The size of the line passed as an argument (i.e. "len") probably does not include the NUL terminator. When you call malloc to copy the line (i.e. "s") you need to allocate an extra byte for the string terminator:

 *(lines + slot) = (char *) malloc((len + 1) * sizeof(char));