From owner-cvs-all@FreeBSD.ORG Sat Oct 29 04:30:00 2005 Return-Path: X-Original-To: cvs-all@freebsd.org Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 27BCD16A41F; Sat, 29 Oct 2005 04:30:00 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailout2.pacific.net.au (mailout2.pacific.net.au [61.8.0.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7430143D46; Sat, 29 Oct 2005 04:29:59 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy2.pacific.net.au (mailproxy2.pacific.net.au [61.8.0.87]) by mailout2.pacific.net.au (8.13.4/8.13.4/Debian-3) with ESMTP id j9T4TwbZ007401; Sat, 29 Oct 2005 14:29:58 +1000 Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailproxy2.pacific.net.au (8.13.4/8.13.4/Debian-3) with ESMTP id j9T4TtDZ028231; Sat, 29 Oct 2005 14:29:56 +1000 Date: Sat, 29 Oct 2005 14:29:55 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: John Baldwin In-Reply-To: <200510281033.48001.jhb@freebsd.org> Message-ID: <20051029140403.O30901@delplex.bde.org> References: <200510281045.j9SAjJmR096150@repoman.freebsd.org> <200510281033.48001.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: cvs-src@freebsd.org, Stefan Farfeleder , src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/bin/sh memalloc.c memalloc.h miscbltin.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 29 Oct 2005 04:30:00 -0000 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