APPROVE COMMIT NOTE: This patch has been committed # HG changeset patch # User Aidan Kehoe <kehoea(a)parhasard.net&gt; # 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&gt; 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&gt; * 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&gt; + + 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&gt; * 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&gt; + * 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)