MechaPrime
MechaPrime

Reputation: 47

Access violation when using free() when running but not when debugging... slowly

As per my previous question (many thanks to Jonathan Leffler), I edited my code (second two blocks of code), but I ran into a rather strange problem.

The following one breaks unpredictably...

void free_array(array_info *A)
{
    int i;
    for(i = 0; i < (A->height); ++i)
    {
        printf("About to free: %x\n", A->dat_ptr[i]);//for debugging purposes
        free(A->dat_ptr[i]);
        printf("Freed row %i\n", i);//for debugging purposes
    }
    free(A->dat_ptr);
}

I initially tested create_array directly followd by free_array and it worked flawlessly with rather big arrays (10^8). However, when I do my calculations in between and then try to free() the arrays, I get an access violation exception (c00000005). When I was debugging it, I noticed that the program would execute perfectly every time if I had a breakpoint within the "free_array" loop and did every line individually. However, the compiled code wouldn't ever run past row6 of my second array on its own. I turned off all optimisations in the compiler, and I still got the error upon execution.

Additional info

typedef struct {
    int    height;
    int    width;
    int    bottom;//position of the bottom tube/slice boundary
    unsigned int**  dat_ptr;//a pointer to a 2d array
    } array_info;

Where the dat_ptr is now a proper 2D pointer. The create_array function that creates the array that is to be put in the structure is (i have stripped NULL checks for readability):

int create_array(array_info *A)
{
int i;
unsigned int **array = malloc(sizeof(*array) * A->height);

for (i = 0; i < A->height; ++i)
{
    array[i] = malloc(sizeof(**array) * A->width);
}

A->dat_ptr = array;
return 0;
}

This function works exactly as expected.

More Additional Info

Added after the responses of Jonathan, Chris, and rharrison33

Thank you so much, Jonathan, with every one of your posts I find out so much about programming :) I finally found the culprit. The code causing the exception was the following:

void fill_number(array_info* array,  int value,  int x1,  int y1,  int x2,  int y2)//fills a rectangular part of the array with `value`
    {
    int i, j;
    for(i=y1 ; ((i<=y2)&&(i<array->height)) ; i++)//start seeding the values by row (as in vertically)
    {
        for(j=x1 ; ((i<=x2)&&(i<array->width)) ; j++)//seed the values by columns (as in horizontally)
        {
        array->dat_ptr[i][j]=value;
        }
    }
}

And ((i<=x2)&&(i<=array->width)) wasn't being evaluated as I expected (Chris Dodd, you were right). I thought that it would evaluate both conditions in that order or stop if either was "FALSE", independent of their order. However, it turned out it didn't work that way and it was simply refusing to evaluate the (i<array->width) part correctly. Also, I assumed that it would trigger an exception upon trying to access memory outside of the array range, but it didn't. Anyway,

I changed the code to:

void fill_number(array_info* array,  int value,  int x1,  int y1,
                                                    int x2,  int y2)
{
    int i, j;
    if(y1>=array->height){ y1=array->height-1;}
    if(y2>=array->height){ y1=array->height-1;}
    if(x1>=array->width) { x2=array->width-1;}
    if(x2>=array->width) { x2=array->width-1;}
    for(i=y1 ; i<=y2 ; i++)//start seeding the values by row
    {
        for(j=x1 ; j<=x2 ; j++)//seed the values by column
        {
        array->dat_ptr[i][j]=value;
        }
    }
}

And now it works. The block of if()s is there because I won't be calling the function very often compared to the rest of the code and I need a visual way to remind me that the check is there.

Again, thank you so much Jonathan Leffler, Chris Dodd, and rharrison33 :)

Upvotes: 2

Views: 1154

Answers (2)

Jonathan Leffler
Jonathan Leffler

Reputation: 754690

This code, closely based on what you've gotten from me and what you wrote above, seems to be working as expected. Note the use of <inttypes.h> and PRIXPTR (and the cast to (uintptr_t)). It avoids making assumptions about the size of pointers and works equally well on 32-bit and 64-bit systems (though the %.8 means you get full 8-digit hex values on 32-bit compilations, and 12 (out of a maximum of 16) on this specific 64-bit platform).

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <inttypes.h>

typedef struct
{
    int    height;
    int    width;
    int    bottom;
    unsigned int **dat_ptr;  // Double pointer, not triple pointer
} array_info;

static void create_array(array_info *A)
{
    unsigned int **array = malloc(sizeof(*array) * A->height);
    printf("array (%zu) = 0x%.8" PRIXPTR "\n",
           sizeof(*array) * A->height, (uintptr_t)array);
    for (int i = 0; i < A->height; ++i)
    {
        array[i] = malloc(sizeof(**array) * A->width);
        printf("array[%d] (%zu) = 0x%.8" PRIXPTR "\n",
               i, sizeof(**array) * A->width, (uintptr_t)array[i]);
    }
    A->dat_ptr = array;
}

static void free_array(array_info *A)
{
    int i;
    for(i = 0; i < (A->height); ++i)
    {
        printf("About to free %d: 0x%.8" PRIXPTR "\n",
               i, (uintptr_t)A->dat_ptr[i]);
        free(A->dat_ptr[i]);
    }
    printf("About to free: 0x%.8" PRIXPTR "\n", (uintptr_t)A->dat_ptr);
    free(A->dat_ptr);
}

int main(void)
{
    array_info array = { .height = 5, .width = 10, .dat_ptr = 0 };
    create_array(&array);
    if (array.dat_ptr == 0)
    {
        fprintf(stderr, "Out of memory\n");
        exit(1);
    }
    free_array(&array);
    puts("OK");
    return(0);
}

Sample output

array (40) = 0x7FAFB3C03980
array[0] (40) = 0x7FAFB3C039B0
array[1] (40) = 0x7FAFB3C039E0
array[2] (40) = 0x7FAFB3C03A10
array[3] (40) = 0x7FAFB3C03A40
array[4] (40) = 0x7FAFB3C03A70
About to free 0: 0x7FAFB3C039B0
About to free 1: 0x7FAFB3C039E0
About to free 2: 0x7FAFB3C03A10
About to free 3: 0x7FAFB3C03A40
About to free 4: 0x7FAFB3C03A70
About to free: 0x7FAFB3C03980
OK

I've not got valgrind on this machine, but the addresses being allocated and freed can be eyeballed to show that there's no obvious problem there. It's coincidence that I sized the arrays such that they're all 40 bytes (on a 64-bit machine).

Follow-up Questions

  • What else are you doing with your data?
  • How big are the arrays that you're allocating?
  • Are you sure you're not running into arithmetic overflows?

Testing on Mac OS X 10.8.2 and the XCode version of GCC/Clang:

i686-apple-darwin11-llvm-gcc-4.2 (GCC) 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2336.11.00)


Array setting and printing functions

static void init_array(array_info *A)
{
    unsigned int ctr = 0;
    printf("D       = 0x%.8" PRIXPTR "\n", (uintptr_t)A->dat_ptr);
    for (int i = 0; i < A->height; i++)
    {
        printf("D[%d]    = 0x%.8" PRIXPTR "\n",i, (uintptr_t)A->dat_ptr[i]);
        for (int j = 0; j < A->width; j++)
        {
            printf("D[%d][%d] = 0x%.8" PRIXPTR " (%u)\n",
                   i, j, (uintptr_t)&A->dat_ptr[i][j], ctr);
            A->dat_ptr[i][j] = ctr;
            ctr += 7;
        }
    }
}

static void print_array(array_info *A)
{
    printf("D       = 0x%.8" PRIXPTR "\n", (uintptr_t)A->dat_ptr);
    for (int i = 0; i < A->height; i++)
    {
        printf("D[%d]    = 0x%.8" PRIXPTR "\n",i, (uintptr_t)A->dat_ptr[i]);
        for (int j = 0; j < A->width; j++)
        {
            printf("D[%d][%d] = 0x%.8" PRIXPTR " (%u)\n",
                   i, j, (uintptr_t)&A->dat_ptr[i][j], A->dat_ptr[i][j]);
        }
    }
}

With a call init_array(&array); in main() after the successful create_array() and a call to print_array(&array); after that, I got the expected output. It's too boring to show here.

Upvotes: 2

rharrison33
rharrison33

Reputation: 1282

I believe you are malloc'ing incorrectly. Try modifying your create_array function to this:

int create_array(array_info *A)
{
  int i;
  unsigned int **array = malloc(sizeof(unsigned int*) * A->height);

  for (i = 0; i < A->height; ++i)
  {
    array[i] = malloc(sizeof(unsigned int) * A->width);
  }

  A->dat_ptr = array;
  return 0;
}

Upvotes: 2

Related Questions