Newlib local patch: Add weak symbols for locking functions

[projectgus]

Hi Max,

I don't know how you'll feel about this patch, but I thought I'd ask. It lets me produce a thread safe newlib that I can link with FreeRTOS' locking primitives for esp-open-rtos.

There might be a better way to do this that I don't know of. Most of the people using FreeRTOS with newlib seem to just modify it directly and compile a FreeRTOS-specific libc, but I figured that was a last resort.

Cheers,

Angus

---

This allows a lock implementation to be optionally added at link time.

Small performance hit if using single threaded, in that locks are now a call to a no-op dummy function rather than a compiled in no-op.

[jcmvbkbc]

Hi Angus, so, for the FreeRTOS you'll configure it with --enable-newlib-multithread and provide locking functions. People that don't need multithreading are not affected at all by this change. And this is to separate locking implementation from the libc, right?

This looks a bit awkward, because then you've no control over the lock structure, it's fixed at the compile time. Maybe instead of implementing such locking for xtensa it'd be better to implement locking for FreeRTOS in a way similar to linux implementation?

[projectgus]

Yes, I agree it's a bit awkward.

To explain my current understanding, I think crosstool builds newlib with --enable-newlib-multithread=yes at the moment (it's the default), but for all non-Linux platforms these are dummied out to no-ops in /include/sys/lock.h.

Just to check I understand the "similar to Linux" suggestion - the Linux implementation ends up weak linking against pthread functions. So a FreeRTOS-like implementation of this would mean weak linking against FreeRTOS mutex functions?

I was hoping to avoid having to compile a FreeRTOS-specific libc. Maybe this is a mistaken priority, though.

If you'd be happy merging a weak-linked FreeRTOS locking implementation into the "mainline" esp crosstool as default then that'd be great from my point of view! ie libc will link FreeRTOS symbols if they're present at link time, or no locking if they're not. I didn't think that'd be a good idea from your/crosstool's point of view though, it seems very implementation-specific for a generic toolchain project.

If it came down to requiring a specifically FreeRTOS-compiled libc, I think it's probably easier for everyone if I I just fork/build a FreeRTOS-specific newlib and put it into esp-open-rtos. There are already two "special cases" that esp-open-rtos needs - the other one is putting .text in .irom, which currently happens via a special target in esp-open-sdk. So it probably makes sense to isolate those in a custom libc (hopefully tracking upstream closely for everything else), and then people can use any toolchain.

I was initially reluctant to do that, but maybe that's actually the best outcome here.

Does that reasoning make sense? I guess I'm happy to do any of these 3 approaches mentioned (current implementation-agnostic approach, compile a weak linked FreeRTOS libc by default in crosstool, or split off a FreeRTOS libc just for esp-open-rtos). I'd be interested to know what you think is the best approach.

Although after writing it all out I'm leaning a lot towards just forking a FreeRTOS-specific libc now - seems the most usable approach, and it could always merge later.

Cheers!

[projectgus]

This looks a bit awkward, because then you've no control over the lock structure, it's fixed at the compile time. Maybe instead of implementing such locking for xtensa it'd be better to implement locking for FreeRTOS in a way similar to linux implementation?

I went to write a FreeRTOS-specific locking implementation but it ended up looking generic anyway, just because the FreeRTOS locking primitives don't match the newlib API (you can't statically initialise a FreeRTOS mutex, but newlib's lock.h API assumes you can.)

So I stuck with this generic approach. For now I'm just bundling a newlib fork with esp-open-rtos. I also forward-ported your xtensa patches to newlib 2.2.0. Seems OK so far. https://github.com/SuperHouse/esp-open-rtos/commit/689cf874b2b76aeed769395bb5659dc04c60d8cf & http://github.com/projectgus/newlib-xtensa

Please close this PR if you like, it seems reasonable to use a forked newlib for this.