Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile newlib with -DMALLOC_PROVIDED #81

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Conversation

rojer
Copy link

@rojer rojer commented Aug 15, 2015

ESP provides its own malloc, so whenever newlib tries to use its own implementation, bad things happen.
This way, malloc symbols remain external in libc.a and can be redirected to the ESP versions.
Right now this is left to the user, though an argument can be made for including them.

This change should not break any existing users because newlib's functions that use memory allocation would not work correctly anyway. This change makes use of those functions possible with appropriate malloc function definitions (trivial shims to the pvPort* variants).

ESP provides its own malloc, so whenever newlib tries to use its own
implementation, bad things happen.
This way, *alloc symbols remain external in libc.a and can be redirected
to the ESP versions.
Right now this is left to the user.
@pfalcon
Copy link
Owner

pfalcon commented Aug 15, 2015

Which library of which SDK version defines malloc?

@rojer
Copy link
Author

rojer commented Aug 15, 2015

as it is, none. to make use of any functions from newlib that use malloc, user has to define his own shims (as we do in our code). but i did not want to complicate this PR further. what i'm doing here is the first essential step to make proper memory management possible at all (with shims).

@rojer
Copy link
Author

rojer commented Aug 15, 2015

in our code we define
_malloc_r, _free_r and _realloc_r that delegate to the corresponding pvPort* variants.

@pfalcon
Copy link
Owner

pfalcon commented Aug 15, 2015

So, I guess that's the case when whole solution should be presented, one step looks pretty random. It's otherwise known that newlib config for esp8266 needs tweaking: jcmvbkbc/crosstool-NG#2 . And good way to do it would be doing it upstream, instead of patching it here (though if there're reasons it can't be done upstream, then sure).

@rojer
Copy link
Author

rojer commented Aug 15, 2015

what do you mean by upstream? espressif can't fix it in their SDK, crosstool-ng is built outside it.
i can add a .c file with malloc shims similar to how user_rf_pre_init is handled. would that be acceptable?

@pfalcon
Copy link
Owner

pfalcon commented Aug 15, 2015

By upstream I mean project whose ticket linked above - https://github.com/jcmvbkbc/crosstool-NG , which is crosstool-NG with Xtensa support maintained by @jcmvbkbc. I just use it, mostly as-is. So, please discuss this matter with him and see what he says.

@rojer
Copy link
Author

rojer commented Aug 15, 2015

and that is not the matter for them either. it may be perfectly ok to use newlib in reentrant mode with its own allocator in other projects with xtensa chips. it's just that ESP clearly has their own allocator and thus this is what newlib should be using.
come to think of it, reentrancy should be turned off too, as none of the existing calls in the sdk are reentrant.

@rojer
Copy link
Author

rojer commented Aug 15, 2015

in fact, it might've been better to use newlib and its allocator, but given the situation with ESP where we have an allocator burned into rom and rom functions using it, we have no choice: there shall only be one system allocator, and in our case it has to be the one in ROM. this may not necessarily be the case for other users of xtensa chips.

@rojer
Copy link
Author

rojer commented Nov 9, 2015

@jcmvbkbc can you give your opinion here? i think the default for crosstool should be kept as is and here , in ESP integration, we should change that to existing malloc.

@jcmvbkbc
Copy link
Contributor

Better late than never?
jcmvbkbc/crosstool-NG@46f160f

@rojer
Copy link
Author

rojer commented Mar 28, 2016

indeed! thank you! :)

rojer and others added 6 commits July 8, 2016 12:40
In other words, %ll, %f and %z.
Enable long long, float and C99 formats in Newlib
Update to new Crosstool-NG, SDK 2.0.0, -mforce-l32
Update to latest available SDK
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants