Page 1 of 1

Memory parts are overlapping

Posted: Sat Jan 16, 2016 11:00 am
by osdever
Some days ago I'm realized that the memory parts are overlapping. So any call can wreck up all data. There's an example:
If I add 1 small printf, something will mess up. For example, without that call my WIP font drawing engine is working normal, except for some drawing errors:
Screenshot from 2016-01-16 19-54-46.png
Screenshot from 2016-01-16 19-54-46.png (1.71 KiB) Viewed 1718 times
But if I call it, it'll mess all up:
Screenshot from 2016-01-16 19-55-55.png
Screenshot from 2016-01-16 19-55-55.png (1.12 KiB) Viewed 1718 times
This is my memory.c:

Code: Select all

//Memory management.
#include "../include/memory.h"

#include <stddef.h>
#include <stdbool.h>
#include <stdint.h>
#include "../include/fio.h"

#define isBitSet(var, n) ((var & (1 << n)))

// Struct page is only for internal usage and should not be used anywhere else
#define PAGE_SIZE 4096
#define PAGE_MAX 4092 // (PAGE_SIZE - 2 * sizeof(uint16_t))
typedef struct
{
    uint16_t free_bytes;
    uint16_t free_offset;
    uint8_t  memory[PAGE_MAX];
} page;

size_t page_count;
page*  pages;

void* memcpy(void* dst, const void* src, size_t n)
{
    size_t i;
    if((uint8_t*)(dst) < (uint8_t*)(src))
    {
        for(i = 0; i < n; ++i)
        {
            ((uint8_t*)(dst))[i] = ((uint8_t*)(src))[i];
        }
    }
    else
    {
        for(i = 0; i < n; ++i)
        {
            ((uint8_t*)(dst))[n - i - 1] = ((uint8_t*)(src))[n - i - 1];
        }
    }
    return dst;
}

void* memset(void* ptr, uint8_t val, size_t n)
{
    size_t i;
    for(i = 0; i < n; ++i)
    {
        ((uint8_t*)(ptr))[i] = val;
    }
    return ptr;
}

void* memmove(void* dst, void* src, size_t n)
{
    if(dst != src)
    {
        memcpy(dst, src, n);
        free(src);
    }
    return dst;
}

/*
 * malloc function.
 * Allocating num bytes of memory and returning pointer to it.
 * Memory is located at the end of the kernel memory.
*/
extern uint16_t kernel_heap_start;
void* malloc(size_t num)
{
    size_t i;
    if(num < PAGE_MAX)
    {
        for(i = 0; i < page_count; ++i)
        {
            if(pages[i].free_bytes >= num + sizeof(uint16_t))
            {
                void* free_space = (void*)((size_t)(pages + i) + pages[i].free_offset);
                *((uint16_t*)(free_space)) = num;
                pages[i].free_bytes  -= num + sizeof(uint16_t);
                pages[i].free_offset += num + sizeof(uint16_t);
                return (void*)((uint16_t*)(free_space) + 1);
            }
        }
        perr("malloc: no free pages left");
        return 0;
    }
    else
    {
        perr("malloc: more than one page requested");
        return 0;
    }
}

void* calloc(size_t num, size_t size)
{
    size *= num;
    void* result = malloc(size);
    memset(result, 0, size);
    return result;
}

void *realloc(void* memblock, size_t size)
{
    if(memblock < (void*)(pages))
    {
        return malloc(size);
    }
    else
    {
        uint16_t oldsize = *((uint16_t*)(memblock) - 1);
        void* newmem = malloc(size);
        memmove(newmem, memblock, oldsize);
    }
    return memblock;
}

void free(void* ptr)
{
    uint16_t size = *((uint16_t*)(ptr) - 1);
    page* pg = (page*)((size_t)(ptr) - (size_t)(ptr) % PAGE_SIZE);
    pg->free_bytes -= size;
    if(pg->free_offset == (uint16_t)((size_t)(ptr) - (size_t)(pg) + size))
    {
        pg->free_offset -= size;
    }
}

uint8_t init_page(page* this)
{
    this->free_bytes = PAGE_MAX;
    this->free_offset = 2;
    if(*(uint16_t*)(this) != PAGE_MAX || *((uint16_t*)(this) + 1) != 2)
    {
        return 0; // false
    }
    return 1; // true
}


size_t init_memory()
{
    size_t pgptr = (size_t)&kernel_heap_start;
    pgptr += (PAGE_SIZE - pgptr % PAGE_SIZE);

    page_count = 0;
    pages = (page*)(pgptr);
    while(init_page(pages + page_count) && page_count < 2048) ++page_count;
    return page_count;
}


Re: Memory parts are overlapping

Posted: Sat Jan 16, 2016 12:48 pm
by Combuster
There's a method called unit testing. You can even use that to test memory management from userspace - just grab yourself some scratch memory and play with it. For now, I renamed malloc/free to my_malloc/my_free so that I can use them in conjunction and dump some stats about memory management.

Ergo, I tried to strategically cover a few usage options with the following:

Code: Select all

void main(void)
{
    // this replaces the start of init_memory so that it groks our test memory addresses
    void * memory = malloc(2049*4096);
    uintptr_t memaddr = (intptr_t) memory;
    memaddr = (memaddr + 0xFFF) & ~0xFFF;
    pages = (page *) memaddr;
    page_count = 0;
    // initialize the data structures
    size_t result = init_memory();

    printf("mem at 0x%lx, aligned 0x%lx, pages 0x%lx, result 0x%lx\n", (long)memory, (long)pages, page_count, result);

    printf("start: %d %d\n", (int)(pages[0].free_bytes), (int)(pages[0].free_offset));

    void * p1 = my_malloc(16);
    printf("alloc #1@16b: 0x%lx-0x%lx (%d %d)\n", (long)p1 & 0xffff, ((long)p1 & 0xffff) + 15, (int)(pages[0].free_bytes), (int)(pages[0].free_offset));

    void * p2 = my_malloc(16);
    printf("alloc #2@16b: 0x%lx-0x%lx (%d %d)\n", (long)p2 & 0xffff, ((long)p2 & 0xffff) + 15, (int)(pages[0].free_bytes), (int)(pages[0].free_offset));

	my_free(p1);
    printf("free #1: %d %d\n", (int)(pages[0].free_bytes), (int)(pages[0].free_offset));

    void * p3 = my_malloc(32);
    printf("alloc #3@32b: 0x%lx-0x%lx (%d %d)\n", (long)p3 & 0xffff, ((long)p3 & 0xffff) + 31, (int)(pages[0].free_bytes), (int)(pages[0].free_offset));

	my_free(p3);
    printf("free #3: %d %d\n", (int)(pages[0].free_bytes), (int)(pages[0].free_offset));

	my_free(p2);
    printf("free #2: %d %d\n", (int)(pages[0].free_bytes), (int)(pages[0].free_offset));
}
This gets me the following. A real unit test would actually test the values and fail with a message, but finishing it properly is left as an exercise. Instead some hand-written comments:
mem at 0x7f6d01fa6010, aligned 0x7f6d01fa7000, pages 0x800, result 0x800
start: 4092 2 [should be 4092 4, the preamble is 4 bytes]
alloc #1@16b: 0x7004-0x7013 (4074 34) [expected: 4074 20, a difference of 16+2 bytes]
alloc #2@16b: 0x7024-0x7033 (4056 52)
free #1: 4004 52 [expected: 4056 52, same as after alloc #2]
alloc #3@32b: 0x7036-0x7055 (3970 86)
free #3: 3938 54 [expected: 4056 52, same as after alloc #2]
free #2: 3922 54 [expected: 4074 34, same as after alloc #1]
Since I can't even seem to allocate the first piece of memory properly, I stopped looking for your overlap issue, but it might magically vanish by the time you sort these base cases out.

Re: Memory parts are overlapping

Posted: Sun Jan 17, 2016 2:27 am
by alexfru
JFYI.
catnikita255 wrote:

Code: Select all

#define isBitSet(var, n) ((var & (1 << n)))
This will only work for 0 <= n < 30, not for n >= 31. Because 1 is a signed int and shifting it into position 31 causes signed integer overflow and undefined behavior. At the very least use 1u instead of plain 1.
catnikita255 wrote:

Code: Select all

void* memcpy(void* dst, const void* src, size_t n)
{
    size_t i;
    if((uint8_t*)(dst) < (uint8_t*)(src))
    ...
}
You can't use relational operators to compare arbitrary pointers. It can cause undefined behavior.
catnikita255 wrote:

Code: Select all

void* memmove(void* dst, void* src, size_t n)
{
    if(dst != src)
    {
        memcpy(dst, src, n);
        free(src);
    }
    return dst;
}
The standard memmove() does not free any memory.
catnikita255 wrote:

Code: Select all

void* calloc(size_t num, size_t size)
{
    size *= num;
    void* result = malloc(size);
    memset(result, 0, size);
    return result;
}
What if malloc() returns NULL?
catnikita255 wrote:

Code: Select all

void *realloc(void* memblock, size_t size)
{
    if(memblock < (void*)(pages))
    {
        return malloc(size);
    }
    else
    {
        uint16_t oldsize = *((uint16_t*)(memblock) - 1);
        void* newmem = malloc(size);
        memmove(newmem, memblock, oldsize);
    }
    return memblock;
}
Relational operators again. Also using non-standard GNU extensions (pointers to void should not be used in pointer arithmetic and comparison other than for equality/inequality without first being cast to pointers to char).

Check realloc() documentation. There's special behavior with NULL pointers.

Also consider size 0. See the documentation for malloc() and realloc().