bug#7952: 24.0.50; crash in find_interval

bug#7952: 24.0.50; crash in find_interval

[Romain Francoise]

I tried to update Emacs again, and this bug is still there.

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Romain Francoise <romain@orebokech.com>
> Date: Wed, 09 Mar 2011 13:25:05 +0100
> Cc: 
> 
> I tried to update Emacs again, and this bug is still there.

This crash happens here:

  if (INTERVAL_HAS_OBJECT (tree))
    {
      Lisp_Object parent;
      GET_INTERVAL_OBJECT (parent, tree);
      if (BUFFERP (parent))
	relative_position -= BUF_BEG (XBUFFER (parent));
    }

  if (relative_position > TOTAL_LENGTH (tree))
    abort ();   <<<<<<<<<<<<<<<<<<<<<<<<<<

Can you show the value of relative_position and of TOTAL_LENGTH (tree)?

FWIW, I couldn't crash today's build on MS-Windows using your recipe.
Perhaps I didn't move around enough -- does it take you a lot?

bug#7952: 24.0.50; crash in find_interval

[Romain Francoise]

Hi Eli,

Thanks for your help.

Eli Zaretskii <eliz@gnu.org> writes:

> Can you show the value of relative_position and of TOTAL_LENGTH
> (tree)?

As you can see from the backtrace in my original report, tree is
NULL when `find_interval' is called, so TOTAL_LENGTH(tree) is 0.

(gdb) f 5
#5  0x00000000006612b8 in find_interval (tree=0x0, position=1626800)
    at intervals.c:635
635         abort ();                   /* Paranoia */
(gdb) p relative_position
$1 = 1626799
(gdb) p tree
$2 = (INTERVAL) 0x0
(gdb) p position
$3 = 1626800
(gdb)

> Perhaps I didn't move around enough -- does it take you a lot?

It's usually enough to do M-> and M-v, but sometimes it takes a
little more moving around to get it to crash (but it never takes
more than a few seconds).

bug#7952: 24.0.50; crash in find_interval

[Romain Francoise]

Still crashes as of revision 103683, but now it hits the abort at
marker.c:130 rather than the one at intervals.c:635. I tried
bisecting it, without success so far.

Program terminated with signal 6, Aborted.
#0  0x00007f5b748c0447 in kill () at ../sysdeps/unix/syscall-template.S:82
82      ../sysdeps/unix/syscall-template.S: No such file or directory.
        in ../sysdeps/unix/syscall-template.S
(gdb) bt
#0  0x00007f5b748c0447 in kill () at ../sysdeps/unix/syscall-template.S:82
#1  0x0000000000558617 in fatal_error_signal (sig=6) at emacs.c:342
#2  <signal handler called>
#3  0x00007f5b748c0447 in kill () at ../sysdeps/unix/syscall-template.S:82
#4  0x0000000000558636 in abort () at emacs.c:371
#5  0x00000000005956cd in buf_charpos_to_bytepos (b=0x11511c0, charpos=1332719)
    at marker.c:130
#6  0x00000000005b7a51 in fast_looking_at (regexp=13575105, pos=1331082,
    pos_byte=1331194, limit=1332719, limit_byte=-1, string=12593666)
    at search.c:566
#7  0x000000000066fc57 in autocmp_chars (rule=16637925, charpos=1331082,
    bytepos=1331194, limit=1332719, win=0x117e910, face=0x114e3d0,
    string=12593666) at composite.c:916
#8  0x0000000000671e46 in composition_reseat_it (cmp_it=0x7fff61245610,
    charpos=1331082, bytepos=1331194, endpos=1332719, w=0x117e910,
    face=0x114e3d0, string=12593666) at composite.c:1266
#9  0x000000000044114e in next_element_from_buffer (it=0x7fff61244de0)
    at xdisp.c:6693
#10 0x0000000000440f5e in next_element_from_buffer (it=0x7fff61244de0)
    at xdisp.c:6660
#11 0x000000000043dd2c in get_next_display_element (it=0x7fff61244de0)
    at xdisp.c:5630
#12 0x0000000000441a43 in move_it_in_display_line_to (it=0x7fff61244de0,
    to_charpos=1332455, to_x=-1, op=MOVE_TO_POS) at xdisp.c:6956
#13 0x0000000000443274 in move_it_to (it=0x7fff61244de0, to_charpos=1332455,
    to_x=-1, to_y=25, to_vpos=-1, op=10) at xdisp.c:7406
#14 0x0000000000443e5f in move_it_vertically (it=0x7fff61244de0, dy=8)
    at xdisp.c:7691
#15 0x0000000000443d94 in move_it_vertically_backward (it=0x7fff61244de0,
    dy=25) at xdisp.c:7665
#16 0x0000000000453c5e in redisplay_window (window=18344213, just_this_one_p=0)
    at xdisp.c:14205
#17 0x000000000044e4ed in redisplay_window_0 (window=18344213) at xdisp.c:12362
#18 0x00000000005f72c8 in internal_condition_case_1 (
    bfun=0x44e4ae <redisplay_window_0>, arg=18344213, handlers=12564150,
    hfun=0x44e47f <redisplay_window_error>) at eval.c:1455
#19 0x000000000044e460 in redisplay_windows (window=18344213) at xdisp.c:12342
#20 0x000000000044d47d in redisplay_internal (preserve_echo_area=1)
    at xdisp.c:11919
#21 0x000000000044dc97 in redisplay_preserve_echo_area (from_where=12)
    at xdisp.c:12170
#22 0x0000000000651789 in wait_reading_process_output (time_limit=120,
    microsecs=0, read_kbd=-1, do_display=1, wait_for_cell=12593666,
    wait_proc=0x0, just_wait_proc=0) at process.c:4974
#23 0x0000000000421e9f in sit_for (timeout=480, reading=1, do_display=1)
    at dispnew.c:6017
...

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Romain Francoise <romain@orebokech.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Fri, 18 Mar 2011 20:19:55 +0100
> 
> Still crashes as of revision 103683, but now it hits the abort at
> marker.c:130 rather than the one at intervals.c:635.

Is this repeatable enough to present a recipe starting from
"emacs -Q"?

This backtrace doesn't look similar to the previous one at all, FWIW.
It happens in a totally different and unrelated place of the display
engine.  So why do you think it's the same problem?

bug#7952: 24.0.50; crash in find_interval

[Romain Francoise]

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:

> Is this repeatable enough to present a recipe starting from
> "emacs -Q"?

Sure, the recipe is the same as in my opening message:

1) Start ./src/emacs -Q -nw in the top-level of the source tree
2) Do M-x grep-find RET emacs RET
3) Do C-x 0 to maximize the window with the *grep* buffer
4) Move around with C-v, M-v, M-< or M-> until it crashes (e.g. M-> M-v)

Also, this is not specific to my machine because I can
reproduce it on fencepost. I generated a core file for you
(~rfrancoise/emacs/core) if you want to have a closer look.

> This backtrace doesn't look similar to the previous one at all, FWIW.
> It happens in a totally different and unrelated place of the display
> engine.  So why do you think it's the same problem?

Because it happens in the same circumstances. But actually, I've now
tried it a few more times and hit the other abort again, once (the
one at intervals.c:635)... so it could be two bugs, or a bug that
results in two different aborts.

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Romain Francoise <romain@orebokech.com>
> Cc: 7952@debbugs.gnu.org
> Date: Fri, 18 Mar 2011 21:45:30 +0100
> 
> Also, this is not specific to my machine because I can
> reproduce it on fencepost. I generated a core file for you
> (~rfrancoise/emacs/core) if you want to have a closer look.

Now, that's one weird crash.  Observe:

  (gdb) bt 10
  #0  0x00007fd2af56dd57 in kill () from /lib/libc.so.6
  #1  0x000000000053bb1d in fatal_error_signal (sig=6) at emacs.c:342
  #2  <signal handler called>
  #3  0x00007fd2af56dd57 in kill () from /lib/libc.so.6
  #4  0x000000000053bb3c in abort () at emacs.c:371
  #5  0x0000000000648688 in find_interval (tree=0x0, position=255426) at intervals.c:635
  #6  0x000000000064af6f in set_point_both (charpos=255426, bytepos=255426) at intervals.c:2007
  #7  0x000000000048a9b2 in window_scroll_line_based (window=18497157, n=-49, whole=1, noerror=0) at window.c:5123
  #8  0x00000000004896cf in window_scroll (window=18497157, n=-1, whole=1, noerror=0) at window.c:4713
  #9  0x000000000048ad6d in scroll_command (n=12466818, direction=-1) at window.c:5246
  (More stack frames follow...)
  (gdb) frame 6
  #6  0x000000000064af6f in set_point_both (charpos=255426, bytepos=255426) at intervals.c:2007
  (gdb) l
  2007      to = find_interval (BUF_INTERVALS (current_buffer), charpos);
  2008      if (charpos == BEGV)
  2009        toprev = 0;
  2010      else if (to && to->position == charpos)
  2011        toprev = previous_interval (to);
  2012      else
  2013        toprev = to;
  2014
  2015      buffer_point = (PT == ZV ? ZV - 1 : PT);
  2016
  (gdb) p current_buffer->text->intervals
  $24 = (INTERVAL) 0x314b4c0
  (gdb) down
  #5  0x0000000000648688 in find_interval (tree=0x0, position=255426) at intervals.c:635
  (gdb) info address tree
  Symbol "tree" is a variable in $rax.
  (gdb)

current_buffer->text->intervals is the expansion of
"BUF_INTERVALS (current_buffer)" (btw, please use -g3 when you compile
an unoptimized version, because then the macro information is present
in the debug info in a form that GDB can use, rather than me having to
look up the macro in the sources and manually expand it).

So BUF_INTERVALS (current_buffer) is 0x314b4c0 in set_point_both, but
when find_interval is called with that value as the first argument,
the value winds up as zero inside find_interval!  The code in
find_interval leading to the abort is this:

  INTERVAL
  find_interval (register INTERVAL tree, register EMACS_INT position)
  {
    /* The distance from the left edge of the subtree at TREE
		      to POSITION.  */
    register EMACS_INT relative_position;

    if (NULL_INTERVAL_P (tree))
      return NULL_INTERVAL;

    relative_position = position;
    if (INTERVAL_HAS_OBJECT (tree))
      {
	Lisp_Object parent;
	GET_INTERVAL_OBJECT (parent, tree);
	if (BUFFERP (parent))
	  relative_position -= BUF_BEG (XBUFFER (parent));
      }

    if (relative_position > TOTAL_LENGTH (tree))
      abort ();			/* Paranoia */

There's nothing in this code that modifies `tree' in any way.  (I even
disassembled the code to make sure.)  So how come a non-NULL value
becomes NULL here?  Since this value is passed in a register by the
caller and kept in a register from the very beginning of the function,
not even some missing GCPRO somewhere could explain this.  What am I
missing?

Ideas are welcome.

bug#7952: 24.0.50; crash in find_interval

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> There's nothing in this code that modifies `tree' in any way.  (I even
> disassembled the code to make sure.)  So how come a non-NULL value
> becomes NULL here?

It isn't, otherwise you would get a crash.

> Since this value is passed in a register by the caller and kept in a
> register from the very beginning of the function, not even some
> missing GCPRO somewhere could explain this.  What am I missing?

Probably your toolchain is too old to be able to produce complete unwind
information.  Try setting a breakpoint at the abort line to get a better
picture.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Romain Francoise <romain@orebokech.com>,  7952@debbugs.gnu.org
> Date: Sat, 19 Mar 2011 13:14:48 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > There's nothing in this code that modifies `tree' in any way.  (I even
> > disassembled the code to make sure.)  So how come a non-NULL value
> > becomes NULL here?
> 
> It isn't, otherwise you would get a crash.

Unless it happens after the place where `tree' is dereferenced.

> > Since this value is passed in a register by the caller and kept in a
> > register from the very beginning of the function, not even some
> > missing GCPRO somewhere could explain this.  What am I missing?
> 
> Probably your toolchain is too old to be able to produce complete unwind
> information.

I doubt that, since it's GDB 7.2.  Maybe it's a GCC problem.

> Try setting a breakpoint at the abort line to get a better picture.

It's a core file.  Romain, could you try that, perhaps?

In any case, we could look at TOTAL_LENGTH of the pointer in the frame
where it has a non-NULL value.

bug#7952: 24.0.50; crash in find_interval

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> I doubt that, since it's GDB 7.2.  Maybe it's a GCC problem.

Of course, the unwind information is generated by the compiler.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#7952: 24.0.50; crash in find_interval

[Romain Francoise]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Try setting a breakpoint at the abort line to get a better
> picture.

Thanks, that helped.

(gdb) p tree
$1 = (INTERVAL) 0x102cdc8
(gdb) p *tree
$2 = {
  total_length = 222096,
  position = 129685,
  left = 0x11f4d58,
  right = 0x1059098,
  up = {
    interval = 0xeee915,
    obj = 15657237
  },
  up_obj = 1,
  gcmarkbit = 0,
  write_protect = 0,
  visible = 0,
  front_sticky = 0,
  rear_sticky = 0,
  plist = 20446582
}
(gdb) p relative_position
$3 = 222657
(gdb) p TOTAL_LENGTH (tree)
$4 = 222096
(gdb) l
630           if (BUFFERP (parent))
631             relative_position -= BUF_BEG (XBUFFER (parent));
632         }
633
634       if (relative_position > TOTAL_LENGTH (tree))
635         abort ();                   /* Paranoia */
636
637       if (!handling_signal)
638         tree = balance_possible_root_interval (tree);
639
(gdb)

> Probably your toolchain is too old to be able to produce complete
> unwind information.

It happens with GCC 4.4.3 (fencepost) and 4.5.2 (my machine), both
are x86_64.

bug#7952: 24.0.50; crash in find_interval

[Chong Yidong]

Romain Francoise <romain@orebokech.com> writes:

> 1) Start ./src/emacs -Q -nw in the top-level of the source tree
> 2) Do M-x grep-find RET emacs RET
> 3) Do C-x 0 to maximize the window with the *grep* buffer
> 4) Move around with C-v, M-v, M-< or M-> until it crashes (e.g. M-> M-v)

I can reproduce it too, and have bisected the crash down to

103013: Stefan Monnier 2011-01-28 [merge]
 * progmodes/compile.el: Don't use font-lock any more.
  revno: 103013 [merge]
  committer: Stefan Monnier <monnier@iro.umontreal.ca>
  branch nick: trunk
  timestamp: Fri 2011-01-28 17:12:05 -0500
  message:
  * progmodes/compile.el: Don't use font-lock any more.

Unfortunately, this is a change in only compile.el, which means the
problem somewhere in the C code did not originate in this commit, and is
only triggered by it.

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 7952@debbugs.gnu.org
> Date: Wed, 13 Apr 2011 17:06:32 -0400
> 
> Unfortunately, this is a change in only compile.el, which means the
> problem somewhere in the C code did not originate in this commit, and is
> only triggered by it.

Do you succeed in understanding how the problem in intervals.c came
into existence?  E.g., did something change BUF_BEG behind the back of
intervals.c?  How about adding some tracing code to intervals.c and
see what comes up there?

Btw, does this problem happen only while grep-find runs, or also after
it exits?

bug#7952: 24.0.50; crash in find_interval

[Romain Francoise]

Eli Zaretskii <eliz@gnu.org> writes:

> Btw, does this problem happen only while grep-find runs, or also
> after it exits?

It also happens when the subprocess is dead. In fact, I just tried
saving the *grep* buffer to a file, opening it in a fresh Emacs and
it crashed when I did M-> M-v, the first time.

bug#7952: 24.0.50; crash in find_interval

[Romain Francoise]

Any chance some intervals expert could look at this bug? I'm still
using an old Emacs copy from January because of it and I would like
to update.

Thanks.

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Romain Francoise <romain@orebokech.com>
> Cc: Chong Yidong <cyd@stupidchicken.com>,  7952@debbugs.gnu.org
> Date: Tue, 26 Apr 2011 10:39:10 +0200
> 
> Any chance some intervals expert could look at this bug?

I'm no expert on this, but I will try this weekend, if no one beats me
to it.

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> Date: Tue, 26 Apr 2011 20:52:35 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: cyd@stupidchicken.com, 7952@debbugs.gnu.org
> 
> > From: Romain Francoise <romain@orebokech.com>
> > Cc: Chong Yidong <cyd@stupidchicken.com>,  7952@debbugs.gnu.org
> > Date: Tue, 26 Apr 2011 10:39:10 +0200
> > 
> > Any chance some intervals expert could look at this bug?
> 
> I'm no expert on this, but I will try this weekend, if no one beats me
> to it.

I found the reason.  It had nothing to do with intervals: in an Emacs
compiled with -DENABLE_CHECKING the crash happens earlier, inside
set_point_both, because the value of point passed to it is greater
than the buffer size.

The problem is that the new fontification in Grep buffers can modify
buffer text, e.g. when it finds an escape sequence emitted by Grep.
The other part of the puzzle is that vertical-motion, called from
window_scroll_line_based as part of handling M-v or C-v, enters
redisplay, which triggers JIT Lock fontification.  Here's the
Lisp-level backtrace from GDB; note the call to replace-match:

"replace-match" (0x82d760)
"progn" (0x82d940)
"eval" (0x82da14)
"font-lock-fontify-keywords-region" (0x82dc54)
"font-lock-default-fontify-region" (0x82de94)
"font-lock-fontify-region" (0x82e1f8)
"run-hook-with-args" (0x82e1f4)
"byte-code" (0x82e3a0)
"jit-lock-fontify-now" (0x82e774)
"jit-lock-function" (0x82eae4)
"scroll-down" (0x82f674)
"scroll-down-command" (0x82f8f4)
"call-interactively" (0x82fb84)

So the value of point saved by window_scroll_line_based becomes
invalid after vertical-motion returns, and the rest is history.

I fixed this on the trunk (revision 104055).  Emacs-23 branch has the
same problem, but I'd like to hear Stefan's and Chong's opinion
whether to install this change there as well (since Grep buffer
fontifications that trigger this problem were only introduced on the
trunk, and since the code in question survived without changes since
the 1990s).

bug#7952: 24.0.50; crash in find_interval

[Romain Francoise]

Eli Zaretskii <eliz@gnu.org> writes:

> I fixed this on the trunk (revision 104055).

I'm happy to confirm that rev 104055 doesn't crash anymore, thank
you very much!

Not sure if it's related, but using grep results in lots of those in
the Messages buffer:

| Error during redisplay: (args-out-of-range 26100 26140)
| Error during redisplay: (args-out-of-range 55792 55803)
| Error during redisplay: (args-out-of-range 89118 89155)
| Error during redisplay: (args-out-of-range 107767 107804)
| Error during redisplay: (args-out-of-range 119160 119176)
| Error during redisplay: (args-out-of-range 152422 152434)

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Romain Francoise <romain@orebokech.com>
> Cc: cyd@stupidchicken.com,  7952@debbugs.gnu.org
> Date: Fri, 29 Apr 2011 22:42:21 +0200
> 
> Not sure if it's related, but using grep results in lots of those in
> the Messages buffer:
> 
> | Error during redisplay: (args-out-of-range 26100 26140)
> | Error during redisplay: (args-out-of-range 55792 55803)
> | Error during redisplay: (args-out-of-range 89118 89155)
> | Error during redisplay: (args-out-of-range 107767 107804)
> | Error during redisplay: (args-out-of-range 119160 119176)
> | Error during redisplay: (args-out-of-range 152422 152434)

It's unrelated to the crash, but it's caused by the same reason:
jit-lock's function jit-lock-fontify-now also assumes that buffer
positions don't change as result of fontification.  The patch below,
which uses markers for those positions that can change, seems to fix
that.

Before I commit this, I'd appreciate a review by Stefan (and anyone
else who cares to comment), especially wrt to semi-kludgey updating of
jit-lock-context-unfontify-pos (I wasn't sure making it a marker would
be TRT).

=== modified file 'lisp/jit-lock.el'
--- lisp/jit-lock.el	2011-01-25 04:08:28 +0000
+++ lisp/jit-lock.el	2011-04-30 08:45:26 +0000
@@ -315,7 +315,8 @@ Defaults to the whole buffer.  END can b
   (with-buffer-prepared-for-jit-lock
    (save-excursion
      (unless start (setq start (point-min)))
-     (setq end (if end (min end (point-max)) (point-max)))
+     (setq end (copy-marker
+		(if end (min end (point-max)) (point-max))))
      ;; This did bind `font-lock-beginning-of-syntax-function' to
      ;; nil at some point, for an unknown reason.  Don't do this; it
      ;; can make highlighting slow due to expensive calls to
@@ -324,15 +325,16 @@ Defaults to the whole buffer.  END can b
      ;; from the end of a buffer to its start, can do repeated
      ;; `parse-partial-sexp' starting from `point-min', which can
      ;; take a long time in a large buffer.
-     (let ((orig-start start) next)
+     (let ((orig-start start)
+	   (next (make-marker)))
        (save-match-data
 	 ;; Fontify chunks beginning at START.  The end of a
 	 ;; chunk is either `end', or the start of a region
 	 ;; before `end' that has already been fontified.
 	 (while (and start (< start end))
 	   ;; Determine the end of this chunk.
-	   (setq next (or (text-property-any start end 'fontified t)
-			  end))
+	   (move-marker next (or (text-property-any start end 'fontified t)
+				 end))
 
 	   ;; Decide which range of text should be fontified.
 	   ;; The problem is that START and NEXT may be in the
@@ -340,7 +342,7 @@ Defaults to the whole buffer.  END can b
 	   ;; Until someone has a better idea, let's start
 	   ;; at the start of the line containing START and
 	   ;; stop at the start of the line following NEXT.
-	   (goto-char next)  (setq next (line-beginning-position 2))
+	   (goto-char next)  (move-marker next (line-beginning-position 2))
 	   (goto-char start) (setq start (line-beginning-position))
 
            ;; Make sure the contextual refontification doesn't re-refontify
@@ -353,7 +355,7 @@ Defaults to the whole buffer.  END can b
                       ;; it past the end of the multiline property and thus
                       ;; forget about this multiline region altogether.
                       (not (get-text-property start 'jit-lock-defer-multiline)))
-             (setq jit-lock-context-unfontify-pos next))
+             (setq jit-lock-context-unfontify-pos (marker-position next)))
 
 	   ;; Fontify the chunk, and mark it as fontified.
 	   ;; We mark it first, to make sure that we don't indefinitely
@@ -366,6 +368,13 @@ Defaults to the whole buffer.  END can b
 	     ;; before displaying the block again.
 	     (quit (put-text-property start next 'fontified nil)
 		   (funcall 'signal (car err) (cdr err))))
+	   ;; If NEXT moved as result of fontifying this chunk, update
+	   ;; jit-lock-context-unfontify-pos.
+	   (when (and jit-lock-context-unfontify-pos
+                      (>= jit-lock-context-unfontify-pos start)
+                      (> jit-lock-context-unfontify-pos next)
+                      (not (get-text-property start 'jit-lock-defer-multiline)))
+             (setq jit-lock-context-unfontify-pos (marker-position next)))
 
            ;; The redisplay engine has already rendered the buffer up-to
            ;; `orig-start' and won't notice if the above jit-lock-functions

bug#7952: 24.0.50; crash in find_interval

[Stefan Monnier]

>> Not sure if it's related, but using grep results in lots of those in
>> the Messages buffer:
>> 
>> | Error during redisplay: (args-out-of-range 26100 26140)
>> | Error during redisplay: (args-out-of-range 55792 55803)
>> | Error during redisplay: (args-out-of-range 89118 89155)
>> | Error during redisplay: (args-out-of-range 107767 107804)
>> | Error during redisplay: (args-out-of-range 119160 119176)
>> | Error during redisplay: (args-out-of-range 152422 152434)

> It's unrelated to the crash, but it's caused by the same reason:
> jit-lock's function jit-lock-fontify-now also assumes that buffer
> positions don't change as result of fontification.  The patch below,
> which uses markers for those positions that can change, seems to fix
> that.

> Before I commit this, I'd appreciate a review by Stefan (and anyone
> else who cares to comment), especially wrt to semi-kludgey updating of
> jit-lock-context-unfontify-pos (I wasn't sure making it a marker would
> be TRT).

As can be seen by a comment in grep.el, I consider grep's updating of
the buffer as a problem.  Also, we've seen other reasons why grep's
handling of escape sequences should be performed in the process filter
rather than in font-lock.  So I'd rather say that grep's use of
font-lock is wrong, rather than change jit-lock to accommodate it.

Your earlier fix that eliminates the crash is not affected by this
decision, because even bad Lisp code should not be able to crash Emacs
so easily, so even if it's triggered by grep.el's bad code, it still
needs to be fixed.


        Stefan

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Romain Francoise <romain@orebokech.com>,  cyd@stupidchicken.com,  7952@debbugs.gnu.org
> Date: Sat, 30 Apr 2011 10:16:44 -0300
> 
> As can be seen by a comment in grep.el, I consider grep's updating of
> the buffer as a problem.  Also, we've seen other reasons why grep's
> handling of escape sequences should be performed in the process filter
> rather than in font-lock.  So I'd rather say that grep's use of
> font-lock is wrong, rather than change jit-lock to accommodate it.

So you are saying that, as a matter of policy, font-lock should never
modify the text of its buffer, is that right?

bug#7952: 24.0.50; crash in find_interval

[Stefan Monnier]

>> As can be seen by a comment in grep.el, I consider grep's updating of
>> the buffer as a problem.  Also, we've seen other reasons why grep's
>> handling of escape sequences should be performed in the process filter
>> rather than in font-lock.  So I'd rather say that grep's use of
>> font-lock is wrong, rather than change jit-lock to accommodate it.
> So you are saying that, as a matter of policy, font-lock should never
> modify the text of its buffer, is that right?

[ Actually, we're talking about jit-lock, really.  ]
Yes.  We should not crash if it does, but any non-crashing behavior
(including minor redisplay glitches) is fair game, I think.


        Stefan

bug#7952: 24.0.50; crash in find_interval

[Chong Yidong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> As can be seen by a comment in grep.el, I consider grep's updating of
> the buffer as a problem.  Also, we've seen other reasons why grep's
> handling of escape sequences should be performed in the process filter
> rather than in font-lock.  So I'd rather say that grep's use of
> font-lock is wrong, rather than change jit-lock to accommodate it.

I've committed a fix for grep.el.

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Romain Francoise <romain@orebokech.com>,  7952@debbugs.gnu.org
> Date: Sun, 08 May 2011 01:18:04 -0400
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > As can be seen by a comment in grep.el, I consider grep's updating of
> > the buffer as a problem.  Also, we've seen other reasons why grep's
> > handling of escape sequences should be performed in the process filter
> > rather than in font-lock.  So I'd rather say that grep's use of
> > font-lock is wrong, rather than change jit-lock to accommodate it.
> 
> I've committed a fix for grep.el.

Thanks, but will that solution work when start-process is not
available (MS-DOS)?  I'm not sure (I think compilation-filter is not
run in that case).

bug#7952: 24.0.50; crash in find_interval

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, but will that solution work when start-process is not
> available (MS-DOS)?  I'm not sure (I think compilation-filter is not
> run in that case).

Good point.  This problem isn't limited to grep.  For the synchronous
case, compilation-start should probably run compilation-filter-hook
after call-process returns.  WDYT?

*** lisp/progmodes/compile.el	2011-05-08 05:17:17 +0000
--- lisp/progmodes/compile.el	2011-05-08 20:25:38 +0000
***************
*** 1623,1630 ****
--- 1623,1632 ----
  	    ;; regardless of where the user sees point.
  	    (goto-char (point-max))
  	    (let* ((inhibit-read-only t) ; call-process needs to modify outbuf
+ 		   (compilation-filter-start (point))
  		   (status (call-process shell-file-name nil outbuf nil "-c"
  					 command)))
+ 	      (run-hooks 'compilation-filter-hook)
  	      (cond ((numberp status)
  		     (compilation-handle-exit
  		      'exit status

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: monnier@iro.umontreal.ca, romain@orebokech.com, 7952@debbugs.gnu.org
> Date: Sun, 08 May 2011 16:27:15 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks, but will that solution work when start-process is not
> > available (MS-DOS)?  I'm not sure (I think compilation-filter is not
> > run in that case).
> 
> Good point.  This problem isn't limited to grep.  For the synchronous
> case, compilation-start should probably run compilation-filter-hook
> after call-process returns.  WDYT?

Looks good, thanks.  Go ahead and commit, I will test it when I have
time.

bug#7952: 24.0.50; crash in find_interval

[Stefan Monnier]

>> > Thanks, but will that solution work when start-process is not
>> > available (MS-DOS)?  I'm not sure (I think compilation-filter is not
>> > run in that case).
>> Good point.  This problem isn't limited to grep.  For the synchronous
>> case, compilation-start should probably run compilation-filter-hook
>> after call-process returns.  WDYT?
> Looks good, thanks.  Go ahead and commit, I will test it when I have
> time.

BTW, wouldn't it be preferable to pass the `start' position as an
argument rather passing it implicitly via a dynamic variable?


        Stefan

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Chong Yidong <cyd@stupidchicken.com>,  romain@orebokech.com,  7952@debbugs.gnu.org
> Date: Mon, 09 May 2011 11:06:52 -0300
> 
> BTW, wouldn't it be preferable to pass the `start' position as an
> argument rather passing it implicitly via a dynamic variable?

??? It's a hook we are going to call, right?  It's not just any
function.

bug#7952: 24.0.50; crash in find_interval

[Stefan Monnier]

>> BTW, wouldn't it be preferable to pass the `start' position as an
>> argument rather passing it implicitly via a dynamic variable?

> ??? It's a hook we are going to call, right?  It's not just any
> function.

run-hook-with-args (...-hook would need to be renamed to ...-functions,
of course).


        Stefan

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: cyd@stupidchicken.com,  romain@orebokech.com,  7952@debbugs.gnu.org
> Date: Mon, 09 May 2011 12:32:29 -0300
> 
> >> BTW, wouldn't it be preferable to pass the `start' position as an
> >> argument rather passing it implicitly via a dynamic variable?
> 
> > ??? It's a hook we are going to call, right?  It's not just any
> > function.
> 
> run-hook-with-args (...-hook would need to be renamed to ...-functions,
> of course).

Which breaks back-compatibility.  Unless we introduce
compilation-filter-start-functions _in_addition_to_ the existing
hook.  But that sounds gross, just to avoid a single let-bound dynamic
variable, doesn't it?

bug#7952: 24.0.50; crash in find_interval

[Stefan Monnier]

>> >> BTW, wouldn't it be preferable to pass the `start' position as an
>> >> argument rather passing it implicitly via a dynamic variable?
>> 
>> > ??? It's a hook we are going to call, right?  It's not just any
>> > function.
>> 
>> run-hook-with-args (...-hook would need to be renamed to ...-functions,
>> of course).

> Which breaks back-compatibility.

Ah, I didn't realize that compilation-filter-hook already existed in
Emacs-23, so that's the reason why "it wouldn't be preferable ...".
Thanks for the explanation.


        Stefan

bug#7952: 24.0.50; crash in find_interval

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> Looks good, thanks.  Go ahead and commit, I will test it when I have
> time.

Committed.  Tested OK by replacing (fboundp 'start-process) with nil to
force the async code path.  The only difference was that when called
through call-process, grep doesn't emit the highlighting escape
sequences by default; I had to pass "--color=always".  This may have
something to do with the way grep's --color=auto detection works.

bug#7952: 24.0.50; crash in find_interval

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: romain@orebokech.com, monnier@iro.umontreal.ca, 7952@debbugs.gnu.org
> Date: Mon, 09 May 2011 15:46:39 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Looks good, thanks.  Go ahead and commit, I will test it when I have
> > time.
> 
> Committed.  Tested OK by replacing (fboundp 'start-process) with nil to
> force the async code path.

Thanks.

> The only difference was that when called
> through call-process, grep doesn't emit the highlighting escape
> sequences by default; I had to pass "--color=always".  This may have
> something to do with the way grep's --color=auto detection works.

On MS-DOS, we already pass --color=always, see grep-compute-defaults.

bug#7952: 24.0.50; crash in find_interval

[Romain Francoise]

Emacs from Git as of revision 4817a832f49, which corresponds to the
bzr trunk as of revision 103065, crashes in find_interval() when I
move around grep buffers:

| Core was generated by `./src/emacs -nw'.
| Program terminated with signal 6, Aborted.
| #0  0x00007fc45c00c447 in kill () at ../sysdeps/unix/syscall-template.S:82
| 82      ../sysdeps/unix/syscall-template.S: No such file or directory.
|         in ../sysdeps/unix/syscall-template.S
| (gdb) bt
| #0  0x00007fc45c00c447 in kill () at ../sysdeps/unix/syscall-template.S:82
| #1  0x000000000055b203 in fatal_error_signal (sig=6) at emacs.c:343
| #2  <signal handler called>
| #3  0x00007fc45c00c447 in kill () at ../sysdeps/unix/syscall-template.S:82
| #4  0x000000000055b222 in abort () at emacs.c:372
| #5  0x0000000000669960 in find_interval (tree=0x0, position=1559343)
|     at intervals.c:635
| #6  0x000000000066c116 in set_point_both (charpos=1559343, bytepos=1559964)
|     at intervals.c:2008
| #7  0x000000000048da2d in window_scroll_line_based (window=43271749, n=-49,
|     whole=1, noerror=0) at window.c:5125
| #8  0x000000000048c726 in window_scroll (window=43271749, n=-1, whole=1,
|     noerror=0) at window.c:4715
| #9  0x000000000048dde8 in scroll_command (n=12601730, direction=-1)
|     at window.c:5248
| #10 0x000000000048deb6 in Fscroll_down (arg=12601730) at window.c:5282
| #11 0x00000000005ffb32 in Ffuncall (nargs=2, args=0x7fff5c4c84b0)
|     at eval.c:2842
| #12 0x000000000064bac7 in Fbyte_code (bytestr=10424209, vector=10424245,
|     maxdepth=12) at bytecode.c:676
| #13 0x0000000000600430 in funcall_lambda (fun=10424149, nargs=1,
|     arg_vector=0x7fff5c4c8958) at eval.c:3028
| #14 0x00000000005ffd3f in Ffuncall (nargs=2, args=0x7fff5c4c8950)
|     at eval.c:2891
| #15 0x00000000005fa4ad in Fcall_interactively (function=12963906,
|     record_flag=12601730, keys=12649141) at callint.c:842
| #16 0x00000000005ffb88 in Ffuncall (nargs=4, args=0x7fff5c4c8cd0)
|     at eval.c:2849
| #17 0x00000000005ff539 in call3 (fn=12788898, arg1=12963906, arg2=12601730,
|     arg3=12601730) at eval.c:2674
| #18 0x000000000057250c in Fcommand_execute (cmd=12963906,
|     record_flag=12601730, keys=12601730, special=12601730) at keyboard.c:10180
| #19 0x000000000055ff27 in command_loop_1 () at keyboard.c:1528
| #20 0x00000000005fccad in internal_condition_case (
|     bfun=0x55f711 <command_loop_1>, handlers=12653922,
|     hfun=0x55f019 <cmd_error>) at eval.c:1408
| #21 0x000000000055f412 in command_loop_2 (ignore=12601730) at keyboard.c:1129
| #22 0x00000000005fc64a in internal_catch (tag=12649938,
|     func=0x55f3ec <command_loop_2>, arg=12601730) at eval.c:1152
| #23 0x000000000055f3c5 in command_loop () at keyboard.c:1108
| #24 0x000000000055eb50 in recursive_edit_1 () at keyboard.c:731
| #25 0x000000000055ed03 in Frecursive_edit () at keyboard.c:793
| #26 0x000000000055d04d in main (argc=2, argv=0x7fff5c4c96d8) at emacs.c:1682
| (gdb)

Steps to reproduce:
1) Start ./src/emacs -nw in the top-level of the source tree
2) Do M-x grep-find RET emacs RET
3) Do C-x 0 to maximize the window with *grep* buffer
4) Move around with C-v/M-v until it crashes

This may be related bug #7920, but I'm not sure.