Confusion about possibly unsafe malloc_r?

Confusion about possibly unsafe malloc_r?

[Dave Nadler]

Please excuse newbie here if this is a dumb question...
I'm trying to sort adaptation of newlib to work with FreeRTOS.
I'm using wrappers to force any use of the malloc family to use 
FreeRTOS's (thread-safe) routines.
FreeRTOS (with configUSE_NEWLIB_REENTRANT defined as 1) allocates space 
for a struct _reent in each task's control block,
and sets _impure_ptr to point to the task-specific _reent area prior 
starting/resuming each task.

For some reason, the very first time printf is called (_after_ FreeRTOS 
sets _impure_ptr correctly),
printf -> __sinit (via _REENT_SMALL_CHECK_INIT macro ?) -> __sfp -> 
__sinit -> __sfp -> __sfmoreglue -> malloc_r
and the struct _reent * eventually passed into malloc_r is
that of impure_ptr (apparently a globally allocated _reent structure).

Is this OK? I'm paranoid about thread safety!
Shouldn't this be the task-specific _reent?

Subsequent calls to malloc_r all use the _reent structure from the task 
context as I expected.

Thanks in advance for any clarification/explanation!
Best Regards, Dave

PS: Here's how newlib is configured (latest ARM toolchain):
/* newlib.h.  Generated from newlib.hin by configure.  */
/* newlib.hin.  Manually edited from the output of autoheader to
    remove all PACKAGE_ macros which will collide with any user
    package using newlib header files and having its own package name,
    version, etc...  */
#ifndef __NEWLIB_H__

#define __NEWLIB_H__ 1

/* EL/IX level */
/* #undef _ELIX_LEVEL */

/* Newlib version */
#include <_newlib_version.h>

/* C99 formats support (such as %a, %zu, ...) in IO functions like
  * printf/scanf enabled */
/* #undef _WANT_IO_C99_FORMATS */

/* long long type support in IO functions like printf/scanf enabled */
#define _WANT_IO_LONG_LONG 1

/* Register application finalization function using atexit. */
#define _WANT_REGISTER_FINI 1

/* long double type support in IO functions like printf/scanf enabled */
/* #undef _WANT_IO_LONG_DOUBLE */

/* Positional argument support in printf functions enabled.  */
/* #undef _WANT_IO_POS_ARGS */

/* Optional reentrant struct support.  Used mostly on platforms with
    very restricted storage.  */
/* #undef _WANT_REENT_SMALL */

/* Multibyte supported */
/* #undef _MB_CAPABLE */

/* MB_LEN_MAX */
#define _MB_LEN_MAX 1

/* ICONV enabled */
/* #undef _ICONV_ENABLED */

/* Enable ICONV external CCS files loading capabilities */
/* #undef _ICONV_ENABLE_EXTERNAL_CCS */

/* Define if the linker supports .preinit_array/.init_array/.fini_array
  * sections.  */
#define HAVE_INITFINI_ARRAY 1

/* True if atexit() may dynamically allocate space for cleanup
    functions.  */
#define _ATEXIT_DYNAMIC_ALLOC 1

/* True if long double supported.  */
#define _HAVE_LONG_DOUBLE 1

/* Define if compiler supports -fno-tree-loop-distribute-patterns. */
#define _HAVE_CC_INHIBIT_LOOP_TO_LIBCALL 1

/* True if long double supported and it is equal to double.  */
#define _LDBL_EQ_DBL 1

/* Define if ivo supported in streamio.  */
#define _FVWRITE_IN_STREAMIO 1

/* Define if fseek functions support seek optimization.  */
#define _FSEEK_OPTIMIZATION 1

/* Define if wide char orientation is supported.  */
#define _WIDE_ORIENT 1

/* Define if unbuffered stream file optimization is supported.  */
#define _UNBUF_STREAM_OPT 1

/* Define if lite version of exit supported.  */
/* #undef _LITE_EXIT */

/* Define if declare atexit data as global.  */
/* #undef _REENT_GLOBAL_ATEXIT */

/* Define if small footprint nano-formatted-IO implementation used. */
/* #undef _NANO_FORMATTED_IO */

/*
  * Iconv encodings enabled ("to" direction)
  */
/* #undef _ICONV_TO_ENCODING_BIG5 */
/* #undef _ICONV_TO_ENCODING_CP775 */
/* #undef _ICONV_TO_ENCODING_CP850 */
/* #undef _ICONV_TO_ENCODING_CP852 */
/* #undef _ICONV_TO_ENCODING_CP855 */
/* #undef _ICONV_TO_ENCODING_CP866 */
/* #undef _ICONV_TO_ENCODING_EUC_JP */
/* #undef _ICONV_TO_ENCODING_EUC_TW */
/* #undef _ICONV_TO_ENCODING_EUC_KR */
/* #undef _ICONV_TO_ENCODING_ISO_8859_1 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_10 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_11 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_13 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_14 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_15 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_2 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_3 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_4 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_5 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_6 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_7 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_8 */
/* #undef _ICONV_TO_ENCODING_ISO_8859_9 */
/* #undef _ICONV_TO_ENCODING_ISO_IR_111 */
/* #undef _ICONV_TO_ENCODING_KOI8_R */
/* #undef _ICONV_TO_ENCODING_KOI8_RU */
/* #undef _ICONV_TO_ENCODING_KOI8_U */
/* #undef _ICONV_TO_ENCODING_KOI8_UNI */
/* #undef _ICONV_TO_ENCODING_UCS_2 */
/* #undef _ICONV_TO_ENCODING_UCS_2_INTERNAL */
/* #undef _ICONV_TO_ENCODING_UCS_2BE */
/* #undef _ICONV_TO_ENCODING_UCS_2LE */
/* #undef _ICONV_TO_ENCODING_UCS_4 */
/* #undef _ICONV_TO_ENCODING_UCS_4_INTERNAL */
/* #undef _ICONV_TO_ENCODING_UCS_4BE */
/* #undef _ICONV_TO_ENCODING_UCS_4LE */
/* #undef _ICONV_TO_ENCODING_US_ASCII */
/* #undef _ICONV_TO_ENCODING_UTF_16 */
/* #undef _ICONV_TO_ENCODING_UTF_16BE */
/* #undef _ICONV_TO_ENCODING_UTF_16LE */
/* #undef _ICONV_TO_ENCODING_UTF_8 */
/* #undef _ICONV_TO_ENCODING_WIN_1250 */
/* #undef _ICONV_TO_ENCODING_WIN_1251 */
/* #undef _ICONV_TO_ENCODING_WIN_1252 */
/* #undef _ICONV_TO_ENCODING_WIN_1253 */
/* #undef _ICONV_TO_ENCODING_WIN_1254 */
/* #undef _ICONV_TO_ENCODING_WIN_1255 */
/* #undef _ICONV_TO_ENCODING_WIN_1256 */
/* #undef _ICONV_TO_ENCODING_WIN_1257 */
/* #undef _ICONV_TO_ENCODING_WIN_1258 */

/*
  * Iconv encodings enabled ("from" direction)
  */
/* #undef _ICONV_FROM_ENCODING_BIG5 */
/* #undef _ICONV_FROM_ENCODING_CP775 */
/* #undef _ICONV_FROM_ENCODING_CP850 */
/* #undef _ICONV_FROM_ENCODING_CP852 */
/* #undef _ICONV_FROM_ENCODING_CP855 */
/* #undef _ICONV_FROM_ENCODING_CP866 */
/* #undef _ICONV_FROM_ENCODING_EUC_JP */
/* #undef _ICONV_FROM_ENCODING_EUC_TW */
/* #undef _ICONV_FROM_ENCODING_EUC_KR */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_1 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_10 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_11 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_13 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_14 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_15 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_2 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_3 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_4 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_5 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_6 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_7 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_8 */
/* #undef _ICONV_FROM_ENCODING_ISO_8859_9 */
/* #undef _ICONV_FROM_ENCODING_ISO_IR_111 */
/* #undef _ICONV_FROM_ENCODING_KOI8_R */
/* #undef _ICONV_FROM_ENCODING_KOI8_RU */
/* #undef _ICONV_FROM_ENCODING_KOI8_U */
/* #undef _ICONV_FROM_ENCODING_KOI8_UNI */
/* #undef _ICONV_FROM_ENCODING_UCS_2 */
/* #undef _ICONV_FROM_ENCODING_UCS_2_INTERNAL */
/* #undef _ICONV_FROM_ENCODING_UCS_2BE */
/* #undef _ICONV_FROM_ENCODING_UCS_2LE */
/* #undef _ICONV_FROM_ENCODING_UCS_4 */
/* #undef _ICONV_FROM_ENCODING_UCS_4_INTERNAL */
/* #undef _ICONV_FROM_ENCODING_UCS_4BE */
/* #undef _ICONV_FROM_ENCODING_UCS_4LE */
/* #undef _ICONV_FROM_ENCODING_US_ASCII */
/* #undef _ICONV_FROM_ENCODING_UTF_16 */
/* #undef _ICONV_FROM_ENCODING_UTF_16BE */
/* #undef _ICONV_FROM_ENCODING_UTF_16LE */
/* #undef _ICONV_FROM_ENCODING_UTF_8 */
/* #undef _ICONV_FROM_ENCODING_WIN_1250 */
/* #undef _ICONV_FROM_ENCODING_WIN_1251 */
/* #undef _ICONV_FROM_ENCODING_WIN_1252 */
/* #undef _ICONV_FROM_ENCODING_WIN_1253 */
/* #undef _ICONV_FROM_ENCODING_WIN_1254 */
/* #undef _ICONV_FROM_ENCODING_WIN_1255 */
/* #undef _ICONV_FROM_ENCODING_WIN_1256 */
/* #undef _ICONV_FROM_ENCODING_WIN_1257 */
/* #undef _ICONV_FROM_ENCODING_WIN_1258 */

#endif /* !__NEWLIB_H__ */

-- 
Dave Nadler, USA East Coast voice (978) 263-0097,drn@nadler.com, Skype
  Dave.Nadler1

Re: Confusion about possibly unsafe malloc_r?

[Freddie Chopin]

On Sat, 2017-06-24 at 18:42 -0400, Dave Nadler wrote:
> Is this OK? I'm paranoid about thread safety!

Then it's worth mentioning that newlib and FreeRTOS will _NEVER_ be
fully thread safe unless you are using a toolchain with retargetable
locks and your project has support code for these locks.

printf()-style families partially use global reent structure, this is
expected. Trace the calls of the mentioned functions in newlib source
and you'll see that sometimes _GLOBAL_REENT is used, sometimes thread's
reent.

Regards,
FCh

Re: Confusion about possibly unsafe malloc_r?

[Dave Nadler]

Thanks Freddie. If I understand your answer correctly, printf's call chain's
use of _GLOBAL_REENT should be OK because this particular memory is
protected by a global lock.

I studied the newlib documentation and looked in syscalls, but did not find
anything explaining how to implement "retargetable locks" as you mention.
I did understand that I must implement env_locks to use set_env/get_env,
and that I did not need to implement the corresponding malloc locks.
I read a half-dozen articles about embedding/porintg newlib and failed
to find this...

Could you point me at documentation (or sources) explaining how
to implement "retargetable locks"?

Thanks for the help!
Best Regards, Dave

On 6/25/2017 2:15 AM, Freddie Chopin wrote:
> On Sat, 2017-06-24 at 18:42 -0400, Dave Nadler wrote:
>> Is this OK? I'm paranoid about thread safety!
> Then it's worth mentioning that newlib and FreeRTOS will _NEVER_ be
> fully thread safe unless you are using a toolchain with retargetable
> locks and your project has support code for these locks.
>
> printf()-style families partially use global reent structure, this is
> expected. Trace the calls of the mentioned functions in newlib source
> and you'll see that sometimes _GLOBAL_REENT is used, sometimes thread's
> reent.
>
> Regards,
> FCh

-- 
Dave Nadler, USA East Coast voice (978) 263-0097,drn@nadler.com, Skype
  Dave.Nadler1

Re: Confusion about possibly unsafe malloc_r?

[Freddie Chopin]

On Sun, 2017-06-25 at 10:22 -0400, Dave Nadler wrote:
> Could you point me at documentation (or sources) explaining how
> to implement "retargetable locks"?

"Retargetable locks" are a new feature, introduced at the beginning of
this year. You can see an example implementation in my C++ RTOS here:

https://github.com/DISTORTEC/distortos/blob/master/source/newlib/locking.cpp

Regards,
FCh

Re: Confusion about possibly unsafe malloc_r?

[Dave Nadler]

Now I'm really confused.
I'm working with newlib version 2.5 as distributed in the GNU ARM Toolchain.
I implemented everything I could find in the documentation.
Is the following correct and complete WRT newlib 2.5.0 ???
http://www.nadler.com/embedded/draft1_newlibAndFreeRTOS.html
Thanks,
Best Regards, Dave

PS: Kudos for pushing C++; we've been doing so for a couple decades.
But FreeRTOS has a lot of advantages and required by some of our customers,
and works fine for C++ (though I've added some ctor/dtor wrappers
to ensure symmetric acquisition/release of interrupts, locks, 
semaphores, etc.).

On 6/27/2017 2:26 PM, Freddie Chopin wrote:
> On Sun, 2017-06-25 at 10:22 -0400, Dave Nadler wrote:
>> Could you point me at documentation (or sources) explaining how
>> to implement "retargetable locks"?
> "Retargetable locks" are a new feature, introduced at the beginning of
> this year. You can see an example implementation in my C++ RTOS here:
>
> https://github.com/DISTORTEC/distortos/blob/master/source/newlib/locking.cpp
>
> Regards,
> FCh


-- 
Dave Nadler, USA East Coast voice (978) 263-0097, drn@nadler.com, Skype
  Dave.Nadler1