why would freeing calloc'ed memory crash my VC6 project?

Question

Compare these two largely identical functions. In the first, the memory for buff is allocated using _alloca. This works fine. In the second, calloc and free are used instead of _alloca. This crashes.

The weird thing is that I use the calloc/free technique in almost every other GMP wrapping function I have and they all work. Here they don't. Any ideas?

1:

#define Z(x) mpz_t (x); mpz_init( (x) );
#define BUFF_SIZE (1024 * 32)

BSTR __stdcall IBIGDIV(BSTR p1, BSTR p2 ) { 
    USES_CONVERSION;

    Z(n1);
    Z(n2);
    Z(res);

    char * buff =  (char *) _alloca( mpz_sizeinbase( res, 10 ) + 2 );

    LPSTR sNum1 = W2A( p1 );
    LPSTR sNum2 = W2A( p2 );

    mpz_set_str( n1, sNum1, 10 );
    mpz_set_str( n2, sNum2, 10 );

    if ( mpz_sgn( n2 ) != 0 ) { 
        mpz_div( res, n1, n2 );
        mpz_get_str(buff, 10, res);
    } else {
        strcpy( buff, "-0" );
    }

    BSTR bResult = _com_util::ConvertStringToBSTR( buff );
    return bResult;
}

2:

#define Z(x) mpz_t (x); mpz_init( (x) );
#define BUFF_SIZE (1024 * 32)

BSTR __stdcall IBIGDIV(BSTR p1, BSTR p2 ) { 
    USES_CONVERSION;

    Z(n1);
    Z(n2);
    Z(res);

    char * buff =  (char *) calloc( mpz_sizeinbase( res, 10 ) + 2, sizeof( char ) );

    LPSTR sNum1 = W2A( p1 );
    LPSTR sNum2 = W2A( p2 );

    mpz_set_str( n1, sNum1, 10 );
    mpz_set_str( n2, sNum2, 10 );

    if ( mpz_sgn( n2 ) != 0 ) { 
        mpz_div( res, n1, n2 );
        mpz_get_str(buff, 10, res);
    } else {
        strcpy( buff, "-0" );
    }

    BSTR bResult = _com_util::ConvertStringToBSTR( buff );
    free( buff );
    return bResult;
}

Answer 1

It may be unrelated, but this type of "works one way but not the other" often indicates a bug that just happens to squeak by in one situation but causes a fatal error in another.

If you suspect a memory overwrite may be occurring you could try allocating an extra 8-bytes in the buffer and writing 4-byte start and end sentinels which you then check for before freeing.

Answer 2

I once spent a week trying to figure out a similar thing. It was a buffer overrun that trashed the pointer so free was going off into the woods. Rational purify found the issue in a minute.

Answer 3

Add logging and dump everything along the way to find what goes wrong. This is usually more efficient than trying to guess.

Answer 4

calloc could potentially return NULL if there's an error (such as lack of memory). I would recommend checking the result of any memory allocation function against NULL. If it is NULL, print a message and then exit(1).

Answer 5

_alloca returns stack memory, so stomping past the end of it may not necessarily overwrite something important. Writing past the end of a heap memory allocation will more likely overwrite something important.

Your code does nothing to ensure that the buffer is at least as big as res would be formatted to after dividing n1 by n2 (or vice versa, as I don't know what the actual function does); it only ensures that it has enough memory for an initialiazed res, which is probably 1. If n1/n2 has more digits than that, welcome to crashville.

Answer 6

@johnny pointed out something rather embarrassing, which necessitated a rewrite of the code. (Here's where being able to tick a comment would be useful.)

BSTR __stdcall IBIGDIV(BSTR p1, BSTR p2 ) { 
 USES_CONVERSION;

 Z(n1);
 Z(n2);
 Z(res);

 char * buff;

 LPSTR sNum1 = W2A( p1 );
 LPSTR sNum2 = W2A( p2 );

 mpz_set_str( n1, sNum1, 10 );
 mpz_set_str( n2, sNum2, 10 );

 if ( mpz_sgn( n2 ) != 0 ) { 
  mpz_div( res, n1, n2 );
  buff =  (char *) calloc( mpz_sizeinbase( res, 10 ) + 2, sizeof( char ) );
  mpz_get_str(buff, 10, res);
 } else {
  buff =  (char *) calloc( 3, sizeof( char ) );
  strcpy( buff, "-0" );
 }

 BSTR bResult = _com_util::ConvertStringToBSTR( buff );
 free( buff );
 return bResult;
}

In the previous incarnations, the memory was being allocated according to the value of res at the point in the code where it contained zero. Thus I was trying to calloc zero bytes and free didn't like it. In the above code, res actually contains something that mpz_sizeinbase can work with.