lunixbochs / lib43

portable libc optimized for code size and readability
MIT License
80 stars 6 forks source link

Add ARM support, fix bug exposed with realloc under ARM #4

Closed tmcarthur closed 9 years ago

tmcarthur commented 9 years ago

tl;dr - ARM support, a fix for realloc

I tested this on my raspberry pi to ensure it worked - I cannot at this time verify arm64 works, but once I have a device I will verify/add this support

I had to link against gcc lib on this platform to grab a few symbols because a given ARM processor may or may not support native integer division, if not we have to pull in implementation from GCC (or do so ourselves, but for another day =) - it doesn't import _Start or anything, I verified this myself by removing arch code, failed build, and then manually setting exit status in _start.s to ensure it was using our _start implementation. Subsequently also had to define raise() because gcc's integer division implementations expect it to raise floating point exception is required - verified case of division by 0 sends correct signal to process.

tmcarthur commented 9 years ago

Oh one quick note - I have some more changes for mman.c to add some additional error handling - but in subsequent commit/pull request once I test it more extensively.

lunixbochs commented 9 years ago

Can you see if part of the mmap contiguous problem on ARM is due to starting the addresses on an aligned offset? (for example, I mmap 1 byte and it gives me 0x08, I try to mmap another byte at 0x09 and it gives me 0x12)

I'm also not happy with linking a gcc lib, so I'd rather just not have ARMv6 support for now.

tmcarthur commented 9 years ago

Yeah - I'm fairly certain it is alignment related - that's the only reason mmap should choose another case for this.

The only place we need integer division for is some ops in num.c - are you okay with me implementing a simple integer division algorithm in either arm asm or C and just including it instead of linking with gcc?

tmcarthur commented 9 years ago

Oh - one other note - the problem is if we don't implement some form of idiv we can't really support armv7 either - only variants of armv7 that support idiv natively, the base armv7 idiv is optional. The only architecture (I believe?) that supports it in the base instruction set is armv8/arm64.

So let me know what you think - if you're fine with it I don't mind just implementing Goldschmidt's algorithm with iterations based on precision of architecture. If not I can turn off support for everything that doesn't support native idiv.

tmcarthur commented 9 years ago

Also - to conclusively answer question on mmap problem - it doesn't appear to be strictly alignment related - if I only realloc 800, it's fine, when I start getting > 4000 it runs into issues. I still think we should handle this case regardless of alignment or other problem (though we could aggressively optimize for alignment if that had turned out to be primary problem) because mmap does not guarantee it will map it from desired start location.

(And sorry for multiple comments - :)

lunixbochs commented 9 years ago

I think we should switch the path from /linux/i386 to /i386/linux folder form so we can put cpu-specific code in /arch. More specific comments inline. I do prefer separate microcommits for things like the int num = mmap() fix. (git add -p is your friend).

I'm fine with a C wrapper for int division. I'm assuming it's named per arch though, like armeabi_idiv()? Maybe an arch/common folder would make sense, then armeabi etc can just wrap it (MIPS support next? :D I have a couple of test devices.)

tmcarthur commented 9 years ago

Thanks for the comments! I'll have a look at these sometime this week :D. I'll just update this pull request with changes unless you'd prefer me to split it up.

And micro commits sound good - different people have different styles so wasn't sure what you'd prefer - I'll fix it up.

Agree on arch/ reorg too.

For C wrapper for int division, sounds good, I'll write one and update this request. And yeah - it could be a common implementation for the most part.

MIPS sounds good. Have any suggestions based on ones you have that aren't expensive? I've been looking for one anyway.

lunixbochs commented 9 years ago

The MIPS Creator is probably your best bet. All of my current MIPS devices are handhelds or routers.

lunixbochs commented 9 years ago

I tested and it always malloc'd on grow for me (even with alignment), so e647c963fd61031f368db0442b688dc6fda2e076 switches realloc to just malloc on grow.

tmcarthur commented 9 years ago

Yeah - I saw e647c96 - it didn't happen for me for small amounts - I'm test on a raspberry pi w/ armv6 - so your mileage may vary - and honestly having it remalloc is probably fine.

And yeah - SYS_mmap is not defined on ARM.

tmcarthur commented 9 years ago

Will have update shortly - working on this now - thanks again for comments and suggestions for MIPS stuff.

tmcarthur commented 9 years ago

Added integer division test to ensure it works for ARM since it's important - hope that's alright, I can split into separate commit if you'd rather.

I only did 7+ so we get hardware integer division - I want to cover all the corner cases for hardware integer division on older ARM hardware, so going to take a little more time to make sure I do it right (without just copying the implementation gcc uses :P)

Oh - and -nostdlib I don't believe is required for the as - it won't even assemble with that option on ARM - and on x86 it doesn't appear to do anything.

tmcarthur commented 9 years ago

Updated - I am using a C shim for sys calls instead of asm for now because I want to make sure an assembly version will work across various ARM variants without significant difficulty (e.g.: calling conventions on various processors, they aren't 100% backwards compatible like i386 if I remember correctly).

Integer division forthcoming as well - for now only supports armv7, v8, and 64.

tmcarthur commented 9 years ago

Comment addressed, thanks.