Subject: 26.1; Fixnum overflow on dpyinfo->last_user_time
From: Stefan Monnier <[email-masked]>
To: [email-masked]
Date: Thu, 17 Oct 2019 12:25:43 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

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);