bug#24368: 25.1; Assertion failure in attach_marker

[Eli Zaretskii]

> Date: Tue, 06 Sep 2016 08:44:52 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: 24368@debbugs.gnu.org
> 
> I now wrote a better patch for the attach_marker issue (attached).

On second thought, I don't think attach_marker should be touched at
all.  It is too low-level, and the assertions compile to nothing in a
build without --enable-checking, so we will be left without any
protection in the production version.

Instead, I think the callers of attach_marker should make sure charpos
and bytepos come from the same buffer, and are therefore in sync.  The
patch below implements that; WDYT?

> For the remainder, the problem is as follows (with emacs -Q):
> Evaluating

> (progn
>   (describe-function 'describe-function)
>   (let* ((help (get-buffer "*Help*"))
>          (marker (with-current-buffer help
>                    (copy-marker (point-max)))))
>     (goto-char marker)))
> 
> fails with (error "Marker points into wrong buffer" #<marker at 271 in 
> *Help*>)
> while evaluating
> 
> (progn
>   (describe-function 'describe-function)
>   (let* ((help (get-buffer "*Help*"))
>          (marker (with-current-buffer help
>                    (copy-marker (point-max)))))
>     (set-window-start (selected-window) marker)))
> 
> crashes in init_iterator and
> 
> (progn
>   (describe-function 'describe-function)
>   (let* ((help (get-buffer "*Help*"))
>          (marker (with-current-buffer help
>                    (copy-marker (point-max)))))
>     (with-selected-window (get-buffer-window help)
>       (set-window-point (get-buffer-window "*scratch*") marker))))
> 
> crashes in set_point_both.
> 
> We can either make these fail as in the ‘goto-char’ case or handle them
> as with attach_marker.
> 
> The former is simpler but would have to be motivated somehow.  IMHO
> ‘goto-char’ should not signal an error in that case - it's perfectly
> legitimate to use the position from another buffer's marker as value.

I agree that erroring out of goto-char in this case is sub-optimal at
best.

> The latter is tedious to implement but inherently more correct.
> 
> Your choice ;-)

I chose neither ;-)  The patch below seems to fix all of your test
cases, including the original problem with window-state-get, even
before it was fixed.  It also lifts the restriction on goto-char.

Comments?

P.S. After fixing the crashes and correcting window--state-get-1,
running your original test case causes "C-x 1" stop working: it has no
effect.  Looks like this is because Emacs thinks the window is the
major non-side window, although actually I have *scratch* and *Help*,
so the command ought to work.  Is this related or unrelated to the
issue at hand?

--- src/marker.c~0      2016-05-08 07:05:54.000000000 +0300
+++ src/marker.c        2016-09-06 12:25:46.480269500 +0300
@@ -507,7 +507,11 @@ set_marker_internal (Lisp_Object marker,
       charpos = clip_to_bounds
        (restricted ? BUF_BEGV (b) : BUF_BEG (b), charpos,
         restricted ? BUF_ZV (b) : BUF_Z (b));
-      if (bytepos == -1)
+      /* Don't believe BYTEPOS if it comes from a different buffer,
+        since that buffer might have a very different correspondence
+        between character and byte positions.  */
+      if (bytepos == -1
+         || !(MARKERP (position) && XMARKER (position)->buffer == b))
        bytepos = buf_charpos_to_bytepos (b, charpos);
       else
        bytepos = clip_to_bounds
--- src/intervals.c~0   2016-03-20 14:34:33.000000000 +0200
+++ src/intervals.c     2016-09-06 12:09:54.663524800 +0300
@@ -1821,11 +1821,16 @@ set_point (ptrdiff_t charpos)
 void
 set_point_from_marker (Lisp_Object marker)
 {
+  ptrdiff_t charpos = clip_to_bounds (BEGV, marker_position (marker), ZV);
+  ptrdiff_t bytepos = marker_byte_position (marker);
+
+  /* Don't trust the byte position if the marker belongs to a
+     different buffer.  */
   if (XMARKER (marker)->buffer != current_buffer)
-    signal_error ("Marker points into wrong buffer", marker);
-  set_point_both
-    (clip_to_bounds (BEGV, marker_position (marker), ZV),
-     clip_to_bounds (BEGV_BYTE, marker_byte_position (marker), ZV_BYTE));
+    bytepos = buf_charpos_to_bytepos (current_buffer, charpos);
+  else
+    bytepos = clip_to_bounds (BEGV_BYTE, bytepos, ZV_BYTE);
+  set_point_both (charpos, bytepos);
 }
 
 /* If there's an invisible character at position POS + TEST_OFFS in the