-
Notifications
You must be signed in to change notification settings - Fork 3k
Thread safe #1765
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
Thread safe #1765
Conversation
@@ -66,10 +66,10 @@ def __init__(self, target, options=None, notify=None, macros=None, silent=False, | |||
self.asm = [join(IAR_BIN, "iasmarm")] + ["--cpu", cpuchoice] | |||
if not "analyze" in self.options: | |||
self.cc = [main_cc] + c_flags | |||
self.cppc = [main_cc, "--c++", "--no_rtti", "--no_exceptions"] + c_flags | |||
self.cppc = [main_cc, "--c++", "--no_rtti", "--no_exceptions", "--guard_calls"] + c_flags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to myself: we need to clean up IAR flags. otherwise when you export you wont get all these new flags in the project file 😭
This is really odd, but I don't see the same errors as the CI when I run workspace_tools/build_travis.py on this branch. |
I was using gcc-arm-embedded, whoops. I can reproduce the error with gcc-arm-none-eabi (version 4.9.3.2015q3-1trusty1). Also, I have the full assembly file if anyone would like to see it. |
removing this line: https://github.com/c1728p9/mbed/blob/807ef62af47b2ff31a8e85a9420b3db3691de631/rtos/rtx/TARGET_CORTEX_M/RTX_CM_lib.h#L646 fixes the CI errors. Tested with both ARMCC and GCC, the only compilers that build that line. |
Using assembler directives in the inline assembly language code is probably not a good idea since it can leave the assembler in an incorrect state when it attempts to assemble later code which was generated by the C/C++ compiler. Dropping the .syntax and .thumb directives would be best. We hit that problem previously with pre_main() and fixed it by just dropping the assembly language altogether and re-writing in C. |
Thanks for looking into this @theotherjimmy! That was a tricky bug to find. Updated the PR to remove these assembler directives, along with adding a definition for the _reent structure. |
@mbed-bot: TEST HOST_OSES=ALL |
[Build 397] |
// Opaque declaration of _reent structure | ||
struct _reent; | ||
|
||
void __malloc_lock( struct _reent *_r ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix the formatting : ( struct _reent *_r )
, no spacing around ( ) - (struct _reent *_r)
I am happy with this in general but not that gcc flag. To proceed, I would propose to keep that gcc as it now (we got boilerplate, we just need to figure out how to switch nano on/off), and discuss how to switch nano lib on and off (my comment #1765 (comment)). What about ARMCC, everything is setup correctly as it is now? With this PR (besides gcc nano flag), we would have armcc and iar thread safe standard library |
Add the locks and flags necessary to make the IAR standard library thread safe. These changes consist of: -Add compiler flag "--guard_calls" to ensure C++ function-static variables with dynamic initializers are initialized in a thread safe manner -Add the linker flag "--threaded_lib" so the thread safe version of the standard library is used -Implement mutex functions required for IAR thread safety -Create a set of stub functions in retarget.c for when the rtos is not present
Add lock functions so that malloc and environment variable access are thread safe. Add the compiler option "-o thread-safe" to use the full version of newlib which is thread safe. Note that this patch does NOT make file access thread safe.
Remove the assembler directives from the inline assembly in RTX_CM_lib.h since these can have unintended side effects in the surrounding C code.
@mbed-bot: TEST HOST_OSES=ALL |
Updated the patch to keep backwards compatibility. The option "-o thread-safe" can be used to build in the thread safe version of the standard library. In the future toolchain handling will need to be overhauled so by default all targets which can use the thread safe version of the standard library do. |
[Build 425] |
Update IAR and GCC to improve thread safety.