This is the mail archive of the newlib@sources.redhat.com mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Queries concerning RTOS protection in newllib


Hi Corinna,

Which part of the patch was causing the problem ? Was it just the introduction of _global_impure_ptr ? I recently discovered a problem which was exposed by this patch when the _cleanup_r function was changed from walking the FILE object list with fflush() to fclose(). I wonder if this is the cause of the unexpected truncation.

Note that I have also submitted another patch, http://sources.redhat.com/ml/newlib/2004/msg00302.html, which changes the effect of _fwalk (called via _reclaim_reent) to only walk the FILE objects in current task's reent structure and not the global reentrancy structure (aka _GLOBAL_REENT), which is handled via the exit() function.

Since the problem seems to be with the capture of stdout, I wonder if the problem is related to the fact that the standard i/o handles (0, 1, 2) are shared by all tasks and so when _reclaim_reent is called it will be closing these shared handles and thus, unless protected in some way, i/o on these handles by other tasks will be lost. In our system the implementation of _close (actually _close_r) prevents the closure of the standard handles, e.g.:

int
_close_r (ptr, fd)
     struct _reent *ptr;
     int fd;
{
  /*
   * Since the stdin, stdout & stderr file handles
   * are shared by all tasks, we do not want to allow
   * any one task to close them - hence the following
   * filter.
   */
  if ((fd != __sfileno(_stdin_r(ptr))) &&
      (fd != __sfileno(_stdout_r(ptr))) &&
      (fd != __sfileno(_stderr_r(ptr))))
  {
    return _SH_posix_Close (fd);
  }
  else
  {
    return -1;
  }
}

This is not very subtle but at least it is safe. I suspect that this solution is not applicable to Cygwin since it will prevent the re-use of the standard handles via open/dup.

If your problems are related to "walking" with fclose instead of fflush then perhaps what is needed is a method by which the implementation of _cleanup_r can be configured to select the function to "walk" over the FILE object list. This option could be set by a config file or through configure, e.g. (in libc/stdio/findfp.c):

#ifndef _STDIO_CLEANUP_FUNCTION
#define _STDIO_CLEANUP_FUNCTION fflush	/* `cheating' */
#endif

_VOID
_DEFUN(_cleanup_r, (ptr),
       struct _reent *ptr)
{
  _CAST_VOID _fwalk (ptr, _STDIO_CLEANUP_FUNCTION);
}

The above approach would restore the original behaviour (i.e. flushing the stdio streams) and would also allow other systems to select their desired behaviour.

I have also had a quick look in the cygwin sources (1.5.9-1) and the use of _REENT, _GLOBAL_REENT and _impure_ptr is a little tricky to follow given what Cygwin is doing when handling forking, shared libraries and thread local storage. If Cygwin is overriding libc/reent/impure.c (which it seems to be doing in winsup/cygwin/lib/_cygwin_crt0_common.cc) then in addition to the above change (in _cleanup_r) perhaps the definition of _GLOBAL_REENT can also be conditional on whether newlib is being targeted for Cygwin, e.g. (in libc/include/sys/reent.h):

#ifdef __CYGWIN__
#define _GLOBAL_REENT _impure_ptr
#else
#define _GLOBAL_REENT _global_impure_ptr
#endif

which may restore the behaviour you need. Alternatively as well as manipulating _impure_ptr Cygwin can manipulate _global_impure_ptr as well.

Cheers,

Antony.

Corinna Vinschen wrote:
I just found that the below patch broke Cygwin.  While running a build
in bison, I got an error message that I'm using an old version of m4.
Which wasn't true.  The configure script asks for a specific string in
the --help output of m4, roughly

  case `m4 --help` in
  *reload-state*) ...
  esac

The reason that this doesn't work anymore is that the output of the
backquoted command is truncated.  I found that Anthony's patch is the
culprit.  If I back it out, Cygwin works fine again.

The reason seem to be the invention of the _global_impure_ptr which
then in turn is used in _GLOBAL_REENT.  Cygwin already implements
changing _impure_ptr in case of context switches and it relies on
this functionality.

The below described patch apparently breaks backward compatibility.

However, I'm not fluent enough in the impure_ptr business to offer
the "right" solution except for backing out this patch again.


Corinna



On Jun 11 16:44, Jeff Johnston wrote:


Thanks Antony. Patch checked in. A patch was also required to get x86-linux building with these changes so I added that as well. In the future, it would be helpful to me if you could add a ChangeLog entry for any changes you make.

-- Jeff J.

Antony KING wrote:

Hi,

I am currently in the process of adapting newlib co-operate with an ST
developed embedded RTOS. FYI this RTOS is pretty much standard in terms
of capability (e.g. tasks, mutexes, semaphores, interrupt handlers) and
an application is RTOS enabled by simply linking in a static library and
using the API. The target is an SH4 based device.

My goal in adapting newlib for this RTOS is that the same static C
library can be used in applications which either use the RTOS library or
do not. Also, I would like to reduce the impact of changes I need to
make to newlib. As a result I have provided my own implementation of
sys/lock.h which implements the locking API as stub functions (instead
of empty macros) which will be overridden when a application is linked
against the RTOS static library. Also, I am aiming to get the RTOS to
switch the task re-entrancy structures by modifying _impure_ptr at each
context switch (instead of using a __getreent() function).

Enough of the background, now my queries and solutions:

1) There is a macro _GLOBAL_REENT which is used to obtain the global
re-entrancy structure used for managing the atexit handlers and FILE
objects. This is currently defined to be _impure_ptr which of course is
incompatible with changing _impure_ptr on each context switch.

My solution is to add a new variable, _global_impure_ptr, to impure.c
which initially has the same value as _impure_ptr and is the value
returned by _GLOBAL_REENT. The value of this variable should never change.

Files affected: impure.c, sys/reent.h

2) Given that I am providing an alternative implementation of sys/lock.h
the current type definition for the FILE lock (_flock_t) is incorrect,
especially in light of the explicit casting (and de-referencing) to
_LOCK_RECURSIVE_T in a number of files when invoking the locking API.
The reason is that the definition of _LOCK_RECURSIVE_T may not be the
same size as the current type (int) for _flock_t.

I think _flock_t should be redefined to be of type _LOCK_RECURSIVE_T
(instead of int) and all the explicit casting be removed.

Files affected: sys/_types.h, fclose.h findfp.c fopen.c, freopen.c,
fopen64.c, vfprintf.c

3) It would be useful if the stub implementations of the specialised
locking APIs __malloc_lock/unlock and __env_lock/unlock use the locking
API defined in sys/lock.h. This would then mean that these functions
would not need overriding by an RTOS if sys/lock.h is RTOS ready.
(Actually I do have to override the malloc lock API since I need to
pre-allocate the malloc lock object under the control of the RTOS since
the type is opaque and cannot be fully initialised statically).

Files affected: envlock.c, mlock.c.

Similarly it would also be useful for the stdio file locking API
_flockfile/_funlockfile to be modified in the same way.

Files affected: sys/stdio.h.

I have attached a patch file incorporating all the changes I have made
to the newlib library (against a CVS snapshot dated 03 June 2004). The
implementation of the locking API (lock.h and lock.c) is also attached
for reference.

Please note that the patch includes some additional changes in the file
sys/_reent.h where the re-entrancy structure initialisation macros have
been slightly re-worked to make them more readable (for me :-), complete
(I hope) and efficient (greater use of memset). Also the implementation
of _cleanup_r has been altered so that fclose is called instead of
fflush when walking the FILE object list.

Comments please (and sorry for the overlong email).

Cheers,

Antony.

-- ----------------------------------------------------------------- Antony King | Email: antony.king@st.com STMicroelectronics (R&D) Ltd. | Bristol, BS32 4SQ. United Kingdom. | Tel: +44 (0)1454 462646


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]