views:

682

answers:

8

I am trying to write a custom allocator for debugging purposes (as an exercise) in C, where I will be using a single linked list to hold together the free list of memory using the First Fit Algorithm. I've shown below the structure I would like to create in an "Empty Memory Node".

How do I write the header block (a union to be specific) at the first few bytes of the memory, I obtain (I am using malloc() to initially get a chunk of memory) so that the remaining bytes are free?

This is the union I am using:

/*Define Header Structure for proper alignment*/
union header {
struct{
 union header* next;
 unsigned size ; /*Make it size_t*/
}s; 
double dummy_align_var;
};

-------------------------------------------------------------------------------
|Next        |Size of  |16Byte| User is concerned only about |16Byte|         |
|Free Memory |Allocated|Header| this portion  of memory      |Footer|Checksum |
|Address     |Block    |Picket| and has no knowledge of rest |Picket|         |
-------------------------------------------------------------------------------
|-------Header---------|      ^Address Returned to user
                           ^------User Requested Size-----^
^-------------Memory Obtained From The Operating System-----------------------^
*/

[EDIT] Changed block structure according to suggestions provided.

+2  A: 

Why are you using a union? Just use a struct and return &dummy_align_var to the user as the start of the free block.

Oh, and since this is for debugging, I suggest that you add a mungwall: Put 16 bytes on either side of the user area and fill them with some pattern (for example 0xdeadbeef, repeated four times). During free() check that these bytes didn't change.

[EDIT] Here is some pseudocode:

struct header {
    struct header * next;
    unsigned size;
    // put mungwall here
    double user_data;
};

init()
    int blockSize = 1024;
    char * bigMem = original_malloc(blockSize);
    struct header * first = (struct header *)bigMem;
    first->next = NULL;
    first->size = blockSize - (sizeof(struct header) - sizeof(double));
Aaron Digulla
As you assign the malloc'ed buffer to a `struct header *` all you have to do is assign the attribute values. Do you plan to malloc 1 large block of memory and deal out blocks from that or malloc every request you get (essenctially adding only a small overhead size for your own administration?)
rsp
Just use the code you posted in the comment. Since the pointer `p` will point to the right memory, the data will end in the right places. Your bug is that you use a union: The user will get the same address as `p` and will overwrite the structure you just filled out.
Aaron Digulla
" Do you plan to malloc 1 large block of memory and deal out blocks from that or malloc every request you get (essenctially adding only a small overhead size for your own administration? "Yes , until the initial chunk in used up completely I don't call malloc again. All the memory users request are assigned from the initial chunk I collect from the OS (Using malloc or system calls )
BumbleBee
See the code which I posted in my edit.
Aaron Digulla
+1  A: 

You might also want to declare the dummy_align_var as union header* prev so that you can put the free memory blocks in a doubly linked list.

This helps a lot on performance when you want to merge a freed block with the previous and next blacks in case they are free too.

Lastly, you don't mention it, keeping the list sorted on size makes it faster to find the best block to allocate for a given request while sorted on address makes it easier to merge freed blocks. If you want to do both, make the user portion at least 3 header* large it will fit te pointers needed when the block is freed.

In addition to the borders Aaron mentioned, overwrite freed buffers with the same pattern. This makes it easier to recognize code that uses already freed buffers.

rsp
A: 

If you want not to use malloc(), you should have a look on sbrk

Aif
+1  A: 

I suggest it would be useful: Some years ago I needed to backup malloc() facility for debugging purpose (allocation tracer and so on)... And it was pretty easy to simple take FreeBSD implementation from their libstdc. It was as I remember FreeBSSD 5.0 or even 4.x late releases but the funny thing was their facility was isolated in simple library malloc.o module so overloading of this layer was very simple copy'n'paste and implementation was really good.

Do you really to do all of this work? Yes, it is only point to check, I don't pretend this solution is the best.

Roman Nikitchenko
A: 

[EDIT]Deleted my own Answer.This is the first time I landed up here Please Ignore :-)

BumbleBee
Please don't post answers; edit your original question :)
Aaron Digulla
@Bumblebee: we were all new once...you can actually delete your answer completely, rather than just editing the text so that the original is not longer visible. By the 'edit' option/link, there should be a 'delete' option/link too.
Jonathan Leffler
+2  A: 

I would do something like

#define MEM_ALIGN 4 // 8 on 64b eventually

struct header {
    union aligned_header {
        struct _s {
            union aligned_header* next;
            size_t size;
        } data;
        char dummy_align_var[sizeof(struct _s) + sizeof(struct _s)%MEM_ALIGN];
    } header_data;
    char user_address;
};

and return &user_address.

Aszarsha
Could you please explain it?
BumbleBee
What kind of explanation do you need ?
Aszarsha
What's the purpose of the union `aligned_header`?
Aaron Digulla
A few questions actually :-1. dummy_align_var[] is of type char and not simply a double because user_address is of same type (IMO structure padding issues) , right ?2.Why dummy_align_var[sizeof(struct _s) + sizeof(struct _s)%MEM_ALIGN]; and not dummy_align_var[MEM_ALIGN]; I guess I need to understand alignment properly.3.char user_address is used only to take the address, (one past the header) to be passed to the user and only because of this the outer structure is used .Right ?I will appreciate if you could validate/disprove my understanding.Regards
BumbleBee
Rational behind `union aligned_header` and `char dummy_align_var[sizeof(struct _s) + sizeof(struct _s)%MEM_ALIGN]` is: you want user data aligned (see bus errors, it may incur a performance hit depending on target too). MEM_ALIGN is used to define this alignment. In x86, union + dummy var is not mandatory since pointer + size_t align.dummy_align_var is array of bytes of this size since it need to be at least `sizeof(struct _s)` in the union to be useful, plus `sizeof(struct _s)%MEM_ALIGN` bytes in case `sizeof(struct _s)` is not a multiple of MEM_ALIGN.yes user_address is only for address
Aszarsha
you should use sizeof(void*) instead of hard coding 4 and commenting that it should be 8 on 64-bit... ... ... ACTUALLY, come to think of it, you should hard code 8, since you want 64-bit quantities to be aligned even on 32-bit, and the caller might want to use compxchg8b (64-bit compare and swap) or similar on a heap address.
asveikau
I would use something like `char user_address[0]` so the structure doesn't allocate memory for the `char` but you can still use it to refer to data after the structure. Also, your `dummy_align_var` looks pretty fragile... I think `(sizeof(struct _s) + MEM_ALIGN - 1) / MEM_ALIGN * MEM_ALIGN` would work better (unless I'm misunderstanding your intent).
strager
+1  A: 

You can use your original union if you want, like so:

union header *hdr = malloc(total_size);
void *user_ptr = hdr + 1;
char *trailer_ptr = (char *)user_ptr + user_size;

That will set user_ptr to where the next union header would begin, if the malloced block was treated as an array of those unions. So that's the value you return to the user.

It also sets trailer_ptr to point to the first byte after the user's allocation, where you can put your checksum.

caf
Could you please explain : void *user_ptr = hdr + 1; I thought , i could get there as follows : #define HEADERSIZE (align(sizeof(union header))) void* unser_ptr = (void*)((char*)hdr + HEADERSIZE); Am I terribly wrong?
BumbleBee
align( ) is defined as size_t align(size_t size){return ((size + ALIGNMENT - 1) }
BumbleBee
That might work too, but the whole point of your `double dummy_align_var;` is to make your union the right size for the correct alignment, *without* those tricks (it assumes that `double` is the type with the most strict alignment requirements). So that lets you use the simpler code.
caf
+3  A: 

For a debugging malloc, consider putting padding space between your control structure and the start of the user data, and also between the end of the user data and the checksum. One byte of the padding should be a zero byte 0x00 - so string operations stop; consider putting another as 0xFF. If you have a fixed pattern and spot that it has changed, you know something went trampling out of bounds -- but there's a better chance that your sensitive control data was not trampled. If you use 16 bytes of padding either side of the space allocated to the user, you might go as far as to put 4 bytes of zeroes suitably aligned (hence a zero 4-byte integer) and maybe 0xFFFFFFFF for -1. Also, since you will probably round up the requested size to a multiple of your basic block size, set the bytes that are not for the user to use to a known value - and validate that they remain unchanged. That will detect modifications of 'one over the allocated length', or just a few bytes over the allocated length, that can otherwise go undetected.

The only disadvantage of the zero byte in padding is that you won't readily detect read operations that do not stop at the end of the allocated memory when looking for a null byte. You could get insight into those by have an alternative option that using padding with no zero bytes in it.

Another option to consider is trying to separate your control data altogether from the memory returned to the user. Of course, complete separation is impossible, but at least maintain a list of allocations (with sizes and pointers) separately from the blocks allocated. Again, this gives you protection by putting your precious control data further away from uncontrolled memory trampling operations. You aren't completely protected from errant pointers, but you are better protected. (And you can still provide buffer zones around the allocated space to detect out-of-control writing.) However, this design is noticably different from the question.


Assuming you get your memory block from 'malloc()', then you would do - roughly:

void *my_malloc(size_t nbytes)
{
    size_t reqblocks = (nbytes + sizeof(header) - 1) / sizeof(header);
    size_t reqspace  = (reqblocks + 2) * sizeof(header) + 2 * sizeof(padding);
    void *space = malloc(reqspace);
    if (space == 0)
        return space;
    void *retval = (char *)space + sizeof(header) + sizeof(padding);
    header *head = space;
    head->next = ...next...;
    head->size = nbytes;
    ...set head padding to chosen value...
    ...set tail padding to chosen value...
    ...set gap between nbytes and block boundary to chosen value...
    return retval;
}

There is some interpretation left to do...

Jonathan Leffler