* bug#37795: 26.1; Fixnum overflow on dpyinfo->last_user_time
@ 2019-10-17 16:25 Stefan Monnier
2019-10-18 20:33 ` Paul Eggert
0 siblings, 1 reply; 2+ messages in thread
From: Stefan Monnier @ 2019-10-17 16:25 UTC (permalink / raw)
To: 37795
I just got an assertion failure:
lisp.h:1151: Emacs fatal error: assertion failed: !FIXNUM_OVERFLOW_P (n)
where the backtrace looks like:
#0 0x0817867e in terminate_due_to_signal (sig=6, backtrace_limit=2147483647)
at emacs.c:371
#1 0x081edce8 in die
(msg=0x82e20f1 "!FIXNUM_OVERFLOW_P (n)", file=0x82e2008 "lisp.h", line=1151) at alloc.c:7374
#2 0x0813a4ee in make_fixnum (n=<optimized out>) at lisp.h:1152
#3 0x0813bb41 in list2i (x=x@entry=1, y=<optimized out>) at lisp.h:3938
#4 0x08148f7c in x_ewmh_activate_frame (f=f@entry=0x8d67980) at xterm.c:11614
#5 0x0814906f in x_focus_frame (f=0x8d67980, noactivate=false)
at xterm.c:11664
The relevant data being:
(gdb) p dpyinfo->last_user_time
$1 = 537117447
(gdb)
which was passed to list2i via:
x_send_client_event (frame, make_fixnum (0), frame,
dpyinfo->Xatom_net_active_window,
make_fixnum (32),
list2i (1, dpyinfo->last_user_time));
Obviously, on 64bit systems this is not a problem, but on 32bit systems
such overflows can happen as I just found out.
I changed `list2i` to use `make_int` instead of `make_fixnum` and it
seems to have fixed the immediate problem, but the same problem showed
up further down in make_lispy_position because the event's timestamp was
similarly large. So I'm now using the patch below, which seems "good
enough" but I also see other places where we do:
selection_data = list4 (selection_name, selection_value,
INT_TO_INTEGER (timestamp), frame);
so maybe we should be using `INT_TO_INTEGER` rather than `make_int`?
Now, AFAICT the exact value of those timestamps doesn't really matter,
so rather than make_int we could use a wrap-around version of
make_fixnum which truncates the higher bits instead of signaling an
error on overflow.
Stefan
diff --git a/src/keyboard.c b/src/keyboard.c
index d07376e8bea..fef2c094f26 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -5301,7 +5301,7 @@ make_lispy_position (struct frame *f, Lisp_Object x, Lisp_Object y,
Fcons (posn,
Fcons (Fcons (make_fixnum (xret),
make_fixnum (yret)),
- Fcons (make_fixnum (t),
+ Fcons (make_int (t),
extra_info))));
}
@@ -5326,7 +5326,7 @@ static Lisp_Object
make_scroll_bar_position (struct input_event *ev, Lisp_Object type)
{
return list5 (ev->frame_or_window, type, Fcons (ev->x, ev->y),
- make_fixnum (ev->timestamp),
+ make_int (ev->timestamp),
builtin_lisp_symbol (scroll_bar_parts[ev->part]));
}
@@ -5639,7 +5639,7 @@ make_lispy_event (struct input_event *event)
position = list4 (event->frame_or_window,
Qmenu_bar,
Fcons (event->x, event->y),
- make_fixnum (event->timestamp));
+ make_int (event->timestamp));
return list2 (item, position);
}
diff --git a/src/lisp.h b/src/lisp.h
index 66e631392e4..fd41b1b97b1 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3929,26 +3929,26 @@ extern void visit_static_gc_roots (struct gc_root_visitor visitor);
INLINE Lisp_Object
list1i (EMACS_INT x)
{
- return list1 (make_fixnum (x));
+ return list1 (make_int (x));
}
INLINE Lisp_Object
list2i (EMACS_INT x, EMACS_INT y)
{
- return list2 (make_fixnum (x), make_fixnum (y));
+ return list2 (make_int (x), make_int (y));
}
INLINE Lisp_Object
list3i (EMACS_INT x, EMACS_INT y, EMACS_INT w)
{
- return list3 (make_fixnum (x), make_fixnum (y), make_fixnum (w));
+ return list3 (make_int (x), make_int (y), make_int (w));
}
INLINE Lisp_Object
list4i (EMACS_INT x, EMACS_INT y, EMACS_INT w, EMACS_INT h)
{
- return list4 (make_fixnum (x), make_fixnum (y),
- make_fixnum (w), make_fixnum (h));
+ return list4 (make_int (x), make_int (y),
+ make_int (w), make_int (h));
}
extern Lisp_Object make_uninit_bool_vector (EMACS_INT);
^ permalink raw reply related [flat|nested] 2+ messages in thread
* bug#37795: 26.1; Fixnum overflow on dpyinfo->last_user_time
2019-10-17 16:25 bug#37795: 26.1; Fixnum overflow on dpyinfo->last_user_time Stefan Monnier
@ 2019-10-18 20:33 ` Paul Eggert
0 siblings, 0 replies; 2+ messages in thread
From: Paul Eggert @ 2019-10-18 20:33 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 37795-done
[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]
Thanks for reporting that. I installed the attached patches, which are
along the lines that you suggested. They also fix a similar bug in
xterm.c's x_ewmh_activate_frame.
> I also see other places where we do:
>
> selection_data = list4 (selection_name, selection_value,
> INT_TO_INTEGER (timestamp), frame);
>
> so maybe we should be using `INT_TO_INTEGER` rather than `make_int`?
Yes for Time values, since Time might be (usually is?) unsigned and
might exceed INTMAX_MAX. However, list1i etc. accept signed integers so
make_int is fine for them.
Changing list1i etc. to use intmax_t and make_int is a small performance
hit in some cases, but is probably worth it given the reliability
implications of ignoring integer overflow.
> AFAICT the exact value of those timestamps doesn't really matter,
Some Emacs code subtracts Time values and assumes wraparound overflow,
so if we shoehorn them into fixnums we would need to take that into
account. Probably better to leave things be.
[-- Attachment #2: 0001-Fix-integer-overflow-bug-in-Time-conversion.patch --]
[-- Type: text/x-patch, Size: 2084 bytes --]
From a7478d4768081efe8abc787e250acfd231b738d2 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 18 Oct 2019 13:07:49 -0700
Subject: [PATCH 1/2] Fix integer-overflow bug in Time conversion
Problem reported by Stefan Monnier (Bug#37795).
* src/keyboard.c (make_lispy_position)
(make_scroll_bar_position, make_lispy_event):
* src/xterm.c (x_ewmh_activate_frame):
Use INT_TO_INTEGER to convert Time to a Lisp integer,
since the value might not be a fixnum.
---
src/keyboard.c | 6 +++---
src/xterm.c | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/keyboard.c b/src/keyboard.c
index d67d18a801..db583ec530 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -5242,7 +5242,7 @@ make_lispy_position (struct frame *f, Lisp_Object x, Lisp_Object y,
Fcons (posn,
Fcons (Fcons (make_fixnum (xret),
make_fixnum (yret)),
- Fcons (make_fixnum (t),
+ Fcons (INT_TO_INTEGER (t),
extra_info))));
}
@@ -5267,7 +5267,7 @@ toolkit_menubar_in_use (struct frame *f)
make_scroll_bar_position (struct input_event *ev, Lisp_Object type)
{
return list5 (ev->frame_or_window, type, Fcons (ev->x, ev->y),
- make_fixnum (ev->timestamp),
+ INT_TO_INTEGER (ev->timestamp),
builtin_lisp_symbol (scroll_bar_parts[ev->part]));
}
@@ -5579,7 +5579,7 @@ make_lispy_event (struct input_event *event)
position = list4 (event->frame_or_window,
Qmenu_bar,
Fcons (event->x, event->y),
- make_fixnum (event->timestamp));
+ INT_TO_INTEGER (event->timestamp));
return list2 (item, position);
}
diff --git a/src/xterm.c b/src/xterm.c
index 5d8b1482a6..045589534f 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -11589,7 +11589,8 @@ x_ewmh_activate_frame (struct frame *f)
x_send_client_event (frame, make_fixnum (0), frame,
dpyinfo->Xatom_net_active_window,
make_fixnum (32),
- list2i (1, dpyinfo->last_user_time));
+ list2 (make_fixnum (1),
+ INT_TO_INTEGER (dpyinfo->last_user_time)));
}
}
--
2.21.0
[-- Attachment #3: 0002-Generalize-list1i-etc.-to-all-signed-integer-types.patch --]
[-- Type: text/x-patch, Size: 1693 bytes --]
From c963f6b7bd4cfffd98894ea05220a6fb80abfb3e Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 18 Oct 2019 13:21:11 -0700
Subject: [PATCH 2/2] Generalize list1i etc. to all signed integer types
* src/lisp.h (list1i, list2i, list3i, list4i):
Accept intmax_t instead of EMACS_INT, and use make_int instead
of make_fixnum. This should help avoid integer-overflow
problems akin to the Time bug (Bug#37795).
---
src/lisp.h | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/lisp.h b/src/lisp.h
index fe20add2d7..04fa1d64ea 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3862,28 +3862,27 @@ #define pure_list(...) \
/* Build a frequently used 1/2/3/4-integer lists. */
INLINE Lisp_Object
-list1i (EMACS_INT x)
+list1i (intmax_t a)
{
- return list1 (make_fixnum (x));
+ return list1 (make_int (a));
}
INLINE Lisp_Object
-list2i (EMACS_INT x, EMACS_INT y)
+list2i (intmax_t a, intmax_t b)
{
- return list2 (make_fixnum (x), make_fixnum (y));
+ return list2 (make_int (a), make_int (b));
}
INLINE Lisp_Object
-list3i (EMACS_INT x, EMACS_INT y, EMACS_INT w)
+list3i (intmax_t a, intmax_t b, intmax_t c)
{
- return list3 (make_fixnum (x), make_fixnum (y), make_fixnum (w));
+ return list3 (make_int (a), make_int (b), make_int (c));
}
INLINE Lisp_Object
-list4i (EMACS_INT x, EMACS_INT y, EMACS_INT w, EMACS_INT h)
+list4i (intmax_t a, intmax_t b, intmax_t c, intmax_t d)
{
- return list4 (make_fixnum (x), make_fixnum (y),
- make_fixnum (w), make_fixnum (h));
+ return list4 (make_int (a), make_int (b), make_int (c), make_int (d));
}
extern Lisp_Object make_uninit_bool_vector (EMACS_INT);
--
2.21.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-10-18 20:33 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-17 16:25 bug#37795: 26.1; Fixnum overflow on dpyinfo->last_user_time Stefan Monnier
2019-10-18 20:33 ` Paul Eggert
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.