APPROVE COMMIT
NOTE: This patch has been committed
# HG changeset patch
# User Aidan Kehoe <kehoea(a)parhasard.net>
# Date 1510076651 0
# Tue Nov 07 17:44:11 2017 +0000
# Node ID 6c493b7790729ec2b805a5967c9374258998613d
# Parent a4a1ae4830fadfd483403399af7d04b496b72138
valid_ichar_p() needs an EMACS_INT argument, not Ichar. Fix some 64-bit bugs.
src/ChangeLog addition:
2017-11-07 Aidan Kehoe <kehoea(a)parhasard.net>
valid_ichar_p() needs an EMACS_INT argument, not an Ichar
argument, since it is usually used to examine values extracted
from a Lisp_Object, and such values will be truncated giving
incorrect values on a 64-bit build.
Correct this, update associated C code.
* doprnt.c (emacs_doprnt):
Check whether the EMACS_INT value of OBJ is a valid Ichar, rather
than truncating and giving incorrect values.
* lisp.h (XCHAR_1):
Ditto.
* mule-ccl.c:
* mule-ccl.c (ccl_driver):
* mule-ccl.h:
Most integer values in mule-ccl.{c,h} are stored to using XFIXNUM()
or XCHAR_OR_FIXNUM(), and so declaring them as int will lead to
truncation and errors down the line.
Update their declaration to EMACS_INT, which will always have
enough bits to handle the result of XFIXNUM().
Use emacs_snprintf() when debug-printing these values, since we
have %lx and %ld which fits the width of an EMACS_INT explicitly.
Make STATUS in struct ccl_program{} into an enum.
Make EOL_TYPE into an enum.
Comment out a couple of fields not used in XEmacs.
* symbols.c (store_symval_forwarding):
#if HAVE_BIGNUM => #ifdef HAVE_BIGNUM.
* text.c:
* text.c (old_mule_non_ascii_valid_ichar_p):
* text.h (valid_unicode_codepoint_p):
Make these two functions return Boolints.
tests/ChangeLog addition:
2017-11-07 Aidan Kehoe <kehoea(a)parhasard.net>
* automated/mule-tests.el:
Check that (int-char most-negative-fixnum) gives nil.
diff -r a4a1ae4830fa -r 6c493b779072 src/ChangeLog
--- a/src/ChangeLog Tue Nov 07 16:13:01 2017 +0000
+++ b/src/ChangeLog Tue Nov 07 17:44:11 2017 +0000
@@ -1,3 +1,37 @@
+2017-11-07 Aidan Kehoe <kehoea(a)parhasard.net>
+
+ valid_ichar_p() needs an EMACS_INT argument, not an Ichar
+ argument, since it is usually used to examine values extracted
+ from a Lisp_Object, and such values will be truncated giving
+ incorrect values on a 64-bit build.
+ Correct this, update associated C code.
+
+ * doprnt.c (emacs_doprnt):
+ Check whether the EMACS_INT value of OBJ is a valid Ichar, rather
+ than truncating and giving incorrect values.
+
+ * lisp.h (XCHAR_1):
+ Ditto.
+ * mule-ccl.c:
+ * mule-ccl.c (ccl_driver):
+ * mule-ccl.h:
+ Most integer values in mule-ccl.{c,h} are stored to using XFIXNUM()
+ or XCHAR_OR_FIXNUM(), and so declaring them as int will lead to
+ truncation and errors down the line.
+ Update their declaration to EMACS_INT, which will always have
+ enough bits to handle the result of XFIXNUM().
+ Use emacs_snprintf() when debug-printing these values, since we
+ have %lx and %ld which fits the width of an EMACS_INT explicitly.
+ Make STATUS in struct ccl_program{} into an enum.
+ Make EOL_TYPE into an enum.
+ Comment out a couple of fields not used in XEmacs.
+ * symbols.c (store_symval_forwarding):
+ #if HAVE_BIGNUM => #ifdef HAVE_BIGNUM.
+ * text.c:
+ * text.c (old_mule_non_ascii_valid_ichar_p):
+ * text.h (valid_unicode_codepoint_p):
+ Make these two functions return Boolints.
+
2017-11-02 Aidan Kehoe <kehoea(a)parhasard.net>
* editfns.c (make_time):
diff -r a4a1ae4830fa -r 6c493b779072 src/doprnt.c
--- a/src/doprnt.c Tue Nov 07 16:13:01 2017 +0000
+++ b/src/doprnt.c Tue Nov 07 17:44:11 2017 +0000
@@ -1984,12 +1984,13 @@
}
else if (FIXNUMP (obj))
{
- a = XREALFIXNUM (obj);
- if (!valid_ichar_p (a))
+ EMACS_INT fa = XREALFIXNUM (obj);
+ if (!valid_ichar_p (fa))
{
UNGCPRO;
syntax_error ("Invalid integer value for %c spec", obj);
}
+ a = (Ichar) fa;
}
else
{
diff -r a4a1ae4830fa -r 6c493b779072 src/lisp.h
--- a/src/lisp.h Tue Nov 07 16:13:01 2017 +0000
+++ b/src/lisp.h Tue Nov 07 17:44:11 2017 +0000
@@ -3170,11 +3170,11 @@
XCHAR_1 (Lisp_Object obj, const Ascbyte *file, int line)
)
{
- Ichar ch;
+ EMACS_INT ch;
assert_at_line (CHARP (obj), file, line);
ch = XCHARVAL (obj);
assert_at_line (valid_ichar_p (ch), file, line);
- return ch;
+ return (Ichar) ch;
}
#define XCHAR(x) XCHAR_1 (x, __FILE__, __LINE__)
diff -r a4a1ae4830fa -r 6c493b779072 src/mule-ccl.c
--- a/src/mule-ccl.c Tue Nov 07 16:13:01 2017 +0000
+++ b/src/mule-ccl.c Tue Nov 07 17:44:11 2017 +0000
@@ -953,8 +953,8 @@
#ifdef CCL_DEBUG
/* Currently enabled when DEBUG_XEMACS, i.e. configure --with-debug */
#define CCL_DEBUG_BACKTRACE_LEN 256
-int ccl_backtrace_table[CCL_DEBUG_BACKTRACE_LEN];
-int ccl_backtrace_idx;
+EMACS_INT ccl_backtrace_table[CCL_DEBUG_BACKTRACE_LEN];
+EMACS_INT ccl_backtrace_idx;
#endif
struct ccl_prog_stack
@@ -976,18 +976,18 @@
int *consumed,
int conversion_mode)
{
- register int *reg = ccl->reg;
- register int ic = ccl->ic;
- register int code = -1;
- register int field1, field2;
+ register EMACS_INT *reg = ccl->reg;
+ register EMACS_INT ic = ccl->ic;
+ register EMACS_INT code = -1;
+ register EMACS_INT field1, field2;
register Lisp_Object *ccl_prog = ccl->prog;
const unsigned char *src = source, *src_end = src + src_bytes;
int jump_address;
int stack_idx = ccl->stack_idx;
/* Instruction counter of the current CCL code. */
- int this_ic = 0;
- int eof_ic = ccl->eof_ic;
- int eof_hit = 0;
+ EMACS_INT this_ic = 0;
+ EMACS_INT eof_ic = ccl->eof_ic;
+ Boolint eof_hit = 0;
if (ic >= eof_ic)
ic = CCL_HEADER_MAIN;
@@ -1004,7 +1004,7 @@
for (;;)
{
- int i, j, op;
+ EMACS_INT i, j, op;
ccl_repeat:
#ifdef CCL_DEBUG
@@ -1057,7 +1057,7 @@
/* #### it's non-obvious to me that we need these casts,
but the left one was already there so clearly the intention
was an unsigned comparison. --ben */
- if ((unsigned int) i < (unsigned int) j)
+ if ((EMACS_UINT) i < (EMACS_UINT) j)
reg[rrr] = XCHAR_OR_FIXNUM (ccl_prog[ic + i]);
ic += j;
break;
@@ -1110,7 +1110,7 @@
i = reg[rrr];
j = XCHAR_OR_FIXNUM (ccl_prog[ic]);
/* #### see comment at CCL_SetArray */
- if ((unsigned int) i < (unsigned int) j)
+ if ((EMACS_UINT) i < (EMACS_UINT) j)
{
i = XCHAR_OR_FIXNUM (ccl_prog[ic + 1 + i]);
CCL_WRITE_CHAR (i);
@@ -1130,7 +1130,7 @@
/* fall through ... */
case CCL_Branch: /* CCCCCCCCCCCCCCCCCCCCrrrXXXXX */
/* #### see comment at CCL_SetArray */
- if ((unsigned int) reg[rrr] < (unsigned int) field1)
+ if ((EMACS_UINT) reg[rrr] < (EMACS_UINT) field1)
ic += XCHAR_OR_FIXNUM (ccl_prog[ic + reg[rrr]]);
else
ic += XCHAR_OR_FIXNUM (ccl_prog[ic + field1]);
@@ -1178,7 +1178,7 @@
case CCL_Call: /* 1:CCCCCCCCCCCCCCCCCCCCFFFXXXXX */
{
Lisp_Object slot;
- int prog_id;
+ EMACS_INT prog_id;
/* If FFF is nonzero, the CCL program ID is in the
following code. */
@@ -1229,7 +1229,7 @@
case CCL_WriteArray: /* CCCCCCCCCCCCCCCCCCCCrrrXXXXX */
i = reg[rrr];
/* #### see comment at CCL_SetArray */
- if ((unsigned int) i < (unsigned int) field1)
+ if ((EMACS_UINT) i < (EMACS_UINT) field1)
{
j = XCHAR_OR_FIXNUM (ccl_prog[ic + i]);
CCL_WRITE_CHAR (j);
@@ -1492,20 +1492,21 @@
op = translate_char (GET_TRANSLATION_TABLE (op), i, -1, 0, 0);
{
Lisp_Object charset;
+ int ii = -1, jj = -1;
/* @@#### Get rid of 7-bit stuff */
buffer_filtered_ichar_to_charset_codepoint (op, buf,
charset_7bit_p,
- &charset, &i, &j,
+ &charset, &ii, &jj,
CONVERR_FAIL);
if (NILP (charset))
CCL_CONVERSION_ERROR;
reg[RRR] = XCHARSET_ID (charset);
}
- if (j != -1)
- i = (i << 7) | j;
+ if (jj != -1)
+ ii = (ii << 7) | jj;
- reg[rrr] = i;
+ reg[rrr] = ii;
#endif
break;
@@ -1517,7 +1518,8 @@
int ucs;
CCL_MAKE_CHAR (reg[rrr], reg[RRR], ich);
- /* @@#### Is USE_PRIVATE correct? or should I fail? */
+ /* [@@#### Is USE_PRIVATE correct? or should I fail?]
+ I should fail. */
ucs = ichar_to_unicode (ich, CONVERR_USE_PRIVATE);
reg[rrr] = ucs;
break;
@@ -1527,26 +1529,28 @@
{
int error = 0;
- /* @@#### Is UNICODE_OFFICIAL_ONLY correct? */
+ /* [@@#### Is UNICODE_OFFICIAL_ONLY correct?]
+ Yes. */
if (!valid_unicode_codepoint_p (reg[rrr],
UNICODE_OFFICIAL_ONLY))
error = 1;
else
{
Lisp_Object charset;
+ int ii = -1, jj = -1;
/* @@#### This 7-bit stuff is awful, change it */
buffer_filtered_unicode_to_charset_codepoint
- (reg[rrr], buf, charset_7bit_p, &charset, &i, &j,
+ (reg[rrr], buf, charset_7bit_p, &charset, &ii, &jj,
CONVERR_FAIL);
if (NILP (charset))
error = 1;
else
{
reg[RRR] = XCHARSET_ID (charset);
- i &= 0x7f;
- j &= 0x7f;
- reg[rrr] = (i << 7) | j;
+ ii &= 0x7f;
+ jj &= 0x7f;
+ reg[rrr] = (ii << 7) | jj;
}
}
@@ -1568,22 +1572,23 @@
if (!HTENTRY_CLEAR_P (e))
{
+ int ii = -1, jj = -1;
op = XCHARVAL (e->value);
if (!valid_ichar_p (op))
CCL_INVALID_CMD;
/* @@#### Get rid of 7-bit stuff */
buffer_filtered_ichar_to_charset_codepoint
- (op, buf, charset_7bit_p, &charset, &i, &j,
+ (op, buf, charset_7bit_p, &charset, &ii, &jj,
CONVERR_FAIL);
if (NILP (charset))
CCL_CONVERSION_ERROR;
reg[RRR] = XCHARSET_ID (charset);
- if (j != 0)
+ if (jj != 0)
{
- i = (i << 7) | j;
+ ii = (ii << 7) | jj;
}
- reg[rrr] = i;
+ reg[rrr] = ii;
reg[7] = 1; /* r7 true for success */
}
else
@@ -1615,7 +1620,7 @@
case CCL_IterateMultipleMap:
{
Lisp_Object map, content, attrib, value;
- int point, size, fin_ic;
+ EMACS_INT point, size, fin_ic;
j = XCHAR_OR_FIXNUM (ccl_prog[ic++]); /* number of maps. */
fin_ic = ic + j;
@@ -1713,9 +1718,9 @@
case CCL_MapMultiple:
{
Lisp_Object map, content, attrib, value;
- int point, size, map_vector_size;
- int map_set_rest_length, fin_ic;
- int current_ic = this_ic;
+ EMACS_INT point, size, map_vector_size;
+ EMACS_INT map_set_rest_length, fin_ic;
+ EMACS_INT current_ic = this_ic;
/* inhibit recursive call on MapMultiple. */
if (stack_idx_of_map_multiple > 0)
@@ -1987,30 +1992,33 @@
/* We can insert an error message only if DESTINATION is
specified and we still have a room to store the message
there. */
- char msg[256];
+ Ibyte msg[256];
switch (ccl->status)
{
case CCL_STAT_INVALID_CMD:
- sprintf (msg, "\nCCL: Invalid command %x (ccl_code = %x) at %d.",
- code & 0x1F, code, this_ic);
+ emacs_snprintf (msg, sizeof (msg),
+ "\nCCL: Invalid command %lx (ccl_code = %lx) at %ld.",
+ code & 0x1F, code, this_ic);
goto ccl_error_continue;
case CCL_STAT_INVALID_CHARSET:
- sprintf (msg, "\nCCL: Invalid charset (command %x, ccl_code = %x) at %d.",
- code & 0x1F, code, this_ic);
+ emacs_snprintf (msg, sizeof (msg),
+ "\nCCL: Invalid charset (command %lx, ccl_code = %lx) at %ld.",
+ (code & 0x1F), code, this_ic);
goto ccl_error_continue;
case CCL_STAT_CONVERSION_ERROR:
- sprintf (msg, "\nCCL: Conversion error (command %x, ccl_code = %x) at %d.",
- code & 0x1F, code, this_ic);
+ emacs_snprintf (msg, sizeof (msg),
+ "\nCCL: Conversion error (command %lx, ccl_code = %lx) at %ld.",
+ code & 0x1F, code, this_ic);
goto ccl_error_continue;
ccl_error_continue:
#ifdef CCL_DEBUG
{
- int i = ccl_backtrace_idx - 1;
- int j;
+ EMACS_INT i = ccl_backtrace_idx - 1;
+ EMACS_INT j;
Dynarr_add_many (destination, (unsigned char *) msg, strlen (msg));
@@ -2019,8 +2027,9 @@
if (i < 0) i = CCL_DEBUG_BACKTRACE_LEN - 1;
if (ccl_backtrace_table[i] == 0)
break;
- sprintf (msg, " %d", ccl_backtrace_table[i]);
- Dynarr_add_many (destination, (unsigned char *) msg, strlen (msg));
+ Dynarr_add_many (destination, msg,
+ emacs_snprintf (msg, sizeof (msg),
+ " %ld", ccl_backtrace_table[i]))
}
goto ccl_finish;
}
@@ -2028,14 +2037,16 @@
break;
case CCL_STAT_QUIT:
- sprintf(msg, "\nCCL: Exited.");
+ emacs_snprintf (msg, sizeof (msg), "\nCCL: Exited.");
break;
default:
- sprintf(msg, "\nCCL: Unknown error type (%d).", ccl->status);
+ emacs_snprintf (msg, sizeof (msg),
+ "\nCCL: Unknown error type (%d).", ccl->status);
}
- Dynarr_add_many (destination, (unsigned char *) msg, strlen (msg));
+ Dynarr_add_many (destination, (unsigned char *) msg,
+ qxestrlen (msg));
}
ccl_finish:
@@ -2340,7 +2351,7 @@
{
Lisp_Object val;
struct ccl_program ccl;
- int i, produced;
+ EMACS_INT i, produced;
unsigned_char_dynarr *outbuf;
struct gcpro gcpro1, gcpro2, gcpro3;
diff -r a4a1ae4830fa -r 6c493b779072 src/mule-ccl.h
--- a/src/mule-ccl.h Tue Nov 07 16:13:01 2017 +0000
+++ b/src/mule-ccl.h Tue Nov 07 17:44:11 2017 +0000
@@ -38,45 +38,50 @@
7-bit charset is wanted, etc.). */
};
+enum ccl_coding_eol
+ {
+ CCL_CODING_EOL_LF, /* Line-feed only, same as Emacs' */
+ CCL_CODING_EOL_CRLF, /* Sequence of carriage-return and
+ line-feed. */
+ CCL_CODING_EOL_CR, /* Carriage-return only. */
+ };
+
/* Structure to hold information about running CCL code. Read
comments in the file ccl.c for the detail of each field. */
struct ccl_program {
Elemcount size; /* Size of the compiled code. */
Lisp_Object *prog; /* Pointer into the compiled code. */
- int ic; /* Instruction Counter (index for PROG). */
- int eof_ic; /* Instruction Counter for end-of-file
+ EMACS_INT ic; /* Instruction Counter (index for PROG). */
+ EMACS_INT eof_ic; /* Instruction Counter for end-of-file
processing code. */
- int reg[8]; /* CCL registers, reg[7] is used for
+ EMACS_INT reg[8]; /* CCL registers, reg[7] is used for
condition flag of relational
operations. */
- int private_state; /* CCL instruction may use this
+ /* Not used in XEmacs: */
+ /* int private_state; */ /* CCL instruction may use this
for private use, mainly for saving
internal states on suspending.
This variable is set to 0 when ccl is
set up. */
- int last_block; /* Set to 1 while processing the last
+
+ Boolint last_block; /* Set to 1 while processing the last
block. */
- int status; /* Exit status of the CCL program. */
- int buf_magnification; /* Output buffer magnification. How
+ enum ccl_status status; /* Exit status of the CCL program. */
+ EMACS_INT buf_magnification; /* Output buffer magnification. How
many times bigger the output buffer
should be than the input buffer. */
int stack_idx; /* How deep the call of CCL_Call is nested. */
- int eol_type; /* When the CCL program is used for
+ enum ccl_coding_eol eol_type; /* When the CCL program is used for
encoding by a coding system, set to
the eol_type of the coding
system. */
- int multibyte; /* 1 if the source text is multibyte. */
+ /* Not used in XEmacs: */
+ /* int multibyte; */ /* 1 if the source text is multibyte. */
};
#define CCL_MODE_ENCODING 0
#define CCL_MODE_DECODING 1
-#define CCL_CODING_EOL_LF 0 /* Line-feed only, same as Emacs'
- internal format. */
-#define CCL_CODING_EOL_CRLF 1 /* Sequence of carriage-return and
- line-feed. */
-#define CCL_CODING_EOL_CR 2 /* Carriage-return only. */
-
#ifdef DEBUG_XEMACS
#define CCL_DEBUG
#endif
diff -r a4a1ae4830fa -r 6c493b779072 src/symbols.c
--- a/src/symbols.c Tue Nov 07 16:13:01 2017 +0000
+++ b/src/symbols.c Tue Nov 07 17:44:11 2017 +0000
@@ -1048,7 +1048,7 @@
CHECK_INTEGER (newval);
if (magicfun)
magicfun (sym, &newval, Qnil, 0);
-#if HAVE_BIGNUM
+#ifdef HAVE_BIGNUM
if (BIGNUMP (newval))
{
if (bignum_fits_emacs_int_p (XBIGNUM_DATA (newval)))
diff -r a4a1ae4830fa -r 6c493b779072 src/text.c
--- a/src/text.c Tue Nov 07 16:13:01 2017 +0000
+++ b/src/text.c Tue Nov 07 17:44:11 2017 +0000
@@ -1498,11 +1498,11 @@
#ifndef UNICODE_INTERNAL
-/* Return whether CH is a valid Ichar, assuming it's >= 0x100.
- Do not call this directly. Use the macro valid_ichar_p() instead. */
-
-int
-old_mule_non_ascii_valid_ichar_p (Ichar ch)
+/* Return whether CH corresponds to a valid Ichar. Do not call this
+ directly. Use the macro valid_ichar_p() instead. */
+
+Boolint
+old_mule_non_ascii_valid_ichar_p (EMACS_INT ch)
{
int f1, f2, f3;
diff -r a4a1ae4830fa -r 6c493b779072 src/text.h
--- a/src/text.h Tue Nov 07 16:13:01 2017 +0000
+++ b/src/text.h Tue Nov 07 17:44:11 2017 +0000
@@ -242,7 +242,7 @@
#define UNICODE_OFFICIAL_MAX 0x10FFFF
DECLARE_INLINE_HEADER (
-int
+Boolint
valid_unicode_codepoint_p (EMACS_INT ch, enum unicode_allow allow)
)
{
@@ -527,13 +527,13 @@
enum
converr);
#elif defined (MULE)
-MODULE_API INT_32_BIT old_mule_non_ascii_valid_ichar_p (Ichar ch);
+MODULE_API Boolint old_mule_non_ascii_valid_ichar_p (EMACS_INT ch);
#endif
-/* Return whether the given Ichar is valid. */
+/* Return whether the given EMACS_INT can be made into a valid Ichar. */
DECLARE_INLINE_HEADER (
Boolint
-valid_ichar_p (Ichar ch)
+valid_ichar_p (EMACS_INT ch)
)
{
#ifdef UNICODE_INTERNAL
diff -r a4a1ae4830fa -r 6c493b779072 tests/ChangeLog
--- a/tests/ChangeLog Tue Nov 07 16:13:01 2017 +0000
+++ b/tests/ChangeLog Tue Nov 07 17:44:11 2017 +0000
@@ -1,5 +1,8 @@
2017-11-07 Aidan Kehoe <kehoea(a)parhasard.net>
+ * automated/mule-tests.el:
+ Check that (int-char most-negative-fixnum) gives nil.
+
* automated/format-tests.el:
Create a ratio at runtime, not load time, on a build without ratio
support.
diff -r a4a1ae4830fa -r 6c493b779072 tests/automated/mule-tests.el
--- a/tests/automated/mule-tests.el Tue Nov 07 16:13:01 2017 +0000
+++ b/tests/automated/mule-tests.el Tue Nov 07 17:44:11 2017 +0000
@@ -35,6 +35,11 @@
(require 'bytecomp)
+;; This would give ?\x00 for a while on 64-bit XEmacs.
+
+(Assert (eq (int-char most-negative-fixnum) nil)
+ "checking for a bug with valid_ichar_p() on 64-bit builds")
+
;;-----------------------------------------------------------------
;; Test whether all legal chars may be safely inserted to a buffer.
;;-----------------------------------------------------------------
--
‘As I sat looking up at the Guinness ad, I could never figure out /
How your man stayed up on the surfboard after forty pints of stout’
(C. Moore)