bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Noam Postavsky]

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

The circumstances that trigger this seem to be a combination of
overlays and timers. Attached is an init.el file that installs several
packages needed to trigger the problem. I have not yet managed to find
a simpler way of reproducing this. After installing the packages, and
running M-x irony-install-server (requires libclang), the problem can
be triggered by a C++ mode buffer with the following contents:

void func(int x);
int main() {
  fun|
}

where | represents the cursor. Press TAB y C-f C-b C-f C-b, this
results in a (wrong-type-argument overlayp nil) which is due to
yas--on-field-overlay-modification being called with *Messages* as the
current buffer, rather than the C++ buffer where the yasnippet's
overlay is in. The begin and end positions passed correspond to
*Messages* buffer positions (i.e. they are too big to be from the C++
buffer). According to the lisp backtrace (see below), it seems to be
triggered by a call to `message' via a timer.

Original report is at https://github.com/capitaomorte/yasnippet/issues/607

Debugger entered--Lisp error: (wrong-type-argument overlayp nil)
  overlay-get(nil yas--snippet)
  (let* ((inhibit-modification-hooks t) (field (overlay-get overlay
(quote yas--field))) (snippet (overlay-get yas--active-field-overlay
(quote yas--snippet)))) (cond (after\? (yas--advance-end-maybe field
(overlay-end overlay)) (save-excursion (yas--field-update-display
field)) (yas--update-mirrors snippet)) (field (if (and (not after\?)
(not (progn nil (or ... ...) (aref field 7))) (eq (point) (if (markerp
...) (marker-position ...) (progn nil ... ...)))) (progn
(yas--skip-and-clear field))) (progn nil (or (progn nil (and (vectorp
field) (>= ... 9) (memq ... cl-struct-yas--field-tags) t)) (signal
(quote wrong-type-argument) (list (quote yas--field) field))) (let*
((v field)) (aset v 7 t))))))
  (if (or yas--inhibit-overlay-hooks (yas--undo-in-progress)) nil
(message "yas--on-field-ov-mod(%S %S %S %S %S): buffer = %s" overlay
after\? _beg _end _length (current-buffer)) (let*
((inhibit-modification-hooks t) (field (overlay-get overlay (quote
yas--field))) (snippet (overlay-get yas--active-field-overlay (quote
yas--snippet)))) (cond (after\? (yas--advance-end-maybe field
(overlay-end overlay)) (save-excursion (yas--field-update-display
field)) (yas--update-mirrors snippet)) (field (if (and (not after\?)
(not (progn nil ... ...)) (eq (point) (if ... ... ...))) (progn
(yas--skip-and-clear field))) (progn nil (or (progn nil (and ... ...
... t)) (signal (quote wrong-type-argument) (list ... field))) (let*
((v field)) (aset v 7 t)))))))
  yas--on-field-overlay-modification(#<overlay from 41 to 42 in
foo.cpp> t 3008 3008 33)
  message("%s" "use of undeclared identifier 'y'")
  display-message-or-buffer("use of undeclared identifier 'y'"
"*Flycheck error messages*")
  (let ((messages (mapcar (function
flycheck-error-format-message-and-id) errors)))
(display-message-or-buffer (string-join messages "\n\n")
flycheck-error-message-buffer))
  (progn (let ((messages (mapcar (function
flycheck-error-format-message-and-id) errors)))
(display-message-or-buffer (string-join messages "\n\n")
flycheck-error-message-buffer)))
  (if (and errors (flycheck-may-use-echo-area-p)) (progn (let
((messages (mapcar (function flycheck-error-format-message-and-id)
errors))) (display-message-or-buffer (string-join messages "\n\n")
flycheck-error-message-buffer))))
  flycheck-display-error-messages(([cl-struct-flycheck-error #<buffer
foo.cpp> irony "/tmp/el-get-test-home/foo.cpp" 5 8 "use of undeclared
identifier 'y'" error nil]))
  funcall(flycheck-display-error-messages ([cl-struct-flycheck-error
#<buffer foo.cpp> irony "/tmp/el-get-test-home/foo.cpp" 5 8 "use of
undeclared identifier 'y'" error nil]))
  (progn (funcall flycheck-display-errors-function errors))
  (if flycheck-display-errors-function (progn (funcall
flycheck-display-errors-function errors)))
  flycheck-display-errors(([cl-struct-flycheck-error #<buffer foo.cpp>
irony "/tmp/el-get-test-home/foo.cpp" 5 8 "use of undeclared
identifier 'y'" error nil]))
  (progn (flycheck-display-errors errors))
  (if errors (progn (flycheck-display-errors errors)))
  (let ((errors (flycheck-overlay-errors-at (point)))) (if errors
(progn (flycheck-display-errors errors))))
  (progn (let ((errors (flycheck-overlay-errors-at (point)))) (if
errors (progn (flycheck-display-errors errors)))))
  (if flycheck-mode (progn (let ((errors (flycheck-overlay-errors-at
(point)))) (if errors (progn (flycheck-display-errors errors))))))
  (progn (flycheck-cancel-error-display-error-at-point-timer) (if
flycheck-mode (progn (let ((errors (flycheck-overlay-errors-at
(point)))) (if errors (progn (flycheck-display-errors errors)))))))
  (condition-case err (progn
(flycheck-cancel-error-display-error-at-point-timer) (if flycheck-mode
(progn (let ((errors (flycheck-overlay-errors-at ...))) (if errors
(progn (flycheck-display-errors errors))))))) ((debug error) (message
"Flycheck error display error: %s" err) nil))
  flycheck-display-error-at-point()
  apply(flycheck-display-error-at-point nil)
  timer-event-handler([t 22073 29035 116958 nil
flycheck-display-error-at-point nil nil 308000])


In GNU Emacs 25.0.50.1 (x86_64-unknown-linux-gnu, X toolkit)
 of 2015-11-03
Repository revision: 2390d890765983ed1e5ece1711c470351705e7c8
Windowing system distributor 'The X.Org Foundation', version 11.0.11702000
Configured using:
 'configure --cache-file=../config.cache 'CFLAGS=-O2 -march=native'
 --with-crt-dir=/usr/lib/x86_64-linux-gnu --with-x-toolkit=lucid
 --without-toolkit-scroll-bars --with-gif=no'

Configured features:
XPM JPEG TIFF PNG SOUND DBUS NOTIFY GNUTLS LIBXML2 ZLIB LUCID X11

Important settings:
  value of $LANG: en_CA.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Debugger

Minor modes in effect:
  global-flycheck-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  global-company-mode: t
  company-mode: t
  global-highlight-parentheses-mode: t
  highlight-parentheses-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-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
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
[yas] Loading snippet files from
/tmp/el-get-test-home/.emacs.d/elpa/yasnippet-20151101.1535/snippets/c++-mode
[yas] Loading for `cc-mode', just-in-time: (lambda nil
(yas--load-directory-1 (quote
/tmp/el-get-test-home/.emacs.d/elpa/yasnippet-20151101.1535/snippets/cc-mode)
(quote cc-mode)))!
[yas] Loading snippet files from
/tmp/el-get-test-home/.emacs.d/elpa/yasnippet-20151101.1535/snippets/cc-mode
Debug on Error enabled globally
[yas] snippet expanded.
yas--on-field-ov-mod(#<overlay from 41 to 46 in foo.cpp> nil 41 41
nil): buffer = foo.cpp
yas--on-field-ov-mod(#<overlay from 41 to 42 in foo.cpp> t 41 42 0):
buffer = foo.cpp
use of undeclared identifier 'y'
yas--on-field-ov-mod(#<overlay from 41 to 42 in foo.cpp> t 3008 3008
33): buffer = *Messages*
Entering debugger...

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message dired format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns mail-prsvr mail-utils debug cus-start cus-load
thingatpt lisp-mnt irony-cdb-libclang irony-cdb-json pp json
irony-cdb-clang-complete irony-cdb cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs company-files
company-oddmuse company-keywords company-etags etags xref project eieio
byte-opt bytecomp byte-compile cconv eieio-core company-gtags
company-dabbrev-code company-dabbrev company-capf company-cmake
company-xcode company-clang company-semantic company-eclim
company-template company-css company-nxml company-bbdb company-irony
irony-completion irony-snippet flycheck-irony irony-diagnostics irony
flycheck find-func rx subr-x dash jka-compr compile comint ansi-color
ring let-alist yasnippet cl-extra derived help-mode company edmacro
kmacro highlight-parentheses cl-macs easy-mmode cl gv cl-seq finder-inf
info cl-loaddefs pcase cl-lib package easymenu epg-config time-date
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel x-win term/common-win x-dnd tool-bar dnd fontset
image regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame 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 charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer 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 dbusbind inotify dynamic-setting
x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 300138 13183)
 (symbols 48 31710 0)
 (miscs 40 474 275)
 (strings 32 53496 8101)
 (string-bytes 1 1682695)
 (vectors 16 23094)
 (vector-slots 8 660438 6306)
 (floats 8 740 352)
 (intervals 56 288 0)
 (buffers 976 16)
 (heap 1024 31399 1018))

[-- Attachment #2: init.el --]
[-- Type: application/octet-stream, Size: 3037 bytes --]

;;; emacs --- Emacs configuration -*- mode: emacs-lisp -*-

;; Author: Yutian Li <hotpxless@gmail.com>

;;; Commentary:

;;; Code:

;; # Packages
(require 'package)
(require 'cl-lib)
;; (add-to-list 'package-archives
;;              '("elpy" . "https://jorgenschaefer.github.io/packages/"))
(add-to-list 'package-archives
             '("melpa" . "http://melpa.org/packages/") t)
(package-initialize)
(defvar required-packages
  '(highlight-parentheses
    ;;diff-hl
    company
    flycheck
    yasnippet
    irony
    company-irony
    flycheck-irony
    )
  "Required packages.")

(defun install-required-packages ()
  "Install required packages if needed."
  (interactive)
  (package-refresh-contents)
  (mapc (lambda (p)
          (unless (package-installed-p p)
            (package-install p)))
        required-packages))

(when (cl-find-if-not 'package-installed-p required-packages)
  (install-required-packages))

(defmacro after (mode &rest body)
  "`eval-after-load` MODE evaluate BODY."
  (declare (indent defun))
  `(eval-after-load ,mode
     '(progn ,@body)))

;; # Show and highlight parentheses
;; (require 'paren)
;; (setq show-paren-delay 0)
;; (set-face-attribute 'show-paren-match nil
;;                     :foreground (face-background 'default)
;;                     :background "dim gray")
;; (show-paren-mode 1)
(require 'highlight-parentheses)
(global-highlight-parentheses-mode t)
(setq hl-paren-delay 0.001)

;; ;; # Linum
;; (global-linum-mode 1)

;; ;; # Highlight diff
;; (global-diff-hl-mode)

;; ;; # Company mode
(require 'company)
(add-hook 'after-init-hook 'global-company-mode)
(setq company-idle-delay 0)
(global-set-key (kbd "<S-tab>") 'company-complete-common)
(global-set-key (kbd "<backtab>") 'company-complete-common)

;; # YASnippet
(require 'yasnippet)
(define-key yas-minor-mode-map (kbd"<tab>") nil)
(define-key yas-minor-mode-map (kbd "TAB") nil)
(global-set-key (kbd "C-c y") 'helm-yas-complete)
(yas-global-mode 1)

;; # Flycheck
(require 'flycheck)
(add-hook 'after-init-hook #'global-flycheck-mode)
(setq flycheck-display-errors-delay 0.001)

;; # Irony mode
(require 'irony)
(add-hook 'c++-mode-hook 'irony-mode)
(add-hook 'c-mode-hook 'irony-mode)
(add-hook 'objc-mode-hook 'irony-mode)
(defun my-irony-mode-hook ()
  "Irony mode hook."
  (define-key irony-mode-map [remap completion-at-point]
    'irony-completion-at-point-async)
  (define-key irony-mode-map [remap complete-symbol]
    'irony-completion-at-point-async))
(add-hook 'irony-mode-hook 'my-irony-mode-hook)
(add-hook 'irony-mode-hook 'irony-cdb-autosetup-compile-options)
(add-hook 'irony-mode-hook 'company-irony-setup-begin-commands)
(after 'flycheck
  (add-hook 'flycheck-mode-hook #'flycheck-irony-setup))
(defvar flycheck-irony-error-filter)
(setq
 flycheck-irony-error-filter
 (lambda (errors)
   (cl-remove-if
    (lambda (i)
      (string= (flycheck-error-message i) "#pragma once in main file"))
    errors)))
(require 'flycheck-irony)

(after 'company
  (add-to-list
   'company-backends 'company-irony))

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Eli Zaretskii]

> Date: Tue, 3 Nov 2015 22:06:20 -0500
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> 
> The circumstances that trigger this seem to be a combination of
> overlays and timers. Attached is an init.el file that installs several
> packages needed to trigger the problem. I have not yet managed to find
> a simpler way of reproducing this. After installing the packages, and
> running M-x irony-install-server (requires libclang)

Thanks.  Unfortunately, I don't really have enough time at the moment
to install all of that, certainly not libclang.  Is it okay if I ask
questions and make observations, and you then try digging into this
problem, find the answers, and post them here?

> the problem can be triggered by a C++ mode buffer with the following
> contents:

> void func(int x);
> int main() {
>   fun|
> }

> where | represents the cursor. Press TAB y C-f C-b C-f C-b, this
> results in a (wrong-type-argument overlayp nil) which is due to
> yas--on-field-overlay-modification being called with *Messages* as the
> current buffer, rather than the C++ buffer where the yasnippet's
> overlay is in. The begin and end positions passed correspond to
> *Messages* buffer positions (i.e. they are too big to be from the C++
> buffer). According to the lisp backtrace (see below), it seems to be
> triggered by a call to `message' via a timer.

I see that yas-global-mode is turned on, which (AFAIU) means yasnippet
tries to do its thing in every buffer, including *Messages*, right?

That in turn means every modification of the *Messages* buffer will,
quite correctly, call yas--on-field-overlay-modification, provided
that the buffer has overlays in it.  Normally, *Messages* doesn't have
any overlays, but in this case it evidently did, or Emacs somehow
thought it did, which might be part of the problem.

A call to 'message' switches to the *Messages* buffer as part of doing
its job, which explains why yas--on-field-overlay-modification is
called with buffer positions from *Messages* -- assuming there are
overlays there, this all is expected behavior.

So far so good, and now we come to the problematic part.
yas--on-field-overlay-modification is called thusly:

   yas--on-field-overlay-modification(#<overlay from 41 to 42 in foo.cpp> t 3008 3008 33)

IOW, the overlay it is called with belongs to buffer foo.cpp, not to
*Messages*.  So the question now becomes: how come an overlay whose
buffer is foo.cpp winds up in *Messages*?  Can you try finding out?
(Is that even possible?)

After this error happens, what does the following produce if invoked
with *Messages* being the current buffer:

  M-: (overlays-in (point-min) (point-max)) RET

Maybe instrument yas--on-field-overlay-modification to show the
current buffer when it is called, and see if it is invoked with
*Messages* as the current buffer in this scenario.

Thanks.

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Noam Postavsky]

On Wed, Nov 4, 2015 at 10:31 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> Thanks.  Unfortunately, I don't really have enough time at the moment
> to install all of that, certainly not libclang.  Is it okay if I ask
> questions and make observations, and you then try digging into this
> problem, find the answers, and post them here?

Yup, totally understandable.

> I see that yas-global-mode is turned on, which (AFAIU) means yasnippet
> tries to do its thing in every buffer, including *Messages*, right?
>

I can reproduce the problem with yas-minor-mode enabled only in the C++ buffer.

But I *can't* reproduce the problem with highlight-parenthesis-mode
only in the C++ buffer. It seems global-highlight-parenthesis-mode is
required.

> That in turn means every modification of the *Messages* buffer will,
> quite correctly, call yas--on-field-overlay-modification, provided
> that the buffer has overlays in it.

To clarify, yas--on-field-overlay-modification is *only* called as a
modification-hooks property of overlays that yasnippet creates.
Yasnippet never calls it directly.

Yasnippet only creates overlays after expanding a snippet, so this
should never occur in the *Messages* buffer (unless the user decides
to start typing and expanding snippets there). However, there are
several other packages involved.

> After this error happens, what does the following produce if invoked
> with *Messages* being the current buffer:
>
>   M-: (overlays-in (point-min) (point-max)) RET

Evaluating in the stackframe where the error is triggered:

(#<overlay from 1 to 1 in *Messages*> #<overlay from 1 to 1 in
*Messages*> #<overlay from 1 to 1 in *Messages*> #<overlay from 1 to 1
in *Messages*> #<overlay from 1 to 1 in *Messages*> #<overlay from 1
to 1 in *Messages*> #<overlay from 1 to 1 in *Messages*> #<overlay
from 1 to 1 in *Messages*>)

Afterwards it gives nil.

The "from 1 to 1" suggests overlays created by highlight-parentheses,
as I see it has a (make-overlay 0 0 nil t) call in a loop.

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Noam Postavsky]

I've boiled it down to just highlight-parentheses and flycheck (plus
some overlay creating code), will try to remove flycheck tomorrow.

https://gist.githubusercontent.com/npostavs/50517534125b8d48029f/raw/cd049ecf58a2ac9fb73536490ac3d6296807f31a/.emacs

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Eli Zaretskii]

> Date: Wed, 4 Nov 2015 22:33:45 -0500
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: 21824@debbugs.gnu.org
> 
> I've boiled it down to just highlight-parentheses and flycheck (plus
> some overlay creating code), will try to remove flycheck tomorrow.

Thanks.  If yasnippet is no longer part of the issue, can you post the
backtrace you get without it?  If possible, please load the *.el files
of the add-on packages, so that the backtrace includes more Lisp and
less byte-code.

TIA

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Noam Postavsky]

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

Okay, turned out flycheck was basically irrelevant except that it
calls message a lot. And highlight-parentheses' only significance is
that it makes overlays all over the place. I have a simple reproducer,
run as

     emacs -Q -l bug-21824.el -f 21824-trigger

The required circumstances appear to be that after a modification-hook
has been run and there is an overlay in the *Messages* buffer, the
message coalescing (the "[2 times]" thing) wrongly causes runs the
modification-hook of the overlay in the 1st buffer, but with
parameters from the  *Messages* buffer modification.

A sample backtrace (I put a call to `debug' when the current-buffer is
incorrect).

Debugger entered: (#<overlay from 2 to 5 in foo.cpp> t 223 223 10
#<buffer *Messages*>)
  (if (eq (current-buffer) (overlay-buffer overlay)) (message
"21842-mod-overlay(%s %s %s %s %s), buf = %s" overlay afterp beg end
len (current-buffer)) (setq 21842-repeat-message) (debug nil overlay
afterp beg end len (current-buffer)))
  21824-mod-overlay(#<overlay from 2 to 5 in foo.cpp> t 223 223 10)
  message("a message")
  21824-trigger()
  command-line-1(("-l" "bug-21824.el" "-f" "21824-trigger"))
  command-line()
  normal-top-level()

[-- Attachment #2: bug-21824.el --]
[-- Type: application/octet-stream, Size: 945 bytes --]

;;; emacs -Q -l bug-21824.el -f 21824-trigger

(defun 21824-mod-overlay (overlay afterp beg end &optional len)
  (if (eq (current-buffer) (overlay-buffer overlay))
      (message "21842-mod-overlay(%s %s %s %s %s), buf = %s"
               overlay afterp beg end len (current-buffer))
    (setq 21842-repeat-message)
    (debug nil overlay afterp beg end len (current-buffer))))

(defun 21824-trigger ()
  (switch-to-buffer "foo.cpp")
  (view-echo-area-messages)
  (erase-buffer)
  (insert "xyy")
  (search-backward "yy")
  (let* ((beg (match-beginning 0))
         (end (match-end 0))
         (mod-hook-ov (make-overlay beg end nil nil t)))
    (overlay-put mod-hook-ov 'face 'highlight)
    (overlay-put mod-hook-ov 'modification-hooks '(21824-mod-overlay))
    (goto-char (1- end))
    (insert "y")) ; trigger modification-hooks

  (with-current-buffer "*Messages*"
    (make-overlay 1 2))

  (message "a message")
  (message "a message"))

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Eli Zaretskii]

> Date: Thu, 5 Nov 2015 22:14:28 -0500
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: 21824@debbugs.gnu.org
> 
> Okay, turned out flycheck was basically irrelevant except that it
> calls message a lot. And highlight-parentheses' only significance is
> that it makes overlays all over the place. I have a simple reproducer,
> run as
> 
>      emacs -Q -l bug-21824.el -f 21824-trigger
> 
> The required circumstances appear to be that after a modification-hook
> has been run and there is an overlay in the *Messages* buffer, the
> message coalescing (the "[2 times]" thing) wrongly causes runs the
> modification-hook of the overlay in the 1st buffer, but with
> parameters from the  *Messages* buffer modification.

Thanks for this.  I didn't even need to run the test case to see the
problem very clearly.  (It sounds like a very old bug, as all the
pieces of the puzzle were not touched in at least 15 years.)

Please see if the original bug is solved by the latest master.

Btw, any idea how come overlays wound up in *Messages* in the original
scenario?

Thanks again for working on this.

I will see about adding a test for this tricky failure.

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Eli Zaretskii]

> Date: Fri, 06 Nov 2015 17:23:41 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 21824@debbugs.gnu.org
> 
> I will see about adding a test for this tricky failure.

Actually, would you like converting your test case into a test, and
post a patch to the test suite to that effect?  TIA.

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Noam Postavsky]

On Fri, Nov 6, 2015 at 10:23 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> (It sounds like a very old bug, as all the
> pieces of the puzzle were not touched in at least 15 years.)

Quite likely, the circumstances required to trigger it are pretty specific.

>
> Please see if the original bug is solved by the latest master.

Yes, it's fixed.

>
> Btw, any idea how come overlays wound up in *Messages* in the original
> scenario?

global-highlight-parentheses-mode puts overlays in every buffer.

> Actually, would you like converting your test case into a test, and
> post a patch to the test suite to that effect?  TIA.

Something like this? I'm not sure where to put it.

(ert-deftest overlay-modification-hooks ()
  "Test for bug#21824."
  (let ((buf nil))
    (with-temp-buffer
      (insert "123")
      (overlay-put (make-overlay 1 3) 'modification-hooks
                   (list (lambda (ov &rest _)
                           (setq buf (current-buffer)))))
      (goto-char 2)
      (insert "x")
      (with-current-buffer "*Messages*"
        (make-overlay 1 1))
      (message "a message")
      (message "a message")
      (should (eq buf (current-buffer))))))

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Eli Zaretskii]

> Date: Fri, 6 Nov 2015 21:24:50 -0500
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: 21824-done@debbugs.gnu.org
> 
> On Fri, Nov 6, 2015 at 10:23 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > Please see if the original bug is solved by the latest master.
> 
> Yes, it's fixed.

Thanks for testing.

> > Btw, any idea how come overlays wound up in *Messages* in the original
> > scenario?
> 
> global-highlight-parentheses-mode puts overlays in every buffer.

Does it really mean to do that in *Messages*?  If not, perhaps we
should suggest to the developers of highlight-parentheses to exempt
*Messages* from that, as that buffer is pretty specialized anyway.  In
particular, due to the way the message logging in *Messages* is
implemented, the overlay modification hooks will only ever be called
in that buffer with AFTER non-nil, and even that will only happen when
Emacs decides to delete some text from *Messages*; inserting a new
message there will never call the overlay modification hooks.

> > Actually, would you like converting your test case into a test, and
> > post a patch to the test suite to that effect?  TIA.
> 
> Something like this? I'm not sure where to put it.

Yes, this looks good to me.  How about creating a new file
buffer-tests.el (the offending code is in buffer.c)?

Thanks.

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Noam Postavsky]

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

On Sat, Nov 7, 2015 at 3:28 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> global-highlight-parentheses-mode puts overlays in every buffer.
>
> Does it really mean to do that in *Messages*?  If not, perhaps we
> should suggest to the developers of highlight-parentheses to exempt
> *Messages* from that, as that buffer is pretty specialized anyway.  In
> particular, due to the way the message logging in *Messages* is
> implemented, the overlay modification hooks will only ever be called
> in that buffer with AFTER non-nil, and even that will only happen when
> Emacs decides to delete some text from *Messages*; inserting a new
> message there will never call the overlay modification hooks.

Well, I suppose users may want parentheses highlighted in messages too.

But note that the overlays created by highlight-parentheses-mode do
*not* have any modification-hooks. It's the triggering
modification-hooks of (yasnippet's) overlays in another buffer in
addition to the existence of (highlight-parentheses') overlays in
*Messages* that triggers the bug.

> Yes, this looks good to me.  How about creating a new file
> buffer-tests.el (the offending code is in buffer.c)?

Okay, see attached.

[-- Attachment #2: 0001-Add-test-for-bug-21824.patch --]
[-- Type: text/x-diff, Size: 2507 bytes --]

From 565d8744271292bccb334460a62ff662802e6af3 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@users.sourceforge.net>
Date: Sat, 7 Nov 2015 10:47:54 -0500
Subject: [PATCH] Add test for bug #21824

* test/automated/buffer-tests.el: New file.
(overlay-modification-hooks-message-other-buf): New test.
---
 test/automated/buffer-tests.el | 48 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 test/automated/buffer-tests.el

diff --git a/test/automated/buffer-tests.el b/test/automated/buffer-tests.el
new file mode 100644
index 0000000..bb3c92d
--- /dev/null
+++ b/test/automated/buffer-tests.el
@@ -0,0 +1,48 @@
+;;; buffer-tests.el --- tests for buffer.c functions -*- lexical-binding: t -*-
+
+;; Copyright (C) 2015 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+
+(ert-deftest overlay-modification-hooks-message-other-buf ()
+  "Test for bug#21824.
+After a modification-hook has been run and there is an overlay in
+the *Messages* buffer, the message coalescing [2 times] wrongly
+runs the modification-hook of the overlay in the 1st buffer, but
+with parameters from the *Messages* buffer modification."
+  (let ((buf nil)
+        (msg-ov nil))
+    (with-temp-buffer
+      (insert "123")
+      (overlay-put (make-overlay 1 3)
+                   'modification-hooks
+                   (list (lambda (&rest _)
+                           (setq buf (current-buffer)))))
+      (goto-char 2)
+      (insert "x")
+      (unwind-protect
+          (progn
+            (setq msg-ov (make-overlay 1 1 (get-buffer-create "*Messages*")))
+            (message "a message")
+            (message "a message")
+            (should (eq buf (current-buffer))))
+        (when msg-ov (delete-overlay msg-ov))))))
+
+;;; buffer-tests.el ends here
-- 
2.6.1

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Eli Zaretskii]

> Date: Sat, 7 Nov 2015 11:22:50 -0500
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: 21824@debbugs.gnu.org
> 
> On Sat, Nov 7, 2015 at 3:28 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> global-highlight-parentheses-mode puts overlays in every buffer.
> >
> > Does it really mean to do that in *Messages*?  If not, perhaps we
> > should suggest to the developers of highlight-parentheses to exempt
> > *Messages* from that, as that buffer is pretty specialized anyway.  In
> > particular, due to the way the message logging in *Messages* is
> > implemented, the overlay modification hooks will only ever be called
> > in that buffer with AFTER non-nil, and even that will only happen when
> > Emacs decides to delete some text from *Messages*; inserting a new
> > message there will never call the overlay modification hooks.
> 
> Well, I suppose users may want parentheses highlighted in messages too.
> 
> But note that the overlays created by highlight-parentheses-mode do
> *not* have any modification-hooks. It's the triggering
> modification-hooks of (yasnippet's) overlays in another buffer in
> addition to the existence of (highlight-parentheses') overlays in
> *Messages* that triggers the bug.

Yes.  My point was that *Messages* is a special buffer, and all kinds
of stuff we take for granted behaves differently there.

> > Yes, this looks good to me.  How about creating a new file
> > buffer-tests.el (the offending code is in buffer.c)?
> 
> Okay, see attached.

Thanks, pushed.

Btw, the FSF copyright assignment lists a different email address for
you.  Which one is correct?

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Noam Postavsky]

On Sat, Nov 7, 2015 at 1:07 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> Yes.  My point was that *Messages* is a special buffer, and all kinds
> of stuff we take for granted behaves differently there.

Okay. I've suggested they disable highlight-parentheses in *Messages*.

https://github.com/tsdh/highlight-parentheses.el/issues/14

> Btw, the FSF copyright assignment lists a different email address for
> you.  Which one is correct?

Both, the sourceforge.net one is an alias. Can you update the list?
This is the 2nd time you asked me about this. :)

bug#21824: 25.0.50; overlay modification-hooks called with wrong buffer

[Eli Zaretskii]

> Date: Sat, 7 Nov 2015 20:43:31 -0500
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: 21824@debbugs.gnu.org
> 
> > Btw, the FSF copyright assignment lists a different email address for
> > you.  Which one is correct?
> 
> Both, the sourceforge.net one is an alias. Can you update the list?

I can't, but if the sourceforge.net address is not the real one, the
list already provides the correct address.  Please use it in your
submissions (in the log part, at least), to avoid confusion in the
future.

> This is the 2nd time you asked me about this. :)

At least I'm consistent.