procyberian/emacs
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix another case of freed markers in the undo-list (Bug#30931)

Browse source 
* src/alloc.c (free_marker): Remove.
* src/editfns.c (save_restriction_restore):
* src/insdel.c (signal_before_change): Detach the markers from the
buffer when we're done with them instead of calling free_marker on
them.
* test/src/editfns-tests.el (delete-region-undo-markers-1)
(delete-region-undo-markers-2): New tests.
This commit is contained in:
Noam Postavsky 2018-03-30 16:44:24 -04:00 • committed by Paul Eggert
parent 842b3d7412
commit 96b8747d5c
5 changed files with 62 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
src/alloc.c
View file

@ -3878,15 +3878,6 @@ build_marker (struct buffer *buf, ptrdiff_t charpos, ptrdiff_t bytepos)
return obj;
}
/* Put MARKER back on the free list after using it temporarily. */
void
free_marker (Lisp_Object marker)
{
unchain_marker (XMARKER (marker));
free_misc (marker);
}
/* Return a newly created vector or string with specified arguments as
elements. If all the arguments are characters that can fit

10
src/editfns.c
View file

@ -3899,10 +3899,12 @@ save_restriction_restore (Lisp_Object data)
buf->clip_changed = 1; /* Remember that the narrowing changed. */
}
/* This isn’t needed anymore, so don’t wait for GC.
Do not call free_marker on XCAR (data) or XCDR (data),
though, since record_marker_adjustments may have put
them on the buffer’s undo list (Bug#30931). */
/* This isn’t needed anymore, so don’t wait for GC. Do not call
free_marker on XCAR (data) or XCDR (data), though, since
record_marker_adjustments may have put them on the buffer’s
undo list (Bug#30931). Just detach them instead. */
Fset_marker (XCAR (data), Qnil, Qnil);
Fset_marker (XCDR (data), Qnil, Qnil);
free_cons (XCONS (data));
}
else

7
src/insdel.c
View file

@ -2148,10 +2148,13 @@ signal_before_change (ptrdiff_t start_int, ptrdiff_t end_int,
FETCH_START, FETCH_END, Qnil);
}
/* Detach the markers now that we're done with them. Don't directly
free them, since the change functions could have caused them to
be inserted into the undo list (Bug#30931). */
if (! NILP (start_marker))
free_marker (start_marker);
Fset_marker (start_marker, Qnil, Qnil);
if (! NILP (end_marker))
free_marker (end_marker);
Fset_marker (end_marker, Qnil, Qnil);
RESTORE_VALUE;
unbind_to (count, Qnil);

1
src/lisp.h
View file

@ -3812,7 +3812,6 @@ extern Lisp_Object make_save_funcptr_ptr_obj (void (*) (void), void *,
extern Lisp_Object make_save_memory (Lisp_Object *, ptrdiff_t);
extern void free_save_value (Lisp_Object);
extern Lisp_Object build_overlay (Lisp_Object, Lisp_Object, Lisp_Object);
extern void free_marker (Lisp_Object);
extern void free_cons (struct Lisp_Cons *);
extern void init_alloc_once (void);
extern void init_alloc (void);

51
test/src/editfns-tests.el
View file

@ -288,4 +288,55 @@
(buffer-string)
"foo bar baz qux"))))))
(ert-deftest delete-region-undo-markers-1 ()
"Make sure we don't end up with freed markers reachable from Lisp."
;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30931#40
(with-temp-buffer
(insert "1234567890")
(setq buffer-undo-list nil)
(narrow-to-region 2 5)
;; `save-restriction' in a narrowed buffer creates two markers
;; representing the current restriction.
(save-restriction
(widen)
;; Any markers *within* the deleted region are put onto the undo
;; list.
(delete-region 1 6))
;; (princ (format "%S" buffer-undo-list) #'external-debugging-output)
;; `buffer-undo-list' is now
;; (("12345" . 1) (#<temp-marker1> . -1) (#<temp-marker2> . 1))
;;
;; If temp-marker1 or temp-marker2 are freed prematurely, calling
;; `type-of' on them will cause Emacs to abort. Calling
;; `garbage-collect' will also abort if it finds any reachable
;; freed objects.
(should (eq (type-of (car (nth 1 buffer-undo-list))) 'marker))
(should (eq (type-of (car (nth 2 buffer-undo-list))) 'marker))
(garbage-collect)))
(ert-deftest delete-region-undo-markers-2 ()
"Make sure we don't end up with freed markers reachable from Lisp."
;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30931#55
(with-temp-buffer
(insert "1234567890")
(setq buffer-undo-list nil)
;; signal_before_change creates markers delimiting a change
;; region.
(let ((before-change-functions
(list (lambda (beg end)
(delete-region (1- beg) (1+ end))))))
(delete-region 2 5))
;; (princ (format "%S" buffer-undo-list) #'external-debugging-output)
;; `buffer-undo-list' is now
;; (("678" . 1) ("12345" . 1) (#<marker in no buffer> . -1)
;; (#<temp-marker1> . -1) (#<temp-marker2> . -4))
;;
;; If temp-marker1 or temp-marker2 are freed prematurely, calling
;; `type-of' on them will cause Emacs to abort. Calling
;; `garbage-collect' will also abort if it finds any reachable
;; freed objects.
(should (eq (type-of (car (nth 3 buffer-undo-list))) 'marker))
(should (eq (type-of (car (nth 4 buffer-undo-list))) 'marker))
(garbage-collect)))
;;; editfns-tests.el ends here