What's the problem of my memory management? Because it causes crash that I show in a comment in the code below ("A memory block could not be found when trying to free."). I know that my memory management is not thread safe because I use global variables g_numBlocks and g_blocks that can cause risk when using multiple threads.
Since my memory management code seems too complex, can anyone suggest a stable and better "Memory Management for C " to avoid memory leaks.
The code that contains bug
#include "emc-memory.h" // <-- Declare the functions MALLOC() and FREE() from other library.
#include <vector>
int main() {
printf("HERE(1)\n");
{
std::vector<string> paths = { // <-- Problem, 'std::vector' & 'string' use internal malloc/free & operator new/delete that are overwritten with my own custom memory management.
"/foo/bar.txt",
"/foo/bar.",
"/foo/bar",
"/foo/bar.txt/bar.cc",
"/foo/bar.txt/bar.",
"/foo/bar.txt/bar",
"/foo/.",
"/foo/..",
"/foo/.hidden",
"/foo/..bar",
};
} // <-- It crashes here, error in FREE(): "A memory block could not be found when trying to free.".
printf("HERE(2)\n"); // The reason I know it crashes above is this line is not evaluated, only "HERE(1)" is printed. I'm using [RelWithDebInfo] with blurry debugging info.
return 0;
}
The problem with [GCC 12.1.0 x86_64-w64-mingw32] [RelWithDebInfo]:
When I run the code above with configuration [RelWithDebInfo]
Compilers:
- [Visual Studio 2015] [Debug]: No problem.
- [Visual Studio 2015] [RelWithDebInfo]: No problem.
- [GCC 12.1.0 x86_64-w64-mingw32] [Debug]: No problem.
- [GCC 12.1.0 x86_64-w64-mingw32] [RelWithDebInfo]: Broken which means there's a bug.
In "emc-memory.h" in other library .so file
extern const char* __file;
extern int __line;
#define new (__file = __FILE__, __line = __LINE__, 0) ? 0 : new
enum MEMORYBLOCKTYPE {
MEMORYBLOCKTYPE_MALLOC,
MEMORYBLOCKTYPE_NEW,
};
void *MALLOC(size_t size, MEMORYBLOCKTYPE type);
void *REALLOC(void *block, size_t newSize);
void FREE(void *block, MEMORYBLOCKTYPE type);
#define malloc(size) ((__file = __FILE__, __line = __LINE__, 0) ? 0 : MALLOC(size, MEMORYBLOCKTYPE_MALLOC))
#define realloc(block, newSize) REALLOC(block, newSize)
#define free(block) FREE(block, MEMORYBLOCKTYPE_MALLOC)
In "emc-memory.cpp" in other library .so file
I use this to overwrite the operator new & delete: https://codereview.stackexchange.com/questions/7216/custom-operator-new-and-operator-delete
typedef unsigned long long BlockId; // The reason it's 64-bit is a memory block can be freed and reallocated multiple times, which means that there can be a lot of ids.
BlockId g_blockId = 0;
BlockId newBlockId() {
return g_blockId ;
}
struct Block {
const char *file;
int line;
const char *scope;
char *hint;
size_t size;
BlockId id; // That id is used for comparison because it will never be changed but the block pointer can be changed.
void *block;
MEMORYBLOCKTYPE type;
};
bool g_blocks_initialized = false;
int g_numBlocks;
Block **g_blocks;
void *MALLOC(size_t size, MEMORYBLOCKTYPE type) {
if (g_blocks_initialized == false) {
g_blocks_initialized = true;
_initializeList(g_numBlocks, g_blocks);
}
Block *b = (Block *)malloc(sizeof(*b));
b->file = __file ; __file = nullptr;
b->line = __line ; __line = 0;
b->scope = __scope; __scope = nullptr;
b->hint = allocateMemoryHint(__hint);
b->size = size;
b->id = newBlockId();
b->block = malloc(size);
b->type = type;
_addListItem(g_numBlocks, g_blocks, b);
return b->block;
}
void FREE(void *block, MEMORYBLOCKTYPE type) {
if (block == nullptr) {
return; // 'free' can free a nullptr.
}
for (int i = 0; i < g_numBlocks; i ) {
Block *b = g_blocks[i];
if (b->block == block) {
if (b->type != type) {
switch (type) {
case MEMORYBLOCKTYPE_MALLOC: EMC_ERROR("The memory block type must be MALLOC."); break;
case MEMORYBLOCKTYPE_NEW: EMC_ERROR("The memory block type must be NEW."); break;
default: EMC_ERROR("Error"); break;
}
}
_removeListItem(g_numBlocks, g_blocks, b);
freeMemoryHint(b->hint); b->hint = nullptr;
SAFE_FREE(b->block);
SAFE_FREE(b);
return;
}
}
EMC_ERROR("A memory block could not be found when trying to free.\n\nExamples:\n - Calling free(pointer) where pointer was not set to zero after it's been called twice, the solution was to use SAFE_FREE(). And if possible, replace any free() with SAFE_FREE(). For example, see Lexer::read0() on the original line \"free(out.asIdentifier);\".\n - If an 'Engine' object is destroyed before destroying a Vulkan object then it can cause this error (It can happen with 'Release' or 'RelWithDebInfo' configuration but not with 'Debug' configuration), that problem happened to me and I stuck there for hours until I realized it.");
}
CodePudding user response：
I would humbly suggest that without a very clear reason to think otherwise the best memory management for GCC C is the out-of-the-box default memory management for GCC C .
That would mean your best solution would have been to do nothing or as it is now strip out your overrides of the global operators.
You may find in some area of a system the default memory management is sub-optimal but in 2022 the default options are very effective and if you find a general purpose strategy that is better it's a publishable paper.
However your question tells us nothing about the application in question or your motivations for thinking you should even try to change the memory management let alone give advice on what to.
Sure you can add a global allocation mutex to block memory management and make it thread-safe. I will be surprised if that doesn't turn out to more than throw away whatever advantage you're hoping to gain.
CodePudding user response：
Not sure where SAFE_FREE is coming from. If you see in MALLOC function, they use c runtime malloc. Meaning that if you want to free the block you need to use the corresponding free() function.
Make sure that SAFE_FREE is indeed using the c runtime free with the correct parameters.