How to avoid heap corrupt problem on Windows XP.

[fl]

Hi,

The following part of routine is originally run on a Linux OS. It
works. Now, I want to run it on Microsoft MSC Express 8.0 on a Windows
XP. I find that the heap corrupt at the second time call
distrib_create (before the free function).
Do you have some ideas on how to solve this easily? Thanks.




..................
distrib *distrib_create
( char *c /* String describing distribution over numbers */
)
{
distrib *d;
char *str, *tstr;
int i, n, scan_num, size;
double prop, sum;
double d_tmp;
char junk;

/* Check for special case of a single number. */

if (sscanf(c,"%d%c",&n,&junk)==1 && n>0)
{ tstr = chk_alloc ( (int)(2.1+log10(n)), sizeof(*str));
sprintf(tstr,"1x%d",n);
d = distrib_create(tstr);
free(tstr);
return d;
}
....
....

 

[Ike Naar]

if (sscanf(c,"%d%c",&n,&junk)==1 && n>0)
{ tstr = chk_alloc ( (int)(2.1+log10(n)), sizeof(*str));
sprintf(tstr,"1x%d",n);

That looks like a buffer overrun.
You haven't explained what chk_alloc does, but assuming that
chk_alloc(n,s) allocates an array of n objects of size s,
you allocate insufficient space for the string you want to store in tstr.
Suppose n equals 1, then you allocate (int)(2.1+log10(1)) =
(int)(2.1+0.0) = (int)(2.1) = 2 bytes.
That's not enough for the 4-byte string "1x1" ('1','x','1','\0').

 

[Ben Bacarisse]

The following part of routine is originally run on a Linux OS. It
works.

I think is just looks like it works. The code looks to me to be wrong
on any system -- what varies is if and exactly when you get to see a
problem.

Now, I want to run it on Microsoft MSC Express 8.0 on a Windows
XP. I find that the heap corrupt at the second time call
distrib_create (before the free function).
Do you have some ideas on how to solve this easily? Thanks.

.................
distrib *distrib_create
( char *c /* String describing distribution over numbers */
)
{
distrib *d;
char *str, *tstr;
int i, n, scan_num, size;
double prop, sum;
double d_tmp;
char junk;

/* Check for special case of a single number. */

if (sscanf(c,"%d%c",&n,&junk)==1 && n>0)
{ tstr = chk_alloc ( (int)(2.1+log10(n)), sizeof(*str));

A better pattern is xxx = chk_alloc(N, sizeof *xxx); Your two xxxs are
not the same. Not a big a deal here because I can easily tell that *str
and *tstr have the same type, but it makes your reader do more work.

sprintf(tstr,"1x%d",n);

There is not enough space is tstr. This is the key problem.

d = distrib_create(tstr);
free(tstr);
return d;
}

This is a very complex way of altering the argument. There is no need
for an allocated array and no need for a log function. You could just
use a fixed size array:

char tstr[32];
sprintf(tstr, "1x%d", n);
return distrib_create(tstr);

but it you want the size to (almost) match the type you can use

CHAR_BIT * sizeof n / 3 + 1

as the size (CHAR_BIT is defined in limits.h).

But I'd seriously try to find a way in which you can deal with this kind
of string (one with only one number in it) without having to re-write it
an call yourself recursively. It just seem like such a complex solution
(and, as you've seen, it is error prone).

 

[fl]

That looks like a buffer overrun.
You haven't explained what chk_alloc does, but assuming that
chk_alloc(n,s) allocates an array of n objects of size s,
you allocate insufficient space for the string you want to store in tstr.
Suppose n equals 1, then you allocate (int)(2.1+log10(1)) =
(int)(2.1+0.0) = (int)(2.1) = 2 bytes.
That's not enough for the 4-byte string "1x1" ('1','x','1','\0').

Thanks. I post the chk_alloc in the following. It was written by
someone else. Could you give me explanation in more detail?
The routine is crashed when it is re-enter at:

d = distrib_create(tstr);

in the function: *distrib_create


It puzzles me that it works in Linux and Cygwin without the problem.



.............
typedef struct distrib_entry
{ int num; /* A positive number */
double prop; /* Proportion for this number */
} distrib_entry;

typedef struct distrib
{ struct distrib_entry *list; /* The list of numbers and proportions
*/
int size; /* Number of entries in the list */
} distrib;



.............
/* ALLOCATE SPACE AND CHECK FOR ERROR. Calls 'calloc' to allocate
space,
and then displays an error message and exits if the space couldn't
be
found. */

void *chk_alloc
( unsigned n, /* Number of elements */
unsigned size /* Size of each element */
)
{
void *p;

p = calloc(n,size);

if (p==0)
{ fprintf(stderr,"Ran out of memory (while trying to allocate %d
bytes)\n",
n*size);
exit(1);
}

return p;
}

 

[Ian Collins]

Thanks. I post the chk_alloc in the following. It was written by
someone else. Could you give me explanation in more detail?
The routine is crashed when it is re-enter at:

d = distrib_create(tstr);

in the function: *distrib_create


It puzzles me that it works in Linux and Cygwin without the problem.

Luck.

Once you over run a buffer, anything can happen.

 

[fl]

Thanks. I post the chk_alloc in the following. It was written by
someone else. Could you give me explanation in more detail?
The routine is crashed when it is re-enter at:

    d = distrib_create(tstr);

in the function: *distrib_create

It puzzles me that it works in Linux and Cygwin without the problem.

............
typedef struct distrib_entry
{ int num;                      /* A positive number */
  double prop;                  /* Proportion for this number */

} distrib_entry;

typedef struct distrib
{ struct distrib_entry *list;   /* The list of numbers and proportions
*/
  int size;                     /* Number of entries in the list */

} distrib;

............
/* ALLOCATE SPACE AND CHECK FOR ERROR.  Calls 'calloc' to allocate
space,
   and then displays an error message and exits if the space couldn't
be
   found. */

void *chk_alloc
( unsigned n,           /* Number of elements */
  unsigned size         /* Size of each element */
)
{
  void *p;

  p = calloc(n,size);

  if (p==0)
  { fprintf(stderr,"Ran out of memory (while trying to allocate %d
bytes)\n",
      n*size);
    exit(1);
  }

  return p;



}- Masquer le texte des messages précédents -

- Afficher le texte des messages précédents -

Before the *distrib_create function, there is a paragraph of comment,
see below please. This program processes a large array. I suspect the
author tried to same memory when allocates mem. Thanks.





...............
/* CREATE A DISTRIBUTION AS SPECIFIED IN A STRING. Space for the
distribution
is allocated; the string is not freed.

The string must consist either of a single positive integer,
representing
the distribution over just that number, or have a form such as the
following:

5x2/3.5x1/1.5x4

This specifies a distribution over 3 numbers, 2, 1, and 4,
specified by
the second number in each pair, with proportions of 0.5, 0.35, and
0.15,
respectively, specified by the first number in each pair. The
actual
proportions are found by dividing the first number in each pair by
the sum
of these numbers.

The distrib type represents the distribution list. It stores a
pointer to
an array of distrib_entry elements along with the length of this
array.
Each distrib_entry contains a (number,proportion) pair.
*/
distrib *distrib_create
( char *c /* String describing distribution over numbers */
)
{
distrib *d;
char *str, *tstr;
int i, n, scan_num, size;
unsigned temp0, temp1;
double prop, sum;
double d_tmp;
char junk;

 

[Ben Bacarisse]

fl wrote:
[...]

if (sscanf(c,"%d%c",&n,&junk)==1&& n>0)
{ tstr = chk_alloc ( (int)(2.1+log10(n)), sizeof(*str)); [...]
d = distrib_create(tstr);
free(tstr);
return d;
}

I think that if (sscanf(c, "%d%c", &n, &junk) == 1 && n > 0) at each
call, then the memory regions you recursively allocate will never be
freed. The program never performs the call to free(tstr) in this case.
But it's hard to say if this is the real (and only) reason of your
heap corruption. You should also check a condition for which the
recursion must finish so your function can plain return.

From what's there it seems the recursion ends right away. The newly
written string (starting "1x...") will make the scanf at the top return
2.

 

[J. J. Farrell]

Thanks. I post the chk_alloc in the following. It was written by
someone else. Could you give me explanation in more detail?

That was an extremely detailed explanation. Ike Naar's assumption about
chk_alloc() was correct. One thing he didn't explain is that the
expression sizeof(*str) always has the value 1 since you define str as
type <char *>. Perhaps you knew that, or meant something else here, but
it's not at all obvious why you'd use 'sizeof(*str)' since str doesn't
seem to be relevant to this bit of code or used elsewhere. What else do
you need explained?

It puzzles me that it works in Linux and Cygwin without the problem.

Luck. You're corrupting memory, anything can happen including everything
apparently behaving correctly.