cvs commit: src/bin/sh memalloc.c memalloc.h miscbltin.c

[Bruce Evans]

On Fri, 28 Oct 2005, John Baldwin wrote:

> On Friday 28 October 2005 06:45 am, Stefan Farfeleder wrote:
>> stefanf     2005-10-28 10:45:19 UTC
>>
>>   FreeBSD src repository
>>
>>   Modified files:
>>     bin/sh               memalloc.c memalloc.h miscbltin.c
>>   Log:
>>   Protect malloc, realloc and free calls with INT{ON,OFF} directly in
>> chkalloc, ckrealloc and ckfree (added), respectively.  sh jumps out of the
>> signal handler using longjmp which is obviously a bad idea during malloc
>> calls.
>>
>>   Note: I think there is still a small race here because volatile
>> sig_atomic_t only guarantees atomic reads and writes while we're doing
>> increments and decrements.

Stefan should know that only atomic writes are guaranteed, but there seems
to be no problem in practice since the inc/dec is not done in signal handlers
(never?) and atomic reads work in practice.  From n869.txt:

%        [#2] The type defined is
% 
%                sig_atomic_t
% 
%        which  is  the (possibly volatile-qualified) integer type of
%        an object that can be accessed as an atomic entity, even  in
                                ^^^^^^^^
This may require atomic reads to work...

%        the presence of asynchronous interrupts.
% 
%        [#5]  If  the  signal  occurs  other  than  as the result of
%        calling  the  abort  or  raise  function,  the  behavior  is
%        undefined  if  the  signal handler refers to any object with
                                             ^^^^^^^^^
%        static storage duration other than by assigning a  value  to
                                                ^^^^^^^^^
%        an  object  declared as volatile sig_atomic_t, or the signal

...but I think this has precedence.

sh (ab)uses "volatile sig_atomic_t" for exactly the opposite of what
it is for.  Normally signal handlers set a flag and the flag must be
volatile so that non-signal-handling code sees it changing, but in
sh it is non-signal-handling-code that sets the flag and signal-handling
code that reads the flag.  The standard doesn't cover this, except to
say that the reference to the flag in the signal handler gives undefined
behaviour since the reference is not an assignment.  Volatility makes no
difference as far as the standard says -- the behaviour is still undefined.
Volatility may be needed in theory but not in practice since there is
s sequence point after the assignment in INTOFF and compilers can't defer
assignments to globals for very long since they don't know if other functions
access the globals (especially if the accesses are for undefined behaviour
in signal handlers).

>>   Protect a setmode call with INT{ON,OFF} as it calls malloc internally.
>>
>>   PR:             45478
>>   Patch from:     Nate Eldredge
>
> If you are just doing a simple reference count you can use an int and use
> either the refcount API from sys/refcount.h or the atomic_foo_int()
> operations directly from machine/atomic.h.  Those should all work fine in
> userland.

I think this is not needed in sh, since it doesn't matter if a signal
handler sees an in-between count.

Bruce