malloc() not filling in node structure

[noah]

Okay, so I'm implementing dynamic memory management for my operating system. It's a fairly simple implementation; the heap starts at 0xc000000, which I think is the same as JamesM's tutorials (but I'm not following them). Anyway, the heap is just a linked list, and each node follows this structure:

Code: Select all

typedef struct {
	size_t size;
	void *next;
	uint8_t isfree;
} block_meta_t;	

Again, extremely simple.
So, my malloc() function is simple as well:

Code: Select all

block_meta_t **malloc_last = NULL, **malloc_first = NULL;		// Most recently malloc'd block and first malloc'd block
void *malloc(size_t sz) {
	if (sz) {
		sz = __2pow_rndup(sz);  // Round up to the nearest power of two
		block_meta_t *previous = *malloc_last, *ptr = extend_heap(sz);
		if (!ptr)
			goto returning_null;
		if (malloc_last)
			*malloc_last  = previous->next = ptr;
		else
			*malloc_first = *malloc_last = ptr;
		ptr->size   = sz;
		ptr->next   = NULL;
		ptr->isfree = 0;
		return ptr+1;
	}
	returning_null:
		return NULL;
}

So the allocating part of malloc() works. It extends the heap and returns a pointer to sizeof(block_meta_t) into the block it got. HOWEVER, free() always failed (right now it's very debuggish and doesn't do anything except print the size), because the size in the structure was zero. When I messed around with malloc() after a few days of trying to fix this, not really expecting anything to work, I made ptr a double pointer and dereferenced it every time I needed to get something (I KNOW it's awful, and I didn't expect it to change). But for some reason, when I did that, free() reported the size as 527360 (0x80c00). I don't know if this is where it pointed, or what... but even if I changed where the heap started, I would always get that number.

So yeah... I'm really not sure exactly what's happening. Since I'm using Pure64, the memory is already mapped (but I'm not sure how much exactly), but I don't think that's the issue, seeing as that would just give me a page fault (I do have ISRs enabled and working).

If you need my extend_heap() function, I'll be happy to provide it. Thanks!

Thank you in advance!

[kzinti]

And how do you expect people to help you debug your free() function if you don't provide it?

[noah]

And how do you expect people to help you debug your free() function if you don't provide it?

Sorry about that. I figured the explanation was sufficient. Here:

Code: Select all

void free(void *ptr) {
	if (ptr) {
		block_meta_t *block = (block_meta_t*)ptr-1;
		//printf("\nfree %d\n", block->size);
		printf("%d", block->size);
	}
}

Like I said, for now it just prints the size so I can see if it writes to the struct.

[Octocontrabass]

Since I'm using Pure64, the memory is already mapped

Pure64 maps address space, not memory. There's a very good chance you're trying to put your heap in something that isn't RAM.

[Combuster]

I see way too many NULL dereferences.

[noah]

I see way too many NULL dereferences.

You're right. I'll need to add cases to make sure it doesn't do that.

Pure64 maps address space, not memory. There's a very good chance you're trying to put your heap in something that isn't RAM.

Ah, I'll look into what Pure64 maps addresses to exactly. Thanks!

[Octocontrabass]

Ah, I'll look into what Pure64 maps addresses to exactly. Thanks!

It identity maps the first 64GB of address space. And, you're welcome.

[noah]

It identity maps the first 64GB of address space. And, you're welcome.

You were 100% right. 0xc000000 (~201MB) is outside the 128MB that QEMU allocates. Thanks so much for the identity mapping tip.

[iansjack]

Unless I'm being very silly (which is quite possible) I don't see how your code even compiles.

Code: Select all

ptr->size = size;

Where does "size" on the RHS come from? It isn't declared anywhere. Is it a global variable? If so, have you given it the correct value? Do yu mean:

Code: Select all

ptr->size = sz;

?

[noah]

Where does "size" on the RHS come from?

You're right. During my fiddling, I moved this part to a second function, which took an argument "size". When I pasted my code here, I condensed it down and simplified it, and took all that out of the other function. I just forgot to change "size" to "sz".

[iansjack]

It gets difficult when you are not posting the actual code but a cut and pasted version as there may be other differences that we can't see. It's always better to paste the actual code you are using rather than an "edited highlights" version.

Anyway, this is surely a trivial exercise in debugging. Just single-step through the code and see what happens to the variables. And then use a memory dump to see what the structures actually look like in memory. It should be easy to determine what is going wrong.

[gardinirafael]

And how do you expect people to help you debug your free() function if you don't provide it?

Sorry about that. I figured the explanation was sufficient. Here:

Code: Select all

void free(void *ptr) {
	if (ptr) {
		block_meta_t *block = (block_meta_t*)ptr-1;
		//printf("\nfree %d\n", block->size);
		printf("%d", block->size);
	}
}

Like I said, for now it just prints the size so I can see if it writes to the struct.

In your extend_heap function. Did you use the size of block_meta_t adding sz?

If your answer is yes. follow steps below.

try this

change this in malloc function:

Code: Select all

return ptr+1

for this:

Code: Select all

return ptr + sizeof(block_meta_t);

change your free function too.

Code: Select all

void free(void *ptr) {
	if (ptr) {
                // here
		block_meta_t *block = (block_meta_t*) ((unsigned int)ptr - sizeof(block_meta_t));

		//printf("\nfree %d\n", block->size);
		printf("%d", block->size);
	}
}

[kzinti]

change this in malloc function:

Code: Select all

return ptr+1

for this:

Code: Select all

return ptr + sizeof(block_meta_t);

Terrible, terrible advise. And wrong.

[gardinirafael]

change this in malloc function:

Code: Select all

return ptr+1

for this:

Code: Select all

return ptr + sizeof(block_meta_t);

Terrible, terrible advise. And wrong.

Why?

[Techel]

ptr+1 increments it by sizeof(blockmeta_t), ptr+sizeof(blockmeta_t) does it by sizeof(blockmeta_t)*sizeof(blockmeta_t)