From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template =?UTF-8?Q?=E2=80=98g=E2=80=99:?= Match data clobbered by buffer modification hooks) Date: Tue, 19 Jul 2016 18:36:08 +0300 Message-ID: <838twx1vyv.fsf__34619.0729913816$1468942964$gmane$org@gnu.org> References: <87vb066ejv.fsf@linaro.org> <8360s67qcp.fsf@gnu.org> <87bn1yyaui.fsf@linaro.org> <87mvlhmv0x.fsf_-_@moondust.awandering> <837fcl5zs9.fsf@gnu.org> <87a8hgkwcb.fsf@linaro.org> <8360s42mcb.fsf@gnu.org> <87eg6rgmlg.fsf@gmail.com> <83lh0y24y6.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: ger.gmane.org 1468942964 11917 80.91.229.3 (19 Jul 2016 15:42:44 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 19 Jul 2016 15:42:44 +0000 (UTC) Cc: 23917@debbugs.gnu.org, rpluim@gmail.com, jwiegley@gmail.com, alex.bennee@linaro.org, nljlistbox2@gmail.com To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Jul 19 17:42:32 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1bPXAB-0004Wg-Kx for geb-bug-gnu-emacs@m.gmane.org; Tue, 19 Jul 2016 17:42:27 +0200 Original-Received: from localhost ([::1]:56779 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPXAA-0000V6-J1 for geb-bug-gnu-emacs@m.gmane.org; Tue, 19 Jul 2016 11:42:26 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:54274) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPX54-0002MH-9D for bug-gnu-emacs@gnu.org; Tue, 19 Jul 2016 11:37:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPX51-0008CJ-VV for bug-gnu-emacs@gnu.org; Tue, 19 Jul 2016 11:37:09 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:45116) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPX4w-0008BG-Ku; Tue, 19 Jul 2016 11:37:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bPX4w-0001zT-Gv; Tue, 19 Jul 2016 11:37:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org, emacs-orgmode@gnu.org Resent-Date: Tue, 19 Jul 2016 15:37:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 23917 X-GNU-PR-Package: emacs,org-mode X-GNU-PR-Keywords: Original-Received: via spool by 23917-submit@debbugs.gnu.org id=B23917.14689425907605 (code B ref 23917); Tue, 19 Jul 2016 15:37:02 +0000 Original-Received: (at 23917) by debbugs.gnu.org; 19 Jul 2016 15:36:30 +0000 Original-Received: from localhost ([127.0.0.1]:57452 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bPX4Q-0001yb-E8 for submit@debbugs.gnu.org; Tue, 19 Jul 2016 11:36:30 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:34866) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bPX4O-0001yO-Ag for 23917@debbugs.gnu.org; Tue, 19 Jul 2016 11:36:28 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPX4E-0007yq-VY for 23917@debbugs.gnu.org; Tue, 19 Jul 2016 11:36:23 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:46623) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPX44-0007xT-Hb; Tue, 19 Jul 2016 11:36:08 -0400 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:3433 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1bPX42-0004yy-H2; Tue, 19 Jul 2016 11:36:06 -0400 In-reply-to: (message from Stefan Monnier on Mon, 18 Jul 2016 20:58:35 -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:121262 Archived-At: > From: Stefan Monnier > Cc: Robert Pluim , 23917@debbugs.gnu.org, alex.bennee@linaro.org, jwiegley@gmail.com, nljlistbox2@gmail.com > Date: Mon, 18 Jul 2016 20:58:35 -0400 > > > I think this change performed by save-match-data is harmless: the old > > value (62) was not valid any more anyway. > > In this particular case, yes. But only in this case, because (a) > there's actually only one sub-expression, and (b) it ends exactly at > EOB. Moreover, if the value of 62 was left alone by save-match-data, the adjustment code in replace-match would have fixed it. That's what that code is all about: it fixes the match data due to changes to the buffer made by replacing the old text by the new. Any attempts to "fix" those values under that code's feet are not helpful; quite the contrary. > > So I think a safe fix is to try and relax the check we added to > > replace-match so it doesn't get all worked up when something ≥ EOB gets > > changed to something else that's also ≥ EOB. > > And lose the other sub-expressions in a more general case? Really? What really bothers me is that by just loosening the conditions under which we signal an error we defeat _valid_ code, which did use save-match-data, and yet the match data still ends up being clobbered. I don't mind making the test more loose so that invalid programs have a longer rope to hang themselves, but valid programs should not be failed, IMO. > > Or maybe instead of signaling an error, we could simply skip the "Adjust > > search data for this change". > That would still sweep the problem under the carpet, leaving the match > data bogus, so I don't like doing that. > > This said, I don't fully understand what's going on: bug#23869 reported > > a crash, but AFAICT the match-data here is only used to adjust > > search_regs which seems like it wouldn't cause a crash, even if the new > > values are bogus. > The crash in bug#23869 was due to this: > newpoint = search_regs.start[sub] + SCHARS (newtext); > [...] > /* Now move point "officially" to the start of the inserted replacement. */ > move_if_not_intangible (newpoint); <<<<<<<<<<<<<<<<<<<<<<< > because due to clobbering, newpoint became -1. > > > - '((save-match-data-internal (match-data))) > > > + '((save-match-data-internal (match-data 'integers))) > > > > That looks risky. > > Then how about manually doing the equivalent of save-match-data around > the call to replace_range, calling match-data with non-nil argument? Here are some more alternatives for dealing with this issue: (1) Make match-data use integers instead of markers by default when a call to replace-match is in progress, i.e. when match-data is called from some buffer-modification hook triggered by replace-match (2) Don't signal an error, even if match data seems clobbered, if the new value of point is valid and either (a) there's only one search register, or (b) the adjustment value is zero (i.e. the registers will be left unchanged). I like (1) better than (2) because (1) will let valid programs avoid clobbering match data, but maybe it's too risky for emacs-25. Also, is it plausible that some buffer-modification hook will edit the buffer in the save-match-data forms, and expect the match data to be adjusted, or is this something that a buffer-modification hook should never do? If valid programs can do this, then (1) is probably not a good idea.