gordon@qfagus.OZ (Peter Gordon) (02/27/90)
Please tell me I'm doing something stupid. The following code works on
Microsoft C and a MIPS/1000 runnung UNIX Sys V Release 4_0.
On Turbo C (ver 2) it crashes after a variable number of frees. I've tried
all memory models and far pointers and can't think of much else to
try. Commonsense tells me that such a basic bug would not be
in Version 2, would some kind person point me in the right direction.
(I've tried the code on 8088, and 80386 machines, both with and without
floating point emulation.)
Peter Gordon.
------------------- cut here ------------------------------
echo "Extracting tst.c"
sed -e 's/^X//' > tst.c << END_OF_FILE
X#include <stdio.h>
X#ifdef __TURBOC__
X#include <alloc.h>
X#else
X#include <malloc.h>
X#endif
X#include <stdlib.h>
Xmain()
X{
X char **head, **cp;
X int i;
X head = (char **)malloc(200 * sizeof(char **));
X for(cp = head, i = 0; i < 200; ++i, ++cp)
X {
X fprintf(stdout,"Freeing %d\n", i);
X fflush(stdout);
X free(cp);
X }
X}
END_OF_FILEdfoster@jarthur.Claremont.EDU (Derek R. Foster) (02/28/90)
In article <26316@qfagus.OZ> gordon@qfagus.OZ (Peter Gordon) writes: > >Please tell me I'm doing something stupid. The following code works on >Microsoft C and a MIPS/1000 runnung UNIX Sys V Release 4_0. >On Turbo C (ver 2) it crashes after a variable number of frees. I've tried >------------------- cut here ------------------------------ >X char **head, **cp; >X int i; >X head = (char **)malloc(200 * sizeof(char **)); Wait a minute. How can you have a pointer to a pointer to char aimed at an array of two hundred pointers to pointers to char? This is a type clash. If it is a pointer to a pointer to char, it should point (surprisingly enough) to one, or a block of, pointers to char. shouldn't it be head = (char **) malloc(200 * sizeof(char *)); or if the array IS supposed to be of char **'s, char ***head, ***cp; head = (char ***) malloc(200 * sizeof(char **)); ? >X for(cp = head, i = 0; i < 200; ++i, ++cp) >X { >X fprintf(stdout,"Freeing %d\n", i); >X fflush(stdout); >X free(cp); >X } Think about what you're doing. You've created ONE array of 200 pointers to char pointers, and assigned head to point to the address of it. Then you (200 times) try to free what cp points to (it starts out as head), then increment cp. The first time it goes through the loop, it frees your ENTIRE array of pointers. Every other time, you're not passing 'free' the address of the start of a malloc'ed block of memory. Instead, you're giving it a pointer into the MIDDLE of a block of memory (which has already been freed anyway). If you've only malloc'ed one thing, why are you trying to free more than one thing? Mallocs and frees should be on a one-to-one correspondance. Also, you should never pass an address to 'free' that wasn't returned DIRECTLY, WITHOUT BEING INCREMENTED OR MODIFIED IN ANY WAY, by malloc (or some other allocation routine). Remember, when you 'malloc' a block of memory, the computer considers it ONE BLOCK. You can't dispose of it piecemeal (other than with realloc, etc.) I think I may see what you are trying to do here. I may be wrong, but I would guess that head is supposed to point to an array of pointers (should these be char pointers instead of pointers to char pointers?) and that those pointers point to other chunks of memory which are what you are really trying to free. (for instance, a dynamically allocated array of strings). If this is the case, you are attempting to free WHAT EACH OF THOSE POINTERS POINTS TO. Ignoring the fact that the above program never assigns any memory for those pointers to point to (in fact, does not initialize them at all), there is another mistake in it. You are trying to free the memory that cp points to, when in fact what you want to do is free the memory that what cp POINTS TO points to. In other words, instead of free(cp), use free(*cp). Of course, after you've done all that, your original array of pointers will still have to be freed (via free(head)). Of course, I could be completely wrong about the intent of this code. If, in fact, all you are really trying to do is deallocate the space assigned to your array of head, just free(head). No loop is necessary. Hope this helps! Derek Riippa Foster
CMH117@psuvm.psu.edu (Charles Hannum) (02/28/90)
In article <26316@qfagus.OZ>, gordon@qfagus.OZ (Peter Gordon) says: > >Please tell me I'm doing something stupid. The following code works on Okay. You're doing something really stupid. {B-) JOKE B-) JOKE B-) JOKE} >Microsoft C and a MIPS/1000 runnung UNIX Sys V Release 4_0. It shouldn't. You were lucky. >On Turbo C (ver 2) it crashes after a variable number of frees. I've tried >all memory models and far pointers and can't think of much else to >try. Commonsense tells me that such a basic bug would not be >in Version 2, would some kind person point me in the right direction. Okay, I'll give you a few pointers. ;-) >#include <stdio.h> >#ifdef __TURBOC__ >#include <alloc.h> >#else >#include <malloc.h> >#endif >#include <stdlib.h> >main() >{ > char **head, **cp; ~~~~~~~ A "char **" is a "pointer to a char *" (or, to make Chris Torek cringe, a "pointer to an array??(SIZE_UNKNOWN??) of char *'s) (Note: Excuse the ASCII tryglyphs; I'm using an IBM 3179G terminal right now.) > int i; > head = (char **)malloc(200 * sizeof(char **)); ~~~~~~~ Thus, you really want to allocate space for 200 "char *"s; but this is not why it crashes. (The pointers are the same size, anyway.) > for(cp = head, i = 0; i < 200; ++i, ++cp) > { > fprintf(stdout,"Freeing %d\n", i); > fflush(stdout); > free(cp); I almost cried when I saw this. You are trying to free the space for the 200 "char *"s independently. You can't do that; you allocated them as one block, and you must free them as one block. Please, PLEASE, read K&R2, or the Turbo C user's guide!! > } >} As a side note: No insult intended against you, but this is a prime example of why we need to teach people a programming language before we expect them to use it. The concept of pointers is so basic to the C language that it seems impossible that you've had any formal education in the language. Is it that you've never had the opportunity, never had the time, or never had the need before? And have you considered taking a class (assuming one is available)? Virtually, - Charles Martin Hannum II "Klein bottle for sale ... inquire within." (That's Charles to you!) "To life immortal!" cmh117@psuvm.{bitnet,psu.edu} "No noozzzz izzz netzzzsnoozzzzz..." c9h@psuecl.{bitnet,psu.edu} "Mem'ry, all alone in the moonlight ..."
lmiller@aerospace.aero.org (Lawrence H. Miller) (02/28/90)
In article <26316@qfagus.OZ> gordon@qfagus.OZ (Peter Gordon) writes: > >Asks why the following code produces anomalous results with Turb C. (various header files removed for clarity) main() { char **head, **cp; int i; head = (char **)malloc(200 * sizeof(char **)); for(cp = head, i = 0; i < 200; ++i, ++cp) { fprintf(stdout,"Freeing %d\n", i); fflush(stdout); free(cp); } } From a VERY early draft of the ANSI C standard: void free(void *ptr) ...if the argument does not match a pointer earlier returned by the calloc, malloc, or realloc function, or if the space has been deallocated by a call to free or realloc, the behavior is undefined. Your first call to free frees all the allocated space. Subsequent calls to free using a pointer into freed space, produces undefined behavior. Here is the definition of "Undefined behavior": Behavior for an erroneous program construct, for which the standard imposes no requirements. Permissible undefined behaavior ranges from ignoring the situation completely with unpredictable results...to terminating a[n]...execution with a diagnostic message. Larry Miller Aerospace Corporation lmiller@aerospace.aero.org 213-336-5597
cpcahil@virtech.uucp (Conor P. Cahill) (02/28/90)
In article <26316@qfagus.OZ> gordon@qfagus.OZ (Peter Gordon) writes: >X head = (char **)malloc(200 * sizeof(char **)); >X for(cp = head, i = 0; i < 200; ++i, ++cp) >X { >X fprintf(stdout,"Freeing %d\n", i); >X fflush(stdout); >X free(cp); >X } The problem is that you can only call free with a pointer that is returned by malloc. In this code the only valid call to free is free(head) or free(cp) for the first loop. Due to a small amount of malloc chain info, most of your calls to free are ignored, but you hit just the right circumstances to cause a problem once in a while. -- +-----------------------------------------------------------------------+ | Conor P. Cahill uunet!virtech!cpcahil 703-430-9247 ! | Virtual Technologies Inc., P. O. Box 876, Sterling, VA 22170 | +-----------------------------------------------------------------------+
pdsmith@bbn.com (Peter D. Smith) (02/28/90)
In article <90058.153755CMH117@psuvm.psu.edu> CMH117@psuvm.psu.edu (Charles Hannum) writes: > >As a side note: No insult intended against you, but this is a prime example >of why we need to teach people a programming language before we expect them >to use it. The concept of pointers is so basic to the C language that it seems >impossible that you've had any formal education in the language. Is it that >you've never had the opportunity, never had the time, or never had the need >before? And have you considered taking a class (assuming one is available)? > C'mon lay off the guy. Malloc and Free are tough to get right. A story: at a previous job, we built a run time allocation checker (bounds checking, freeing the same memory twice, freeing a pointer after incrementing it, not freeing memory, allocation statistics... the whole ball of wax). **Every single library** by **every single C engineer** had problems! Most implementations of C are screwed up: they give you a dangerous tool but don't give you a handle on double-checking the results. Peter D. Smith
darcy@druid.uucp (D'Arcy J.M. Cain) (02/28/90)
In article <26316@qfagus.OZ> gordon@qfagus.OZ (Peter Gordon) writes: > >Please tell me I'm doing something stupid. OK. You're doing something stupid. :-) >#include <stdio.h> >#ifdef __TURBOC__ >#include <alloc.h> >#else >#include <malloc.h> >#endif >#include <stdlib.h> >main() >{ > char **head, **cp; > int i; > head = (char **)malloc(200 * sizeof(char **)); > for(cp = head, i = 0; i < 200; ++i, ++cp) > { > fprintf(stdout,"Freeing %d\n", i); > fflush(stdout); > free(cp); > } >} The first call to free(cp) frees up the entire memory allocated by malloc. The functions malloc and free don't have any idea what the allocated space is used for and don't know what size the items are. The value returned by malloc is a pointer and the argument to free is a previously returned pointer. For 199 times in the above code you are freeing pointers that haven't been allocated. in malloc(3C) this is stated to produce undefined results so you got what was promised. Also, even given what you were trying to do, didn't you mean 'cp++' in the for loop statement, not '++cp'? -- D'Arcy J.M. Cain (darcy@druid) | Thank goodness we don't get all D'Arcy Cain Consulting | the government we pay for. West Hill, Ontario, Canada | (416) 281-6094 |
richard@calvin.spp.cornell.edu (Richard Brittain) (03/02/90)
In article <52749@bbn.COM> pdsmith@spca.bbn.com (Peter D. Smith) writes: > >C'mon lay off the guy. Malloc and Free are tough to get right. A story: >at a previous job, we built a run time allocation checker (bounds checking, >freeing the same memory twice, freeing a pointer after incrementing it, not >freeing memory, allocation statistics... the whole ball of wax). **Every >single library** by **every single C engineer** had problems! > >Most implementations of C are screwed up: they give you a dangerous tool >but don't give you a handle on double-checking the results. > > Peter D. Smith As a side note to this side note, I'll just mention that with Turbo-C, the chances of bombing out if you try to use something from a free'd block are VERY high, since the free() routine seems to modify the contents of the block immediately. I have found cases of code ported from various unix compilers, and from Microsoft C, which had been doing this succesfully and showed up the error right away with TurboC. In one case the original author told me the bug had been there over 3 years of development, but no-one noticed on the other systems. On a pc, you have even more incentive to get it right, since getting it wrong often means reaching for the red button. Richard Brittain, School of Elect. Eng., Upson Hall Cornell University, Ithaca, NY 14853 ARPA: richard@calvin.spp.cornell.edu UUCP: {uunet,uw-beaver,rochester,cmcl2}!cornell!calvin!richard
CMH117@psuvm.psu.edu (Charles Hannum) (03/02/90)
In article <52749@bbn.COM>, pdsmith@bbn.com (Peter D. Smith) says: > >Most implementations of C are screwed up: they give you a dangerous tool >but don't give you a handle on double-checking the results. Too much error-checking slows things down. The assumption in C is that the programmer *knows* what {s}he is doing. With power come responsibility ... Virtually, - Charles Martin Hannum II "Klein bottle for sale ... inquire within." (That's Charles to you!) "To life immortal!" cmh117@psuvm.{bitnet,psu.edu} "No noozzzz izzz netzzzsnoozzzzz..." c9h@psuecl.{bitnet,psu.edu} "Mem'ry, all alone in the moonlight ..."