From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#30823: 25.3; modification-hooks of overlays are not run in some cases Date: Tue, 11 Sep 2018 14:59:07 +0300 Message-ID: <83sh2gmeic.fsf@gnu.org> References: <83lgetri7r.fsf@gnu.org> <87in9cpd3a.fsf@gmail.com> <87in48ww9l.fsf@gmail.com> <83o9e0f9uj.fsf@gnu.org> <87tvnluvp4.fsf@gmail.com> <83bm9tb2yj.fsf@gnu.org> <875zzrrzv6.fsf@gmail.com> <83h8ja395r.fsf@gnu.org> <87sh2tqikk.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1536667102 15259 195.159.176.226 (11 Sep 2018 11:58:22 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 11 Sep 2018 11:58:22 +0000 (UTC) Cc: victorhge@gmail.com, 30823@debbugs.gnu.org, monnier@iro.umontreal.ca To: Noam Postavsky Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Sep 11 13:58:18 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 1fzhJB-0003nV-5b for geb-bug-gnu-emacs@m.gmane.org; Tue, 11 Sep 2018 13:58:17 +0200 Original-Received: from localhost ([::1]:57155 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fzhLH-0002gW-QG for geb-bug-gnu-emacs@m.gmane.org; Tue, 11 Sep 2018 08:00:27 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:42911) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fzhKz-0002TT-Ul for bug-gnu-emacs@gnu.org; Tue, 11 Sep 2018 08:00:12 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fzhKt-0002Ix-S4 for bug-gnu-emacs@gnu.org; Tue, 11 Sep 2018 08:00:09 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:60237) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fzhKt-0002Ik-NX for bug-gnu-emacs@gnu.org; Tue, 11 Sep 2018 08:00:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fzhKt-0007N1-M3 for bug-gnu-emacs@gnu.org; Tue, 11 Sep 2018 08:00:03 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 11 Sep 2018 12:00:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 30823 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 30823-submit@debbugs.gnu.org id=B30823.153666715528250 (code B ref 30823); Tue, 11 Sep 2018 12:00:03 +0000 Original-Received: (at 30823) by debbugs.gnu.org; 11 Sep 2018 11:59:15 +0000 Original-Received: from localhost ([127.0.0.1]:36260 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fzhK7-0007LZ-39 for submit@debbugs.gnu.org; Tue, 11 Sep 2018 07:59:15 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:49578) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fzhK5-0007LN-IM for 30823@debbugs.gnu.org; Tue, 11 Sep 2018 07:59:14 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fzhJx-0001N6-Gc for 30823@debbugs.gnu.org; Tue, 11 Sep 2018 07:59:07 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:37746) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fzhJr-0001HH-1O; Tue, 11 Sep 2018 07:58:59 -0400 Original-Received: from [176.228.60.248] (port=2178 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1fzhJq-0002sr-J6; Tue, 11 Sep 2018 07:58:58 -0400 In-reply-to: <87sh2tqikk.fsf@gmail.com> (message from Noam Postavsky on Sat, 01 Sep 2018 12:38:19 -0400) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] 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:150218 Archived-At: > From: Noam Postavsky > Cc: victorhge@gmail.com, 30823@debbugs.gnu.org, monnier@iro.umontreal.ca > Date: Sat, 01 Sep 2018 12:38:19 -0400 > > Eli Zaretskii writes: > > >> From: Noam Postavsky > >> Cc: victorhge@gmail.com, 30823@debbugs.gnu.org, monnier@iro.umontreal.ca > >> Date: Thu, 30 Aug 2018 23:14:53 -0400 > >> > >> This makes the "safety device" redundant, but with the after-change > >> suppression added it doesn't do any harm; so if you insist, we can leave > >> it in. I don't think it's a good idea to have such things cluttering up > >> the source though. > > > > Not sure I follow this part: are you saying that we shouldn't protect > > ourselves from overlay modification hooks that record a wrong buffer? > > Hmm, I'm not sure I follow you on this. As far as I can tell, it rather > protects against a particular bug in the C code: calling modification > hooks without calling prepare_to_modify_buffer. No, the protection was meant to be more general: to avoid calling overlay modification hooks when the overlay in question is from the wrong buffer. The particular bug in C code which unearthed the problem was just one such case, but we have no reason to believe that it's the only such case. I'm not opposed to making the change you suggested for xdisp.c (although maybe it should go to master, not to emacs-26), but I would like to keep the protection in buffer.c. It just needs to be more fine-grained, to avoid causing adverse side effects, such as the problem reported here. With that in mind, WDYT about the patch below, which replaces the buffer.c portion of your patch? I've ran the tests for both bug#21824 and for this bug, and they both pass with the patch installed and with unmodified xdisp.c. Thanks. diff --git a/src/buffer.c b/src/buffer.c index b0cee71..179360c 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -4543,23 +4543,6 @@ report_overlay_modification (Lisp_Object start, Lisp_Object end, bool after, Lisp_Object *copy; ptrdiff_t i; - if (size) - { - Lisp_Object ovl - = XVECTOR (last_overlay_modification_hooks)->contents[1]; - - /* If the buffer of the first overlay in the array doesn't - match the current buffer, then these modification hooks - should not be run in this buffer. This could happen when - some code calls some insdel functions, such as del_range_1, - with the PREPARE argument false -- in that case this - function is never called to record the overlay modification - hook functions in the last_overlay_modification_hooks - array, so anything we find there is not ours. */ - if (XMARKER (OVERLAY_START (ovl))->buffer != current_buffer) - return; - } - USE_SAFE_ALLOCA; SAFE_ALLOCA_LISP (copy, size); memcpy (copy, XVECTOR (last_overlay_modification_hooks)->contents, @@ -4570,7 +4553,12 @@ report_overlay_modification (Lisp_Object start, Lisp_Object end, bool after, Lisp_Object prop_i, overlay_i; prop_i = copy[i++]; overlay_i = copy[i++]; - call_overlay_mod_hooks (prop_i, overlay_i, after, arg1, arg2, arg3); + /* It is possible that the recorded overlay has been deleted + (which makes its markers' buffers be nil), or that (due to + some bug) it belongs to a different buffer. Only run this + hook if the overlay belongs to the current buffer. */ + if (XMARKER (OVERLAY_START (overlay_i))->buffer == current_buffer) + call_overlay_mod_hooks (prop_i, overlay_i, after, arg1, arg2, arg3); } SAFE_FREE ();