From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#8254: race condition in dired.c's scmp function Date: Mon, 14 Mar 2011 23:16:26 -0700 Organization: UCLA Computer Science Department Message-ID: <4D7F043A.5070702@cs.ucla.edu> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1300171073 17553 80.91.229.12 (15 Mar 2011 06:37:53 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Tue, 15 Mar 2011 06:37:53 +0000 (UTC) To: 8254@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Mar 15 07:37:49 2011 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1PzNt5-0002vk-Tr for geb-bug-gnu-emacs@m.gmane.org; Tue, 15 Mar 2011 07:37:48 +0100 Original-Received: from localhost ([127.0.0.1]:40546 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzNt5-0005Kh-4D for geb-bug-gnu-emacs@m.gmane.org; Tue, 15 Mar 2011 02:37:47 -0400 Original-Received: from [140.186.70.92] (port=55209 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzNsQ-00052L-Fp for bug-gnu-emacs@gnu.org; Tue, 15 Mar 2011 02:37:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PzNsN-0004yC-C3 for bug-gnu-emacs@gnu.org; Tue, 15 Mar 2011 02:37:05 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:33735) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PzNsN-0004y1-AH for bug-gnu-emacs@gnu.org; Tue, 15 Mar 2011 02:37:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1PzNZ0-0006lE-Ha; Tue, 15 Mar 2011 02:17:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 15 Mar 2011 06:17:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 8254 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.130016980325956 (code B ref -1); Tue, 15 Mar 2011 06:17:02 +0000 Original-Received: (at submit) by debbugs.gnu.org; 15 Mar 2011 06:16:43 +0000 Original-Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzNYg-0006kb-CP for submit@debbugs.gnu.org; Tue, 15 Mar 2011 02:16:43 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzNYe-0006kQ-FP for submit@debbugs.gnu.org; Tue, 15 Mar 2011 02:16:41 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PzNYY-0008Oj-5f for submit@debbugs.gnu.org; Tue, 15 Mar 2011 02:16:35 -0400 Original-Received: from lists.gnu.org ([199.232.76.165]:38955) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PzNYX-0008Of-WF for submit@debbugs.gnu.org; Tue, 15 Mar 2011 02:16:34 -0400 Original-Received: from [140.186.70.92] (port=60487 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzNYW-0003Yq-Ms for bug-gnu-emacs@gnu.org; Tue, 15 Mar 2011 02:16:33 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PzNYV-0008OE-38 for bug-gnu-emacs@gnu.org; Tue, 15 Mar 2011 02:16:32 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:57996) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PzNYU-0008Ny-Pc for bug-gnu-emacs@gnu.org; Tue, 15 Mar 2011 02:16:31 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 7E36A39E80E1 for ; Mon, 14 Mar 2011 23:16:28 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ShOYv8N2HBZa for ; Mon, 14 Mar 2011 23:16:27 -0700 (PDT) Original-Received: from [192.168.1.10] (pool-71-189-109-235.lsanca.fios.verizon.net [71.189.109.235]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 752D239E8083 for ; Mon, 14 Mar 2011 23:16:27 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.14) Gecko/20110223 Thunderbird/3.1.8 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Tue, 15 Mar 2011 02:17:02 -0400 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:45012 Archived-At: The following code in the Emacs trunk src/dired.c's scmp function has undefined behavior: while (l && (DOWNCASE ((unsigned char) *s1++) == DOWNCASE ((unsigned char) *s2++))) l--; Because the DOWNCASE macro assigns to the global variables case_temp1 and case_temp2, (DOWNCASE (x) == DOWNCASE (y)) is not valid, as the assignments can collide and lead to a race condition. This bug was found by gcc -Wsequence-point (GCC 4.5.2), which warns: dired.c:799:7: error: operation on 'case_temp2' may be undefined [-Wsequence-point] dired.c:799:7: error: operation on 'case_temp1' may be undefined [-Wsequence-point] I plan to work around the problem with something like the following patch. ---- Fix a race condition diagnosed by gcc -Wsequence-point. The expression (DOWNCASE ((unsigned char) *s1++) == DOWNCASE ((unsigned char) *s2++)), found in dired.c's scmp function, had undefined behavior. * lisp.h (DOWNCASE_TABLE, UPCASE_TABLE, DOWNCASE, UPPERCASEP): (NOCASEP, LOWERCASEP, UPCASE, UPCASE1): Move from here ... * buffer.h: ... to here, because these macros use current_buffer, and the new implementation with inline functions needs to have current_buffer in scope now, rather than later when the macros are used. (downcase, upcase1): New static inline functions. (DOWNCASE, UPCASE1): Reimplement using these functions. This avoids undefined behavior in expressions like DOWNCASE (x) == DOWNCASE (y), which previously suffered from race conditions in accessing the global variables case_temp1 and case_temp2. * casetab.c (case_temp1, case_temp2): Remove; no longer needed. * lisp.h (case_temp1, case_temp2): Remove their decls. * character.h (ASCII_CHAR_P): Move from here ... * lisp.h: ... to here, so that the inline functions mentioned above can use them. === modified file 'src/buffer.h' --- src/buffer.h 2011-03-13 22:25:16 +0000 +++ src/buffer.h 2011-03-15 05:52:48 +0000 @@ -1026,4 +1026,47 @@ #define PER_BUFFER_VALUE(BUFFER, OFFSET) \ (*(Lisp_Object *)((OFFSET) + (char *) (BUFFER))) - + +/* Current buffer's map from characters to lower-case characters. */ + +#define DOWNCASE_TABLE BVAR (current_buffer, downcase_table) + +/* Current buffer's map from characters to upper-case characters. */ + +#define UPCASE_TABLE BVAR (current_buffer, upcase_table) + +/* Downcase a character, or make no change if that cannot be done. */ + +static inline EMACS_INT +downcase (int ch) +{ + Lisp_Object down = CHAR_TABLE_REF (DOWNCASE_TABLE, ch); + return NATNUMP (down) ? XFASTINT (down) : ch; +} +#define DOWNCASE(CH) downcase (CH) + +/* 1 if CH is upper case. */ + +#define UPPERCASEP(CH) (DOWNCASE (CH) != (CH)) + +/* 1 if CH is neither upper nor lower case. */ + +#define NOCASEP(CH) (UPCASE1 (CH) == (CH)) + +/* 1 if CH is lower case. */ + +#define LOWERCASEP(CH) (!UPPERCASEP (CH) && !NOCASEP(CH)) + +/* Upcase a character, or make no change if that cannot be done. */ + +#define UPCASE(CH) (!UPPERCASEP (CH) ? UPCASE1 (CH) : (CH)) + +/* Upcase a character known to be not upper case. */ + +static inline EMACS_INT +upcase1 (int ch) +{ + Lisp_Object up = CHAR_TABLE_REF (UPCASE_TABLE, ch); + return NATNUMP (up) ? XFASTINT (up) : ch; +} +#define UPCASE1(CH) upcase1 (CH) === modified file 'src/casetab.c' --- src/casetab.c 2011-02-16 15:02:50 +0000 +++ src/casetab.c 2011-03-15 04:01:35 +0000 @@ -28,11 +28,6 @@ Lisp_Object Vascii_downcase_table, Vascii_upcase_table; Lisp_Object Vascii_canon_table, Vascii_eqv_table; -/* Used as a temporary in DOWNCASE and other macros in lisp.h. No - need to mark it, since it is used only very temporarily. */ -int case_temp1; -Lisp_Object case_temp2; - static void set_canon (Lisp_Object case_table, Lisp_Object range, Lisp_Object elt); static void set_identity (Lisp_Object table, Lisp_Object c, Lisp_Object elt); static void shuffle (Lisp_Object table, Lisp_Object c, Lisp_Object elt); @@ -302,4 +297,3 @@ defsubr (&Sset_case_table); defsubr (&Sset_standard_case_table); } - === modified file 'src/character.h' --- src/character.h 2011-03-08 04:37:19 +0000 +++ src/character.h 2011-03-15 05:52:52 +0000 @@ -128,9 +128,6 @@ XSETCDR ((x), tmp); \ } while (0) -/* Nonzero iff C is an ASCII character. */ -#define ASCII_CHAR_P(c) ((unsigned) (c) < 0x80) - /* Nonzero iff C is a character of code less than 0x100. */ #define SINGLE_BYTE_CHAR_P(c) ((unsigned) (c) < 0x100) === modified file 'src/lisp.h' --- src/lisp.h 2011-03-15 01:32:33 +0000 +++ src/lisp.h 2011-03-15 04:23:51 +0000 @@ -841,6 +841,9 @@ #endif /* not __GNUC__ */ +/* Nonzero iff C is an ASCII character. */ +#define ASCII_CHAR_P(c) ((unsigned) (c) < 0x80) + /* Almost equivalent to Faref (CT, IDX) with optimization for ASCII characters. Do not check validity of CT. */ #define CHAR_TABLE_REF(CT, IDX) \ @@ -2041,50 +2044,6 @@ #define QUITP (!NILP (Vquit_flag) && NILP (Vinhibit_quit)) -/* Variables used locally in the following case handling macros. */ -extern int case_temp1; -extern Lisp_Object case_temp2; - -/* Current buffer's map from characters to lower-case characters. */ - -#define DOWNCASE_TABLE BVAR (current_buffer, downcase_table) - -/* Current buffer's map from characters to upper-case characters. */ - -#define UPCASE_TABLE BVAR (current_buffer, upcase_table) - -/* Downcase a character, or make no change if that cannot be done. */ - -#define DOWNCASE(CH) \ - ((case_temp1 = (CH), \ - case_temp2 = CHAR_TABLE_REF (DOWNCASE_TABLE, case_temp1), \ - NATNUMP (case_temp2)) \ - ? XFASTINT (case_temp2) : case_temp1) - -/* 1 if CH is upper case. */ - -#define UPPERCASEP(CH) (DOWNCASE (CH) != (CH)) - -/* 1 if CH is neither upper nor lower case. */ - -#define NOCASEP(CH) (UPCASE1 (CH) == (CH)) - -/* 1 if CH is lower case. */ - -#define LOWERCASEP(CH) (!UPPERCASEP (CH) && !NOCASEP(CH)) - -/* Upcase a character, or make no change if that cannot be done. */ - -#define UPCASE(CH) (!UPPERCASEP (CH) ? UPCASE1 (CH) : (CH)) - -/* Upcase a character known to be not upper case. */ - -#define UPCASE1(CH) \ - ((case_temp1 = (CH), \ - case_temp2 = CHAR_TABLE_REF (UPCASE_TABLE, case_temp1), \ - NATNUMP (case_temp2)) \ - ? XFASTINT (case_temp2) : case_temp1) - extern Lisp_Object Vascii_downcase_table, Vascii_upcase_table; extern Lisp_Object Vascii_canon_table, Vascii_eqv_table;