* bug#42660: emacs-27.1-rc1 and UBsan findings
2020-08-01 20:34 bug#42660: emacs-27.1-rc1 and UBsan findings Jeffrey Walton
@ 2020-08-09 23:00 ` Lars Ingebrigtsen
[not found] ` <CAH8yC8kR7GyB7vSMjA8cHCVhSsmM1u1+9mhzO2+eohg=N2awyA@mail.gmail.com>
2020-08-18 0:58 ` Paul Eggert
1 sibling, 1 reply; 4+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-09 23:00 UTC (permalink / raw)
To: Jeffrey Walton; +Cc: 42660
Jeffrey Walton <noloader@gmail.com> writes:
> $ cat emacs-27.1/test/lisp/international/ccl-tests.log
> Running 7 tests (2020-08-01 16:28:12-0400, selector `(not (or (tag
> :expensive-test) (tag :unstable)))')
> passed 1/7 ccl-compile-midi (0.000181 sec)
> passed 2/7 ccl-compile-pgg (0.000093 sec)
> passed 3/7 ccl-dump-midi (0.005956 sec)
> passed 4/7 ccl-dump-pgg (0.000111 sec)
> Package pgg-def is deprecated
> Package pgg-parse is deprecated
> Package pgg is deprecated
> ccl.c:1146:29: runtime error: left shift of 1246883553 by 1 places
> cannot be represented in type 'int'
I get the same, basically:
Running 7 tests (2020-08-10 00:55:35+0200, selector `(not (tag :unstable))')
passed 1/7 ccl-compile-midi (0.000614 sec)
passed 2/7 ccl-compile-pgg (0.000371 sec)
passed 3/7 ccl-dump-midi (0.001032 sec)
passed 4/7 ccl-dump-pgg (0.000531 sec)
Package pgg-def is deprecated
Package pgg-parse is deprecated
Package pgg is deprecated
ccl.c:1153:29: runtime error: left shift of 1239426054 by 1 places cannot be represented in type 'int'
make[1]: *** [Makefile:183: lisp/international/ccl-tests.log] Error 1
This is the test that fails:
(ert-deftest pgg-parse-crc24 ()
;; Compiler
(require 'pgg)
(should (equal pgg-parse-crc24 prog-pgg-code))
;; Interpreter
(should (equal (pgg-parse-crc24-string "foo") (concat [#x4f #xc2 #x55])))
(should (equal (pgg-parse-crc24-string "bar") (concat [#x51 #xd9 #x53])))
(should (equal (pgg-parse-crc24-string "baz") (concat [#xf0 #x58 #x6a]))))
All three of those pgg-parse calls make Emacs signal a runtime error
with this checking turned on.
So this is with:
CFLAGS="-fsanitize=undefined -fno-sanitize-recover=all" ./configure && make
Now, pgg is obsolete, so that's a test that probably should go away.
So is this a bug? The thing that fails is this:
case CCL_LSH: reg[rrr] <<= i; break;
So it's doing a left shift on a too-high number... which is something
that pgg does, but since that's obsolete, do we care?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 4+ messages in thread* bug#42660: emacs-27.1-rc1 and UBsan findings
2020-08-01 20:34 bug#42660: emacs-27.1-rc1 and UBsan findings Jeffrey Walton
2020-08-09 23:00 ` Lars Ingebrigtsen
@ 2020-08-18 0:58 ` Paul Eggert
1 sibling, 0 replies; 4+ messages in thread
From: Paul Eggert @ 2020-08-18 0:58 UTC (permalink / raw)
To: noloader; +Cc: Lars Ingebrigtsen, 42660-done
[-- Attachment #1: Type: text/plain, Size: 179 bytes --]
The bug is unlikely to lead to a real problem, so I installed the attached
low-priority patch into the master branch rather than into emacs-27. Closing the
bug report as fixed.
[-- Attachment #2: 0001-Fix-glitch-uncovered-by-gcc-fsanitize-undefined.patch --]
[-- Type: text/x-patch, Size: 4910 bytes --]
From 9905001e4b0c9dc0a90cefdd9530a90d07a17b99 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 17 Aug 2020 17:54:44 -0700
Subject: [PATCH] Fix glitch uncovered by gcc -fsanitize=undefined
* src/ccl.c (ccl_driver): Defend against signed integer
overflow (Bug#42660). Perhaps some of this is unnecessary,
but it is safe and ccl.c is low-priority these days.
---
src/ccl.c | 104 ++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 85 insertions(+), 19 deletions(-)
diff --git a/src/ccl.c b/src/ccl.c
index e85cfa6cdf..86debeef0e 100644
--- a/src/ccl.c
+++ b/src/ccl.c
@@ -1142,19 +1142,52 @@ #define EXCMD (field1 >> 6)
ccl_expr_self:
switch (op)
{
- case CCL_PLUS: reg[rrr] += i; break;
- case CCL_MINUS: reg[rrr] -= i; break;
- case CCL_MUL: reg[rrr] *= i; break;
- case CCL_DIV: reg[rrr] /= i; break;
+ case CCL_PLUS: INT_ADD_WRAPV (reg[rrr], i, ®[rrr]); break;
+ case CCL_MINUS: INT_SUBTRACT_WRAPV (reg[rrr], i, ®[rrr]); break;
+ case CCL_MUL: INT_MULTIPLY_WRAPV (reg[rrr], i, ®[rrr]); break;
+ case CCL_DIV:
+ if (!i)
+ CCL_INVALID_CMD;
+ if (!INT_DIVIDE_OVERFLOW (reg[rrr], i))
+ reg[rrr] /= i;
+ break;
case CCL_MOD: reg[rrr] %= i; break;
+ if (!i)
+ CCL_INVALID_CMD;
+ reg[rrr] = i == -1 ? 0 : reg[rrr] % i;
+ break;
case CCL_AND: reg[rrr] &= i; break;
case CCL_OR: reg[rrr] |= i; break;
case CCL_XOR: reg[rrr] ^= i; break;
- case CCL_LSH: reg[rrr] <<= i; break;
- case CCL_RSH: reg[rrr] >>= i; break;
- case CCL_LSH8: reg[rrr] <<= 8; reg[rrr] |= i; break;
+ case CCL_LSH:
+ if (i < 0)
+ CCL_INVALID_CMD;
+ reg[rrr] = i < UINT_WIDTH ? (unsigned) reg[rrr] << i : 0;
+ break;
+ case CCL_RSH:
+ if (i < 0)
+ CCL_INVALID_CMD;
+ reg[rrr] = reg[rrr] >> min (i, INT_WIDTH - 1);
+ break;
+ case CCL_LSH8:
+ reg[rrr] = (unsigned) reg[rrr] << 8;
+ reg[rrr] |= i;
+ break;
case CCL_RSH8: reg[7] = reg[rrr] & 0xFF; reg[rrr] >>= 8; break;
- case CCL_DIVMOD: reg[7] = reg[rrr] % i; reg[rrr] /= i; break;
+ case CCL_DIVMOD:
+ if (!i)
+ CCL_INVALID_CMD;
+ if (i == -1)
+ {
+ reg[7] = 0;
+ INT_SUBTRACT_WRAPV (0, reg[rrr], ®[rrr]);
+ }
+ else
+ {
+ reg[7] = reg[rrr] % i;
+ reg[rrr] /= i;
+ }
+ break;
case CCL_LS: reg[rrr] = reg[rrr] < i; break;
case CCL_GT: reg[rrr] = reg[rrr] > i; break;
case CCL_EQ: reg[rrr] = reg[rrr] == i; break;
@@ -1204,19 +1237,52 @@ #define EXCMD (field1 >> 6)
ccl_set_expr:
switch (op)
{
- case CCL_PLUS: reg[rrr] = i + j; break;
- case CCL_MINUS: reg[rrr] = i - j; break;
- case CCL_MUL: reg[rrr] = i * j; break;
- case CCL_DIV: reg[rrr] = i / j; break;
- case CCL_MOD: reg[rrr] = i % j; break;
+ case CCL_PLUS: INT_ADD_WRAPV (i, j, ®[rrr]); break;
+ case CCL_MINUS: INT_SUBTRACT_WRAPV (i, j, ®[rrr]); break;
+ case CCL_MUL: INT_MULTIPLY_WRAPV (i, j, ®[rrr]); break;
+ case CCL_DIV:
+ if (!j)
+ CCL_INVALID_CMD;
+ if (!INT_DIVIDE_OVERFLOW (i, j))
+ i /= j;
+ reg[rrr] = i;
+ break;
+ case CCL_MOD:
+ if (!j)
+ CCL_INVALID_CMD;
+ reg[rrr] = j == -1 ? 0 : i % j;
+ break;
case CCL_AND: reg[rrr] = i & j; break;
case CCL_OR: reg[rrr] = i | j; break;
case CCL_XOR: reg[rrr] = i ^ j; break;
- case CCL_LSH: reg[rrr] = i << j; break;
- case CCL_RSH: reg[rrr] = i >> j; break;
- case CCL_LSH8: reg[rrr] = (i << 8) | j; break;
+ case CCL_LSH:
+ if (j < 0)
+ CCL_INVALID_CMD;
+ reg[rrr] = j < UINT_WIDTH ? (unsigned) i << j : 0;
+ break;
+ case CCL_RSH:
+ if (j < 0)
+ CCL_INVALID_CMD;
+ reg[rrr] = i >> min (j, INT_WIDTH - 1);
+ break;
+ case CCL_LSH8:
+ reg[rrr] = ((unsigned) i << 8) | j;
+ break;
case CCL_RSH8: reg[rrr] = i >> 8; reg[7] = i & 0xFF; break;
- case CCL_DIVMOD: reg[rrr] = i / j; reg[7] = i % j; break;
+ case CCL_DIVMOD:
+ if (!j)
+ CCL_INVALID_CMD;
+ if (j == -1)
+ {
+ INT_SUBTRACT_WRAPV (0, reg[rrr], ®[rrr]);
+ reg[7] = 0;
+ }
+ else
+ {
+ reg[rrr] = i / j;
+ reg[7] = i % j;
+ }
+ break;
case CCL_LS: reg[rrr] = i < j; break;
case CCL_GT: reg[rrr] = i > j; break;
case CCL_EQ: reg[rrr] = i == j; break;
@@ -1225,7 +1291,7 @@ #define EXCMD (field1 >> 6)
case CCL_NE: reg[rrr] = i != j; break;
case CCL_DECODE_SJIS:
{
- i = (i << 8) | j;
+ i = ((unsigned) i << 8) | j;
SJIS_TO_JIS (i);
reg[rrr] = i >> 8;
reg[7] = i & 0xFF;
@@ -1233,7 +1299,7 @@ #define EXCMD (field1 >> 6)
}
case CCL_ENCODE_SJIS:
{
- i = (i << 8) | j;
+ i = ((unsigned) i << 8) | j;
JIS_TO_SJIS (i);
reg[rrr] = i >> 8;
reg[7] = i & 0xFF;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread