From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Noam Postavsky Newsgroups: gmane.emacs.bugs Subject: bug#30931: abort() due to CHECK_ALLOCATED_AND_LIVE failure during GC Date: Fri, 30 Mar 2018 16:50:53 -0400 Message-ID: <87tvsxp9si.fsf@gmail.com> References: <0eb14aa5-8110-8336-5d3d-fac46ccc23f3@zoho.com> <9829542a-15bb-6a83-20b0-2fd4dc42b8d4@cs.ucla.edu> <87woxtpvrv.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1522443014 32598 195.159.176.226 (30 Mar 2018 20:50:14 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 30 Mar 2018 20:50:14 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.90 (gnu/linux) Cc: 30931@debbugs.gnu.org, =?UTF-8?Q?Micha=C5=82?= Kondraciuk To: Paul Eggert Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Mar 30 22:50:10 2018 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1f20yO-0008MZ-QO for geb-bug-gnu-emacs@m.gmane.org; Fri, 30 Mar 2018 22:50:09 +0200 Original-Received: from localhost ([::1]:41722 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f210S-00016c-Ge for geb-bug-gnu-emacs@m.gmane.org; Fri, 30 Mar 2018 16:52:16 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:56711) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f210J-00014X-Td for bug-gnu-emacs@gnu.org; Fri, 30 Mar 2018 16:52:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f210F-0006Pv-LB for bug-gnu-emacs@gnu.org; Fri, 30 Mar 2018 16:52:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:52574) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1f210F-0006PX-Fq for bug-gnu-emacs@gnu.org; Fri, 30 Mar 2018 16:52:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1f210F-0002dS-93 for bug-gnu-emacs@gnu.org; Fri, 30 Mar 2018 16:52:03 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Noam Postavsky Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 30 Mar 2018 20:52:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 30931 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: confirmed Original-Received: via spool by 30931-submit@debbugs.gnu.org id=B30931.152244306410037 (code B ref 30931); Fri, 30 Mar 2018 20:52:03 +0000 Original-Received: (at 30931) by debbugs.gnu.org; 30 Mar 2018 20:51:04 +0000 Original-Received: from localhost ([127.0.0.1]:60466 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1f20zI-0002bp-B4 for submit@debbugs.gnu.org; Fri, 30 Mar 2018 16:51:04 -0400 Original-Received: from mail-io0-f174.google.com ([209.85.223.174]:43750) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1f20zG-0002bI-3x for 30931@debbugs.gnu.org; Fri, 30 Mar 2018 16:51:02 -0400 Original-Received: by mail-io0-f174.google.com with SMTP id q84so12140707iod.10 for <30931@debbugs.gnu.org>; Fri, 30 Mar 2018 13:51:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=V5qujwuIudlZoXmH2OL1a6hTzQFoDKi48wCzmPrQnMs=; b=s1RnrWjM7EDEChuzPlMcGBXhaxsXn0gjrXpR6j+fZQOyaaaaoSD9XSdI3if2t2w6Lr kRo2Cx0YPjHSujRl/jWK+wVtY4mRPpGwqLt6onuTJVkLnNSrBZPXlOVO6HQOSbutUa5j XpGTfjhvdiwuLarGNe7lcZyq14+5UDGIhC6VZtgrJ9pwzFsGbYJ6QtRYWZ4R8VUI1uN8 H2ZQSKnBUj0pObKC/Si6EmrehbzEOKZwXcAk8zafets9XcEI4rkXo0cX+r63/ETaBj7I mB5ppGNcV0ypftq1qGNL8HXQUaxQInclbIu1/a7tlE7qZ0bKBftBGu7t92O3dc6IGAOz KxWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=V5qujwuIudlZoXmH2OL1a6hTzQFoDKi48wCzmPrQnMs=; b=RLi9yinjhPyVYOyYl+Tr0my5Un+BeLFNbBKXd7ON93tFEfcgbFckGEVJ3R0dpJwBSr FWBxj7tXiVr7KKM4olYceTFCJ77+QATSPwhzLmJlt/W5c0YR4yifjYX1+qjAOiFl36Jo v20HQiCSxRKTeDiJtSaZxlw33povScFTA9LVl/9jeDcM8n9hQ6LwObCKr8QxWMtWn1on eyNi+0JfgZUnGb8BgtTwZPMDAGjZ7zDyZhDCJkDIGuNMp8mfWjM7KJ6tw4g54pMs/eVh X2MU2NPen3SoHb3Hxl73Spu1RdAlokwOCp0PAQ1LEhVGp5wL+perL/DtsaQAUx6Io3ru jizA== X-Gm-Message-State: ALQs6tA2Nrd3Zx0EWLIZAL0gPayMpBYVZJnGQodotOYzIOH3pMF0btFR 0vC+osI0wAT0H2B1PqzqKhQ= X-Google-Smtp-Source: AIpwx49S6w8TuBrzl3AXIXztKIrRbUAS0nnLwpTpuS+OuP3WWlEIfgMWYB4Kx0BJubKxmR3BgHSx1Q== X-Received: by 10.107.84.8 with SMTP id i8mr519766iob.254.1522443056488; Fri, 30 Mar 2018 13:50:56 -0700 (PDT) Original-Received: from zebian (cbl-45-2-119-34.yyz.frontiernetworks.ca. [45.2.119.34]) by smtp.googlemail.com with ESMTPSA id 199-v6sm1280936itl.1.2018.03.30.13.50.53 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 30 Mar 2018 13:50:54 -0700 (PDT) In-Reply-To: (Paul Eggert's message of "Fri, 30 Mar 2018 09:23:03 -0700") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:144750 Archived-At: --=-=-= Content-Type: text/plain Paul Eggert writes: > On 03/30/2018 05:56 AM, Noam Postavsky wrote: >> Shouldn't we at least do set-marker? >> >> Fset_marker (XCAR (data), Qnil, Qnil) >> Fset_marker (XCDR (data), Qnil, Qnil) > > Possibly. And perhaps we should making similar changes to the other > two calls to free_marker (in insdel.c's signal_before_change), Oh, good catch. Here's a test-case for that one: (with-temp-buffer (insert "1234567890") (setq buffer-undo-list nil) (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) (type-of (car (nth 3 buffer-undo-list)))) This prints (("678" . 1) ("12345" . 1) (# . -1) (# . -1) (# . -4)) and then aborts. > and removing the definition of free_marker while we're at it. I think that is sensible, see following patch. --=-=-= Content-Type: text/x-diff; charset=utf-8 Content-Disposition: inline; filename=0001-Fix-another-case-of-freed-markers-in-the-undo-list-B.patch Content-Transfer-Encoding: quoted-printable Content-Description: patch >From cf327524c9d66969051f429da6f004003850ffef Mon Sep 17 00:00:00 2001 From: Noam Postavsky Date: Fri, 30 Mar 2018 16:44:24 -0400 Subject: [PATCH] Fix another case of freed markers in the undo-list (Bug#30931) * 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. --- src/alloc.c | 9 --------- src/editfns.c | 10 ++++++---- src/insdel.c | 7 +++++-- src/lisp.h | 1 - test/src/editfns-tests.el | 51 +++++++++++++++++++++++++++++++++++++++++++= ++++ 5 files changed, 62 insertions(+), 16 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index 369592d70e..9fdcc7306a 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -3878,15 +3878,6 @@ build_marker (struct buffer *buf, ptrdiff_t charpos,= ptrdiff_t bytepos) return obj; } =20 -/* 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 diff --git a/src/editfns.c b/src/editfns.c index 727f2d0080..84de679273 100644 --- a/src/editfns.c +++ b/src/editfns.c @@ -3899,10 +3899,12 @@ save_restriction_restore (Lisp_Object data) =20 buf->clip_changed =3D 1; /* Remember that the narrowing changed. */ } - /* This isn=E2=80=99t needed anymore, so don=E2=80=99t 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=E2=80=99s undo list (Bug#30931). */ + /* This isn=E2=80=99t needed anymore, so don=E2=80=99t 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=E2=80= =99s + 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 diff --git a/src/insdel.c b/src/insdel.c index 02e3f41bc9..697395c507 100644 --- a/src/insdel.c +++ b/src/insdel.c @@ -2148,10 +2148,13 @@ signal_before_change (ptrdiff_t start_int, ptrdiff_= t end_int, FETCH_START, FETCH_END, Qnil); } =20 + /* 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; =20 unbind_to (count, Qnil); diff --git a/src/lisp.h b/src/lisp.h index b931d23bf3..f471b21658 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3812,7 +3812,6 @@ make_uninit_sub_char_table (int depth, int min_char) 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); diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el index 442ad08937..2e20c9dd12 100644 --- a/test/src/editfns-tests.el +++ b/test/src/editfns-tests.el @@ -288,4 +288,55 @@ transpose-test-get-byte-positions (buffer-string) "foo bar baz qux")))))) =20 +(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=3D30931#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) (# . -1) (# . 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=3D30931#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) (# . -1) + ;; (# . -1) (# . -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 --=20 2.11.0 --=-=-=--