From 1153e583667e48c638f7ba5715ffd92d6be29b9f Mon Sep 17 00:00:00 2001 From: Olaf Barthel Date: Sun, 27 Nov 2005 09:26:55 +0000 Subject: [PATCH] - In libunix.a malloc(), calloc() and realloc() no longer treat a request to allocate 0 bytes as an error, returning NULL. They all return a pointer sized memory chunk (= four bytes) initialized to NULL (= 0) instead. - The alloca() implementation which allocates memory from the system rather than the local stack frame is thread-safe now. It also interacts with the realloc(), calloc(), free() and malloc() functions in that the alloca() cleanup routine is called once alloca() has done its job. If all the memory allocated through alloca() has been released no further calls to the cleanup function will be made. - In the thread-safe library, realloc() permitted two different overlapping calls to succeed in trying to reallocate the same chunk of memory due to a race condition. Fixed. git-svn-id: file:///Users/olsen/Code/migration-svn-zu-git/logical-line-staging/clib2/trunk@15070 87f5fb63-7c3d-0410-a384-fd976d0f7a62 --- library/GNUmakefile.68k | 3 ++- library/GNUmakefile.os4 | 3 ++- library/changes | 14 +++++++++++- library/smakefile | 3 ++- library/stdlib_alloca.c | 47 +++++++++++++++++++++++++++------------- library/stdlib_free.c | 28 ++++++++++-------------- library/stdlib_headers.h | 6 ++++- library/stdlib_malloc.c | 19 +++++++++------- library/stdlib_memory.h | 5 +---- library/stdlib_protos.h | 7 ++---- library/stdlib_realloc.c | 9 +++++++- 11 files changed, 90 insertions(+), 54 deletions(-) diff --git a/library/GNUmakefile.68k b/library/GNUmakefile.68k index f1651e0..d91bb53 100644 --- a/library/GNUmakefile.68k +++ b/library/GNUmakefile.68k @@ -1,5 +1,5 @@ # -# $Id: GNUmakefile.68k,v 1.75 2005-11-20 17:00:22 obarthel Exp $ +# $Id: GNUmakefile.68k,v 1.76 2005-11-27 09:26:55 obarthel Exp $ # # :ts=8 # @@ -283,6 +283,7 @@ C_LIB = \ stdlib_abort.o \ stdlib_abs.o \ stdlib_alloca.o \ + stdlib_alloca_cleanup.o \ stdlib_assertion_failure.o \ stdlib_atexit.o \ stdlib_atoi.o \ diff --git a/library/GNUmakefile.os4 b/library/GNUmakefile.os4 index 1c6cdae..5aa522a 100644 --- a/library/GNUmakefile.os4 +++ b/library/GNUmakefile.os4 @@ -1,5 +1,5 @@ # -# $Id: GNUmakefile.os4,v 1.87 2005-11-20 17:00:22 obarthel Exp $ +# $Id: GNUmakefile.os4,v 1.88 2005-11-27 09:26:55 obarthel Exp $ # # :ts=8 # @@ -317,6 +317,7 @@ C_LIB = \ stdlib_abort.o \ stdlib_abs.o \ stdlib_alloca.o \ + stdlib_alloca_cleanup.o \ stdlib_assertion_failure.o \ stdlib_atexit.o \ stdlib_atoi.o \ diff --git a/library/changes b/library/changes index e962ddb..3ce18bb 100644 --- a/library/changes +++ b/library/changes @@ -8,7 +8,19 @@ - In libunix.a malloc(), calloc() and realloc() no longer treat a request to allocate 0 bytes as an error, returning NULL. They all - return a 1 byte memory chunk initialized to 0 instead. + return a pointer sized memory chunk (= four bytes) initialized to + NULL (= 0) instead. + +- The alloca() implementation which allocates memory from the system + rather than the local stack frame is thread-safe now. It also + interacts with the realloc(), calloc(), free() and malloc() functions + in that the alloca() cleanup routine is called once alloca() has + done its job. If all the memory allocated through alloca() has been + released no further calls to the cleanup function will be made. + +- In the thread-safe library, realloc() permitted two different overlapping + calls to succeed in trying to reallocate the same chunk of memory due to + a race condition. Fixed. c.lib 1.197 (4.11.2005) diff --git a/library/smakefile b/library/smakefile index 665c95c..8f7c3d5 100644 --- a/library/smakefile +++ b/library/smakefile @@ -1,5 +1,5 @@ # -# $Id: smakefile,v 1.56 2005-11-19 17:11:22 obarthel Exp $ +# $Id: smakefile,v 1.57 2005-11-27 09:26:55 obarthel Exp $ # # :ts=8 # @@ -482,6 +482,7 @@ STDLIB_OBJ = \ stdlib_abort.o \ stdlib_abs.o \ stdlib_alloca.o \ + stdlib_alloca_cleanup.o \ stdlib_assertion_failure.o \ stdlib_atexit.o \ stdlib_atof.o \ diff --git a/library/stdlib_alloca.c b/library/stdlib_alloca.c index 73fb9ac..b13373f 100644 --- a/library/stdlib_alloca.c +++ b/library/stdlib_alloca.c @@ -1,5 +1,5 @@ /* - * $Id: stdlib_alloca.c,v 1.7 2005-11-20 17:00:22 obarthel Exp $ + * $Id: stdlib_alloca.c,v 1.8 2005-11-27 09:26:55 obarthel Exp $ * * :ts=4 * @@ -55,16 +55,17 @@ extern void * alloca(size_t size); /****************************************************************************/ -struct MemoryContextNode -{ - struct MinNode mcn_MinNode; - void * mcn_StackPointer; - void * mcn_Memory; -}; +static struct MinList alloca_memory_list; /****************************************************************************/ -static struct MinList alloca_memory_list; +struct MemoryContextNode +{ + struct MinNode mcn_MinNode; /* The usual linkage. */ + void * mcn_StackPointer; /* Points to stack frame this allocation + is associated with. */ + void * mcn_Memory; /* Points to the memory allocated. */ +}; /****************************************************************************/ @@ -80,9 +81,14 @@ CLIB_DESTRUCTOR(alloca_exit) /****************************************************************************/ -void -__alloca_cleanup(const char * file,int line) +/* Cleans up after all alloca() allocations that have been made so far. */ +static void +alloca_cleanup(const char * file,int line) { + void * stack_pointer = __get_sp(); + + __memory_lock(); + /* Initialize this if it hasn't been taken care of yet. */ if(alloca_memory_list.mlh_Head == NULL) NewList((struct List *)&alloca_memory_list); @@ -90,7 +96,6 @@ __alloca_cleanup(const char * file,int line) /* Is this worth cleaning up? */ if(NOT IsListEmpty((struct List *)&alloca_memory_list)) { - void * stack_pointer = __get_sp(); struct MemoryContextNode * mcn_prev; struct MemoryContextNode * mcn; @@ -109,10 +114,17 @@ __alloca_cleanup(const char * file,int line) Remove((struct Node *)mcn); - __force_free(mcn->mcn_Memory,file,line); - __free(mcn,file,line); + __free_memory(mcn->mcn_Memory,TRUE,file,line); + __free_memory(mcn,FALSE,file,line); } + + /* Drop the cleanup callback if there's nothing to be cleaned + up any more. */ + if(IsListEmpty((struct List *)&alloca_memory_list)) + __alloca_cleanup = NULL; } + + __memory_unlock(); } /****************************************************************************/ @@ -124,9 +136,12 @@ __alloca(size_t size,const char * file,int line) struct MemoryContextNode * mcn; void * result = NULL; - __alloca_cleanup(file,line); + __memory_lock(); - mcn = __malloc(sizeof(*mcn),file,line); + __alloca_cleanup = alloca_cleanup; + (*__alloca_cleanup)(file,line); + + mcn = __allocate_memory(sizeof(*mcn),FALSE,file,line); if(mcn == NULL) { SHOWMSG("not enough memory"); @@ -153,6 +168,8 @@ __alloca(size_t size,const char * file,int line) out: + __memory_unlock(); + return(result); } diff --git a/library/stdlib_free.c b/library/stdlib_free.c index 675262b..b56c978 100644 --- a/library/stdlib_free.c +++ b/library/stdlib_free.c @@ -1,5 +1,5 @@ /* - * $Id: stdlib_free.c,v 1.10 2005-03-20 11:18:06 obarthel Exp $ + * $Id: stdlib_free.c,v 1.11 2005-11-27 09:26:55 obarthel Exp $ * * :ts=4 * @@ -446,16 +446,13 @@ __free_memory_node(struct MemoryNode * mn,const char * UNUSED file,int UNUSED li /****************************************************************************/ -STATIC VOID -free_memory(void * ptr,BOOL force,const char * file,int line) +VOID +__free_memory(void * ptr,BOOL force,const char * file,int line) { struct MemoryNode * mn; assert(ptr != NULL); - /* Try to get rid of now unused memory. */ - /*__alloca_cleanup(file,line);*/ - #ifdef __MEM_DEBUG { /*if((rand() % 16) == 0) @@ -499,20 +496,19 @@ free_memory(void * ptr,BOOL force,const char * file,int line) /****************************************************************************/ -void -__force_free(void * ptr,const char * file,int line) -{ - if(ptr != NULL) - free_memory(ptr,TRUE,file,line); -} - -/****************************************************************************/ - __static void __free(void * ptr,const char * file,int line) { + __memory_lock(); + + /* Try to get rid of now unused memory. */ + if(__alloca_cleanup != NULL) + (*__alloca_cleanup)(file,line); + + __memory_unlock(); + if(ptr != NULL) - free_memory(ptr,FALSE,file,line); + __free_memory(ptr,FALSE,file,line); } /****************************************************************************/ diff --git a/library/stdlib_headers.h b/library/stdlib_headers.h index 17e5c97..7d439cb 100644 --- a/library/stdlib_headers.h +++ b/library/stdlib_headers.h @@ -1,5 +1,5 @@ /* - * $Id: stdlib_headers.h,v 1.18 2005-07-03 10:36:47 obarthel Exp $ + * $Id: stdlib_headers.h,v 1.19 2005-11-27 09:26:55 obarthel Exp $ * * :ts=4 * @@ -154,6 +154,10 @@ extern BOOL NOCOMMON __exit_blocked; /****************************************************************************/ +extern void NOCOMMON (*__alloca_cleanup)(const char * file,int line); + +/****************************************************************************/ + extern unsigned int NOCOMMON (* __get_default_stack_size)(void); /****************************************************************************/ diff --git a/library/stdlib_malloc.c b/library/stdlib_malloc.c index 613a995..f8fe21b 100644 --- a/library/stdlib_malloc.c +++ b/library/stdlib_malloc.c @@ -1,5 +1,5 @@ /* - * $Id: stdlib_malloc.c,v 1.16 2005-11-20 19:04:10 obarthel Exp $ + * $Id: stdlib_malloc.c,v 1.17 2005-11-27 09:26:55 obarthel Exp $ * * :ts=4 * @@ -106,9 +106,9 @@ __allocate_memory(size_t size,BOOL never_free,const char * UNUSED unused_file,in original_size = size; /* The libunix.a flavour accepts zero length memory allocations - and quietly turns them into 1 byte allocations. */ + and quietly turns them into a pointer sized allocations. */ if(size == 0) - size++; + size = sizeof(char *); } #endif /* UNIX_PATH_SEMANTICS */ @@ -210,11 +210,9 @@ __allocate_memory(size_t size,BOOL never_free,const char * UNUSED unused_file,in #if defined(UNIX_PATH_SEMANTICS) { - /* Zero length memory allocations in libunix.a quietly - return one byte of memory each, and that byte is - set to '\0'. */ + /* Set the zero length allocation contents to NULL. */ if(original_size == 0) - *(char *)result = '\0'; + *(char **)result = NULL; } #endif /* UNIX_PATH_SEMANTICS */ @@ -245,8 +243,13 @@ __malloc(size_t size,const char * file,int line) { void * result = NULL; + __memory_lock(); + /* Try to get rid of now unused memory. */ - /*__alloca_cleanup(file,line);*/ + if(__alloca_cleanup != NULL) + (*__alloca_cleanup)(file,line); + + __memory_unlock(); #ifdef __MEM_DEBUG { diff --git a/library/stdlib_memory.h b/library/stdlib_memory.h index 9146c9d..42f0710 100644 --- a/library/stdlib_memory.h +++ b/library/stdlib_memory.h @@ -1,5 +1,5 @@ /* - * $Id: stdlib_memory.h,v 1.2 2005-03-20 11:18:06 obarthel Exp $ + * $Id: stdlib_memory.h,v 1.3 2005-11-27 09:26:55 obarthel Exp $ * * :ts=4 * @@ -114,11 +114,8 @@ #define __static -#define __alloca_cleanup __alloca_cleanup_debug - #define __find_memory_node __find_memory_node_debug #define __free_memory_node __free_memory_node_debug -#define __force_free __force_free_debug #define __get_allocation_size __get_allocation_size_debug #define __allocate_memory __allocate_memory_debug diff --git a/library/stdlib_protos.h b/library/stdlib_protos.h index 5f5815c..bcd9e03 100644 --- a/library/stdlib_protos.h +++ b/library/stdlib_protos.h @@ -1,5 +1,5 @@ /* - * $Id: stdlib_protos.h,v 1.15 2005-07-03 10:36:47 obarthel Exp $ + * $Id: stdlib_protos.h,v 1.16 2005-11-27 09:26:55 obarthel Exp $ * * :ts=4 * @@ -139,13 +139,10 @@ extern void * __allocate_memory(size_t size,BOOL never_free,const char * file,in /* stdlib_free.c */ extern struct MemoryNode * __find_memory_node(void * address); -extern void __force_free(void * ptr,const char * file,int line); extern void __check_memory_allocations(const char * file,int line); +extern void __free_memory(void * ptr,BOOL force,const char * file,int line); extern void __free_memory_node(struct MemoryNode * mn,const char * file,int line); -/* stdlib_alloca.c */ -extern void __alloca_cleanup(const char * file,int line); - /****************************************************************************/ /* signal_checkabort.c */ diff --git a/library/stdlib_realloc.c b/library/stdlib_realloc.c index 93e470f..62fd309 100644 --- a/library/stdlib_realloc.c +++ b/library/stdlib_realloc.c @@ -1,5 +1,5 @@ /* - * $Id: stdlib_realloc.c,v 1.7 2005-11-20 17:00:22 obarthel Exp $ + * $Id: stdlib_realloc.c,v 1.8 2005-11-27 09:26:55 obarthel Exp $ * * :ts=4 * @@ -51,6 +51,7 @@ __static void * __realloc(void *ptr,size_t size,const char * file,int line) { void * result = NULL; + BOOL locked = FALSE; ENTER(); @@ -80,6 +81,9 @@ __realloc(void *ptr,size_t size,const char * file,int line) assert( ptr != NULL ); + __memory_lock(); + locked = TRUE; + /* Try to find the allocation in the list. */ mn = __find_memory_node(ptr); @@ -182,6 +186,9 @@ __realloc(void *ptr,size_t size,const char * file,int line) out: + if(locked) + __memory_unlock(); + if(result == NULL) SHOWMSG("ouch! realloc failed");