OSDev.org

Posted: **Fri Nov 09, 2007 11:26 am**

Hi,
I'm having a problem with a free() function (stolen from PDClib), which seems to be causing my OS to pagefault. Is it possible if anyone can point out to me what is going wrong? (the error occurs in the free function itself: I've checked!)

This is the code for the function (located at 0x80000eff):

Code: Select all

void free( void * ptr )
{
    ptr = (void *)( (char *)ptr - sizeof( struct _PDCLIB_memnode_t ) );
    ( (struct _PDCLIB_memnode_t *)ptr )->next = NULL;
    if ( _PDCLIB_memlist.last != NULL )
    {
        _PDCLIB_memlist.last->next = ptr;
    }
    else
    {
        _PDCLIB_memlist.first = ptr;
    }
    _PDCLIB_memlist.last = ptr;
}

Here's from the function that called it:

Code: Select all

parameters_pointer = malloc(temp_size);
memcpy(parameters_pointer, &parameters, temp_size);
p_procedure = object_table[i].object_data.procedure.function;
p_procedure((void*) parameters_pointer);
free(parameters_pointer);

CPU dump:

Code: Select all

00006932077i[CPU  ] CS.d_b = 32 bit
00006932077i[CPU  ] SS.d_b = 32 bit
00006932077i[CPU  ] | EAX=fffffff8  EBX=80000000  ECX=00000007  EDX=000b829f
00006932077i[CPU  ] | ESP=80306f27  EBP=80306f27  ESI=000263bc  EDI=000263d7
00006932077i[CPU  ] | IOPL=0 NV UP EI NG NZ AC PO CY
00006932077i[CPU  ] | SEG selector     base    limit G D
00006932077i[CPU  ] | SEG sltr(index|ti|rpl)     base    limit G D
00006932077i[CPU  ] |  DS:0010( 0002| 0|  0) 00000000 000fffff 1 1
00006932077i[CPU  ] |  ES:0010( 0002| 0|  0) 00000000 000fffff 1 1
00006932077i[CPU  ] |  FS:0010( 0002| 0|  0) 00000000 000fffff 1 1
00006932077i[CPU  ] |  GS:0010( 0002| 0|  0) 00000000 000fffff 1 1
00006932077i[CPU  ] |  SS:0010( 0002| 0|  0) 00000000 000fffff 1 1
00006932077i[CPU  ] |  CS:0008( 0001| 0|  0) 00000000 000fffff 1 1
00006932077i[CPU  ] | EIP=80000eff (80000eff)
00006932077i[CPU  ] | CR0=0xe0000011 CR1=0x00000000 CR2=0xfffffffc
00006932077i[CPU  ] | CR3=0x00110000 CR4=0x00000000
00006932077i[CPU  ] >> c7
00006932077i[CPU  ] >> 40
00006932077i[CPU  ] >> 04
00006932077i[CPU  ] >> 00
00006932077i[CPU  ] >> 00
00006932077i[CPU  ] >> 00
00006932077i[CPU  ] >> 00
00006932077i[CPU  ] >> : mov dword ptr ds:[eax+0x4], 0x0

Thanks for your help in advance,
OScoder

Posted: **Fri Nov 09, 2007 5:23 pm**

When I had a problem with page faults in my free() function I discovered that I was somehow overwriting the next pointer in the malloc block. Upon calling free it would look for a block somewhere where there was no memory mapped. Try just calling malloc and immediately calling free on that block and see if it causes a page fault.

Posted: **Sat Nov 10, 2007 5:42 am**

euhm, free(NULL); ?!

edit: have you checked your support function for malloc() - the error is probably there... (and some bad coding practice for assuming malloc() does not return NULL)

Posted: **Sat Nov 10, 2007 10:48 am**

Combuster wrote:euhm, free(NULL); ?!

edit: have you checked your support function for malloc() - the error is probably there... (and some bad coding practice for assuming malloc() does not return NULL)

Heh - you can tell how inexperienced a coder I am, not checking that! Well, it seems that my alloc_pages function is returning a null value, and I don't see how that's possible: the code seems (on reading at least) watertight enough.

Here it is anyway:

Code: Select all


#define	STD_HEAP_START		0xC0000000
#define	STD_HEAP_PAGE_START	(STD_HEAP_START/PAGE_SIZE)

 static void *membreak;	/*Where the heap ends*/
 unsigned long membreak_page = 0;

void* alloc_pages(int const n)
{
 void *oldbreak;
 int i;
	if(n>0)
	{
	/*if this is the first call:*/
		if(membreak_page == 0)
		{
		 membreak = (void *)STD_HEAP_START;
		 membreak_page = STD_HEAP_PAGE_START;
		}
	 /*enable each page:*/
		for(i=0; i<n; i++)
		{
		 k_enable_memory(membreak_page);/*enable the memory at the end of the heap*/
		 membreak_page++;				/*increase heap end*/
		}
	 oldbreak = membreak;		/*store the old heap end pointer*/
	 membreak = (void *)(membreak_page * PAGE_SIZE);	/*update the heap end pointer*/
	 return(oldbreak);			/*return the old heap end pointer (the beginning of the new block)*/
	}
	
	if(n<0)
	{
	/*if this is the first call, return error --> must have something to free!*/
		if(membreak == 0)
		 return(0);
	/*check the number of pages to free is not too high*/
		if((membreak_page-n)<STD_HEAP_START)
		 return(0);
	/*disable each page:*/
		for(i=0; i>n; i--)
		{
		 k_disable_memory(membreak_page);/*enable the memory at the end of the heap*/
		 membreak_page--;				/*increase heap end*/
		}
	 oldbreak = membreak;		/*store the old heap end pointer*/
	 membreak = (void *)(membreak_page * PAGE_SIZE);	/*update the heap end pointer*/
	 return(oldbreak);			/*return the old heap end pointer (the beginning of the new block)*/
	}
 return(0);
}

Can you see what's going wrong here?
I really appreciate your help,
OScoder

Posted: **Sat Nov 10, 2007 12:18 pm**

What's the relationship between malloc and alloc_pages? If alloc_pages is an sbrk-like function, then it should _always_ return the old end-of-heap pointer, which you are correctly doing, as long as n != 0. The return should be (void *)-1 in the case of error. In the case of n == 0 you are returning 0 when you should return membreak, or (void *)-1 in the case that membreak == 0.

Regards,
John.

Posted: **Sat Nov 10, 2007 5:37 pm**

alloc_pages() is a helper function for pdclib - it passes a number of needed pages, and the function returns the base address where that memory is added.

I can't tell for sure, but it seems I have a different version of pdclib. You can check my implementation if you want.

but please, enable warnings (pass -Wall -Wextra -pedantic -Werror to gcc) to fix your sloppy coding practices, maybe that fixes the bug as well...

OSDev.org

free function causes page-fault

free function causes page-fault