bug#33083: 27.0.50; Bignums and message-checksum

bug#33083: 27.0.50; Bignums and message-checksum

[Stefan Monnier]

Package: Emacs
Version: 27.0.50

I just got this error:

    Debugger entered--Lisp error: (overflow-error)
      ash(100.....0170 1)
      message-checksum()
      message--yank-original-internal(nil)
      (let nil (message--yank-original-internal 'nil))
      eval((let nil (message--yank-original-internal 'nil)))
      message-yank-original()
      [...]
      command-execute(gnus-summary-followup-with-original)

Where the "....." is fairly long.  The problem is simple:
In message-checksum we do

      (while (not (eobp))
	(when (not (looking-at "[ \t\n]"))
	  (setq sum (logxor (ash sum 1) (if (natnump sum) 0 1)
			    (char-after))))
	(forward-char 1)))

where the code assumes an `ash` where the MSB bit would overflow into
the sign bit when the result can't fit in a fixnum, and the (if (natnump
sum) 0 1) ends up feeding that sign bit back to the LSB, thus doing
a "rotate".

Of course, with bignums we don't overflow until we reach the 65536 bits
and when we do, we signal an error instead of dropping the extra bits.

So the above error is signaled when the yanked text is longer than 20Kchars
(or non-whitespace chars).  Note that even before we reach this error,
we have another problem: this function is basically used at one spot
where we do

      (not (eq (message-checksum) message-checksum))

and if the checksum is larger than a fixnum, then this `eq` will always
return false even if both numbers are equal.

So, ever since we switched to bignums, this equality is basically always
false (except when yanking less 8 non-whitespace chars).

What should we do:
- just get rid of this message-checksum feature (which has been broken
  in master for a few months now).
- replace message-checksum with something like md5 (which will be faster
  but won't ignore whitespace chars like the current code does).
- try to get back the original semantics (e.g. by introducing a proper
  bit-rotate primitive).


        Stefan

bug#33083: 27.0.50; Bignums and message-checksum

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 183 bytes --]

Thanks for reporting the bug. I installed the attached, which is a conservative 
patch that should fix the bug, though it doesn't address the more-general issues 
that you mentioned.

[-- Attachment #2: 0001-Port-message-checksum-to-bignums.patch --]
[-- Type: text/x-patch, Size: 1546 bytes --]

From b9c0ea6a6bc01c0034d00d685cab0de5953fdcc9 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 26 Dec 2018 01:11:58 -0800
Subject: [PATCH] Port message-checksum to bignums

* lisp/gnus/message.el (message--rotate-fixnum-left): New function.
(message-checksum): Use it instead of assuming fixnum-only arithmetic.
This should fix Bug#33083.
---
 lisp/gnus/message.el | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
index 03f80616d9..dc15769a4f 100644
--- a/lisp/gnus/message.el
+++ b/lisp/gnus/message.el
@@ -5400,6 +5400,17 @@ message-check-news-body-syntax
 	     (message "Denied posting -- only quoted text.")
 	     nil)))))))
 
+(defun message--rotate-fixnum-left (n)
+  "Rotate the fixnum N left by one bit in a fixnum word.
+The result is a fixnum."
+  (logior (if (natnump n) 0 1)
+	  (ash (cond ((< (ash most-positive-fixnum -1) n)
+		      (logior n most-negative-fixnum))
+		     ((< n (ash most-negative-fixnum -1))
+		      (logand n most-positive-fixnum))
+		     (n))
+	       1)))
+
 (defun message-checksum ()
   "Return a \"checksum\" for the current buffer."
   (let ((sum 0))
@@ -5409,7 +5420,7 @@ message-checksum
        (concat "^" (regexp-quote mail-header-separator) "$"))
       (while (not (eobp))
 	(when (not (looking-at "[ \t\n]"))
-	  (setq sum (logxor (ash sum 1) (if (natnump sum) 0 1)
+	  (setq sum (logxor (message--rotate-fixnum-left sum)
 			    (char-after))))
 	(forward-char 1)))
     sum))
-- 
2.17.1