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#23869: 25.0.95; replace-match can crash emacs Date: Thu, 30 Jun 2016 18:04:13 +0300 Message-ID: <83twgavhua.fsf@gnu.org> References: <837fd8vwf5.fsf@gnu.org> <8360ssvv17.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1467299130 1831 80.91.229.3 (30 Jun 2016 15:05:30 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 30 Jun 2016 15:05:30 +0000 (UTC) Cc: 23869@debbugs.gnu.org To: Leo Liu , Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Jun 30 17:05:19 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 1bIdWl-00021K-9U for geb-bug-gnu-emacs@m.gmane.org; Thu, 30 Jun 2016 17:05:15 +0200 Original-Received: from localhost ([::1]:50822 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bIdWk-0003PP-HD for geb-bug-gnu-emacs@m.gmane.org; Thu, 30 Jun 2016 11:05:14 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:49054) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bIdWd-0003Nj-4A for bug-gnu-emacs@gnu.org; Thu, 30 Jun 2016 11:05:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bIdWY-0001aM-LD for bug-gnu-emacs@gnu.org; Thu, 30 Jun 2016 11:05:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:48642) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bIdWY-0001aI-HT for bug-gnu-emacs@gnu.org; Thu, 30 Jun 2016 11:05:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bIdWY-0001OX-B0 for bug-gnu-emacs@gnu.org; Thu, 30 Jun 2016 11:05:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 30 Jun 2016 15:05:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 23869 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 23869-submit@debbugs.gnu.org id=B23869.14672990865323 (code B ref 23869); Thu, 30 Jun 2016 15:05:02 +0000 Original-Received: (at 23869) by debbugs.gnu.org; 30 Jun 2016 15:04:46 +0000 Original-Received: from localhost ([127.0.0.1]:60979 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bIdWH-0001Nm-0Y for submit@debbugs.gnu.org; Thu, 30 Jun 2016 11:04:45 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:58004) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bIdWF-0001NS-9O for 23869@debbugs.gnu.org; Thu, 30 Jun 2016 11:04:43 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bIdW6-0001YY-Po for 23869@debbugs.gnu.org; Thu, 30 Jun 2016 11:04:38 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:41202) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bIdW6-0001YS-Mi; Thu, 30 Jun 2016 11:04:34 -0400 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:4540 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1bIdW4-0003zi-Kp; Thu, 30 Jun 2016 11:04:33 -0400 In-reply-to: (message from Leo Liu on Thu, 30 Jun 2016 11:27:07 +0800) 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:120228 Archived-At: > From: Leo Liu > Cc: 23869@debbugs.gnu.org > Date: Thu, 30 Jun 2016 11:27:07 +0800 > > 1. Save the attached bug.el and bug.m to disk > 2. start emacs like this: > emacs -q -l ./bug.el ./bug.m > 3. Make sure we are in the bug.m buffer and eval > (unexport-defun "is_odd/1") > > Emacs should crash. The code which triggers the bug is this (from your bug.el): (add-hook 'first-change-hook (lambda () (and (derived-mode-p 'octave-mode) (smie-config-guess)))) When replace-match modifies the buffer, this hook gets run, and smie-config-guess it calls feels free to call functions that clobber match-data right under replace-match's feet, something that we don't expect, because replace-match needs to use the match data after the replacement, to adjust point for the replacement. Morale: a buffer-modification hook should always save-match-data. Of course, Emacs shouldn't crash if a hook doesn't, so I propose the patch below. This is a nasty bug, and I'd like to install it on the release branch. So I'd like to hear Stefan's opinion on (a) whether the fix looks correct and safe for the release branch, and (b) whether we might want to fix this on master in some other way. > There may be another bug in replace-match. But it was something I saw > nearly half a year ago. I was in a hurry and chose a quick workaround to > move on. > > The bug is this: replace-match may move point to point-min when changed > region doesn't involve point-min. The trigger is similar to this bug > i.e. smie-config-guess in first-change-hook. Unfortunately I forgot how > to reproduce this bug. I thought I mention it in case it is obvious from > the code. I believe this is a manifestation of the same bug. Once a modification hook clobbers match-data, it's sheer luck (or lack thereof) where point will wind up when replace-match returns, because replace-match moves point to adjust it after replacement -- this is what caused the crash in the first place, because the recipe causes replace-match to try to move point to buffer position of -1. Here's the proposed patch: --- src/search.c~0 2016-06-06 10:08:54.000000000 +0300 +++ src/search.c 2016-06-30 17:26:23.678839500 +0300 @@ -2684,19 +2684,35 @@ since only regular expressions have dist xfree (substed); } + /* The functions below modify the buffer, so they could trigger + various modification hooks (see signal_before_change and + signal_after_change), which might clobber the match data we need + to adjust after the replacement. So we save and restore the + match data around the calls. */ + ptrdiff_t sub_start = search_regs.start[sub]; + ptrdiff_t sub_end = search_regs.end[sub]; + save_search_regs (); + /* Replace the old text with the new in the cleanest possible way. */ - replace_range (search_regs.start[sub], search_regs.end[sub], - newtext, 1, 0, 1); - newpoint = search_regs.start[sub] + SCHARS (newtext); + replace_range (sub_start, sub_end, newtext, 1, 0, 1); + + newpoint = sub_start + SCHARS (newtext); if (case_action == all_caps) - Fupcase_region (make_number (search_regs.start[sub]), + Fupcase_region (make_number (sub_start), make_number (newpoint), Qnil); else if (case_action == cap_initial) - Fupcase_initials_region (make_number (search_regs.start[sub]), + Fupcase_initials_region (make_number (sub_start), make_number (newpoint)); + restore_search_regs (); + /* Last line of defense, in case search registers were actually not + saved (because someone else already occupied the save slots). */ + if (search_regs.start[sub] != sub_start + || search_regs.end[sub] != sub_end) + error ("Match data clobbered by buffer modification hooks"); + /* Adjust search data for this change. */ { ptrdiff_t oldend = search_regs.end[sub]; @@ -3017,8 +3033,10 @@ static bool search_regs_saved; static struct re_registers saved_search_regs; static Lisp_Object saved_last_thing_searched; -/* Called from Flooking_at, Fstring_match, search_buffer, Fstore_match_data - if asynchronous code (filter or sentinel) is running. */ +/* Called from Flooking_at, Fstring_match, search_buffer, + Fstore_match_data if asynchronous code (filter or sentinel) is + running. Also called from Freplace_match, to countermand possible + clobbering of match data by buffer-modification hooks. */ static void save_search_regs (void) { @@ -3037,7 +3055,8 @@ save_search_regs (void) } } -/* Called upon exit from filters and sentinels. */ +/* Called upon exit from filters and sentinels, and before updating + the match data in Freplace_match. */ void restore_search_regs (void) {