From 16b24ed9a877ffb671286ed7f50a172118b9179d Mon Sep 17 00:00:00 2001 From: Volodymyr Shymanskyy Date: Sat, 20 Mar 2021 02:27:46 +0200 Subject: [PATCH] Review memory allocation. Fix #208 --- source/m3_bind.c | 4 +- source/m3_code.c | 4 +- source/m3_compile.c | 13 +++--- source/m3_core.c | 105 +++++++++++++++++++------------------------- source/m3_core.h | 26 +++++------ source/m3_env.c | 47 ++++++++++---------- source/m3_module.c | 22 +++++----- source/m3_parse.c | 21 ++++----- 8 files changed, 118 insertions(+), 124 deletions(-) diff --git a/source/m3_bind.c b/source/m3_bind.c index 86ea9f5..acd4e1c 100644 --- a/source/m3_bind.c +++ b/source/m3_bind.c @@ -86,7 +86,7 @@ _ (AllocFuncType (& funcType, umaxNumArgs)); } _catch: if (result) - m3Free (funcType); // nulls funcType + m3_Free (funcType); * o_functionType = funcType; @@ -112,7 +112,7 @@ _ (SignatureToFuncType (& ftype, i_linkingSignature)); _catch: - m3Free (ftype); + m3_Free (ftype); return result; } diff --git a/source/m3_code.c b/source/m3_code.c index f625caa..37569b3 100644 --- a/source/m3_code.c +++ b/source/m3_code.c @@ -26,7 +26,7 @@ IM3CodePage NewCodePage (u32 i_minNumLines) u32 pageSize = sizeof (M3CodePageHeader) + sizeof (code_t) * i_minNumLines; pageSize = (pageSize + (d_m3CodePageAlignSize-1)) & ~(d_m3CodePageAlignSize-1); // align - m3Alloc ((void **) & page, u8, pageSize); + page = (IM3CodePage)m3_Malloc (pageSize); if (page) { @@ -62,7 +62,7 @@ void FreeCodePages (IM3CodePage * io_list) #if d_m3RecordBacktraces FreeCodeMappingPage (page->info.mapping); #endif // d_m3RecordBacktraces - m3Free (page); + m3_Free (page); page = next; } diff --git a/source/m3_compile.c b/source/m3_compile.c index d566168..5d1d4fc 100644 --- a/source/m3_compile.c +++ b/source/m3_compile.c @@ -687,8 +687,10 @@ M3Result AcquirePatch (IM3Compilation o, IM3BranchPatch * o_patch) o->releasedPatches = patch->next; patch->next = NULL; } - else -_ (m3Alloc (& patch, M3BranchPatch, 1)); + else { + patch = m3_AllocStruct(M3BranchPatch); + _throwifnull(patch); + } * o_patch = patch; @@ -1239,7 +1241,7 @@ _ (EmitOp (o, op)); if (IsValidSlot (valueSlot)) EmitSlotOffset (o, valueSlot); - IM3BranchPatch patch; + IM3BranchPatch patch = NULL; _ (AcquirePatch (o, & patch)); patch->location = (pc_t *) ReservePointer (o); @@ -1308,7 +1310,7 @@ _ (EmitOp (o, op_ContinueLoop)); } else { - IM3BranchPatch patch; + IM3BranchPatch patch = NULL; _ (AcquirePatch (o, & patch)); patch->location = (pc_t *) ReservePointer (o); @@ -2413,7 +2415,8 @@ _ (Compile_BlockStatements (o)); if (numConstantSlots) { -_ (m3CopyMem (& io_function->constants, o->constants, io_function->numConstantBytes)); + io_function->constants = m3_CopyMem(o->constants, io_function->numConstantBytes); + _throwifnull(io_function->constants); } } _catch: diff --git a/source/m3_core.c b/source/m3_core.c index 1b6bf84..8c781e2 100644 --- a/source/m3_core.c +++ b/source/m3_core.c @@ -37,7 +37,7 @@ static u8* fixedHeapLast = NULL; # define HEAP_ALIGN_PTR(P) #endif -M3Result m3_Malloc (void ** o_ptr, size_t i_size) +void * m3_Malloc (size_t i_size) { u8 * ptr = fixedHeapPtr; @@ -46,117 +46,105 @@ M3Result m3_Malloc (void ** o_ptr, size_t i_size) if (fixedHeapPtr >= fixedHeapEnd) { - * o_ptr = NULL; - - return m3Err_mallocFailed; + return NULL; } memset (ptr, 0x0, i_size); - * o_ptr = ptr; fixedHeapLast = ptr; //printf("== alloc %d => %p\n", i_size, ptr); - return m3Err_none; + return ptr; } -void m3_Free (void ** io_ptr) +void m3_FreeImpl (void * i_ptr) { - if (!io_ptr) return; - // Handle the last chunk - if (io_ptr == fixedHeapLast) { + if (i_ptr && i_ptr == fixedHeapLast) { fixedHeapPtr = fixedHeapLast; fixedHeapLast = NULL; //printf("== free %p\n", io_ptr); } else { //printf("== free %p [failed]\n", io_ptr); } - - * io_ptr = NULL; } -M3Result m3_Realloc (void ** io_ptr, size_t i_newSize, size_t i_oldSize) +void * m3_Realloc (void * i_ptr, size_t i_newSize, size_t i_oldSize) { //printf("== realloc %p => %d\n", io_ptr, i_newSize); - void * ptr = *io_ptr; - if (i_newSize == i_oldSize) return m3Err_none; + if (UNLIKELY(i_newSize == i_oldSize)) return i_ptr; + + void * newPtr; // Handle the last chunk - if (ptr && ptr == fixedHeapLast) { + if (i_ptr && i_ptr == fixedHeapLast) { fixedHeapPtr = fixedHeapLast + i_newSize; HEAP_ALIGN_PTR(fixedHeapPtr); - return m3Err_none; + if (fixedHeapPtr >= fixedHeapEnd) + { + return NULL; + } + newPtr = i_ptr; + } else { + newPtr = m3_Malloc(i_newSize); + if (!newPtr) { + return NULL; + } + if (i_ptr) { + memcpy(newPtr, i_ptr, i_oldSize); + } } - M3Result result = m3_Malloc(&ptr, i_newSize); - if (result) return result; - - if (*io_ptr) { - memcpy(ptr, *io_ptr, i_oldSize); + if (i_newSize > i_oldSize) { + memset ((u8 *) newPtr + i_oldSize, 0x0, i_newSize - i_oldSize); } - *io_ptr = ptr; - return m3Err_none; + return newPtr; } #else -M3Result m3_Malloc (void ** o_ptr, size_t i_size) +void * m3_Malloc (size_t i_size) { - M3Result result = m3Err_none; - void * ptr = calloc (i_size, 1); - if (not ptr) - result = m3Err_mallocFailed; - - * o_ptr = ptr; // printf("== alloc %d => %p\n", (u32) i_size, ptr); - return result; + return ptr; } -void m3_Free (void ** io_ptr) +void m3_FreeImpl (void * io_ptr) { // if (io_ptr) printf("== free %p\n", io_ptr); - free (* io_ptr); - * io_ptr = NULL; + free ((void*)io_ptr); } -M3Result m3_Realloc (void ** io_ptr, size_t i_newSize, size_t i_oldSize) +void * m3_Realloc (void * i_ptr, size_t i_newSize, size_t i_oldSize) { - M3Result result = m3Err_none; + if (UNLIKELY(i_newSize == i_oldSize)) return i_ptr; - if (i_newSize != i_oldSize) - { - void * newPtr = realloc (* io_ptr, i_newSize); + void * newPtr = realloc (i_ptr, i_newSize); - if (newPtr) - { - if (i_newSize > i_oldSize) - memset ((u8 *) newPtr + i_oldSize, 0x0, i_newSize - i_oldSize); - - * io_ptr = newPtr; + if (LIKELY(newPtr)) + { + if (i_newSize > i_oldSize) { + memset ((u8 *) newPtr + i_oldSize, 0x0, i_newSize - i_oldSize); } - else result = m3Err_mallocFailed; - -// printf("== realloc %p -> %p => %d\n", io_ptr, io_ptr, (u32) i_newSize); + return newPtr; } - - return result; + return NULL; } #endif -M3Result m3_CopyMem (void ** o_to, const void * i_from, size_t i_size) +void * m3_CopyMem (const void * i_from, size_t i_size) { - M3Result result = m3_Malloc(o_to, i_size); - if (!result) { - memcpy (*o_to, i_from, i_size); + void * ptr = m3_Malloc(i_size); + if (ptr) { + memcpy (ptr, i_from, i_size); } - return result; + return ptr; } //-------------------------------------------------------------------------------------------- @@ -471,10 +459,9 @@ M3Result Read_utf8 (cstr_t * o_utf8, bytes_t * io_bytes, cbytes_t i_end) if (end <= i_end) { - char * utf8; - result = m3_Malloc ((void **) & utf8, utf8Length + 1); + char * utf8 = (char *)m3_Malloc (utf8Length + 1); - if (not result) + if (utf8) { memcpy (utf8, ptr, utf8Length); utf8 [utf8Length] = 0; diff --git a/source/m3_core.h b/source/m3_core.h index 10b4d01..3cac1a3 100644 --- a/source/m3_core.h +++ b/source/m3_core.h @@ -121,8 +121,8 @@ const void * const cvptr_t; # endif -# if (defined(DEBUG) || defined(ASSERTS)) && !defined(NASSERTS) -# define d_m3Assert(ASS) assert (ASS) +# if (defined(DEBUG)) +# define d_m3Assert(ASS) if (!(ASS)) { printf("Assertion failed at %s:%d : %s\n", __FILE__, __LINE__, #ASS); abort(); } # else # define d_m3Assert(ASS) # endif @@ -202,17 +202,17 @@ int m3StackGetMax (); #define m3StackGetMax() 0 #endif -void m3_Abort (const char* message); -M3Result m3_Malloc (void ** o_ptr, size_t i_size); -M3Result m3_Realloc (void ** io_ptr, size_t i_newSize, size_t i_oldSize); -void m3_Free (void ** io_ptr); -M3Result m3_CopyMem (void ** o_to, const void * i_from, size_t i_size); - -#define m3Alloc(OPTR, STRUCT, NUM) m3_Malloc ((void **) OPTR, sizeof (STRUCT) * (NUM)) -#define m3ReallocArray(PTR, STRUCT, NEW, OLD) m3_Realloc ((void **) (PTR), sizeof (STRUCT) * (NEW), sizeof (STRUCT) * (OLD)) -#define m3Reallocate(_ptr, _newSize, _oldSize) m3_Realloc ((void **) _ptr, _newSize, _oldSize) -#define m3Free(P) m3_Free ((void **)(& P)); -#define m3CopyMem(_to, _from, _size) m3_CopyMem ((void **) _to, (void *) _from, _size) +void m3_Abort (const char* message); +void * m3_Malloc (size_t i_size); +void * m3_Realloc (void *i_ptr, size_t i_newSize, size_t i_oldSize); +void m3_FreeImpl (void * i_ptr); +void * m3_CopyMem (const void * i_from, size_t i_size); + +#define m3_AllocStruct(STRUCT) (STRUCT *)m3_Malloc (sizeof (STRUCT)) +#define m3_AllocArray(STRUCT, NUM) (STRUCT *)m3_Malloc (sizeof (STRUCT) * (NUM)) +#define m3_ReallocArray(STRUCT, PTR, NEW, OLD) (STRUCT *)m3_Realloc ((void *)(PTR), sizeof (STRUCT) * (NEW), sizeof (STRUCT) * (OLD)) +#define m3_Free(P) do { m3_FreeImpl ((void*)(P)); (P) = NULL; } while(0) +#define _throwifnull(PTR) _throwif (m3Err_mallocFailed, !(PTR)) M3Result NormalizeType (u8 * o_type, i8 i_convolutedWasmType); diff --git a/source/m3_env.c b/source/m3_env.c index 6940152..93ccad8 100644 --- a/source/m3_env.c +++ b/source/m3_env.c @@ -16,7 +16,8 @@ M3Result AllocFuncType (IM3FuncType * o_functionType, u32 i_numTypes) { - return m3Alloc (o_functionType, u8, sizeof (M3FuncType) + i_numTypes); + *o_functionType = (IM3FuncType)m3_Malloc (sizeof (M3FuncType) + i_numTypes); + return (*o_functionType) ? m3Err_none : m3Err_mallocFailed; } @@ -39,14 +40,14 @@ void Runtime_ReleaseCodePages (IM3Runtime i_runtime) void Function_Release (IM3Function i_function) { - m3Free (i_function->constants); + m3_Free (i_function->constants); for (int i = 0; i < i_function->numNames; i++) { // name can be an alias of fieldUtf8 if (i_function->names[i] != i_function->import.fieldUtf8) { - m3Free (i_function->names[i]); + m3_Free (i_function->names[i]); } } @@ -169,15 +170,15 @@ u32 GetFunctionNumArgsAndLocals (IM3Function i_function) void FreeImportInfo (M3ImportInfo * i_info) { - m3Free (i_info->moduleUtf8); - m3Free (i_info->fieldUtf8); + m3_Free (i_info->moduleUtf8); + m3_Free (i_info->fieldUtf8); } IM3Environment m3_NewEnvironment () { - IM3Environment env = NULL; - m3Alloc (& env, M3Environment, 1); + IM3Environment env = m3_AllocStruct (M3Environment); + if (!env) return NULL; // create FuncTypes for all simple block return ValueTypes for (int t = c_m3Type_none; t <= c_m3Type_f64; t++) @@ -204,7 +205,7 @@ void Environment_Release (IM3Environment i_environment) while (ftype) { IM3FuncType next = ftype->next; - m3Free (ftype); + m3_Free (ftype); ftype = next; } for (int t = c_m3Type_none; t <= c_m3Type_f64; t++) @@ -212,7 +213,7 @@ void Environment_Release (IM3Environment i_environment) d_m3Assert (t < 5); ftype = i_environment->retFuncTypes[t]; d_m3Assert (ftype->next == NULL); - m3Free (ftype); + m3_Free (ftype); } m3log (runtime, "freeing %d pages from environment", CountCodePages (i_environment->pagesReleased)); FreeCodePages (& i_environment->pagesReleased); @@ -224,7 +225,7 @@ void m3_FreeEnvironment (IM3Environment i_environment) if (i_environment) { Environment_Release (i_environment); - m3Free (i_environment); + m3_Free (i_environment); } } @@ -238,7 +239,7 @@ void Environment_AddFuncType (IM3Environment i_environment, IM3FuncType * io_f { if (AreFuncTypesEqual (newType, addType)) { - m3Free (addType); + m3_Free (addType); break; } @@ -316,8 +317,7 @@ void Environment_ReleaseCodePages (IM3Environment i_environment, IM3CodePage i IM3Runtime m3_NewRuntime (IM3Environment i_environment, u32 i_stackSizeInBytes, void * i_userdata) { - IM3Runtime runtime = NULL; - m3Alloc (& runtime, M3Runtime, 1); + IM3Runtime runtime = m3_AllocStruct (M3Runtime); if (runtime) { @@ -326,13 +326,13 @@ IM3Runtime m3_NewRuntime (IM3Environment i_environment, u32 i_stackSizeInBytes runtime->environment = i_environment; runtime->userdata = i_userdata; - m3Alloc (& runtime->stack, u8, i_stackSizeInBytes); + runtime->stack = m3_Malloc (i_stackSizeInBytes); if (runtime->stack) { runtime->numStackSlots = i_stackSizeInBytes / sizeof (m3slot_t); m3log (runtime, "new stack: %p", runtime->stack); } - else m3Free (runtime); + else m3_Free (runtime); } return runtime; @@ -378,7 +378,7 @@ void FreeCompilationPatches (IM3Compilation o) while (patches) { IM3BranchPatch next = patches->next; - m3Free (patches); + m3_Free (patches); patches = next; } } @@ -393,8 +393,8 @@ void Runtime_Release (IM3Runtime i_runtime) FreeCompilationPatches (& i_runtime->compilation); - m3Free (i_runtime->stack); - m3Free (i_runtime->memory.mallocated); + m3_Free (i_runtime->stack); + m3_Free (i_runtime->memory.mallocated); } @@ -405,7 +405,7 @@ void m3_FreeRuntime (IM3Runtime i_runtime) m3_PrintProfilerInfo (); Runtime_Release (i_runtime); - m3Free (i_runtime); + m3_Free (i_runtime); } } @@ -531,7 +531,10 @@ M3Result ResizeMemory (IM3Runtime io_runtime, u32 i_numPages) if (numPreviousBytes) numPreviousBytes += sizeof (M3MemoryHeader); -_ (m3Reallocate (& memory->mallocated, numBytes, numPreviousBytes)); + void* newMem = m3_Realloc (memory->mallocated, numBytes, numPreviousBytes); + _throwifnull(newMem); + + memory->mallocated = (M3MemoryHeader*)newMem; # if d_m3LogRuntime M3MemoryHeader * oldMallocated = memory->mallocated; @@ -646,8 +649,8 @@ _ (ReadLEB_u32 (& numElements, & bytes, end)); u32 endElement = numElements + offset; _throwif ("table overflow", offset >= endElement); // TODO: check this, endElement depends on offset -_ (m3ReallocArray (& io_module->table0, IM3Function, endElement, io_module->table0Size)); - + io_module->table0 = m3_ReallocArray (IM3Function, io_module->table0, endElement, io_module->table0Size); + _throwifnull(io_module->table0); io_module->table0Size = endElement; for (u32 e = 0; e < numElements; ++e) diff --git a/source/m3_module.c b/source/m3_module.c index 849856f..68a9a1b 100644 --- a/source/m3_module.c +++ b/source/m3_module.c @@ -28,16 +28,16 @@ void m3_FreeModule (IM3Module i_module) Module_FreeFunctions (i_module); - m3Free (i_module->functions); - //m3Free (i_module->imports); - m3Free (i_module->funcTypes); - m3Free (i_module->dataSegments); - m3Free (i_module->table0); + m3_Free (i_module->functions); + //m3_Free (i_module->imports); + m3_Free (i_module->funcTypes); + m3_Free (i_module->dataSegments); + m3_Free (i_module->table0); // TODO: free importinfo - m3Free (i_module->globals); + m3_Free (i_module->globals); - m3Free (i_module); + m3_Free (i_module); } } @@ -47,8 +47,8 @@ M3Result Module_AddGlobal (IM3Module io_module, IM3Global * o_global, u8 i_typ M3Result result = m3Err_none; _try { u32 index = io_module->numGlobals++; -_ (m3ReallocArray (& io_module->globals, M3Global, io_module->numGlobals, index)); - + io_module->globals = m3_ReallocArray (M3Global, io_module->globals, io_module->numGlobals, index); + _throwifnull(io_module->globals); M3Global * global = & io_module->globals [index]; global->type = i_type; @@ -68,8 +68,8 @@ M3Result Module_AddFunction (IM3Module io_module, u32 i_typeIndex, IM3ImportIn M3Result result = m3Err_none; _try { u32 index = io_module->numFunctions++; -_ (m3ReallocArray (& io_module->functions, M3Function, io_module->numFunctions, index)); - + io_module->functions = m3_ReallocArray (M3Function, io_module->functions, io_module->numFunctions, index); + _throwifnull(io_module->functions); _throwif("type sig index out of bounds", i_typeIndex >= io_module->numFuncTypes); IM3FuncType ft = io_module->funcTypes [i_typeIndex]; diff --git a/source/m3_parse.c b/source/m3_parse.c index eb3cda6..347cb5e 100644 --- a/source/m3_parse.c +++ b/source/m3_parse.c @@ -49,7 +49,8 @@ _ (ReadLEB_u32 (& numTypes, & i_bytes, i_end)); if (numTypes) { // table of IM3FuncType (that point to the actual M3FuncType struct in the Environment) -_ (m3Alloc (& io_module->funcTypes, IM3FuncType, numTypes)); + io_module->funcTypes = m3_AllocArray (IM3FuncType, numTypes); + _throwifnull(io_module->funcTypes); io_module->numFuncTypes = numTypes; for (u32 i = 0; i < numTypes; ++i) @@ -107,8 +108,8 @@ _ (NormalizeType (& retType, wasmType)); if (result) { - m3Free (ftype); - m3Free (io_module->funcTypes); + m3_Free (ftype); + m3_Free (io_module->funcTypes); io_module->numFuncTypes = 0; } @@ -237,7 +238,7 @@ _ (ReadLEB_u32 (& index, & i_bytes, i_end)); } } - m3Free (utf8); + m3_Free (utf8); } _catch: return result; @@ -371,8 +372,8 @@ M3Result ParseSection_Data (M3Module * io_module, bytes_t i_bytes, cbytes_t i_ u32 numDataSegments; _ (ReadLEB_u32 (& numDataSegments, & i_bytes, i_end)); m3log (parse, "** Data [%d]", numDataSegments); -_ (m3Alloc (& io_module->dataSegments, M3DataSegment, numDataSegments)); - + io_module->dataSegments = m3_AllocArray (M3DataSegment, numDataSegments); + _throwifnull(io_module->dataSegments); io_module->numDataSegments = numDataSegments; for (u32 i = 0; i < numDataSegments; ++i) @@ -456,7 +457,7 @@ _ (Read_utf8 (& name, & i_bytes, i_end)); if (strcmp (name, "name") != 0) i_bytes = i_end; - m3Free (name); + m3_Free (name); while (i_bytes < i_end) { @@ -489,7 +490,7 @@ _ (Read_utf8 (& name, & i_bytes, i_end)); // else m3log (parse, "prenamed: %s", io_module->functions [index].name); } - m3Free (name); + m3_Free (name); } } @@ -548,8 +549,8 @@ M3Result m3_ParseModule (IM3Environment i_environment, IM3Module * o_module, c IM3Module module; _try { -_ (m3Alloc (& module, M3Module, 1)); - + module = m3_AllocStruct (M3Module); + _throwifnull(module); module->name = ".unnamed"; m3log (parse, "load module: %d bytes", i_numBytes); module->startFunction = -1; //module->hasWasmCodeCopy = false;