From owner-freebsd-threads@FreeBSD.ORG Mon Jan 4 11:07:07 2010 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 74311106568B for ; Mon, 4 Jan 2010 11:07:07 +0000 (UTC) (envelope-from owner-bugmaster@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 596FE8FC1A for ; Mon, 4 Jan 2010 11:07:07 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id o04B77p6065039 for ; Mon, 4 Jan 2010 11:07:07 GMT (envelope-from owner-bugmaster@FreeBSD.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id o04B76oO065037 for freebsd-threads@FreeBSD.org; Mon, 4 Jan 2010 11:07:06 GMT (envelope-from owner-bugmaster@FreeBSD.org) Date: Mon, 4 Jan 2010 11:07:06 GMT Message-Id: <201001041107.o04B76oO065037@freefall.freebsd.org> X-Authentication-Warning: freefall.freebsd.org: gnats set sender to owner-bugmaster@FreeBSD.org using -f From: FreeBSD bugmaster To: freebsd-threads@FreeBSD.org Cc: Subject: Current problem reports assigned to freebsd-threads@FreeBSD.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Jan 2010 11:07:07 -0000 Note: to view an individual PR, use: http://www.freebsd.org/cgi/query-pr.cgi?pr=(number). The following is a listing of current problems submitted by FreeBSD users. These represent problem reports covering all versions including experimental development code and obsolete releases. S Tracker Resp. Description -------------------------------------------------------------------------------- o threa/141721 threads rtprio(1): (id|rt)prio priority resets when new thread o threa/141198 threads [libc] src/lib/libc/stdio does not properly initialize o threa/136345 threads Recursive read rwlocks in thread A cause deadlock with o threa/135673 threads databases/mysql50-server - MySQL query lock-ups on 7.2 p threa/135462 threads [PATCH] _thread_cleanupspecific() doesn't handle delet o threa/133734 threads 32 bit libthr failing pthread_create() o threa/128922 threads threads hang with xorg running o threa/127225 threads bug in lib/libthr/thread/thr_init.c o threa/122923 threads 'nice' does not prevent background process from steali o threa/121336 threads lang/neko threading ok on UP, broken on SMP (FreeBSD 7 o threa/118715 threads kse problem o threa/116668 threads can no longer use jdk15 with libthr on -stable SMP o threa/116181 threads /dev/io-related io access permissions are not propagat o threa/115211 threads pthread_atfork misbehaves in initial thread o threa/110636 threads [request] gdb(1): using gdb with multi thread applicat o threa/110306 threads apache 2.0 segmentation violation when calling gethost o threa/103975 threads Implicit loading/unloading of libpthread.so may crash o threa/101323 threads [patch] fork(2) in threaded programs broken. s threa/100815 threads FBSD 5.5 broke nanosleep in libc_r s threa/94467 threads send(), sendto() and sendmsg() are not correct in libc s threa/84483 threads problems with devel/nspr and -lc_r on 4.x o threa/83914 threads [libc] popen() doesn't work in static threaded program o threa/80992 threads abort() sometimes not caught by gdb depending on threa o threa/80435 threads panic on high loads o threa/79887 threads [patch] freopen() isn't thread-safe o threa/79683 threads svctcp_create() fails if multiple threads call at the s threa/76694 threads fork cause hang in dup()/close() function in child (-l s threa/76690 threads fork hang in child for -lc_r o threa/75374 threads pthread_kill() ignores SA_SIGINFO flag o threa/75273 threads FBSD 5.3 libpthread (KSE) bug o threa/72953 threads fork() unblocks blocked signals w/o PTHREAD_SCOPE_SYST o threa/70975 threads [sysvipc] unexpected and unreliable behaviour when usi s threa/69020 threads pthreads library leaks _gc_mutex s threa/49087 threads Signals lost in programs linked with libc_r s threa/48856 threads Setting SIGCHLD to SIG_IGN still leaves zombies under s threa/40671 threads pthread_cancel doesn't remove thread from condition qu s threa/39922 threads [threads] [patch] Threaded applications executed with s threa/37676 threads libc_r: msgsnd(), msgrcv(), pread(), pwrite() need wra s threa/34536 threads accept() blocks other threads s threa/32295 threads [libc_r] [patch] pthread(3) dont dequeue signals s threa/30464 threads pthread mutex attributes -- pshared s threa/24632 threads libc_r delicate deviation from libc in handling SIGCHL s threa/24472 threads libc_r does not honor SO_SNDTIMEO/SO_RCVTIMEO socket o 43 problems total. From owner-freebsd-threads@FreeBSD.ORG Tue Jan 5 06:48:21 2010 Return-Path: Delivered-To: threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 51B7E1065676; Tue, 5 Jan 2010 06:48:21 +0000 (UTC) (envelope-from davidxu@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 4342B8FC13; Tue, 5 Jan 2010 06:48:21 +0000 (UTC) Received: from apple.my.domain (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id o056mIpo017411; Tue, 5 Jan 2010 06:48:19 GMT (envelope-from davidxu@freebsd.org) Message-ID: <4B42E0B2.4010507@freebsd.org> Date: Tue, 05 Jan 2010 14:48:18 +0800 From: David Xu User-Agent: Thunderbird 2.0.0.9 (X11/20080612) MIME-Version: 1.0 To: threads@freebsd.org, Daniel Eischen Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Subject: Underline version of semaphore functions X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Jan 2010 06:48:21 -0000 I saw underline version of semaphore functions in libc/include/namespace.h: #define sem_close _sem_close #define sem_destroy _sem_destroy #define sem_getvalue _sem_getvalue #define sem_init _sem_init #define sem_open _sem_open #define sem_post _sem_post #define sem_timedwait _sem_timedwait #define sem_trywait _sem_trywait #define sem_unlink _sem_unlink #define sem_wait _sem_wait but libc never has them: pe6300:~> readelf -s /lib/libc.so.7 | grep _sem 35: 000459d4 0 FUNC GLOBAL DEFAULT 10 __sys_semsys@@FBSDprivate_1.0 154: 000457d4 0 FUNC WEAK DEFAULT 10 ___semctl@@FBSDprivate_1.0 161: 0003a5e0 81 FUNC GLOBAL DEFAULT 10 __sem_wait@@FBSDprivate_1.0 814: 0003a2e0 232 FUNC GLOBAL DEFAULT 10 __sem_destroy@@FBSDprivate_1.0 850: 000457d4 0 FUNC WEAK DEFAULT 10 __semctl@@FBSD_1.0 903: 0003a0c0 418 FUNC GLOBAL DEFAULT 10 __sem_open@@FBSDprivate_1.0 1005: 00039f00 35 FUNC GLOBAL DEFAULT 10 __sem_unlink@@FBSDprivate_1.0 1173: 000457b4 0 FUNC GLOBAL DEFAULT 10 __sys_semget@@FBSDprivate_1.0 1451: 0003a270 108 FUNC GLOBAL DEFAULT 10 __sem_init@@FBSDprivate_1.0 1520: 00039f80 183 FUNC GLOBAL DEFAULT 10 __sem_close@@FBSDprivate_1.0 1578: 0003a3d0 147 FUNC GLOBAL DEFAULT 10 __sem_getvalue@@FBSDprivate_1.0 1614: 0003a530 164 FUNC GLOBAL DEFAULT 10 __sem_trywait@@FBSDprivate_1.0 2152: 0003a470 81 FUNC GLOBAL DEFAULT 10 __sem_post@@FBSDprivate_1.0 2177: 00045794 0 FUNC GLOBAL DEFAULT 10 __sys_semop@@FBSDprivate_1.0 2323: 00045794 0 FUNC WEAK DEFAULT 10 _semop@@FBSDprivate_1.0 2452: 000457d4 0 FUNC GLOBAL DEFAULT 10 __sys___semctl@@FBSDprivate_1.0 2455: 000457b4 0 FUNC WEAK DEFAULT 10 _semget@@FBSDprivate_1.0 2511: 0003a4d0 88 FUNC GLOBAL DEFAULT 10 __sem_timedwait@@FBSDprivate_1.0 2543: 000459d4 0 FUNC WEAK DEFAULT 10 _semsys@@FBSDprivate_1.0 however libthr provided them: pe6300:~> readelf -s /lib/libthr.so.3 | grep _sem 46: 000062d0 326 FUNC GLOBAL DEFAULT 10 _sem_timedwait@@FBSDprivate_1.0 148: 00006420 244 FUNC GLOBAL DEFAULT 10 _sem_wait@@FBSDprivate_1.0 192: 00006250 118 FUNC GLOBAL DEFAULT 10 _sem_destroy@@FBSDprivate_1.0 206: 00006060 107 FUNC GLOBAL DEFAULT 10 _sem_getvalue@@FBSDprivate_1.0 252: 000060d0 210 FUNC GLOBAL DEFAULT 10 _sem_init@@FBSDprivate_1.0 303: 000061b0 153 FUNC GLOBAL DEFAULT 10 _sem_post@@FBSDprivate_1.0 354: 00006520 146 FUNC GLOBAL DEFAULT 10 _sem_trywait@@FBSDprivate_1.0 From owner-freebsd-threads@FreeBSD.ORG Tue Jan 5 14:37:04 2010 Return-Path: Delivered-To: threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D5D6E1065696; Tue, 5 Jan 2010 14:37:04 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from mail.netplex.net (mail.netplex.net [204.213.176.10]) by mx1.freebsd.org (Postfix) with ESMTP id 970808FC2C; Tue, 5 Jan 2010 14:37:04 +0000 (UTC) Received: from sea.ntplx.net (sea.ntplx.net [204.213.176.11]) by mail.netplex.net (8.14.3/8.14.3/NETPLEX) with ESMTP id o05Eb21c011123; Tue, 5 Jan 2010 09:37:03 -0500 (EST) X-Virus-Scanned: by AMaViS and Clam AntiVirus (mail.netplex.net) X-Greylist: Message whitelisted by DRAC access database, not delayed by milter-greylist-4.2.2 (mail.netplex.net [204.213.176.10]); Tue, 05 Jan 2010 09:37:03 -0500 (EST) Date: Tue, 5 Jan 2010 09:37:02 -0500 (EST) From: Daniel Eischen X-X-Sender: eischen@sea.ntplx.net To: David Xu In-Reply-To: <4B42E0B2.4010507@freebsd.org> Message-ID: References: <4B42E0B2.4010507@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: threads@freebsd.org Subject: Re: Underline version of semaphore functions X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Daniel Eischen List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Jan 2010 14:37:04 -0000 On Tue, 5 Jan 2010, David Xu wrote: > I saw underline version of semaphore functions in > libc/include/namespace.h: > > #define sem_close _sem_close > #define sem_destroy _sem_destroy > #define sem_getvalue _sem_getvalue > #define sem_init _sem_init > #define sem_open _sem_open > #define sem_post _sem_post > #define sem_timedwait _sem_timedwait > #define sem_trywait _sem_trywait > #define sem_unlink _sem_unlink > #define sem_wait _sem_wait > > > but libc never has them: Yes, because libpthread has them and the thread library was needed in order to provide semaphores. These single underscore functions are for libc internal usage so that libthr (libpthread) may know the difference between semaphores created/used by libc and those created/used by an application. The provider of semaphores (and mutexes, condvars, etc) may need to know who created and uses these objects (libc or application), so for example on a fork() or perhaps thread_safe_fork() all those objects created by libc can be automatically reinitialized. Or perhaps so that we know whether to delay thread cancellation until a thread releases these internal locks. -- DE From owner-freebsd-threads@FreeBSD.ORG Wed Jan 6 21:25:27 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 13F7E106568F; Wed, 6 Jan 2010 21:25:27 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id C78BD8FC26; Wed, 6 Jan 2010 21:25:26 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 64B0746B51; Wed, 6 Jan 2010 16:25:26 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 9E18D8A01F; Wed, 6 Jan 2010 16:25:25 -0500 (EST) From: John Baldwin To: freebsd-threads@freebsd.org Date: Wed, 6 Jan 2010 16:00:47 -0500 User-Agent: KMail/1.12.1 (FreeBSD/7.2-CBSD-20091103; KDE/4.3.1; amd64; ; ) References: <200912052034.nB5KYfaY000395@www.freebsd.org> <200912070850.37112.jhb@freebsd.org> In-Reply-To: <200912070850.37112.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201001061600.47628.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Wed, 06 Jan 2010 16:25:25 -0500 (EST) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-gnats-submit@freebsd.org, Jeremy Huddleston Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jan 2010 21:25:27 -0000 On Monday 07 December 2009 8:50:37 am John Baldwin wrote: > On Saturday 05 December 2009 3:34:41 pm Jeremy Huddleston wrote: > > > > >Number: 141198 > > >Category: threads > > >Synopsis: src/lib/libc/stdio does not properly initialize mutexes > > >Confidential: no > > >Severity: non-critical > > >Priority: low > > >Responsible: freebsd-threads > > >State: open > > >Quarter: > > >Keywords: > > >Date-Required: > > >Class: sw-bug > > >Submitter-Id: current-users > > >Arrival-Date: Sat Dec 05 20:40:01 UTC 2009 > > >Closed-Date: > > >Last-Modified: > > >Originator: Jeremy Huddleston > > >Release: 8.0 > > >Organization: > > Apple > > >Environment: > > NA > > >Description: > > libc/stdio assumes PTHREAD_MUTEX_INITIALIZER is NULL (which it is in > FreeBSD), but this makes the code not as portable. > > > > Earlier versions of stdio did properly initialize the lock to > PTHREAD_MUTEX_INITIALIZER in INITEXTRA() when it was part of the _extra > extension substructure. > > This is my fault. I suspect it was more an error of omission on my part than > assuming the default value of PTHREAD_MUTEX_INITIALIZER. :) Hmm, so I > reviewed the change that removed INITEXTRA() and all the places it was used to > construct a 'fake' FILE object it was passed to an internal stdio routine that > did not use locking. One thing I do see that is an old bug is that the three > static FILE structures used for stdin, stdout, and stderr have never had their > internal locks properly initialized. Also, it seems __sfp() never initializes > fp->_lock at all. Oh, it gets set to NULL via 'empty' in moreglue(). That is > also an old bug. I think this will fix those problems: > > Index: stdio/findfp.c > =================================================================== > --- stdio/findfp.c (revision 199969) > +++ stdio/findfp.c (working copy) > @@ -61,6 +61,7 @@ > ._read = __sread, \ > ._seek = __sseek, \ > ._write = __swrite, \ > + ._fl_mutex = PTHREAD_MUTEX_INITIALIZER, \ > } > /* the usual - (stdin + stdout + stderr) */ > static FILE usual[FOPEN_MAX - 3]; > @@ -96,7 +97,7 @@ > int n; > { > struct glue *g; > - static FILE empty; > + static FILE empty = { ._fl_mutex = PTHREAD_MUTEX_INITIALIZER }; > FILE *p; > > g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE)); > @@ -154,7 +155,7 @@ > fp->_ub._size = 0; > fp->_lb._base = NULL; /* no line buffer */ > fp->_lb._size = 0; > -/* fp->_lock = NULL; */ /* once set always set (reused) */ > +/* fp->_fl_mutex = NULL; */ /* once set always set (reused) */ > fp->_orientation = 0; > memset(&fp->_mbstate, 0, sizeof(mbstate_t)); > return (fp); Does this patch address the concerns? -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Wed Jan 6 21:30:09 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3F68B106568B for ; Wed, 6 Jan 2010 21:30:09 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 1588A8FC08 for ; Wed, 6 Jan 2010 21:30:09 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id o06LU81i010838 for ; Wed, 6 Jan 2010 21:30:08 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id o06LU87Q010835; Wed, 6 Jan 2010 21:30:08 GMT (envelope-from gnats) Date: Wed, 6 Jan 2010 21:30:08 GMT Message-Id: <201001062130.o06LU87Q010835@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: John Baldwin Cc: Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: John Baldwin List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jan 2010 21:30:09 -0000 The following reply was made to PR threads/141198; it has been noted by GNATS. From: John Baldwin To: freebsd-threads@freebsd.org Cc: freebsd-gnats-submit@freebsd.org, Jeremy Huddleston Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes Date: Wed, 6 Jan 2010 16:00:47 -0500 On Monday 07 December 2009 8:50:37 am John Baldwin wrote: > On Saturday 05 December 2009 3:34:41 pm Jeremy Huddleston wrote: > > > > >Number: 141198 > > >Category: threads > > >Synopsis: src/lib/libc/stdio does not properly initialize mutexes > > >Confidential: no > > >Severity: non-critical > > >Priority: low > > >Responsible: freebsd-threads > > >State: open > > >Quarter: > > >Keywords: > > >Date-Required: > > >Class: sw-bug > > >Submitter-Id: current-users > > >Arrival-Date: Sat Dec 05 20:40:01 UTC 2009 > > >Closed-Date: > > >Last-Modified: > > >Originator: Jeremy Huddleston > > >Release: 8.0 > > >Organization: > > Apple > > >Environment: > > NA > > >Description: > > libc/stdio assumes PTHREAD_MUTEX_INITIALIZER is NULL (which it is in > FreeBSD), but this makes the code not as portable. > > > > Earlier versions of stdio did properly initialize the lock to > PTHREAD_MUTEX_INITIALIZER in INITEXTRA() when it was part of the _extra > extension substructure. > > This is my fault. I suspect it was more an error of omission on my part than > assuming the default value of PTHREAD_MUTEX_INITIALIZER. :) Hmm, so I > reviewed the change that removed INITEXTRA() and all the places it was used to > construct a 'fake' FILE object it was passed to an internal stdio routine that > did not use locking. One thing I do see that is an old bug is that the three > static FILE structures used for stdin, stdout, and stderr have never had their > internal locks properly initialized. Also, it seems __sfp() never initializes > fp->_lock at all. Oh, it gets set to NULL via 'empty' in moreglue(). That is > also an old bug. I think this will fix those problems: > > Index: stdio/findfp.c > =================================================================== > --- stdio/findfp.c (revision 199969) > +++ stdio/findfp.c (working copy) > @@ -61,6 +61,7 @@ > ._read = __sread, \ > ._seek = __sseek, \ > ._write = __swrite, \ > + ._fl_mutex = PTHREAD_MUTEX_INITIALIZER, \ > } > /* the usual - (stdin + stdout + stderr) */ > static FILE usual[FOPEN_MAX - 3]; > @@ -96,7 +97,7 @@ > int n; > { > struct glue *g; > - static FILE empty; > + static FILE empty = { ._fl_mutex = PTHREAD_MUTEX_INITIALIZER }; > FILE *p; > > g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE)); > @@ -154,7 +155,7 @@ > fp->_ub._size = 0; > fp->_lb._base = NULL; /* no line buffer */ > fp->_lb._size = 0; > -/* fp->_lock = NULL; */ /* once set always set (reused) */ > +/* fp->_fl_mutex = NULL; */ /* once set always set (reused) */ > fp->_orientation = 0; > memset(&fp->_mbstate, 0, sizeof(mbstate_t)); > return (fp); Does this patch address the concerns? -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Thu Jan 7 00:19:52 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 43CDE1065732; Thu, 7 Jan 2010 00:19:52 +0000 (UTC) (envelope-from jeremyhu@apple.com) Received: from mail-out3.apple.com (mail-out3.apple.com [17.254.13.22]) by mx1.freebsd.org (Postfix) with ESMTP id 25C8F8FC20; Thu, 7 Jan 2010 00:19:52 +0000 (UTC) Received: from relay16.apple.com (relay16.apple.com [17.128.113.55]) by mail-out3.apple.com (Postfix) with ESMTP id 8EDAA7F3B677; Wed, 6 Jan 2010 16:00:15 -0800 (PST) X-AuditID: 11807137-b7bd4ae000000f0d-e7-4b45240e6aaf Received: from [17.151.90.43] (Unknown_Domain [17.151.90.43]) by relay16.apple.com (Apple SCV relay) with SMTP id 81.DF.03853.E04254B4; Wed, 6 Jan 2010 16:00:15 -0800 (PST) Mime-Version: 1.0 (Apple Message framework v1077) Content-Type: multipart/signed; boundary=Apple-Mail-17-1047373119; protocol="application/pkcs7-signature"; micalg=sha1 From: Jeremy Huddleston In-Reply-To: <201001061600.47628.jhb@freebsd.org> Date: Wed, 6 Jan 2010 19:00:14 -0500 Message-Id: <56AAD075-8202-40C0-AC75-18A5CDC3B76A@apple.com> References: <200912052034.nB5KYfaY000395@www.freebsd.org> <200912070850.37112.jhb@freebsd.org> <201001061600.47628.jhb@freebsd.org> To: John Baldwin X-Mailer: Apple Mail (2.1077) X-Brightmail-Tracker: AAAAAQAAAZE= X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Cc: freebsd-gnats-submit@freebsd.org, freebsd-threads@freebsd.org Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2010 00:19:52 -0000 --Apple-Mail-17-1047373119 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii I don't think that is sufficient. On Jan 6, 2010, at 16:00, John Baldwin wrote: > On Monday 07 December 2009 8:50:37 am John Baldwin wrote: >> On Saturday 05 December 2009 3:34:41 pm Jeremy Huddleston wrote: >>>=20 >>>> Number: 141198 >>>> Category: threads >>>> Synopsis: src/lib/libc/stdio does not properly initialize = mutexes >>>> Confidential: no >>>> Severity: non-critical >>>> Priority: low >>>> Responsible: freebsd-threads >>>> State: open >>>> Quarter: =20 >>>> Keywords: =20 >>>> Date-Required: >>>> Class: sw-bug >>>> Submitter-Id: current-users >>>> Arrival-Date: Sat Dec 05 20:40:01 UTC 2009 >>>> Closed-Date: >>>> Last-Modified: >>>> Originator: Jeremy Huddleston >>>> Release: 8.0 >>>> Organization: >>> Apple >>>> Environment: >>> NA >>>> Description: >>> libc/stdio assumes PTHREAD_MUTEX_INITIALIZER is NULL (which it is in=20= >> FreeBSD), but this makes the code not as portable. >>>=20 >>> Earlier versions of stdio did properly initialize the lock to=20 >> PTHREAD_MUTEX_INITIALIZER in INITEXTRA() when it was part of the = _extra=20 >> extension substructure. >>=20 >> This is my fault. I suspect it was more an error of omission on my = part than=20 >> assuming the default value of PTHREAD_MUTEX_INITIALIZER. :) Hmm, so = I=20 >> reviewed the change that removed INITEXTRA() and all the places it = was used to=20 >> construct a 'fake' FILE object it was passed to an internal stdio = routine that=20 >> did not use locking. One thing I do see that is an old bug is that = the three=20 >> static FILE structures used for stdin, stdout, and stderr have never = had their=20 >> internal locks properly initialized. Also, it seems __sfp() never = initializes=20 >> fp->_lock at all. Oh, it gets set to NULL via 'empty' in moreglue(). = That is=20 >> also an old bug. I think this will fix those problems: >>=20 >> Index: stdio/findfp.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- stdio/findfp.c (revision 199969) >> +++ stdio/findfp.c (working copy) >> @@ -61,6 +61,7 @@ >> ._read =3D __sread, \ >> ._seek =3D __sseek, \ >> ._write =3D __swrite, \ >> + ._fl_mutex =3D PTHREAD_MUTEX_INITIALIZER, \ >> } >> /* the usual - (stdin + stdout + stderr) = */ >> static FILE usual[FOPEN_MAX - 3]; >> @@ -96,7 +97,7 @@ >> int n; >> { >> struct glue *g; >> - static FILE empty; >> + static FILE empty =3D { ._fl_mutex =3D PTHREAD_MUTEX_INITIALIZER = }; >> FILE *p; >>=20 >> g =3D (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * = sizeof(FILE)); >> @@ -154,7 +155,7 @@ >> fp->_ub._size =3D 0; >> fp->_lb._base =3D NULL; /* no line buffer */ >> fp->_lb._size =3D 0; >> -/* fp->_lock =3D NULL; */ /* once set always set (reused) */ >> +/* fp->_fl_mutex =3D NULL; */ /* once set always set (reused) */ >> fp->_orientation =3D 0; >> memset(&fp->_mbstate, 0, sizeof(mbstate_t)); >> return (fp); >=20 > Does this patch address the concerns? I'm not sure if that is sufficient. I added it there and as part of = INITEXTRA (which we reverted to in darwin) in the following. I can = provide you with the full patches if you want, but there is a lot of = noise in them for our implementation of the _l variants and whatnot. I = think the following might not need to be initialized, but I did it for = good measure: vasprintf.c.patch:+ INITEXTRA(&f); vdprintf.c.patch:+ INITEXTRA(&f); vfprintf.c.patch:+ INITEXTRA(&fake); vfwprintf.c.patch:+ INITEXTRA(&fake); vsnprintf.c.patch:+ INITEXTRA(&f); vsprintf.c.patch:+ INITEXTRA(&f); vsscanf.c.patch:+ INITEXTRA(&f); vswprintf.c.patch:+ INITEXTRA(&f); vswscanf.c.patch:+ INITEXTRA(&f); --Apple-Mail-17-1047373119-- From owner-freebsd-threads@FreeBSD.ORG Thu Jan 7 00:20:03 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1C1A31065744 for ; Thu, 7 Jan 2010 00:20:03 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id E55798FC1E for ; Thu, 7 Jan 2010 00:20:02 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id o070K2Yh058563 for ; Thu, 7 Jan 2010 00:20:02 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id o070K2x2058562; Thu, 7 Jan 2010 00:20:02 GMT (envelope-from gnats) Date: Thu, 7 Jan 2010 00:20:02 GMT Message-Id: <201001070020.o070K2x2058562@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: Jeremy Huddleston Cc: Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Jeremy Huddleston List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2010 00:20:03 -0000 The following reply was made to PR threads/141198; it has been noted by GNATS. From: Jeremy Huddleston To: John Baldwin Cc: freebsd-threads@freebsd.org, freebsd-gnats-submit@freebsd.org Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes Date: Wed, 6 Jan 2010 19:00:14 -0500 --Apple-Mail-17-1047373119 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii I don't think that is sufficient. On Jan 6, 2010, at 16:00, John Baldwin wrote: > On Monday 07 December 2009 8:50:37 am John Baldwin wrote: >> On Saturday 05 December 2009 3:34:41 pm Jeremy Huddleston wrote: >>>=20 >>>> Number: 141198 >>>> Category: threads >>>> Synopsis: src/lib/libc/stdio does not properly initialize = mutexes >>>> Confidential: no >>>> Severity: non-critical >>>> Priority: low >>>> Responsible: freebsd-threads >>>> State: open >>>> Quarter: =20 >>>> Keywords: =20 >>>> Date-Required: >>>> Class: sw-bug >>>> Submitter-Id: current-users >>>> Arrival-Date: Sat Dec 05 20:40:01 UTC 2009 >>>> Closed-Date: >>>> Last-Modified: >>>> Originator: Jeremy Huddleston >>>> Release: 8.0 >>>> Organization: >>> Apple >>>> Environment: >>> NA >>>> Description: >>> libc/stdio assumes PTHREAD_MUTEX_INITIALIZER is NULL (which it is in=20= >> FreeBSD), but this makes the code not as portable. >>>=20 >>> Earlier versions of stdio did properly initialize the lock to=20 >> PTHREAD_MUTEX_INITIALIZER in INITEXTRA() when it was part of the = _extra=20 >> extension substructure. >>=20 >> This is my fault. I suspect it was more an error of omission on my = part than=20 >> assuming the default value of PTHREAD_MUTEX_INITIALIZER. :) Hmm, so = I=20 >> reviewed the change that removed INITEXTRA() and all the places it = was used to=20 >> construct a 'fake' FILE object it was passed to an internal stdio = routine that=20 >> did not use locking. One thing I do see that is an old bug is that = the three=20 >> static FILE structures used for stdin, stdout, and stderr have never = had their=20 >> internal locks properly initialized. Also, it seems __sfp() never = initializes=20 >> fp->_lock at all. Oh, it gets set to NULL via 'empty' in moreglue(). = That is=20 >> also an old bug. I think this will fix those problems: >>=20 >> Index: stdio/findfp.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- stdio/findfp.c (revision 199969) >> +++ stdio/findfp.c (working copy) >> @@ -61,6 +61,7 @@ >> ._read =3D __sread, \ >> ._seek =3D __sseek, \ >> ._write =3D __swrite, \ >> + ._fl_mutex =3D PTHREAD_MUTEX_INITIALIZER, \ >> } >> /* the usual - (stdin + stdout + stderr) = */ >> static FILE usual[FOPEN_MAX - 3]; >> @@ -96,7 +97,7 @@ >> int n; >> { >> struct glue *g; >> - static FILE empty; >> + static FILE empty =3D { ._fl_mutex =3D PTHREAD_MUTEX_INITIALIZER = }; >> FILE *p; >>=20 >> g =3D (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * = sizeof(FILE)); >> @@ -154,7 +155,7 @@ >> fp->_ub._size =3D 0; >> fp->_lb._base =3D NULL; /* no line buffer */ >> fp->_lb._size =3D 0; >> -/* fp->_lock =3D NULL; */ /* once set always set (reused) */ >> +/* fp->_fl_mutex =3D NULL; */ /* once set always set (reused) */ >> fp->_orientation =3D 0; >> memset(&fp->_mbstate, 0, sizeof(mbstate_t)); >> return (fp); >=20 > Does this patch address the concerns? I'm not sure if that is sufficient. I added it there and as part of = INITEXTRA (which we reverted to in darwin) in the following. I can = provide you with the full patches if you want, but there is a lot of = noise in them for our implementation of the _l variants and whatnot. I = think the following might not need to be initialized, but I did it for = good measure: vasprintf.c.patch:+ INITEXTRA(&f); vdprintf.c.patch:+ INITEXTRA(&f); vfprintf.c.patch:+ INITEXTRA(&fake); vfwprintf.c.patch:+ INITEXTRA(&fake); vsnprintf.c.patch:+ INITEXTRA(&f); vsprintf.c.patch:+ INITEXTRA(&f); vsscanf.c.patch:+ INITEXTRA(&f); vswprintf.c.patch:+ INITEXTRA(&f); vswscanf.c.patch:+ INITEXTRA(&f); --Apple-Mail-17-1047373119 Content-Disposition: attachment; filename=smime.p7s Content-Type: application/pkcs7-signature; name=smime.p7s Content-Transfer-Encoding: base64 MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAQAAoIITbjCCAz8w ggKooAMCAQICAQ0wDQYJKoZIhvcNAQEFBQAwgdExCzAJBgNVBAYTAlpBMRUwEwYDVQQIEwxXZXN0 ZXJuIENhcGUxEjAQBgNVBAcTCUNhcGUgVG93bjEaMBgGA1UEChMRVGhhd3RlIENvbnN1bHRpbmcx KDAmBgNVBAsTH0NlcnRpZmljYXRpb24gU2VydmljZXMgRGl2aXNpb24xJDAiBgNVBAMTG1RoYXd0 ZSBQZXJzb25hbCBGcmVlbWFpbCBDQTErMCkGCSqGSIb3DQEJARYccGVyc29uYWwtZnJlZW1haWxA dGhhd3RlLmNvbTAeFw0wMzA3MTcwMDAwMDBaFw0xMzA3MTYyMzU5NTlaMGIxCzAJBgNVBAYTAlpB MSUwIwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5KSBMdGQuMSwwKgYDVQQDEyNUaGF3dGUg UGVyc29uYWwgRnJlZW1haWwgSXNzdWluZyBDQTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA xKY8VXNV+065yplaHmjAdQRwnd/p/6Me7L3N9VvyGna9fww6YfK/Uc4B1OVQCjDXAmNaLIkVcI7d yfArhVqqP3FWy688Cwfn8R+RNiQqE88r1fOCdz0Dviv+uxg+B79AgAJk16emu59l0cUqVIUPSAR/ p7bRPGEEQB5kGXJgt/sCAwEAAaOBlDCBkTASBgNVHRMBAf8ECDAGAQH/AgEAMEMGA1UdHwQ8MDow OKA2oDSGMmh0dHA6Ly9jcmwudGhhd3RlLmNvbS9UaGF3dGVQZXJzb25hbEZyZWVtYWlsQ0EuY3Js MAsGA1UdDwQEAwIBBjApBgNVHREEIjAgpB4wHDEaMBgGA1UEAxMRUHJpdmF0ZUxhYmVsMi0xMzgw DQYJKoZIhvcNAQEFBQADgYEASIzRUIPqCy7MDaNmrGcPf6+svsIXoUOWlJ1/TCG4+DYfqi2fNi/A 9BxQIJNwPP2t4WFiw9k6GX6EsZkbAMUaC4J0niVQlGLH2ydxVyWN3amcOY6MIE9lX5Xa9/eH1sYI Tq726jTlEBpbNU1341YheILcIRk13iSx0x1G/11fZU8wggM/MIICqKADAgECAgENMA0GCSqGSIb3 DQEBBQUAMIHRMQswCQYDVQQGEwJaQTEVMBMGA1UECBMMV2VzdGVybiBDYXBlMRIwEAYDVQQHEwlD YXBlIFRvd24xGjAYBgNVBAoTEVRoYXd0ZSBDb25zdWx0aW5nMSgwJgYDVQQLEx9DZXJ0aWZpY2F0 aW9uIFNlcnZpY2VzIERpdmlzaW9uMSQwIgYDVQQDExtUaGF3dGUgUGVyc29uYWwgRnJlZW1haWwg Q0ExKzApBgkqhkiG9w0BCQEWHHBlcnNvbmFsLWZyZWVtYWlsQHRoYXd0ZS5jb20wHhcNMDMwNzE3 MDAwMDAwWhcNMTMwNzE2MjM1OTU5WjBiMQswCQYDVQQGEwJaQTElMCMGA1UEChMcVGhhd3RlIENv bnN1bHRpbmcgKFB0eSkgTHRkLjEsMCoGA1UEAxMjVGhhd3RlIFBlcnNvbmFsIEZyZWVtYWlsIElz c3VpbmcgQ0EwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMSmPFVzVftOucqZWh5owHUEcJ3f 6f+jHuy9zfVb8hp2vX8MOmHyv1HOAdTlUAow1wJjWiyJFXCO3cnwK4Vaqj9xVsuvPAsH5/EfkTYk KhPPK9Xzgnc9A74r/rsYPge/QIACZNenprufZdHFKlSFD0gEf6e20TxhBEAeZBlyYLf7AgMBAAGj gZQwgZEwEgYDVR0TAQH/BAgwBgEB/wIBADBDBgNVHR8EPDA6MDigNqA0hjJodHRwOi8vY3JsLnRo YXd0ZS5jb20vVGhhd3RlUGVyc29uYWxGcmVlbWFpbENBLmNybDALBgNVHQ8EBAMCAQYwKQYDVR0R BCIwIKQeMBwxGjAYBgNVBAMTEVByaXZhdGVMYWJlbDItMTM4MA0GCSqGSIb3DQEBBQUAA4GBAEiM 0VCD6gsuzA2jZqxnD3+vrL7CF6FDlpSdf0whuPg2H6otnzYvwPQcUCCTcDz9reFhYsPZOhl+hLGZ GwDFGguCdJ4lUJRix9sncVcljd2pnDmOjCBPZV+V2vf3h9bGCE6u9uo05RAaWzVNd+NWIXiC3CEZ Nd4ksdMdRv9dX2VPMIIGcDCCBdmgAwIBAgIQKF0Nr8sW2fhCBNsoUjwm8zANBgkqhkiG9w0BAQUF ADBiMQswCQYDVQQGEwJaQTElMCMGA1UEChMcVGhhd3RlIENvbnN1bHRpbmcgKFB0eSkgTHRkLjEs MCoGA1UEAxMjVGhhd3RlIFBlcnNvbmFsIEZyZWVtYWlsIElzc3VpbmcgQ0EwHhcNMDkwNTA0MDUy OTE0WhcNMTAwNTA0MDUyOTE0WjCCAnAxHzAdBgNVBAMTFlRoYXd0ZSBGcmVlbWFpbCBNZW1iZXIx JDAiBgkqhkiG9w0BCQEWFWplcmVteWh1QGJlcmtlbGV5LmVkdTErMCkGCSqGSIb3DQEJARYcamVy ZW15aHVAdWNsaW5rLmJlcmtlbGV5LmVkdTEsMCoGCSqGSIb3DQEJARYdamVyZW15aHVAdWNsaW5r NC5iZXJrZWxleS5lZHUxJzAlBgkqhkiG9w0BCQEWGGplcmVteWh1QGNzLmJlcmtlbGV5LmVkdTEp MCcGCSqGSIb3DQEJARYaamVyZW15QHVwZS5jcy5iZXJrZWxleS5lZHUxKTAnBgkqhkiG9w0BCQEW GmplcmVteWh1QGVlY3MuYmVya2VsZXkuZWR1MScwJQYJKoZIhvcNAQkBFhhqZXJlbXlodUBmcmVl ZGVza3RvcC5vcmcxJDAiBgkqhkiG9w0BCQEWFWplcmVteWh1QG1hY3BvcnRzLm9yZzElMCMGCSqG SIb3DQEJARYWamVyZW15QG91dGVyc3F1YXJlLm9yZzEgMB4GCSqGSIb3DQEJARYRamVyZW15aHVk QG1hYy5jb20xIzAhBgkqhkiG9w0BCQEWFGplcmVteUBodWRzY2FiaW4uY29tMSEwHwYJKoZIhvcN AQkBFhJqZXJlbXlodUBhcHBsZS5jb20xJTAjBgkqhkiG9w0BCQEWFmplcmVteUBvdXRlcnNxdWFy ZS5jb20xJTAjBgkqhkiG9w0BCQEWFnBheXBhbEBvdXRlcnNxdWFyZS5jb20xHzAdBgkqhkiG9w0B CQEWEGplcmVteWh1ZEBtZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCwVnJ8 XrKgByWkhJhQDk7Kj45PnZYRXJNQfcTyBQsqSqfUh13Limf2qJTxpw8Mdq/SuNkO3ZjLkaYGPB4+ 8uaHdDqGEanq2wf4qKV4dyFEQO92mRQRxLijfBS4CunlSYzHuPd6g5osI0BVpFbNRswqOXWbHd1z XRVvRqpvYKQJFWLf3dqXU3zZO2nv4sabnovbNCKEO6HrxQeawFfwxL20adsK5F1ejK1VRSEsTzd7 BjNs8QTWC4qZKrrNuaPJLVt4LDbRXIqOggrZaOkggIBIIdXubjOrrpR41PvcvibfvYLUpo3bdX5e tWH/VU/ywIS3oIc4d+VtOL/O3YdCpX0FAgMBAAGjggGRMIIBjTCCAXsGA1UdEQSCAXIwggFugRVq ZXJlbXlodUBiZXJrZWxleS5lZHWBHGplcmVteWh1QHVjbGluay5iZXJrZWxleS5lZHWBHWplcmVt eWh1QHVjbGluazQuYmVya2VsZXkuZWR1gRhqZXJlbXlodUBjcy5iZXJrZWxleS5lZHWBGmplcmVt eUB1cGUuY3MuYmVya2VsZXkuZWR1gRpqZXJlbXlodUBlZWNzLmJlcmtlbGV5LmVkdYEYamVyZW15 aHVAZnJlZWRlc2t0b3Aub3JngRVqZXJlbXlodUBtYWNwb3J0cy5vcmeBFmplcmVteUBvdXRlcnNx dWFyZS5vcmeBEWplcmVteWh1ZEBtYWMuY29tgRRqZXJlbXlAaHVkc2NhYmluLmNvbYESamVyZW15 aHVAYXBwbGUuY29tgRZqZXJlbXlAb3V0ZXJzcXVhcmUuY29tgRZwYXlwYWxAb3V0ZXJzcXVhcmUu Y29tgRBqZXJlbXlodWRAbWUuY29tMAwGA1UdEwEB/wQCMAAwDQYJKoZIhvcNAQEFBQADgYEAMtx6 voXn2w2+kaevSb7REuy5TBAQNzwlcwLiaC44HMVhwQGEYG544mBabCqY2+MtLbEn2RDQGHArtuCA Tv9liObLp6UPNKo+8Bcd3edN0dlFSeb0wFPVt71e05dGeyIoBxIrM4ix2BON/SHcGsgt3n1DRXen JLYVV809vRtHQpowggZwMIIF2aADAgECAhBfIA3CIvCJAyf8rsNvgxtuMA0GCSqGSIb3DQEBBQUA MGIxCzAJBgNVBAYTAlpBMSUwIwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5KSBMdGQuMSww KgYDVQQDEyNUaGF3dGUgUGVyc29uYWwgRnJlZW1haWwgSXNzdWluZyBDQTAeFw0wOTA5MTQyMTM2 MjdaFw0xMDA5MTQyMTM2MjdaMIICcDEfMB0GA1UEAxMWVGhhd3RlIEZyZWVtYWlsIE1lbWJlcjEk MCIGCSqGSIb3DQEJARYVamVyZW15aHVAYmVya2VsZXkuZWR1MSswKQYJKoZIhvcNAQkBFhxqZXJl bXlodUB1Y2xpbmsuYmVya2VsZXkuZWR1MSwwKgYJKoZIhvcNAQkBFh1qZXJlbXlodUB1Y2xpbms0 LmJlcmtlbGV5LmVkdTEnMCUGCSqGSIb3DQEJARYYamVyZW15aHVAY3MuYmVya2VsZXkuZWR1MSkw JwYJKoZIhvcNAQkBFhpqZXJlbXlAdXBlLmNzLmJlcmtlbGV5LmVkdTEpMCcGCSqGSIb3DQEJARYa amVyZW15aHVAZWVjcy5iZXJrZWxleS5lZHUxJzAlBgkqhkiG9w0BCQEWGGplcmVteWh1QGZyZWVk ZXNrdG9wLm9yZzEkMCIGCSqGSIb3DQEJARYVamVyZW15aHVAbWFjcG9ydHMub3JnMSUwIwYJKoZI hvcNAQkBFhZqZXJlbXlAb3V0ZXJzcXVhcmUub3JnMSAwHgYJKoZIhvcNAQkBFhFqZXJlbXlodWRA bWFjLmNvbTEjMCEGCSqGSIb3DQEJARYUamVyZW15QGh1ZHNjYWJpbi5jb20xITAfBgkqhkiG9w0B CQEWEmplcmVteWh1QGFwcGxlLmNvbTElMCMGCSqGSIb3DQEJARYWamVyZW15QG91dGVyc3F1YXJl LmNvbTElMCMGCSqGSIb3DQEJARYWcGF5cGFsQG91dGVyc3F1YXJlLmNvbTEfMB0GCSqGSIb3DQEJ ARYQamVyZW15aHVkQG1lLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL+c2RGH leO3G25PQEPEVsV3H/cWDewBCnMbqV0zgEg3hMyoRUG3aRUgH4gWbhVNkx/5t0A+mLQQWNnktg2J ku4MJJhHmarkxQAwITyamyO+37GHFl2d7oe5J7CFwg3Evf/2Lli0mfglfDHBy5YN9yURbSMVRaDV WGHhpYkqTwGXG2Bpai7oqdOlB0hDcRGE4Fv5aurxAuxyIohZMuxhZBzDfmidKsOUTnsz+NCUFIXK cMLYWwvH4XOBC4l0SU523phMyEW0OPas38EWd2NMCYaO1URA944+cS68DUvCqrrRzGmixY03PcaV uJ/+KA3L2u9esq8vt8s5m8aW8MWQWIkCAwEAAaOCAZEwggGNMIIBewYDVR0RBIIBcjCCAW6BFWpl cmVteWh1QGJlcmtlbGV5LmVkdYEcamVyZW15aHVAdWNsaW5rLmJlcmtlbGV5LmVkdYEdamVyZW15 aHVAdWNsaW5rNC5iZXJrZWxleS5lZHWBGGplcmVteWh1QGNzLmJlcmtlbGV5LmVkdYEaamVyZW15 QHVwZS5jcy5iZXJrZWxleS5lZHWBGmplcmVteWh1QGVlY3MuYmVya2VsZXkuZWR1gRhqZXJlbXlo dUBmcmVlZGVza3RvcC5vcmeBFWplcmVteWh1QG1hY3BvcnRzLm9yZ4EWamVyZW15QG91dGVyc3F1 YXJlLm9yZ4ERamVyZW15aHVkQG1hYy5jb22BFGplcmVteUBodWRzY2FiaW4uY29tgRJqZXJlbXlo dUBhcHBsZS5jb22BFmplcmVteUBvdXRlcnNxdWFyZS5jb22BFnBheXBhbEBvdXRlcnNxdWFyZS5j b22BEGplcmVteWh1ZEBtZS5jb20wDAYDVR0TAQH/BAIwADANBgkqhkiG9w0BAQUFAAOBgQBAga5a Jmkyd0TMiY0icyR7j5soyooiP4q9+Iu6lG+s/S+7vF5sDadCq+Y7US091MNT4LmbQehwwhi4jUWy EZ+KP9dhfWMqi51rZDbhWxAqAoKmgWgoQ9UsA4LqaC1wWlrM/DtzZ7+L5ZZ+MWlr94fDNL8qU3+y 3ZfiXgpWBV1x1zGCAxAwggMMAgEBMHYwYjELMAkGA1UEBhMCWkExJTAjBgNVBAoTHFRoYXd0ZSBD b25zdWx0aW5nIChQdHkpIEx0ZC4xLDAqBgNVBAMTI1RoYXd0ZSBQZXJzb25hbCBGcmVlbWFpbCBJ c3N1aW5nIENBAhBfIA3CIvCJAyf8rsNvgxtuMAkGBSsOAwIaBQCgggFvMBgGCSqGSIb3DQEJAzEL BgkqhkiG9w0BBwEwHAYJKoZIhvcNAQkFMQ8XDTEwMDEwNzAwMDAxNFowIwYJKoZIhvcNAQkEMRYE FIfHXVmHFAZiiIB/hCq4qHN7kGDpMIGFBgkrBgEEAYI3EAQxeDB2MGIxCzAJBgNVBAYTAlpBMSUw IwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5KSBMdGQuMSwwKgYDVQQDEyNUaGF3dGUgUGVy c29uYWwgRnJlZW1haWwgSXNzdWluZyBDQQIQKF0Nr8sW2fhCBNsoUjwm8zCBhwYLKoZIhvcNAQkQ AgsxeKB2MGIxCzAJBgNVBAYTAlpBMSUwIwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5KSBM dGQuMSwwKgYDVQQDEyNUaGF3dGUgUGVyc29uYWwgRnJlZW1haWwgSXNzdWluZyBDQQIQKF0Nr8sW 2fhCBNsoUjwm8zANBgkqhkiG9w0BAQEFAASCAQAWjecrp9GigiRDiXVZAwEGni/gPKQmpLDUmUkW NArC3njXUjN2PYQNbal6mwabaaLHnarR0yiy+IhVPDL8fVewqoEU4SbSc7VLIYMHqQNPRSjCjac6 QYrctm6VN003hXkZ0wXu4O7yPLb3XcSGVxWLwF9qBKGsQ9S68WK01+gov5EEeaXEayRQiRS1hFB8 Yt1UYfXdQ95WwjxJZqgnDaWSl0xrU0/u8iLrUF0ahuOBBJlqiaE/xhx5+5z/sVC3XU5ZFUhHsfhl x43Rn0b1qXSLxlhEbIhl+GD9DugJ5audBHCN68LpCcl+isd7U4QKwWeH/8CyOqOo7vwxQyrDPskc AAAAAAAA --Apple-Mail-17-1047373119-- From owner-freebsd-threads@FreeBSD.ORG Thu Jan 7 15:29:20 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6502F106568F; Thu, 7 Jan 2010 15:29:20 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 34ACA8FC24; Thu, 7 Jan 2010 15:29:20 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id DA0FB46B49; Thu, 7 Jan 2010 10:29:19 -0500 (EST) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 1553B8A01D; Thu, 7 Jan 2010 10:29:19 -0500 (EST) From: John Baldwin To: Jeremy Huddleston Date: Thu, 7 Jan 2010 09:56:25 -0500 User-Agent: KMail/1.12.1 (FreeBSD/7.2-CBSD-20091103; KDE/4.3.1; amd64; ; ) References: <200912052034.nB5KYfaY000395@www.freebsd.org> <201001061600.47628.jhb@freebsd.org> <56AAD075-8202-40C0-AC75-18A5CDC3B76A@apple.com> In-Reply-To: <56AAD075-8202-40C0-AC75-18A5CDC3B76A@apple.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201001070956.25906.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Thu, 07 Jan 2010 10:29:19 -0500 (EST) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-gnats-submit@freebsd.org, freebsd-threads@freebsd.org Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2010 15:29:20 -0000 On Wednesday 06 January 2010 7:00:14 pm Jeremy Huddleston wrote: > I don't think that is sufficient. > > On Jan 6, 2010, at 16:00, John Baldwin wrote: > > > On Monday 07 December 2009 8:50:37 am John Baldwin wrote: > >> On Saturday 05 December 2009 3:34:41 pm Jeremy Huddleston wrote: > >>> > >>>> Number: 141198 > >>>> Category: threads > >>>> Synopsis: src/lib/libc/stdio does not properly initialize mutexes > >>>> Confidential: no > >>>> Severity: non-critical > >>>> Priority: low > >>>> Responsible: freebsd-threads > >>>> State: open > >>>> Quarter: > >>>> Keywords: > >>>> Date-Required: > >>>> Class: sw-bug > >>>> Submitter-Id: current-users > >>>> Arrival-Date: Sat Dec 05 20:40:01 UTC 2009 > >>>> Closed-Date: > >>>> Last-Modified: > >>>> Originator: Jeremy Huddleston > >>>> Release: 8.0 > >>>> Organization: > >>> Apple > >>>> Environment: > >>> NA > >>>> Description: > >>> libc/stdio assumes PTHREAD_MUTEX_INITIALIZER is NULL (which it is in > >> FreeBSD), but this makes the code not as portable. > >>> > >>> Earlier versions of stdio did properly initialize the lock to > >> PTHREAD_MUTEX_INITIALIZER in INITEXTRA() when it was part of the _extra > >> extension substructure. > >> > >> This is my fault. I suspect it was more an error of omission on my part than > >> assuming the default value of PTHREAD_MUTEX_INITIALIZER. :) Hmm, so I > >> reviewed the change that removed INITEXTRA() and all the places it was used to > >> construct a 'fake' FILE object it was passed to an internal stdio routine that > >> did not use locking. One thing I do see that is an old bug is that the three > >> static FILE structures used for stdin, stdout, and stderr have never had their > >> internal locks properly initialized. Also, it seems __sfp() never initializes > >> fp->_lock at all. Oh, it gets set to NULL via 'empty' in moreglue(). That is > >> also an old bug. I think this will fix those problems: > >> > >> Index: stdio/findfp.c > >> =================================================================== > >> --- stdio/findfp.c (revision 199969) > >> +++ stdio/findfp.c (working copy) > >> @@ -61,6 +61,7 @@ > >> ._read = __sread, \ > >> ._seek = __sseek, \ > >> ._write = __swrite, \ > >> + ._fl_mutex = PTHREAD_MUTEX_INITIALIZER, \ > >> } > >> /* the usual - (stdin + stdout + stderr) */ > >> static FILE usual[FOPEN_MAX - 3]; > >> @@ -96,7 +97,7 @@ > >> int n; > >> { > >> struct glue *g; > >> - static FILE empty; > >> + static FILE empty = { ._fl_mutex = PTHREAD_MUTEX_INITIALIZER }; > >> FILE *p; > >> > >> g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE)); > >> @@ -154,7 +155,7 @@ > >> fp->_ub._size = 0; > >> fp->_lb._base = NULL; /* no line buffer */ > >> fp->_lb._size = 0; > >> -/* fp->_lock = NULL; */ /* once set always set (reused) */ > >> +/* fp->_fl_mutex = NULL; */ /* once set always set (reused) */ > >> fp->_orientation = 0; > >> memset(&fp->_mbstate, 0, sizeof(mbstate_t)); > >> return (fp); > > > > Does this patch address the concerns? > > I'm not sure if that is sufficient. I added it there and as part of INITEXTRA (which we reverted to in darwin) in the following. I can provide you with the full patches if you want, but there is a lot of noise in them for our implementation of the _l variants and whatnot. I think the following might not need to be initialized, but I did it for good measure: > > vasprintf.c.patch:+ INITEXTRA(&f); > vdprintf.c.patch:+ INITEXTRA(&f); > vfprintf.c.patch:+ INITEXTRA(&fake); > vfwprintf.c.patch:+ INITEXTRA(&fake); > vsnprintf.c.patch:+ INITEXTRA(&f); > vsprintf.c.patch:+ INITEXTRA(&f); > vsscanf.c.patch:+ INITEXTRA(&f); > vswprintf.c.patch:+ INITEXTRA(&f); > vswscanf.c.patch:+ INITEXTRA(&f); Ah, ok. In our stdio at least these are all dummy files that are passed to internal stdio routines that never do any locking (e.g. __vfprintf() which does no locking vs vfprintf() which does use the mutex locks). I'm not sure if that is also true for Darwin, but in theory it should be as these file objects are all private stack variables that no other threads can gain a reference to, so no locking is needed. -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Thu Jan 7 15:30:06 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 71FE0106568F for ; Thu, 7 Jan 2010 15:30:06 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 47BC88FC20 for ; Thu, 7 Jan 2010 15:30:06 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id o07FU6gT092422 for ; Thu, 7 Jan 2010 15:30:06 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id o07FU6tF092417; Thu, 7 Jan 2010 15:30:06 GMT (envelope-from gnats) Date: Thu, 7 Jan 2010 15:30:06 GMT Message-Id: <201001071530.o07FU6tF092417@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: John Baldwin Cc: Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: John Baldwin List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2010 15:30:06 -0000 The following reply was made to PR threads/141198; it has been noted by GNATS. From: John Baldwin To: Jeremy Huddleston Cc: freebsd-threads@freebsd.org, freebsd-gnats-submit@freebsd.org Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes Date: Thu, 7 Jan 2010 09:56:25 -0500 On Wednesday 06 January 2010 7:00:14 pm Jeremy Huddleston wrote: > I don't think that is sufficient. > > On Jan 6, 2010, at 16:00, John Baldwin wrote: > > > On Monday 07 December 2009 8:50:37 am John Baldwin wrote: > >> On Saturday 05 December 2009 3:34:41 pm Jeremy Huddleston wrote: > >>> > >>>> Number: 141198 > >>>> Category: threads > >>>> Synopsis: src/lib/libc/stdio does not properly initialize mutexes > >>>> Confidential: no > >>>> Severity: non-critical > >>>> Priority: low > >>>> Responsible: freebsd-threads > >>>> State: open > >>>> Quarter: > >>>> Keywords: > >>>> Date-Required: > >>>> Class: sw-bug > >>>> Submitter-Id: current-users > >>>> Arrival-Date: Sat Dec 05 20:40:01 UTC 2009 > >>>> Closed-Date: > >>>> Last-Modified: > >>>> Originator: Jeremy Huddleston > >>>> Release: 8.0 > >>>> Organization: > >>> Apple > >>>> Environment: > >>> NA > >>>> Description: > >>> libc/stdio assumes PTHREAD_MUTEX_INITIALIZER is NULL (which it is in > >> FreeBSD), but this makes the code not as portable. > >>> > >>> Earlier versions of stdio did properly initialize the lock to > >> PTHREAD_MUTEX_INITIALIZER in INITEXTRA() when it was part of the _extra > >> extension substructure. > >> > >> This is my fault. I suspect it was more an error of omission on my part than > >> assuming the default value of PTHREAD_MUTEX_INITIALIZER. :) Hmm, so I > >> reviewed the change that removed INITEXTRA() and all the places it was used to > >> construct a 'fake' FILE object it was passed to an internal stdio routine that > >> did not use locking. One thing I do see that is an old bug is that the three > >> static FILE structures used for stdin, stdout, and stderr have never had their > >> internal locks properly initialized. Also, it seems __sfp() never initializes > >> fp->_lock at all. Oh, it gets set to NULL via 'empty' in moreglue(). That is > >> also an old bug. I think this will fix those problems: > >> > >> Index: stdio/findfp.c > >> =================================================================== > >> --- stdio/findfp.c (revision 199969) > >> +++ stdio/findfp.c (working copy) > >> @@ -61,6 +61,7 @@ > >> ._read = __sread, \ > >> ._seek = __sseek, \ > >> ._write = __swrite, \ > >> + ._fl_mutex = PTHREAD_MUTEX_INITIALIZER, \ > >> } > >> /* the usual - (stdin + stdout + stderr) */ > >> static FILE usual[FOPEN_MAX - 3]; > >> @@ -96,7 +97,7 @@ > >> int n; > >> { > >> struct glue *g; > >> - static FILE empty; > >> + static FILE empty = { ._fl_mutex = PTHREAD_MUTEX_INITIALIZER }; > >> FILE *p; > >> > >> g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE)); > >> @@ -154,7 +155,7 @@ > >> fp->_ub._size = 0; > >> fp->_lb._base = NULL; /* no line buffer */ > >> fp->_lb._size = 0; > >> -/* fp->_lock = NULL; */ /* once set always set (reused) */ > >> +/* fp->_fl_mutex = NULL; */ /* once set always set (reused) */ > >> fp->_orientation = 0; > >> memset(&fp->_mbstate, 0, sizeof(mbstate_t)); > >> return (fp); > > > > Does this patch address the concerns? > > I'm not sure if that is sufficient. I added it there and as part of INITEXTRA (which we reverted to in darwin) in the following. I can provide you with the full patches if you want, but there is a lot of noise in them for our implementation of the _l variants and whatnot. I think the following might not need to be initialized, but I did it for good measure: > > vasprintf.c.patch:+ INITEXTRA(&f); > vdprintf.c.patch:+ INITEXTRA(&f); > vfprintf.c.patch:+ INITEXTRA(&fake); > vfwprintf.c.patch:+ INITEXTRA(&fake); > vsnprintf.c.patch:+ INITEXTRA(&f); > vsprintf.c.patch:+ INITEXTRA(&f); > vsscanf.c.patch:+ INITEXTRA(&f); > vswprintf.c.patch:+ INITEXTRA(&f); > vswscanf.c.patch:+ INITEXTRA(&f); Ah, ok. In our stdio at least these are all dummy files that are passed to internal stdio routines that never do any locking (e.g. __vfprintf() which does no locking vs vfprintf() which does use the mutex locks). I'm not sure if that is also true for Darwin, but in theory it should be as these file objects are all private stack variables that no other threads can gain a reference to, so no locking is needed. -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Thu Jan 7 15:44:57 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 710BB106568B; Thu, 7 Jan 2010 15:44:57 +0000 (UTC) (envelope-from jeremyhu@apple.com) Received: from mail-out4.apple.com (mail-out4.apple.com [17.254.13.23]) by mx1.freebsd.org (Postfix) with ESMTP id 531918FC1D; Thu, 7 Jan 2010 15:44:57 +0000 (UTC) Received: from relay15.apple.com (relay15.apple.com [17.128.113.54]) by mail-out4.apple.com (Postfix) with ESMTP id 001FA8567765; Thu, 7 Jan 2010 07:44:56 -0800 (PST) X-AuditID: 11807136-b7bafae000000e8d-35-4b460177a0f1 Received: from [17.151.100.3] (Unknown_Domain [17.151.100.3]) by relay15.apple.com (Apple SCV relay) with SMTP id 87.8D.03725.771064B4; Thu, 7 Jan 2010 07:44:56 -0800 (PST) Mime-Version: 1.0 (Apple Message framework v1077) Content-Type: multipart/signed; boundary=Apple-Mail-20--1043430417; protocol="application/pkcs7-signature"; micalg=sha1 From: Jeremy Huddleston In-Reply-To: <201001070956.25906.jhb@freebsd.org> Date: Thu, 7 Jan 2010 10:44:54 -0500 Message-Id: <8C235DD0-74DA-4E15-A722-17772E8089DE@apple.com> References: <200912052034.nB5KYfaY000395@www.freebsd.org> <201001061600.47628.jhb@freebsd.org> <56AAD075-8202-40C0-AC75-18A5CDC3B76A@apple.com> <201001070956.25906.jhb@freebsd.org> To: John Baldwin X-Mailer: Apple Mail (2.1077) X-Brightmail-Tracker: AAAAAQAAAZE= X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Cc: freebsd-gnats-submit@freebsd.org, freebsd-threads@freebsd.org Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2010 15:44:57 -0000 --Apple-Mail-20--1043430417 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii On Jan 7, 2010, at 09:56, John Baldwin wrote: >> vasprintf.c.patch:+ INITEXTRA(&f); >> vdprintf.c.patch:+ INITEXTRA(&f); >> vfprintf.c.patch:+ INITEXTRA(&fake); >> vfwprintf.c.patch:+ INITEXTRA(&fake); >> vsnprintf.c.patch:+ INITEXTRA(&f); >> vsprintf.c.patch:+ INITEXTRA(&f); >> vsscanf.c.patch:+ INITEXTRA(&f); >> vswprintf.c.patch:+ INITEXTRA(&f); >> vswscanf.c.patch:+ INITEXTRA(&f); >=20 > Ah, ok. In our stdio at least these are all dummy files that are = passed to > internal stdio routines that never do any locking (e.g. __vfprintf() = which > does no locking vs vfprintf() which does use the mutex locks). I'm = not sure > if that is also true for Darwin, but in theory it should be as these = file > objects are all private stack variables that no other threads can gain = a > reference to, so no locking is needed. Yeah, we're just being cautious with these changes. It takes one clock = cycle and maintains the old (FBSD 7?) state of initializing the mutex = during INITEXTRA in those dumies... just in case something gets added = down the line which needs it. If you're confident that won't be the case for FBSD, then I believe your = initial patch is sufficient. --Apple-Mail-20--1043430417-- From owner-freebsd-threads@FreeBSD.ORG Thu Jan 7 15:50:05 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A5EC210656A4 for ; Thu, 7 Jan 2010 15:50:05 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 7B1F88FC13 for ; Thu, 7 Jan 2010 15:50:05 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id o07Fo56A010387 for ; Thu, 7 Jan 2010 15:50:05 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id o07Fo5fp010386; Thu, 7 Jan 2010 15:50:05 GMT (envelope-from gnats) Date: Thu, 7 Jan 2010 15:50:05 GMT Message-Id: <201001071550.o07Fo5fp010386@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: Jeremy Huddleston Cc: Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Jeremy Huddleston List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2010 15:50:05 -0000 The following reply was made to PR threads/141198; it has been noted by GNATS. From: Jeremy Huddleston To: John Baldwin Cc: freebsd-threads@freebsd.org, freebsd-gnats-submit@freebsd.org Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes Date: Thu, 7 Jan 2010 10:44:54 -0500 --Apple-Mail-20--1043430417 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii On Jan 7, 2010, at 09:56, John Baldwin wrote: >> vasprintf.c.patch:+ INITEXTRA(&f); >> vdprintf.c.patch:+ INITEXTRA(&f); >> vfprintf.c.patch:+ INITEXTRA(&fake); >> vfwprintf.c.patch:+ INITEXTRA(&fake); >> vsnprintf.c.patch:+ INITEXTRA(&f); >> vsprintf.c.patch:+ INITEXTRA(&f); >> vsscanf.c.patch:+ INITEXTRA(&f); >> vswprintf.c.patch:+ INITEXTRA(&f); >> vswscanf.c.patch:+ INITEXTRA(&f); >=20 > Ah, ok. In our stdio at least these are all dummy files that are = passed to > internal stdio routines that never do any locking (e.g. __vfprintf() = which > does no locking vs vfprintf() which does use the mutex locks). I'm = not sure > if that is also true for Darwin, but in theory it should be as these = file > objects are all private stack variables that no other threads can gain = a > reference to, so no locking is needed. Yeah, we're just being cautious with these changes. It takes one clock = cycle and maintains the old (FBSD 7?) state of initializing the mutex = during INITEXTRA in those dumies... just in case something gets added = down the line which needs it. If you're confident that won't be the case for FBSD, then I believe your = initial patch is sufficient. --Apple-Mail-20--1043430417 Content-Disposition: attachment; filename=smime.p7s Content-Type: application/pkcs7-signature; name=smime.p7s Content-Transfer-Encoding: base64 MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAQAAoIITbjCCAz8w ggKooAMCAQICAQ0wDQYJKoZIhvcNAQEFBQAwgdExCzAJBgNVBAYTAlpBMRUwEwYDVQQIEwxXZXN0 ZXJuIENhcGUxEjAQBgNVBAcTCUNhcGUgVG93bjEaMBgGA1UEChMRVGhhd3RlIENvbnN1bHRpbmcx KDAmBgNVBAsTH0NlcnRpZmljYXRpb24gU2VydmljZXMgRGl2aXNpb24xJDAiBgNVBAMTG1RoYXd0 ZSBQZXJzb25hbCBGcmVlbWFpbCBDQTErMCkGCSqGSIb3DQEJARYccGVyc29uYWwtZnJlZW1haWxA dGhhd3RlLmNvbTAeFw0wMzA3MTcwMDAwMDBaFw0xMzA3MTYyMzU5NTlaMGIxCzAJBgNVBAYTAlpB MSUwIwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5KSBMdGQuMSwwKgYDVQQDEyNUaGF3dGUg UGVyc29uYWwgRnJlZW1haWwgSXNzdWluZyBDQTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA xKY8VXNV+065yplaHmjAdQRwnd/p/6Me7L3N9VvyGna9fww6YfK/Uc4B1OVQCjDXAmNaLIkVcI7d yfArhVqqP3FWy688Cwfn8R+RNiQqE88r1fOCdz0Dviv+uxg+B79AgAJk16emu59l0cUqVIUPSAR/ p7bRPGEEQB5kGXJgt/sCAwEAAaOBlDCBkTASBgNVHRMBAf8ECDAGAQH/AgEAMEMGA1UdHwQ8MDow OKA2oDSGMmh0dHA6Ly9jcmwudGhhd3RlLmNvbS9UaGF3dGVQZXJzb25hbEZyZWVtYWlsQ0EuY3Js MAsGA1UdDwQEAwIBBjApBgNVHREEIjAgpB4wHDEaMBgGA1UEAxMRUHJpdmF0ZUxhYmVsMi0xMzgw DQYJKoZIhvcNAQEFBQADgYEASIzRUIPqCy7MDaNmrGcPf6+svsIXoUOWlJ1/TCG4+DYfqi2fNi/A 9BxQIJNwPP2t4WFiw9k6GX6EsZkbAMUaC4J0niVQlGLH2ydxVyWN3amcOY6MIE9lX5Xa9/eH1sYI Tq726jTlEBpbNU1341YheILcIRk13iSx0x1G/11fZU8wggM/MIICqKADAgECAgENMA0GCSqGSIb3 DQEBBQUAMIHRMQswCQYDVQQGEwJaQTEVMBMGA1UECBMMV2VzdGVybiBDYXBlMRIwEAYDVQQHEwlD YXBlIFRvd24xGjAYBgNVBAoTEVRoYXd0ZSBDb25zdWx0aW5nMSgwJgYDVQQLEx9DZXJ0aWZpY2F0 aW9uIFNlcnZpY2VzIERpdmlzaW9uMSQwIgYDVQQDExtUaGF3dGUgUGVyc29uYWwgRnJlZW1haWwg Q0ExKzApBgkqhkiG9w0BCQEWHHBlcnNvbmFsLWZyZWVtYWlsQHRoYXd0ZS5jb20wHhcNMDMwNzE3 MDAwMDAwWhcNMTMwNzE2MjM1OTU5WjBiMQswCQYDVQQGEwJaQTElMCMGA1UEChMcVGhhd3RlIENv bnN1bHRpbmcgKFB0eSkgTHRkLjEsMCoGA1UEAxMjVGhhd3RlIFBlcnNvbmFsIEZyZWVtYWlsIElz c3VpbmcgQ0EwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMSmPFVzVftOucqZWh5owHUEcJ3f 6f+jHuy9zfVb8hp2vX8MOmHyv1HOAdTlUAow1wJjWiyJFXCO3cnwK4Vaqj9xVsuvPAsH5/EfkTYk KhPPK9Xzgnc9A74r/rsYPge/QIACZNenprufZdHFKlSFD0gEf6e20TxhBEAeZBlyYLf7AgMBAAGj gZQwgZEwEgYDVR0TAQH/BAgwBgEB/wIBADBDBgNVHR8EPDA6MDigNqA0hjJodHRwOi8vY3JsLnRo YXd0ZS5jb20vVGhhd3RlUGVyc29uYWxGcmVlbWFpbENBLmNybDALBgNVHQ8EBAMCAQYwKQYDVR0R BCIwIKQeMBwxGjAYBgNVBAMTEVByaXZhdGVMYWJlbDItMTM4MA0GCSqGSIb3DQEBBQUAA4GBAEiM 0VCD6gsuzA2jZqxnD3+vrL7CF6FDlpSdf0whuPg2H6otnzYvwPQcUCCTcDz9reFhYsPZOhl+hLGZ GwDFGguCdJ4lUJRix9sncVcljd2pnDmOjCBPZV+V2vf3h9bGCE6u9uo05RAaWzVNd+NWIXiC3CEZ Nd4ksdMdRv9dX2VPMIIGcDCCBdmgAwIBAgIQKF0Nr8sW2fhCBNsoUjwm8zANBgkqhkiG9w0BAQUF ADBiMQswCQYDVQQGEwJaQTElMCMGA1UEChMcVGhhd3RlIENvbnN1bHRpbmcgKFB0eSkgTHRkLjEs MCoGA1UEAxMjVGhhd3RlIFBlcnNvbmFsIEZyZWVtYWlsIElzc3VpbmcgQ0EwHhcNMDkwNTA0MDUy OTE0WhcNMTAwNTA0MDUyOTE0WjCCAnAxHzAdBgNVBAMTFlRoYXd0ZSBGcmVlbWFpbCBNZW1iZXIx JDAiBgkqhkiG9w0BCQEWFWplcmVteWh1QGJlcmtlbGV5LmVkdTErMCkGCSqGSIb3DQEJARYcamVy ZW15aHVAdWNsaW5rLmJlcmtlbGV5LmVkdTEsMCoGCSqGSIb3DQEJARYdamVyZW15aHVAdWNsaW5r NC5iZXJrZWxleS5lZHUxJzAlBgkqhkiG9w0BCQEWGGplcmVteWh1QGNzLmJlcmtlbGV5LmVkdTEp MCcGCSqGSIb3DQEJARYaamVyZW15QHVwZS5jcy5iZXJrZWxleS5lZHUxKTAnBgkqhkiG9w0BCQEW GmplcmVteWh1QGVlY3MuYmVya2VsZXkuZWR1MScwJQYJKoZIhvcNAQkBFhhqZXJlbXlodUBmcmVl ZGVza3RvcC5vcmcxJDAiBgkqhkiG9w0BCQEWFWplcmVteWh1QG1hY3BvcnRzLm9yZzElMCMGCSqG SIb3DQEJARYWamVyZW15QG91dGVyc3F1YXJlLm9yZzEgMB4GCSqGSIb3DQEJARYRamVyZW15aHVk QG1hYy5jb20xIzAhBgkqhkiG9w0BCQEWFGplcmVteUBodWRzY2FiaW4uY29tMSEwHwYJKoZIhvcN AQkBFhJqZXJlbXlodUBhcHBsZS5jb20xJTAjBgkqhkiG9w0BCQEWFmplcmVteUBvdXRlcnNxdWFy ZS5jb20xJTAjBgkqhkiG9w0BCQEWFnBheXBhbEBvdXRlcnNxdWFyZS5jb20xHzAdBgkqhkiG9w0B CQEWEGplcmVteWh1ZEBtZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCwVnJ8 XrKgByWkhJhQDk7Kj45PnZYRXJNQfcTyBQsqSqfUh13Limf2qJTxpw8Mdq/SuNkO3ZjLkaYGPB4+ 8uaHdDqGEanq2wf4qKV4dyFEQO92mRQRxLijfBS4CunlSYzHuPd6g5osI0BVpFbNRswqOXWbHd1z XRVvRqpvYKQJFWLf3dqXU3zZO2nv4sabnovbNCKEO6HrxQeawFfwxL20adsK5F1ejK1VRSEsTzd7 BjNs8QTWC4qZKrrNuaPJLVt4LDbRXIqOggrZaOkggIBIIdXubjOrrpR41PvcvibfvYLUpo3bdX5e tWH/VU/ywIS3oIc4d+VtOL/O3YdCpX0FAgMBAAGjggGRMIIBjTCCAXsGA1UdEQSCAXIwggFugRVq ZXJlbXlodUBiZXJrZWxleS5lZHWBHGplcmVteWh1QHVjbGluay5iZXJrZWxleS5lZHWBHWplcmVt eWh1QHVjbGluazQuYmVya2VsZXkuZWR1gRhqZXJlbXlodUBjcy5iZXJrZWxleS5lZHWBGmplcmVt eUB1cGUuY3MuYmVya2VsZXkuZWR1gRpqZXJlbXlodUBlZWNzLmJlcmtlbGV5LmVkdYEYamVyZW15 aHVAZnJlZWRlc2t0b3Aub3JngRVqZXJlbXlodUBtYWNwb3J0cy5vcmeBFmplcmVteUBvdXRlcnNx dWFyZS5vcmeBEWplcmVteWh1ZEBtYWMuY29tgRRqZXJlbXlAaHVkc2NhYmluLmNvbYESamVyZW15 aHVAYXBwbGUuY29tgRZqZXJlbXlAb3V0ZXJzcXVhcmUuY29tgRZwYXlwYWxAb3V0ZXJzcXVhcmUu Y29tgRBqZXJlbXlodWRAbWUuY29tMAwGA1UdEwEB/wQCMAAwDQYJKoZIhvcNAQEFBQADgYEAMtx6 voXn2w2+kaevSb7REuy5TBAQNzwlcwLiaC44HMVhwQGEYG544mBabCqY2+MtLbEn2RDQGHArtuCA Tv9liObLp6UPNKo+8Bcd3edN0dlFSeb0wFPVt71e05dGeyIoBxIrM4ix2BON/SHcGsgt3n1DRXen JLYVV809vRtHQpowggZwMIIF2aADAgECAhBfIA3CIvCJAyf8rsNvgxtuMA0GCSqGSIb3DQEBBQUA MGIxCzAJBgNVBAYTAlpBMSUwIwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5KSBMdGQuMSww KgYDVQQDEyNUaGF3dGUgUGVyc29uYWwgRnJlZW1haWwgSXNzdWluZyBDQTAeFw0wOTA5MTQyMTM2 MjdaFw0xMDA5MTQyMTM2MjdaMIICcDEfMB0GA1UEAxMWVGhhd3RlIEZyZWVtYWlsIE1lbWJlcjEk MCIGCSqGSIb3DQEJARYVamVyZW15aHVAYmVya2VsZXkuZWR1MSswKQYJKoZIhvcNAQkBFhxqZXJl bXlodUB1Y2xpbmsuYmVya2VsZXkuZWR1MSwwKgYJKoZIhvcNAQkBFh1qZXJlbXlodUB1Y2xpbms0 LmJlcmtlbGV5LmVkdTEnMCUGCSqGSIb3DQEJARYYamVyZW15aHVAY3MuYmVya2VsZXkuZWR1MSkw JwYJKoZIhvcNAQkBFhpqZXJlbXlAdXBlLmNzLmJlcmtlbGV5LmVkdTEpMCcGCSqGSIb3DQEJARYa amVyZW15aHVAZWVjcy5iZXJrZWxleS5lZHUxJzAlBgkqhkiG9w0BCQEWGGplcmVteWh1QGZyZWVk ZXNrdG9wLm9yZzEkMCIGCSqGSIb3DQEJARYVamVyZW15aHVAbWFjcG9ydHMub3JnMSUwIwYJKoZI hvcNAQkBFhZqZXJlbXlAb3V0ZXJzcXVhcmUub3JnMSAwHgYJKoZIhvcNAQkBFhFqZXJlbXlodWRA bWFjLmNvbTEjMCEGCSqGSIb3DQEJARYUamVyZW15QGh1ZHNjYWJpbi5jb20xITAfBgkqhkiG9w0B CQEWEmplcmVteWh1QGFwcGxlLmNvbTElMCMGCSqGSIb3DQEJARYWamVyZW15QG91dGVyc3F1YXJl LmNvbTElMCMGCSqGSIb3DQEJARYWcGF5cGFsQG91dGVyc3F1YXJlLmNvbTEfMB0GCSqGSIb3DQEJ ARYQamVyZW15aHVkQG1lLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL+c2RGH leO3G25PQEPEVsV3H/cWDewBCnMbqV0zgEg3hMyoRUG3aRUgH4gWbhVNkx/5t0A+mLQQWNnktg2J ku4MJJhHmarkxQAwITyamyO+37GHFl2d7oe5J7CFwg3Evf/2Lli0mfglfDHBy5YN9yURbSMVRaDV WGHhpYkqTwGXG2Bpai7oqdOlB0hDcRGE4Fv5aurxAuxyIohZMuxhZBzDfmidKsOUTnsz+NCUFIXK cMLYWwvH4XOBC4l0SU523phMyEW0OPas38EWd2NMCYaO1URA944+cS68DUvCqrrRzGmixY03PcaV uJ/+KA3L2u9esq8vt8s5m8aW8MWQWIkCAwEAAaOCAZEwggGNMIIBewYDVR0RBIIBcjCCAW6BFWpl cmVteWh1QGJlcmtlbGV5LmVkdYEcamVyZW15aHVAdWNsaW5rLmJlcmtlbGV5LmVkdYEdamVyZW15 aHVAdWNsaW5rNC5iZXJrZWxleS5lZHWBGGplcmVteWh1QGNzLmJlcmtlbGV5LmVkdYEaamVyZW15 QHVwZS5jcy5iZXJrZWxleS5lZHWBGmplcmVteWh1QGVlY3MuYmVya2VsZXkuZWR1gRhqZXJlbXlo dUBmcmVlZGVza3RvcC5vcmeBFWplcmVteWh1QG1hY3BvcnRzLm9yZ4EWamVyZW15QG91dGVyc3F1 YXJlLm9yZ4ERamVyZW15aHVkQG1hYy5jb22BFGplcmVteUBodWRzY2FiaW4uY29tgRJqZXJlbXlo dUBhcHBsZS5jb22BFmplcmVteUBvdXRlcnNxdWFyZS5jb22BFnBheXBhbEBvdXRlcnNxdWFyZS5j b22BEGplcmVteWh1ZEBtZS5jb20wDAYDVR0TAQH/BAIwADANBgkqhkiG9w0BAQUFAAOBgQBAga5a Jmkyd0TMiY0icyR7j5soyooiP4q9+Iu6lG+s/S+7vF5sDadCq+Y7US091MNT4LmbQehwwhi4jUWy EZ+KP9dhfWMqi51rZDbhWxAqAoKmgWgoQ9UsA4LqaC1wWlrM/DtzZ7+L5ZZ+MWlr94fDNL8qU3+y 3ZfiXgpWBV1x1zGCAxAwggMMAgEBMHYwYjELMAkGA1UEBhMCWkExJTAjBgNVBAoTHFRoYXd0ZSBD b25zdWx0aW5nIChQdHkpIEx0ZC4xLDAqBgNVBAMTI1RoYXd0ZSBQZXJzb25hbCBGcmVlbWFpbCBJ c3N1aW5nIENBAhBfIA3CIvCJAyf8rsNvgxtuMAkGBSsOAwIaBQCgggFvMBgGCSqGSIb3DQEJAzEL BgkqhkiG9w0BBwEwHAYJKoZIhvcNAQkFMQ8XDTEwMDEwNzE1NDQ1NVowIwYJKoZIhvcNAQkEMRYE FJ+UxYZBHHNMiSz6Pv24bfLGvnAJMIGFBgkrBgEEAYI3EAQxeDB2MGIxCzAJBgNVBAYTAlpBMSUw IwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5KSBMdGQuMSwwKgYDVQQDEyNUaGF3dGUgUGVy c29uYWwgRnJlZW1haWwgSXNzdWluZyBDQQIQKF0Nr8sW2fhCBNsoUjwm8zCBhwYLKoZIhvcNAQkQ AgsxeKB2MGIxCzAJBgNVBAYTAlpBMSUwIwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5KSBM dGQuMSwwKgYDVQQDEyNUaGF3dGUgUGVyc29uYWwgRnJlZW1haWwgSXNzdWluZyBDQQIQKF0Nr8sW 2fhCBNsoUjwm8zANBgkqhkiG9w0BAQEFAASCAQCGTG+uiPHmb1vEewjHH2qKqAEJ1NfjpV1Vos65 40HEWI7Uw/Md41MVn57f8Vg+TTGut/vihLimloWyvVjkK/r3e2VS/VWSiACTi//miJ0dUz1rrh/L kya+NuSqd37alkRjdgk7NdutZrpe//kxE03EMVapAcZXymeTpKL+4ZYPljoTOECOfzGK6mqBQEjs FjzsT081GuenlxHiRB6Dgh2y5Ogyy7/CeVeCrphN9Ww6LRZ85VBJLgkegpQEexJybTP9hbRdmItO 307qkzSWbGiB+jOawrCBUhJ16OhA69xem6VqmGF0lJ9FwNyGppLKMkY2RQlxTpkh/PCHyisYeWJM AAAAAAAA --Apple-Mail-20--1043430417-- From owner-freebsd-threads@FreeBSD.ORG Thu Jan 7 18:15:27 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D7F51106566C; Thu, 7 Jan 2010 18:15:27 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 9BC628FC18; Thu, 7 Jan 2010 18:15:27 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 3183346B46; Thu, 7 Jan 2010 13:15:27 -0500 (EST) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 71CFB8A01D; Thu, 7 Jan 2010 13:15:26 -0500 (EST) From: John Baldwin To: Jeremy Huddleston Date: Thu, 7 Jan 2010 13:15:25 -0500 User-Agent: KMail/1.12.1 (FreeBSD/7.2-CBSD-20091103; KDE/4.3.1; amd64; ; ) References: <200912052034.nB5KYfaY000395@www.freebsd.org> <201001070956.25906.jhb@freebsd.org> <8C235DD0-74DA-4E15-A722-17772E8089DE@apple.com> In-Reply-To: <8C235DD0-74DA-4E15-A722-17772E8089DE@apple.com> MIME-Version: 1.0 Message-Id: <201001071315.26022.jhb@freebsd.org> Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Thu, 07 Jan 2010 13:15:26 -0500 (EST) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-gnats-submit@freebsd.org, freebsd-threads@freebsd.org Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2010 18:15:27 -0000 On Thursday 07 January 2010 10:44:54 am Jeremy Huddleston wrote: > > On Jan 7, 2010, at 09:56, John Baldwin wrote: > > >> vasprintf.c.patch:+ INITEXTRA(&f); > >> vdprintf.c.patch:+ INITEXTRA(&f); > >> vfprintf.c.patch:+ INITEXTRA(&fake); > >> vfwprintf.c.patch:+ INITEXTRA(&fake); > >> vsnprintf.c.patch:+ INITEXTRA(&f); > >> vsprintf.c.patch:+ INITEXTRA(&f); > >> vsscanf.c.patch:+ INITEXTRA(&f); > >> vswprintf.c.patch:+ INITEXTRA(&f); > >> vswscanf.c.patch:+ INITEXTRA(&f); > > > > Ah, ok. In our stdio at least these are all dummy files that are passed to > > internal stdio routines that never do any locking (e.g. __vfprintf() which > > does no locking vs vfprintf() which does use the mutex locks). I'm not sure > > if that is also true for Darwin, but in theory it should be as these file > > objects are all private stack variables that no other threads can gain a > > reference to, so no locking is needed. > > Yeah, we're just being cautious with these changes. It takes one clock cycle > and maintains the old (FBSD 7?) state of initializing the mutex during > INITEXTRA in those dumies... just in case something gets added down the line > which needs it. > > If you're confident that won't be the case for FBSD, then I believe your > initial patch is sufficient. Ok. I actually think it would be nicer to provide some sort of macro to properly initialize the various 'fake' FILE objects. I will probably look at doing that in which case including an initializer for the lock should be simple. I actually don't like the amount of duplicated code to setup fake FILE objects as it is. Here is a larger patch which does this in addition to the earlier patch. Index: stdio/vsnprintf.c =================================================================== --- stdio/vsnprintf.c (revision 201516) +++ stdio/vsnprintf.c (working copy) @@ -47,7 +47,7 @@ size_t on; int ret; char dummy[2]; - FILE f; + FILE f = FAKE_FILE; on = n; if (n != 0) @@ -61,12 +61,9 @@ str = dummy; n = 1; } - f._file = -1; f._flags = __SWR | __SSTR; f._bf._base = f._p = (unsigned char *)str; f._bf._size = f._w = n; - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); ret = __vfprintf(&f, fmt, ap); if (on > 0) *f._p = '\0'; Index: stdio/vswprintf.c =================================================================== --- stdio/vswprintf.c (revision 201516) +++ stdio/vswprintf.c (working copy) @@ -45,7 +45,7 @@ { static const mbstate_t initial; mbstate_t mbs; - FILE f; + FILE f = FAKE_FILE; char *mbp; int ret, sverrno; size_t nwc; @@ -55,7 +55,6 @@ return (-1); } - f._file = -1; f._flags = __SWR | __SSTR | __SALC; f._bf._base = f._p = (unsigned char *)malloc(128); if (f._bf._base == NULL) { @@ -63,8 +62,6 @@ return (-1); } f._bf._size = f._w = 127; /* Leave room for the NUL */ - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); ret = __vfwprintf(&f, fmt, ap); if (ret < 0) { sverrno = errno; Index: stdio/vsscanf.c =================================================================== --- stdio/vsscanf.c (revision 201516) +++ stdio/vsscanf.c (working copy) @@ -55,16 +55,11 @@ vsscanf(const char * __restrict str, const char * __restrict fmt, __va_list ap) { - FILE f; + FILE f = FAKE_FILE; - f._file = -1; f._flags = __SRD; f._bf._base = f._p = (unsigned char *)str; f._bf._size = f._r = strlen(str); f._read = eofread; - f._ub._base = NULL; - f._lb._base = NULL; - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); return (__svfscanf(&f, fmt, ap)); } Index: stdio/snprintf.c =================================================================== --- stdio/snprintf.c (revision 201516) +++ stdio/snprintf.c (working copy) @@ -48,7 +48,7 @@ size_t on; int ret; va_list ap; - FILE f; + FILE f = FAKE_FILE; on = n; if (n != 0) @@ -56,12 +56,9 @@ if (n > INT_MAX) n = INT_MAX; va_start(ap, fmt); - f._file = -1; f._flags = __SWR | __SSTR; f._bf._base = f._p = (unsigned char *)str; f._bf._size = f._w = n; - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); ret = __vfprintf(&f, fmt, ap); if (on > 0) *f._p = '\0'; Index: stdio/xprintf.c =================================================================== --- stdio/xprintf.c (revision 201516) +++ stdio/xprintf.c (working copy) @@ -48,6 +48,7 @@ #include #include "un-namespace.h" +#include "local.h" #include "printf.h" #include "fvwrite.h" @@ -575,7 +576,7 @@ __v3printf(FILE *fp, const char *fmt, int pct, va_list ap) { int ret; - FILE fake; + FILE fake = FAKE_FILE; unsigned char buf[BUFSIZ]; /* copy the important variables */ Index: stdio/vdprintf.c =================================================================== --- stdio/vdprintf.c (revision 201516) +++ stdio/vdprintf.c (working copy) @@ -39,7 +39,7 @@ int vdprintf(int fd, const char * __restrict fmt, va_list ap) { - FILE f; + FILE f = FAKE_FILE; unsigned char buf[BUFSIZ]; int ret; @@ -56,8 +56,6 @@ f._write = __swrite; f._bf._base = buf; f._bf._size = sizeof(buf); - f._orientation = 0; - bzero(&f._mbstate, sizeof(f._mbstate)); if ((ret = __vfprintf(&f, fmt, ap)) < 0) return (ret); Index: stdio/vfprintf.c =================================================================== --- stdio/vfprintf.c (revision 201516) +++ stdio/vfprintf.c (working copy) @@ -169,7 +169,7 @@ __sbprintf(FILE *fp, const char *fmt, va_list ap) { int ret; - FILE fake; + FILE fake = FAKE_FILE; unsigned char buf[BUFSIZ]; /* XXX This is probably not needed. */ Index: stdio/local.h =================================================================== --- stdio/local.h (revision 201516) +++ stdio/local.h (working copy) @@ -110,6 +110,14 @@ } /* + * Structure initializations for 'fake' FILE objects. + */ +#define FAKE_FILE { \ + ._file = -1, \ + ._fl_mutex = PTHREAD_MUTEX_INITIALIZER, \ +} + +/* * Set the orientation for a stream. If o > 0, the stream has wide- * orientation. If o < 0, the stream has byte-orientation. */ Index: stdio/vsprintf.c =================================================================== --- stdio/vsprintf.c (revision 201516) +++ stdio/vsprintf.c (working copy) @@ -44,14 +44,11 @@ vsprintf(char * __restrict str, const char * __restrict fmt, __va_list ap) { int ret; - FILE f; + FILE f = FAKE_FILE; - f._file = -1; f._flags = __SWR | __SSTR; f._bf._base = f._p = (unsigned char *)str; f._bf._size = f._w = INT_MAX; - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); ret = __vfprintf(&f, fmt, ap); *f._p = 0; return (ret); Index: stdio/findfp.c =================================================================== --- stdio/findfp.c (revision 201516) +++ stdio/findfp.c (working copy) @@ -61,6 +61,7 @@ ._read = __sread, \ ._seek = __sseek, \ ._write = __swrite, \ + ._fl_mutex = PTHREAD_MUTEX_INITIALIZER, \ } /* the usual - (stdin + stdout + stderr) */ static FILE usual[FOPEN_MAX - 3]; @@ -96,7 +97,7 @@ int n; { struct glue *g; - static FILE empty; + static FILE empty = { ._fl_mutex = PTHREAD_MUTEX_INITIALIZER }; FILE *p; g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE)); @@ -154,7 +155,7 @@ fp->_ub._size = 0; fp->_lb._base = NULL; /* no line buffer */ fp->_lb._size = 0; -/* fp->_lock = NULL; */ /* once set always set (reused) */ +/* fp->_fl_mutex = NULL; */ /* once set always set (reused) */ fp->_orientation = 0; memset(&fp->_mbstate, 0, sizeof(mbstate_t)); return (fp); Index: stdio/vasprintf.c =================================================================== --- stdio/vasprintf.c (revision 201516) +++ stdio/vasprintf.c (working copy) @@ -42,9 +42,8 @@ __va_list ap; { int ret; - FILE f; + FILE f = FAKE_FILE; - f._file = -1; f._flags = __SWR | __SSTR | __SALC; f._bf._base = f._p = (unsigned char *)malloc(128); if (f._bf._base == NULL) { @@ -53,8 +52,6 @@ return (-1); } f._bf._size = f._w = 127; /* Leave room for the NUL */ - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); ret = __vfprintf(&f, fmt, ap); if (ret < 0) { free(f._bf._base); Index: stdio/vswscanf.c =================================================================== --- stdio/vswscanf.c (revision 201516) +++ stdio/vswscanf.c (working copy) @@ -62,7 +62,7 @@ { static const mbstate_t initial; mbstate_t mbs; - FILE f; + FILE f = FAKE_FILE; char *mbstr; size_t mlen; int r; @@ -80,15 +80,10 @@ free(mbstr); return (EOF); } - f._file = -1; f._flags = __SRD; f._bf._base = f._p = (unsigned char *)mbstr; f._bf._size = f._r = mlen; f._read = eofread; - f._ub._base = NULL; - f._lb._base = NULL; - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); r = __vfwscanf(&f, fmt, ap); free(mbstr); -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Thu Jan 7 18:20:03 2010 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A9DEB1065676 for ; Thu, 7 Jan 2010 18:20:03 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 8EFCB8FC18 for ; Thu, 7 Jan 2010 18:20:03 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id o07IK3TZ039586 for ; Thu, 7 Jan 2010 18:20:03 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id o07IK3ng039585; Thu, 7 Jan 2010 18:20:03 GMT (envelope-from gnats) Date: Thu, 7 Jan 2010 18:20:03 GMT Message-Id: <201001071820.o07IK3ng039585@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: John Baldwin Cc: Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: John Baldwin List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jan 2010 18:20:03 -0000 The following reply was made to PR threads/141198; it has been noted by GNATS. From: John Baldwin To: Jeremy Huddleston Cc: freebsd-threads@freebsd.org, freebsd-gnats-submit@freebsd.org Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes Date: Thu, 7 Jan 2010 13:15:25 -0500 On Thursday 07 January 2010 10:44:54 am Jeremy Huddleston wrote: > > On Jan 7, 2010, at 09:56, John Baldwin wrote: > > >> vasprintf.c.patch:+ INITEXTRA(&f); > >> vdprintf.c.patch:+ INITEXTRA(&f); > >> vfprintf.c.patch:+ INITEXTRA(&fake); > >> vfwprintf.c.patch:+ INITEXTRA(&fake); > >> vsnprintf.c.patch:+ INITEXTRA(&f); > >> vsprintf.c.patch:+ INITEXTRA(&f); > >> vsscanf.c.patch:+ INITEXTRA(&f); > >> vswprintf.c.patch:+ INITEXTRA(&f); > >> vswscanf.c.patch:+ INITEXTRA(&f); > > > > Ah, ok. In our stdio at least these are all dummy files that are passed to > > internal stdio routines that never do any locking (e.g. __vfprintf() which > > does no locking vs vfprintf() which does use the mutex locks). I'm not sure > > if that is also true for Darwin, but in theory it should be as these file > > objects are all private stack variables that no other threads can gain a > > reference to, so no locking is needed. > > Yeah, we're just being cautious with these changes. It takes one clock cycle > and maintains the old (FBSD 7?) state of initializing the mutex during > INITEXTRA in those dumies... just in case something gets added down the line > which needs it. > > If you're confident that won't be the case for FBSD, then I believe your > initial patch is sufficient. Ok. I actually think it would be nicer to provide some sort of macro to properly initialize the various 'fake' FILE objects. I will probably look at doing that in which case including an initializer for the lock should be simple. I actually don't like the amount of duplicated code to setup fake FILE objects as it is. Here is a larger patch which does this in addition to the earlier patch. Index: stdio/vsnprintf.c =================================================================== --- stdio/vsnprintf.c (revision 201516) +++ stdio/vsnprintf.c (working copy) @@ -47,7 +47,7 @@ size_t on; int ret; char dummy[2]; - FILE f; + FILE f = FAKE_FILE; on = n; if (n != 0) @@ -61,12 +61,9 @@ str = dummy; n = 1; } - f._file = -1; f._flags = __SWR | __SSTR; f._bf._base = f._p = (unsigned char *)str; f._bf._size = f._w = n; - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); ret = __vfprintf(&f, fmt, ap); if (on > 0) *f._p = '\0'; Index: stdio/vswprintf.c =================================================================== --- stdio/vswprintf.c (revision 201516) +++ stdio/vswprintf.c (working copy) @@ -45,7 +45,7 @@ { static const mbstate_t initial; mbstate_t mbs; - FILE f; + FILE f = FAKE_FILE; char *mbp; int ret, sverrno; size_t nwc; @@ -55,7 +55,6 @@ return (-1); } - f._file = -1; f._flags = __SWR | __SSTR | __SALC; f._bf._base = f._p = (unsigned char *)malloc(128); if (f._bf._base == NULL) { @@ -63,8 +62,6 @@ return (-1); } f._bf._size = f._w = 127; /* Leave room for the NUL */ - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); ret = __vfwprintf(&f, fmt, ap); if (ret < 0) { sverrno = errno; Index: stdio/vsscanf.c =================================================================== --- stdio/vsscanf.c (revision 201516) +++ stdio/vsscanf.c (working copy) @@ -55,16 +55,11 @@ vsscanf(const char * __restrict str, const char * __restrict fmt, __va_list ap) { - FILE f; + FILE f = FAKE_FILE; - f._file = -1; f._flags = __SRD; f._bf._base = f._p = (unsigned char *)str; f._bf._size = f._r = strlen(str); f._read = eofread; - f._ub._base = NULL; - f._lb._base = NULL; - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); return (__svfscanf(&f, fmt, ap)); } Index: stdio/snprintf.c =================================================================== --- stdio/snprintf.c (revision 201516) +++ stdio/snprintf.c (working copy) @@ -48,7 +48,7 @@ size_t on; int ret; va_list ap; - FILE f; + FILE f = FAKE_FILE; on = n; if (n != 0) @@ -56,12 +56,9 @@ if (n > INT_MAX) n = INT_MAX; va_start(ap, fmt); - f._file = -1; f._flags = __SWR | __SSTR; f._bf._base = f._p = (unsigned char *)str; f._bf._size = f._w = n; - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); ret = __vfprintf(&f, fmt, ap); if (on > 0) *f._p = '\0'; Index: stdio/xprintf.c =================================================================== --- stdio/xprintf.c (revision 201516) +++ stdio/xprintf.c (working copy) @@ -48,6 +48,7 @@ #include #include "un-namespace.h" +#include "local.h" #include "printf.h" #include "fvwrite.h" @@ -575,7 +576,7 @@ __v3printf(FILE *fp, const char *fmt, int pct, va_list ap) { int ret; - FILE fake; + FILE fake = FAKE_FILE; unsigned char buf[BUFSIZ]; /* copy the important variables */ Index: stdio/vdprintf.c =================================================================== --- stdio/vdprintf.c (revision 201516) +++ stdio/vdprintf.c (working copy) @@ -39,7 +39,7 @@ int vdprintf(int fd, const char * __restrict fmt, va_list ap) { - FILE f; + FILE f = FAKE_FILE; unsigned char buf[BUFSIZ]; int ret; @@ -56,8 +56,6 @@ f._write = __swrite; f._bf._base = buf; f._bf._size = sizeof(buf); - f._orientation = 0; - bzero(&f._mbstate, sizeof(f._mbstate)); if ((ret = __vfprintf(&f, fmt, ap)) < 0) return (ret); Index: stdio/vfprintf.c =================================================================== --- stdio/vfprintf.c (revision 201516) +++ stdio/vfprintf.c (working copy) @@ -169,7 +169,7 @@ __sbprintf(FILE *fp, const char *fmt, va_list ap) { int ret; - FILE fake; + FILE fake = FAKE_FILE; unsigned char buf[BUFSIZ]; /* XXX This is probably not needed. */ Index: stdio/local.h =================================================================== --- stdio/local.h (revision 201516) +++ stdio/local.h (working copy) @@ -110,6 +110,14 @@ } /* + * Structure initializations for 'fake' FILE objects. + */ +#define FAKE_FILE { \ + ._file = -1, \ + ._fl_mutex = PTHREAD_MUTEX_INITIALIZER, \ +} + +/* * Set the orientation for a stream. If o > 0, the stream has wide- * orientation. If o < 0, the stream has byte-orientation. */ Index: stdio/vsprintf.c =================================================================== --- stdio/vsprintf.c (revision 201516) +++ stdio/vsprintf.c (working copy) @@ -44,14 +44,11 @@ vsprintf(char * __restrict str, const char * __restrict fmt, __va_list ap) { int ret; - FILE f; + FILE f = FAKE_FILE; - f._file = -1; f._flags = __SWR | __SSTR; f._bf._base = f._p = (unsigned char *)str; f._bf._size = f._w = INT_MAX; - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); ret = __vfprintf(&f, fmt, ap); *f._p = 0; return (ret); Index: stdio/findfp.c =================================================================== --- stdio/findfp.c (revision 201516) +++ stdio/findfp.c (working copy) @@ -61,6 +61,7 @@ ._read = __sread, \ ._seek = __sseek, \ ._write = __swrite, \ + ._fl_mutex = PTHREAD_MUTEX_INITIALIZER, \ } /* the usual - (stdin + stdout + stderr) */ static FILE usual[FOPEN_MAX - 3]; @@ -96,7 +97,7 @@ int n; { struct glue *g; - static FILE empty; + static FILE empty = { ._fl_mutex = PTHREAD_MUTEX_INITIALIZER }; FILE *p; g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE)); @@ -154,7 +155,7 @@ fp->_ub._size = 0; fp->_lb._base = NULL; /* no line buffer */ fp->_lb._size = 0; -/* fp->_lock = NULL; */ /* once set always set (reused) */ +/* fp->_fl_mutex = NULL; */ /* once set always set (reused) */ fp->_orientation = 0; memset(&fp->_mbstate, 0, sizeof(mbstate_t)); return (fp); Index: stdio/vasprintf.c =================================================================== --- stdio/vasprintf.c (revision 201516) +++ stdio/vasprintf.c (working copy) @@ -42,9 +42,8 @@ __va_list ap; { int ret; - FILE f; + FILE f = FAKE_FILE; - f._file = -1; f._flags = __SWR | __SSTR | __SALC; f._bf._base = f._p = (unsigned char *)malloc(128); if (f._bf._base == NULL) { @@ -53,8 +52,6 @@ return (-1); } f._bf._size = f._w = 127; /* Leave room for the NUL */ - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); ret = __vfprintf(&f, fmt, ap); if (ret < 0) { free(f._bf._base); Index: stdio/vswscanf.c =================================================================== --- stdio/vswscanf.c (revision 201516) +++ stdio/vswscanf.c (working copy) @@ -62,7 +62,7 @@ { static const mbstate_t initial; mbstate_t mbs; - FILE f; + FILE f = FAKE_FILE; char *mbstr; size_t mlen; int r; @@ -80,15 +80,10 @@ free(mbstr); return (EOF); } - f._file = -1; f._flags = __SRD; f._bf._base = f._p = (unsigned char *)mbstr; f._bf._size = f._r = mlen; f._read = eofread; - f._ub._base = NULL; - f._lb._base = NULL; - f._orientation = 0; - memset(&f._mbstate, 0, sizeof(mbstate_t)); r = __vfwscanf(&f, fmt, ap); free(mbstr); -- John Baldwin