From: Paul Eggert <eggert@cs.ucla.edu>
To: 8254@debbugs.gnu.org
Subject: bug#8254: race condition in dired.c's scmp function
Date: Mon, 14 Mar 2011 23:16:26 -0700 [thread overview]
Message-ID: <4D7F043A.5070702@cs.ucla.edu> (raw)
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)))
-
+\f
+/* 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))
\f
-/* 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;
\f
next reply other threads:[~2011-03-15 6:16 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-15 6:16 Paul Eggert [this message]
2011-03-15 7:06 ` bug#8254: race condition in dired.c's scmp function Eli Zaretskii
2011-03-15 7:31 ` Paul Eggert
2011-03-15 10:50 ` Eli Zaretskii
2011-03-15 16:53 ` Paul Eggert
2011-03-15 11:36 ` Juanma Barranquero
2011-03-15 16:52 ` Paul Eggert
2011-03-15 16:58 ` Paul Eggert
2011-03-15 19:13 ` Stefan Monnier
2011-03-15 21:27 ` Paul Eggert
2011-03-16 13:19 ` Richard Stallman
2011-03-16 20:02 ` Paul Eggert
2011-03-17 16:56 ` Paul Eggert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D7F043A.5070702@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=8254@debbugs.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.