Re: xdisp.c 1.794 breaks Mac OS X (Carbon)

[storm@cua.dk (Kim F. Storm)]

Steven Tamm <steventamm@mac.com> writes:

> Just updated to latest source and a change in to xdisp.c:row_containing_pos causes the osx build to break reliably after scrolling through a long document.  Reverting to 1.793 fixes everything
> 
> The problem is that row is NULL for set_cursor_from_row in try_window_id.  Should there be a check for row=NULL in try_window_id?  Here is the stack trace.
> 
> Exception:  EXC_BAD_ACCESS (0x0001)
> Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x0000006c
> Thread 0 Crashed:
>  #0   0x00020488 in set_cursor_from_row (xdisp.c:9447)
>  #1   0x00025d5c in try_window_id (xdisp.c:11774)
>  #2   0x000230e4 in redisplay_window (xdisp.c:10478)
>  #3   0x000203f8 in redisplay_window_0 (xdisp.c:9417)
>  #4   0x000c9518 in internal_condition_case_1 (eval.c:1393)
> 
> Let me know if more details are needed...
> 
> -Steven
> 

Hi Steven,

I "fixed" this problem by testing that the "row" was non-NULL before
calling set_cursor_from_row.  However, RMS has asked me to try finding
the actual cause the this error:

> + 2002-12-11  Kim F. Storm  <storm@cua.dk>
> + 
> + 	* xdisp.c (try_window_id): Don't call set_cursor_from_row if
> + 	row_containing_pos returned NULL.
> + 
> 
> This is an obvious way to avoid the crash, but is it a correct fix?  I
> don't know.  If it is not correct, the result is likely to be
> another occasional bug that puts the cursor in the wrong place.
> That would be hard to debug.
> 
> Could you please check if it is correct?  One way to check
> is to add
> 
>         else
> 	  abort ();
> 
> after each of those if statements, and run under GDB.  When it stops
> at `abort', type `return' so it won't die, then either (1) step
> through and see if the subsequent actions look reasonable or (2)
> just continue and see if the cursor comes out in the right place. 
> 

If you can still reproduce the crash (after adding the else abort();),
could you try to follow RMS' instructions above.

Now, if the cursor does indeed not appear in the right position,
could you try the following patch that RMS suggested to fix this:

The change below may fix the problem, but I am not certain it is
correct.  Anyway, can you please run under GDB all the time, with a
breakpoint set in set_cursor_from_row like this?

  b set_cursor_from_row if row==0

If it stops there, please try calling row_containing_pos the same way
it was called by try_window_id, but step through it and see precisely
why it returns zero.  In particular, what line does it return at?
(Show us the code on and around that line, not just the line number!)
What is the value of MATRIX_ROW_BOTTOM_Y (row) for the last row that
it thinks about?  And what is last_y?  Should it have found PT in the
previous row?


*** xdisp.c.~1.794.~	Sun Dec  8 16:06:46 2002
--- xdisp.c	Tue Dec 10 22:22:07 2002
***************
*** 11514,11520 ****
        /* Give up if we have gone too far.  */
        if (end && row >= end)
  	return NULL;
!       if (MATRIX_ROW_BOTTOM_Y (row) >= last_y)
  	return NULL;
  
        /* If it is in this row, return this row.  */
--- 11516,11522 ----
        /* Give up if we have gone too far.  */
        if (end && row >= end)
  	return NULL;
!       if (MATRIX_ROW_BOTTOM_Y (row) > last_y)
  	return NULL;
  
        /* If it is in this row, return this row.  */


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk