bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[Eli Zaretskii]

Today I started Emacs 24.3.93, and it crashed near the end of
restoring the last session from .emacs.desktop, when it was
re-creating the frames recorded in that file.  Here's the backtrace:

  Program received signal SIGSEGV, Segmentation fault.
  _malloc_internal_nolock (size=size@entry=4294967285) at gmalloc.c:897
  897     gmalloc.c: No such file or directory.
  (gdb) bt 10
  #0  _malloc_internal_nolock (size=size@entry=4294967285) at gmalloc.c:897
  #1  0x011eff12 in _realloc_internal_nolock (ptr=0x3e89600, size=4294967285)
      at gmalloc.c:1441
  #2  0x01123f22 in xrealloc (block=0x3e89600, size=4294967285) at alloc.c:717
  #3  0x0100a25e in adjust_decode_mode_spec_buffer (f=<optimized out>)
      at dispnew.c:2106
  #4  adjust_frame_glyphs (f=f@entry=0x418ca80) at dispnew.c:1756
  #5  0x0100abd0 in change_frame_size_1 (f=0x418ca80,
      new_width=<optimized out>, new_height=<optimized out>,
      pretend=pretend@entry=false, delay=delay@entry=false,
      safe=safe@entry=false, pixelwise=pixelwise@entry=true) at dispnew.c:5596
  #6  0x0100cc89 in change_frame_size (pixelwise=true, safe=<optimized out>,
      delay=false, pretend=false, new_height=<optimized out>,
      new_width=<optimized out>, f=<optimized out>) at dispnew.c:5471
  #7  do_pending_window_change (safe=safe@entry=false) at dispnew.c:5432
  #8  0x0100e9e8 in Fset_frame_size (frame=frame@entry=104658605, width=2880,
      height=3740, pixelwise=65550402) at frame.c:2645
  #9  0x010126ad in x_set_frame_parameters (f=f@entry=0x63cf6a8,
      alist=<optimized out>) at frame.c:3002
  (More stack frames follow...)
  (gdb) frame 4
  #4  adjust_frame_glyphs (f=f@entry=0x418ca80) at dispnew.c:1756
  1756    in dispnew.c
  (gdb) p/x f
  $5 = 0x418ca80
  (gdb) p f->text_cols
  $6 = -3  <<<<<<<<<<<<<<<<<<<

As you can see, the text_cols member is negative.  This is the
immediate cause of the crash, because adjust_decode_mode_spec_buffer
does this:

  static void
  adjust_decode_mode_spec_buffer (struct frame *f)
  {
    f->decode_mode_spec_buffer = xrealloc (f->decode_mode_spec_buffer,
                                           FRAME_MESSAGE_BUF_SIZE (f) + 1);
  }

and FRAME_MESSAGE_BUF_SIZE is defined like this:

  #define FRAME_MESSAGE_BUF_SIZE(f) (((int) FRAME_COLS (f)) * 4)

So we pass a negative value to xrealloc, which interprets it as a very
large positive value, with predictable results.

Some digging into this reveals the following:

  . The negative values come from w32term.c, around line 4770, where
    they are derived from the value returned by GetClientRect.
    Evidently, it sometimes returns a (0, 0, 0, 0) rectangle for the
    frame dimensions, from which we then subtract the dimensions of
    frame decorations, like scroll bar etc., and call
    change_frame_size.  (We also don't check errors returned by
    GetClientRect.)

  . change_frame_size internally validates the requested dimensions,
    and doesn't allow them to become too small.  But it does that on
    pixel dimensions, and if those are corrected, the character-unit
    dimensions are not recalculated to reflect those corrections.

Below please find a patch that I intend to commit to the emacs-24
branch if no one objects.  Martin, I'd appreciate your review,
especially for the dispnew.c parts.

TIA

--- src/w32term.c~0     2014-05-24 23:48:43 +0300
+++ src/w32term.c       2014-09-21 17:48:00 +0300
@@ -4754,34 +4754,42 @@ w32_read_socket (struct terminal *termin
              RECT rect;
              int rows, columns, width, height, text_width, text_height;
 
-             GetClientRect (msg.msg.hwnd, &rect);
-
-             height = rect.bottom - rect.top;
-             width = rect.right - rect.left;
-             text_width = FRAME_PIXEL_TO_TEXT_WIDTH (f, width);
-             text_height = FRAME_PIXEL_TO_TEXT_HEIGHT (f, height);
-             rows = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (f, height);
-             columns = FRAME_PIXEL_WIDTH_TO_TEXT_COLS (f, width);
-
-             /* TODO: Clip size to the screen dimensions.  */
-
-             /* Even if the number of character rows and columns has
-                not changed, the font size may have changed, so we need
-                to check the pixel dimensions as well.  */
-
-             if (width != FRAME_PIXEL_WIDTH (f)
-                 || height != FRAME_PIXEL_HEIGHT (f)
-                 || text_width != FRAME_TEXT_WIDTH (f)
-                 || text_height != FRAME_TEXT_HEIGHT (f))
+             if (GetClientRect (msg.msg.hwnd, &rect)
+                 /* GetClientRect evidently returns (0, 0, 0, 0) if
+                    called on a minimized frame.  Such "dimensions"
+                    aren't useful anyway.  */
+                 && !(rect.bottom == 0
+                      && rect.top == 0
+                      && rect.left == 0
+                      && rect.right == 0))
                {
-                 change_frame_size (f, text_width, text_height, 0, 1, 0, 1);
-                 SET_FRAME_GARBAGED (f);
-                 cancel_mouse_face (f);
-                 /* Do we want to set these here ????  */
-/**              FRAME_PIXEL_WIDTH (f) = width; **/
-/**              FRAME_TEXT_WIDTH (f) = text_width; **/
-/**              FRAME_PIXEL_HEIGHT (f) = height; **/
-                 f->win_gravity = NorthWestGravity;
+                 height = rect.bottom - rect.top;
+                 width = rect.right - rect.left;
+                 text_width = FRAME_PIXEL_TO_TEXT_WIDTH (f, width);
+                 text_height = FRAME_PIXEL_TO_TEXT_HEIGHT (f, height);
+                 rows = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (f, height);
+                 columns = FRAME_PIXEL_WIDTH_TO_TEXT_COLS (f, width);
+
+                 /* TODO: Clip size to the screen dimensions.  */
+
+                 /* Even if the number of character rows and columns
+                    has not changed, the font size may have changed,
+                    so we need to check the pixel dimensions as well.  */
+
+                 if (width != FRAME_PIXEL_WIDTH (f)
+                     || height != FRAME_PIXEL_HEIGHT (f)
+                     || text_width != FRAME_TEXT_WIDTH (f)
+                     || text_height != FRAME_TEXT_HEIGHT (f))
+                   {
+                     change_frame_size (f, text_width, text_height, 0, 1, 0, 
1);
+                     SET_FRAME_GARBAGED (f);
+                     cancel_mouse_face (f);
+                     /* Do we want to set these here ????  */
+                     /**               FRAME_PIXEL_WIDTH (f) = width; **/
+                     /**               FRAME_TEXT_WIDTH (f) = text_width; **/
+                     /**               FRAME_PIXEL_HEIGHT (f) = height; **/
+                     f->win_gravity = NorthWestGravity;
+                   }
                }
            }
 


--- src/dispnew.c~1     2014-08-17 07:29:32 +0300
+++ src/dispnew.c       2014-09-22 17:40:15 +0300
@@ -2139,8 +2139,11 @@ adjust_frame_glyphs_for_window_redisplay
 static void
 adjust_decode_mode_spec_buffer (struct frame *f)
 {
+  ssize_t frame_message_buf_size = FRAME_MESSAGE_BUF_SIZE (f);
+
+  eassert (frame_message_buf_size >= 0);
   f->decode_mode_spec_buffer = xrealloc (f->decode_mode_spec_buffer,
-                                        FRAME_MESSAGE_BUF_SIZE (f) + 1);
+                                        frame_message_buf_size + 1);
 }
 
 
@@ -5540,10 +5543,6 @@ change_frame_size_1 (struct frame *f, in
     {
       new_text_width = (new_width == 0) ? FRAME_TEXT_WIDTH (f) : new_width;
       new_text_height = (new_height == 0) ? FRAME_TEXT_HEIGHT (f) : new_height;
-      /* Consider rounding here: Currently, the root window can be
-        larger than the frame in terms of columns/lines.  */
-      new_cols = new_text_width / FRAME_COLUMN_WIDTH (f);
-      new_lines = new_text_height / FRAME_LINE_HEIGHT (f);
     }
   else
     {
@@ -5556,6 +5555,12 @@ change_frame_size_1 (struct frame *f, in
   /* Compute width of windows in F.  */
   /* Round up to the smallest acceptable size.  */
   check_frame_size (f, &new_text_width, &new_text_height, 1);
+  /* Recompute the dimensions in character units, since
+     check_frame_size might have changed the pixel dimensions.  */
+  /* Consider rounding here: Currently, the root window can be
+     larger than the frame in terms of columns/lines.  */
+  new_cols = new_text_width / FRAME_COLUMN_WIDTH (f);
+  new_lines = new_text_height / FRAME_LINE_HEIGHT (f);
 
   /* This is the width of the frame without vertical scroll bars and
      fringe columns.  Do this after rounding - see discussion of




In GNU Emacs 24.3.93.1 (i686-pc-mingw32)
 of 2014-08-15 on HOME-C4E4A596F7
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --prefix=/d/usr --enable-checking=yes,glyphs 'CFLAGS=-Og
 -g3''

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1255

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x r e o p <backspace> <backspace> p o r t - e m <tab> 
<return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns mail-prsvr mail-utils time-date tooltip electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp
w32-common-fns disp-table w32-win w32-vars tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew
greek romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process w32notify w32
multi-tty emacs)

Memory information:
((conses 8 74217 7009)
 (symbols 32 17535 0)
 (miscs 32 33 127)
 (strings 16 10776 4344)
 (string-bytes 1 269654)
 (vectors 8 9550)
 (vector-slots 4 384749 6002)
 (floats 8 57 196)
 (intervals 28 237 95)
 (buffers 508 11))