Fix a race condition diagnosed by gcc -Wsequence-point (Bug#8254).

An expression of the form (DOWNCASE (x) == DOWNCASE (y)), 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.

diff --git a/src/ChangeLog b/src/ChangeLog
index 7468af4..7e873e3 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,26 @@
 2011-03-15  Paul Eggert  <eggert@cs.ucla.edu>
 
+       Fix a race condition diagnosed by gcc -Wsequence-point (Bug#8254).
+       An expression of the form (DOWNCASE (x) == DOWNCASE (y)), 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.
+
        * dired.c (directory_files_internal_unwind): Now static.
 
        * fileio.c (file_name_as_directory, directory_file_name):

diff --git a/src/buffer.h b/src/buffer.h
index 0975d2e..996e4e5 100644 (file)
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -1026,4 +1026,47 @@ extern int last_per_buffer_idx;
 
 #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)

diff --git a/src/casetab.c b/src/casetab.c
index 5207e53..56f6b06 100644 (file)
--- a/src/casetab.c
+++ b/src/casetab.c
@@ -28,11 +28,6 @@ Lisp_Object Qcase_table_p, Qcase_table;
 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 @@ syms_of_casetab (void)
   defsubr (&Sset_case_table);
   defsubr (&Sset_standard_case_table);
 }
-

diff --git a/src/character.h b/src/character.h
index d29ab41..6d5b811 100644 (file)
--- a/src/character.h
+++ b/src/character.h
@@ -128,9 +128,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
     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)
 

diff --git a/src/lisp.h b/src/lisp.h
index 3ba1818..15c13e0 100644 (file)
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -841,6 +841,9 @@ struct Lisp_Vector
 
 #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 @@ extern int pending_signals;
 
 #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