bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Eli Zaretskii]

When I visit a C source file that already has a buffer that visits it,
and that file has been changed behind Emacs's back by "git pull",
Emacs frequently signals an error.  Here's an example:

  Debugger entered--Lisp error: (args-out-of-range #<buffer xfaces.c> 1 217135)
    parse-partial-sexp(1 217135 nil nil nil syntax-table)
    c-after-change-mark-abnormal-strings(214738 215088 0)
    #f(compiled-function (fn) #<bytecode -0x1ffffffff8fc4c10>)(c-after-change-mark-abnormal-strings)
    mapc(#f(compiled-function (fn) #<bytecode -0x1ffffffff8fc4c10>) (c-depropertize-new-text c-after-change-escape-NL-in-string c-parse-quotes-after-change c-after-change-mark-abnormal-strings c-extend-font-lock-region-for-macros c-neutralize-syntax-in-CPP c-change-expand-fl-region))
    c-after-change(214738 215088 0)
    insert-file-contents("branch/src/xfaces.c" t nil nil t)
    revert-buffer-insert-file-contents--default-function("branch/src/xfaces.c" nil)
    revert-buffer--default(t t)
    revert-buffer(t t)
    find-file-noselect("branch/src/xfaces.c" nil nil t)
    find-file("branch/src/xfaces.c" t)
    funcall-interactively(find-file "branch/src/xfaces.c" t)
    call-interactively(find-file nil nil)
    command-execute(find-file)

In this particular case, the buffer's point-max is 216784, which
explains the error, but I have no idea where did the 217135 value come
from, because AFAICT the latest change in Git (the emacs-27 branch)
made that file larger, not smaller.

I hope this can be fixed soon, as this happens quite a lot to me, and
is very annoying.

In GNU Emacs 27.0.90 (build 3, i686-pc-mingw32)
 of 2020-03-06 built on HOME-C4E4A596F7
Windowing system distributor 'Microsoft Corp.', version 5.1.2600
System Description: Microsoft Windows XP Service Pack 3 (v5.1.0.2600)

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure -C --prefix=/d/usr --with-wide-int --with-modules
 'CFLAGS=-Og -gdwarf-4 -g3''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY W32NOTIFY ACL GNUTLS LIBXML2
HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS MODULES THREADS JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1255

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel dos-w32 ls-lisp disp-table term/w32-win w32-win w32-vars
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads w32notify w32 lcms2 multi-tty make-network-process
emacs)

Memory information:
((conses 16 50720 11407)
 (symbols 48 7171 1)
 (strings 16 18862 2117)
 (string-bytes 1 532860)
 (vectors 16 9508)
 (vector-slots 8 127133 8382)
 (floats 8 21 66)
 (intervals 40 252 93)
 (buffers 888 11))

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Damien Cassou]

I face the same problem while editing C# files in GNU Emacs 27.1.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Eli Zaretskii]

> From: Damien Cassou <damien@cassou.me>
> Date: Wed, 16 Sep 2020 14:24:55 +0200
> 
> 
> I face the same problem while editing C# files in GNU Emacs 27.1.

Does that happen with _any_ C# file?  If not, can you post an example
of a file where this happens, and a recipe to reproduce the problem?

Thanks.

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Damien Cassou]

Eli Zaretskii <eliz@gnu.org> writes:
> Does that happen with _any_ C# file?  If not, can you post an example
> of a file where this happens, and a recipe to reproduce the problem?

I don't know how to reproduce, it is infrequent and seems random.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Eli Zaretskii]

> From: Damien Cassou <damien@cassou.me>
> Cc: 40317@debbugs.gnu.org
> Date: Wed, 16 Sep 2020 16:51:42 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Does that happen with _any_ C# file?  If not, can you post an example
> > of a file where this happens, and a recipe to reproduce the problem?
> 
> I don't know how to reproduce, it is infrequent and seems random.

Ah, so it isn't consistent.

And the Lisp backtrace is the same as the one shown in the original
report?

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Damien Cassou]

Eli Zaretskii <eliz@gnu.org> writes:
> Ah, so it isn't consistent.
>
> And the Lisp backtrace is the same as the one shown in the original
> report?


yes it is.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Jeff Norden]

I came across this by searching for args-out-of-range bugs.  I recently found
a bug in forward-comment (which I'll post separately) that was causing
out-of-range errors for me, and I wondered if forward-comment might be
relevant to other issues.  It isn't in this case, but I think I did find the
source of the problem.

The function c-after-change (in cc-mode.el) was changed between 26.3 and 27.1,
to handle more cases where the before and/or after change functions get called
multiple times.  The function now begins (line numbers are from the current
master version) with:

1993   ;; Note: c-just-done-before-change is nil, t, or 'whole-buffer.
1994   (unless (c-called-from-text-property-change-p)
1995     (save-restriction
1996       (widen)
1997       (unless c-just-done-before-change
1998         (c-before-change (point-min) (point-max)))
1999       (unless (eq c-just-done-before-change t)
2000         (setq beg (point-min)
2001               end (point-max)
2002               old-len (- end beg)
2003               c-new-BEG (point-min)
2004               c-new-END (point-max)))
2005       (setq c-just-done-before-change nil)))
2006 
2007   ;; (c-new-BEG c-new-END) will be the region to fontify.  It may become
2008   ;; larger than (beg end).
2009   (setq c-new-END (- (+ c-new-END (- end beg)) old-len))

It looks like it is now possible for the last line above, which increments
c-new-END, to run even if c-new-END has been set to the after-change value
of point-max.  That will make c-new-END point past the end of the buffer.

---

In the backtrace from March,

>   c-after-change(214738 215088 0)

indicates that 350 bytes have been added to the buffer, although not at end.
If point-max was 216785 (which would be the value if the buffer-length was
actually 216784), then line-2009 would set c-new-END to 216785+350 = 217135,
which would then be used by c-after-change-mark-abnormal-strings when calling
parse-partial-sexp.  This fits the args-out-of-range error in the backtrace.

---

A simple fix would be to change line-2009 to 

   (setq c-new-END (min (- (+ c-new-END (- end beg)) old-len) (point-max)))

But, maybe the third 'unless' could be changed to 'if' instead, with the
increment as the else.  I don't know if c-new-END might need to be
incremented when c-called-from-text-property-change-p is true.

Unfortunately, I can't figure out how to trigger this bug myself.  If you want
to be 100% sure about it, you might try adding

  (if (> c-new-END (point-max))
    (error "c-new-END is too big! %d > %d" c-new-END (point-max)))

right after line-2009 and see if it raises an error before it gets to
parse-partial-sexp.


Hope this helps,
-Jeff

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Damien Cassou]

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

Hi Jeff,

Jeff Norden <jnorden@tntech.edu> writes:
> Unfortunately, I can't figure out how to trigger this bug myself.  If you want
> to be 100% sure about it, you might try adding
>
>   (if (> c-new-END (point-max))
>     (error "c-new-END is too big! %d > %d" c-new-END (point-max)))
>
> right after line-2009 and see if it raises an error before it gets to
> parse-partial-sexp.

I wasn't sure if you wanted me to only add the `if` or if you wanted me
to also change the previous line. I've just added the `if` with the
attached patch and will report if I get the new error message.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: emacs-bug-40317.patch --]
[-- Type: text/x-diff, Size: 528 bytes --]

--- a/lisp/progmodes/cc-mode.el.orig	2020-07-29 23:40:41.000000000 +0200
+++ b/lisp/progmodes/cc-mode.el	2020-09-18 15:36:37.409779350 +0200
@@ -2007,6 +2007,10 @@
   ;; larger than (beg end).
   (setq c-new-END (- (+ c-new-END (- end beg)) old-len))
 
+  ;; Trying to fix bug#40317
+  (if (> c-new-END (point-max))
+         (error "c-new-END is too big! %d > %d" c-new-END (point-max)))
+
   (unless (c-called-from-text-property-change-p)
     (setq c-just-done-before-change nil)
     (c-save-buffer-state (case-fold-search)

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Alan Mackenzie]

Hello, Jeff.

Thanks for contributing towards this bug.

On Thu, Sep 17, 2020 at 20:21:47 -0500, Jeff Norden wrote:
> I came across this by searching for args-out-of-range bugs.  I recently found
> a bug in forward-comment (which I'll post separately) that was causing
> out-of-range errors for me, and I wondered if forward-comment might be
> relevant to other issues.  It isn't in this case, but I think I did find the
> source of the problem.

> The function c-after-change (in cc-mode.el) was changed between 26.3 and 27.1,
> to handle more cases where the before and/or after change functions get called
> multiple times.  The function now begins (line numbers are from the current
> master version) with:

> 1993   ;; Note: c-just-done-before-change is nil, t, or 'whole-buffer.
> 1994   (unless (c-called-from-text-property-change-p)
> 1995     (save-restriction
> 1996       (widen)
> 1997       (unless c-just-done-before-change
> 1998         (c-before-change (point-min) (point-max)))
> 1999       (unless (eq c-just-done-before-change t)
> 2000         (setq beg (point-min)
> 2001               end (point-max)
> 2002               old-len (- end beg)
> 2003               c-new-BEG (point-min)
> 2004               c-new-END (point-max)))
> 2005       (setq c-just-done-before-change nil)))
> 2006 
> 2007   ;; (c-new-BEG c-new-END) will be the region to fontify.  It may become
> 2008   ;; larger than (beg end).
> 2009   (setq c-new-END (- (+ c-new-END (- end beg)) old-len))

> It looks like it is now possible for the last line above, which increments
> c-new-END, to run even if c-new-END has been set to the after-change value
> of point-max.  That will make c-new-END point past the end of the buffer.

[ .... ]

> Unfortunately, I can't figure out how to trigger this bug myself.  If you want
> to be 100% sure about it, you might try adding

I've spent quite a long time looking at this, trying various means to
trigger the error (via `insert-file-contents' and `revert-buffer').

Then it suddenly dawned on me that the (setq c-new-END (.....)) is OK.
If the body of the the last `unless' has been run, (- end beg) and
old-len are equal to each other, and to the buffer length.  So c-new-END
doesn't get changed in this case.

Of course, it's hopelessly confusing coding.  It seems to have confused
you, and it certainly confused me, even though I wrote it myself only a
short while ago.

If that code is to remain as it is, it definitely needs commenting.
There seems to be an aesthetic benefit in keeping the (setq c-new-BEG
...) separate from the ~13 line section which deals with the
out-of-sequence calling of before-change-functions and
after-change-functions.  But I'll sleep on that thought.

[ .... ]

> Hope this helps,
> -Jeff

Yes, it has helped, thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Jeff Norden]

> I've spent quite a long time looking at this, trying various means to
> trigger the error (via `insert-file-contents' and `revert-buffer').
>
> Then it suddenly dawned on me that the (setq c-new-END (.....)) is OK.
> If the body of the the last `unless' has been run, (- end beg) and
> old-len are equal to each other, and to the buffer length.  So c-new-END
> doesn't get changed in this case.

Yup, I guess I was more tired than I realized when I sent that off last
night, and jumped to a conclusion.  So, I'll fall back to my original
theory, which I changed when noticed the code that precedes the
(setq c-new-END ...) line.

Somehow, and I sure don't know how, I think that c-after-change gets
called with: c-new-END already set to the value of point-max after the
insertion; and with the other variables set so that that beg, end, and
old-len remain unchanged.  It's the only scenario that I can see that
fits the backtrace that Eli posted.

If Damien and/or Eli can temporarily try out the test that I suggested
and get it to trigger, I think that would verify this.  In fact, maybe
warn would be even better: 

  (if (> c-new-END (point-max))
    (warn "c-new-END is too big! %d > %d" c-new-END (point-max)))

This should produce a warnings window *and* a backtrace with the
args-out-of-range error.  Don't change the line above yet if the goal is
to diagnose this.  Assuming this does cause a combination warning and
backtrace to occur, then I guess there are two choices: 

1) Try to figure out how the after-change function gets called in this
   way, or
2) Just add a min to prevent c-new-END from exceeding point-max, and
   leave it at that. 

Regards,
-Jeff

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Eli Zaretskii]

> From: Jeff Norden <jnorden@tntech.edu>
> Cc: 40317@debbugs.gnu.org, eliz@gnu.org,
>   damien@cassou.me
> Date: Fri, 18 Sep 2020 17:03:07 -0500
> 
> Somehow, and I sure don't know how, I think that c-after-change gets
> called with: c-new-END already set to the value of point-max after the
> insertion; and with the other variables set so that that beg, end, and
> old-len remain unchanged.  It's the only scenario that I can see that
> fits the backtrace that Eli posted.
> 
> If Damien and/or Eli can temporarily try out the test that I suggested
> and get it to trigger, I think that would verify this.  In fact, maybe
> warn would be even better: 
> 
>   (if (> c-new-END (point-max))
>     (warn "c-new-END is too big! %d > %d" c-new-END (point-max)))

Unfortunately, the problem no longer happens to me, not in many
moons.  Not sure why: I didn't change my usage patterns.

Hopefully, Damien will be able to test this theory.  Thanks.

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Alan Mackenzie]

Hello, Eli.

On Sat, Sep 19, 2020 at 10:35:18 +0300, Eli Zaretskii wrote:
> > From: Jeff Norden <jnorden@tntech.edu>
> > Cc: 40317@debbugs.gnu.org, eliz@gnu.org,
> >   damien@cassou.me
> > Date: Fri, 18 Sep 2020 17:03:07 -0500

> > Somehow, and I sure don't know how, I think that c-after-change gets
> > called with: c-new-END already set to the value of point-max after the
> > insertion; and with the other variables set so that that beg, end, and
> > old-len remain unchanged.  It's the only scenario that I can see that
> > fits the backtrace that Eli posted.

> > If Damien and/or Eli can temporarily try out the test that I suggested
> > and get it to trigger, I think that would verify this.  In fact, maybe
> > warn would be even better: 

> >   (if (> c-new-END (point-max))
> >     (warn "c-new-END is too big! %d > %d" c-new-END (point-max)))

> Unfortunately, the problem no longer happens to me, not in many
> moons.  Not sure why: I didn't change my usage patterns.

The reason is the following patch, which was committed slightly before
you reported the bug, but before you had updated your Emacs to include
it:


commit a3c2d186eb514b505e61c2a89a1df886dbfcb06b
Author: Alan Mackenzie <acm@muc.de>
Date:   Wed Mar 4 21:17:04 2020 +0000

    CC Mode: Fix the handling of two adjacent after-change-functionses.

    The bug involved failing to set c-new-END correctly, which lead to an
    args-out-of-range error when after-change-functions was invoked twice without
    an intervening invocation of before-change-functions.

    * lisp/progmodes/cc-mode.el (c-after-change): Correct a coding error in the
    handling of c-just-done-before-change.


What triggered the bug there was insert-file-contents not calling
before-change-functions when called from revert-buffer.

> Hopefully, Damien will be able to test this theory.  Thanks.

What Damien has found appears to be a bug distinct from the one you
reported in March.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Alan Mackenzie]

Hello, Damien.

On Wed, Sep 16, 2020 at 16:51:42 +0200, Damien Cassou wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> > Does that happen with _any_ C# file?  If not, can you post an example
> > of a file where this happens, and a recipe to reproduce the problem?

> I don't know how to reproduce, it is infrequent and seems random.

What do you mean by "infrequent"?  Once a month, once a week, once a
day, several times a day?  Does it happen frequently enough that you
would notice if it stopped happening?  If so ....

csharp-mode has an inappropriate use of syntax-propertize-function,
which might be damaging things.  Could you please disable this with code
like the following in your .emacs (Note: this involves a minor loss of
functionality):

    (defun dc-spf-disable ()
      (setq syntax-propertize-function nil))
    (add-hook 'csharp-mode-hook 'dc-spf-disable)

And then see if the problem with csharp-mode still occurs, and report it
here, please.  Thanks!

> -- 
> Damien Cassou

> "Success is the ability to go from one failure to another without
> losing enthusiasm." --Winston Churchill

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Damien Cassou]

Alan Mackenzie <acm@muc.de> writes:
> On Wed, Sep 16, 2020 at 16:51:42 +0200, Damien Cassou wrote:
>> Eli Zaretskii <eliz@gnu.org> writes:
>> > Does that happen with _any_ C# file?  If not, can you post an example
>> > of a file where this happens, and a recipe to reproduce the problem?
>
>> I don't know how to reproduce, it is infrequent and seems random.
>
> What do you mean by "infrequent"?  Once a month, once a week, once a
> day, several times a day?


This didn't happen for some weeks and happened twice today:

  Warning (emacs): c-new-END is too big! 1217 > 729
  Warning (emacs): c-new-END is too big! 2331 > 1902


> Does it happen frequently enough that you
> would notice if it stopped happening?  If so ....


It would take me a while to realize.


> csharp-mode has an inappropriate use of syntax-propertize-function,
> which might be damaging things.


I don't know if your comment is still relevant because csharp-mode
changed a lot recently. I'm on latest master (commit
fb1f7d57675df8c38bbf9cc3c99e0059c8b16d7f).


> Could you please disable this with code
> like the following in your .emacs (Note: this involves a minor loss of
> functionality):


do you still recommend it?


Best

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Lars Ingebrigtsen]

Damien Cassou <damien@cassou.me> writes:

>> csharp-mode has an inappropriate use of syntax-propertize-function,
>> which might be damaging things.
>
> I don't know if your comment is still relevant because csharp-mode
> changed a lot recently. I'm on latest master (commit
> fb1f7d57675df8c38bbf9cc3c99e0059c8b16d7f).
>
>> Could you please disable this with code
>> like the following in your .emacs (Note: this involves a minor loss of
>> functionality):
>
> do you still recommend it?

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

This was a year ago, and this was the final post in this thread.
Damien, are you still seeing this problem on the current trunk?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Damien Cassou]

Hi Lars,

> (I'm going through old bug reports that unfortunately weren't resolved
> at the time.)

I'm following your blog and love your work. Thank you very much!

> This was a year ago, and this was the final post in this thread.
> Damien, are you still seeing this problem on the current trunk?

I haven't been annoyed by this problem (since my switch to Emacs 28?). I
suggest closing this issue.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

bug#40317: 27.0.90; Reverting a buffer that visits C file signals an error

[Lars Ingebrigtsen]

Damien Cassou <damien@cassou.me> writes:

> I haven't been annoyed by this problem (since my switch to Emacs 28?). I
> suggest closing this issue.

Thanks; closing the issue, then.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no