Newlib HEAD + FILE streams

[Sebastian Huber]

On 11/28/2012 01:45 PM, Ralf Corsepius wrote:
> On 11/28/2012 09:51 AM, Sebastian Huber wrote:
>> Hello,
>>
>> the Newlib HEAD has now a different FILE locking implementation
>> (compared to Newlib 1.20.0).  It uses now pthread_setcancelstate().
>> With the Newlib HEAD the RTEMS shell depends now on the POSIX API.
>
> Correct, newlib-CVS HEAD is unusable for RTEMS.

Ok, we should identify the problems and fix them upstream.  I noticed two issues.

1. The <inttypes.h> of Newlib seems to be broken if we use the GCC provided 
<stdint.h>.  We should add a test case for this to RTEMS.

2. There are locking changes for FILE streams.

There is also an open PR in this area:

https://www.rtems.org/bugzilla/show_bug.cgi?id=1247

The file locking change was introduced in Newlib with with change:

commit 202f892065b93ffd91e9b6c0521c87758fae1d51
Author: Corinna Vinschen <corinna at vinschen.de>
Date:   Wed May 30 08:58:41 2012 +0000

     	* libc/stdio/local.h (_newlib_flockfile_start): New macro to
     	secure stream related critical section against thread cancellation.
     	(_newlib_flockfile_exit): Ditto.
     	(_newlib_sfp_lock_end): Ditto.
     	(_newlib_sfp_lock_start): Ditto for the list of streams.
     	(_newlib_sfp_lock_exit): Ditto.
     	(_newlib_sfp_lock_end): Ditto.
     	Use aforementioned macros in place of _flockfile/_funlockfile
     	and __sfp_lock_acquire/__sfp_lock_release throughout the code.
     	* libc/stdio/fclose.c: Explicitely disable and re-enable thread
     	cancellation.  Explain why.
     	* libc/stdio/freopen.c: Ditto.
     	* libc/stdio64/freopen64.c: Ditto.

Here we have (partial patch content):

diff --git a/newlib/libc/stdio/local.h b/newlib/libc/stdio/local.h
index 187f7d8..aeeb0e8 100644
--- a/newlib/libc/stdio/local.h
+++ b/newlib/libc/stdio/local.h
@@ -32,6 +32,83 @@
  # include <io.h>
  #endif

+/* The following macros are supposed to replace calls to _flockfile/_funlockfile
+   and __sfp_lock_acquire/__sfp_lock_release.  In case of multi-threaded
+   environments using pthreads, it's not sufficient to lock the stdio functions
+   against concurrent threads accessing the same data, the locking must also be
+   secured against thread cancellation.
+
+   The below macros have to be used in pairs.  The _newlib_XXX_start macro
+   starts with a opening curly brace, the _newlib_XXX_end macro ends with a
+   closing curly brace, so the start macro and the end macro mark the code
+   start and end of a critical section.  In case the code leaves the critical
+   section before reaching the end of the critical section's code end, use
+   the appropriate _newlib_XXX_exit macro. */
+
+#if !defined (__SINGLE_THREAD__) && defined (_POSIX_THREADS)
+#include <pthread.h>
+
+/* Start a stream oriented critical section: */
+# define _newlib_flockfile_start(_fp) \
+	{ \
+	  int __oldfpcancel; \
+	  pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, &__oldfpcancel); \
+	  _flockfile (_fp)
+
+/* Exit from a stream oriented critical section prematurely: */
+# define _newlib_flockfile_exit(_fp) \
+	  _funlockfile (_fp); \
+	  pthread_setcancelstate (__oldfpcancel, &__oldfpcancel);
+
+/* End a stream oriented critical section: */
+# define _newlib_flockfile_end(_fp) \
+	  _funlockfile (_fp); \
+	  pthread_setcancelstate (__oldfpcancel, &__oldfpcancel); \
+	}
+
+/* Start a stream list oriented critical section: */
+# define _newlib_sfp_lock_start() \
+	{ \
+	  int __oldsfpcancel; \
+	  pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, &__oldsfpcancel); \
+	  __sfp_lock_acquire ()
+
+/* Exit from a stream list oriented critical section prematurely: */
+# define _newlib_sfp_lock_exit() \
+	  __sfp_lock_release (); \
+	  pthread_setcancelstate (__oldsfpcancel, &__oldsfpcancel);
+
+/* End a stream list oriented critical section: */
+# define _newlib_sfp_lock_end() \
+	  __sfp_lock_release (); \
+	  pthread_setcancelstate (__oldsfpcancel, &__oldsfpcancel); \
+	}
+
+#else /* __SINGLE_THREAD__ || !_POSIX_THREADS */
+
+# define _newlib_flockfile_start(_fp) \
+	{ \
+		_flockfile(_fp)
+
+# define _newlib_flockfile_exit(_fp) \
+		_funlockfile(_fp); \
+
+# define _newlib_flockfile_end(_fp) \
+		_funlockfile(_fp); \
+	}
+
+# define _newlib_sfp_lock_start() \
+	{ \
+		__sfp_lock_acquire ()
+
+# define _newlib_sfp_lock_end() \
+		__sfp_lock_release ();
+
+# define _newlib_sfp_lock_end() \
+		__sfp_lock_release (); \
+	}
+
+#endif /* !__SINGLE_THREAD__ && _POSIX_THREADS */

RTEMS defines _POSIX_THREADS in Newlibs <sys/features.h>.  The problem that in 
fact pthread support is a configure option.  I don't think we need thread 
cancellation support for FILE streams.  Thus we should find a way to restore 
the previous behaviour.

>
> If you had a look into my newlib-patches, you'd have noticed that I reverted
> this change.

Sorry, but the newlib-1.20.0-rtems4.11-20121011.diff patch has 2418216 bytes so 
I didn't consider that.

With this patch everything works well.

-- 
Sebastian Huber, embedded brains GmbH

Address : Obere Lagerstr. 30, D-82178 Puchheim, Germany
Phone   : +49 89 18 90 80 79-6
Fax     : +49 89 18 90 80 79-9
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.