bug#20223: 25.0.50; key-chord.el crashes Emacs

bug#20223: 25.0.50; key-chord.el crashes Emacs

[Jan Tatarik]

This can be reproduced from emacs -Q, with just key-chord.el
loaded. Happens with the latest key-chord available from MELPA
(key-chord-20140929.2246.el). Tried with current emacs master.

(package-install-file "/PATH/TO/key-chord.el")
(key-chord-define-global "vv" ctl-x-r-map)
(key-chord-mode 1)

Pressing [vv b] will execute bookmark-jump, as expected.

But pressing the key-chord with a key that is not defined in the target
map will crash Emacs immediately. Try [vv q], for instance.

Only the fact that matters is the key not defined in the map. The actual
key-chord (tried vv, pf, others) nor the map affect the outcome.



--

In GNU Emacs 25.0.50.4 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8)
 of 2015-03-29 on nb-jtatarik2
Repository revision: e6127d94746e230f95bdf2ad002e4379474e5a8b
Windowing system distributor `The X.Org Foundation', version 11.0.11501000
System Description:	Linux Mint 17.1 Rebecca

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GSETTINGS NOTIFY
GNUTLS LIBXML2 FREETYPE LIBOTF XFT ZLIB

Important settings:
  value of $LC_COLLATE: C
  value of $LC_MONETARY: en_US.UTF-8
  value of $LC_NUMERIC: en_US.UTF-8
  value of $LC_TIME: en_GB.UTF8
  value of $LANG: en_US.UTF8
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  gnus-topic-mode: t
  workgroups-mode: t
  helm-mode: t
  delete-selection-mode: t
  cua-mode: t
  gdb-many-windows: t
  diff-auto-refine-mode: t
  gnus-undo-mode: t
  guide-key-mode: t
  ido-vertical-mode: t
  flx-ido-mode: t
  ido-ubiquitous-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  projectile-global-mode: t
  projectile-mode: t
  keyfreq-autosave-mode: t
  keyfreq-mode: t
  winner-mode: t
  anything-dired-mode: Enable anything completion in Dired functions.
Bindings affected are C, R, S, H.
This is deprecated for Emacs24+ users, use `ac-mode' instead.
  shell-dirtrack-mode: t
  auto-compile-on-load-mode: t
  auto-compile-on-save-mode: t
  override-global-mode: t
  show-paren-mode: t
  savehist-mode: t
  global-auto-revert-mode: t
  auto-insert-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t

Recent messages:
Checking new news...
Reading active file via nnnil...done
Opening connection to localhost via tls...
Decrypting /home/jan/.authinfo.gpg...done
Opening connection to localhost...done
nnimap read 2k from localhost
Reading active file via nndraft...done
Checking new news...done
nnimap read 0k from localhost
Expiring articles...done

Load-path shadows:
/home/jan/.emacs.d/lisp/bbdb3/bbdb-anniv hides ~/.emacs.d/lisp/bbdb/bits/bbdb-anniv
/home/jan/.emacs.d/lisp/bbdb3/bbdb-pgp hides ~/.emacs.d/lisp/bbdb/bits/bbdb-pgp
~/repos/magit/with-editor hides ~/repos/git-modes/with-editor
/home/jan/.dotfiles/src/.emacs.d/.cask/25.0.50.4/elpa/ido-ubiquitous-20150305.2254/ido-ubiquitous hides ~/.emacs.d/lisp/ido-ubiquitous
/home/jan/.dotfiles/src/.emacs.d/.cask/25.0.50.4/elpa/http-post-simple-20131011.358/http-post-simple hides ~/.emacs.d/lisp/http-post-simple

Features:
(shadow sort gnus-cite bbdb-message emacsbug gnus-async mail-extr qp
gnus-ml hl-line disp-table gnus-demon gnus-topic utf-7 nndraft nnmh
epa-file epa epg gnutls network-stream nsm starttls copyright bbdb-gnus
bbdb-mua nnnil gnus-agent gnus-srvr gnus-score score-mode nnvirtual
gnus-msg nntp gnus-cache server my-org my-org-task org-clock find-lisp
xing-settings my:jira-xing my:jira my:bugtracker my:ticket
http-post-simple url-http url-auth midnight mode-settings info-look
company-files company-oddmuse company-keywords company-dabbrev-code
company-dabbrev company-etags company-gtags company-xcode company-clang
company-semantic company-eclim company-template company-css company-nxml
company-elisp company-bbdb paredit checkdoc diff-hl vc-dir ewoc
bug-reference workgroups2 apt-utils helm-apt helm-mode helm-files
image-dired helm-buffers helm-elscreen helm-tags helm-bookmark
helm-adaptive helm-info bookmark helm-locate helm-help helm-org
helm-match-plugin helm-grep helm-regexp helm-plugin helm-external
helm-net helm-utils helm helm-source delsel cua-base sqlup-mode
maven-fetch javadoc-lookup ensime ensime-ui ensime-semantic-highlight
ensime-doc ensime-scalex ensime-search ensime-undo ensime-startup
ensime-refactor ensime-popup ensime-notes ensime-model ensime-mode
ensime-inspector ensime-goto-testfile ensime-editor ensime-debug gdb-mi
bindat ensime-stacktrace ensime-inf ensime-sbt sbt-mode sbt-mode-rgrep
sbt-mode-comint sbt-mode-buffer sbt-mode-project ensime-company
ensime-auto-complete ensime-completion-util ensime-config ensime-vars
ensime-util auto-complete popup flymake url-gw ensime-client
ensime-macros scala-mode-lib scala-mode-constants scala-mode2
scala-mode2-imenu scala-mode2-sbt scala-mode2-map scala-mode2-fontlock
scala-mode2-indent scala-mode2-paragraph scala-mode2-syntax
scala-mode2-lib tabify org-table footnote flyspell ispell org-element
org-man org-w3m org-rmail org-mhe org-irc org-info org-gnus org-bibtex
bibtex org-bbdb cal-china lunar solar cal-dst cal-bahai cal-islam
cal-hebrew holidays hol-loaddefs vc vc-dispatcher vc-git diff-mode appt
notifications dbus mm-url smtpmail sendmail shr dom bbdb-anniv bbdb-com
crm bbdb-loaddefs bbdb bbdb-site timezone gnus-icalendar org-capture
org-location-google-maps org-agenda google-maps google-maps-static
google-maps-geocode google-maps-base org org-macro org-footnote
org-pcomplete org-list org-faces org-entities org-version ob-plantuml
ob-sql ob-octave ob-R ob-sh ob-emacs-lisp ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-src ob-keys ob-comint ob-core ob-eval org-compat
org-macs org-loaddefs icalendar diary-lib diary-loaddefs cal-menu
calendar cal-loaddefs gnus-registry registry eieio-compat eieio-base
gnus-art mm-uu mml2015 mm-view mml-smime smime dig nnir gnus-sum
gnus-group gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source tls
utf7 netrc parse-time gnus-spec gnus-int gnus-range message rfc822 mml
mml-sec mailabbrev gmm-utils mailheader gnus-win nnoo perlbrew
slime-fancy slime-trace-dialog slime-fontifying-fu slime-package-fu
slime-references slime-compiler-notes-tree slime-scratch
slime-presentations bridge slime-fuzzy slime-fancy-trace
slime-fancy-inspector slime-c-p-c slime-editing-commands slime-autodoc
slime-repl elp slime-parse slime derived gud apropos etags xref arc-mode
archive-mode noutline outline pp hyperspec slime-autoloads guide-key
popwin myfold-mode ido-vertical-mode flx-ido flx ido-ubiquitous warnings
yasnippet develock company flycheck find-func help-mode subr-x
indent-guide highlight-symbol custom-dot-emacs key-chord projectile
ibuf-ext ibuffer skeletor let-alist keyfreq ido ess-toolbar ess-mouse
mouseme ess-menu ess-swv ess-noweb ess-noweb-font-lock-mode ess-bugs-l
essd-els ess-sas-d ess-sas-l ess-sas-a ess-sta-d ess-sta-l cc-vars
cc-defs make-regexp ess-sp6-d ess-sp3-d ess-julia ess-r-d
ess-r-completion ess-tracebug ess-roxy hideshow ess-help ess-developer
ess-s-l ess ess-inf ess-mode ess-noweb-mode ess-utils ess-custom
executable ess-compat ess-site smartparens iedit iedit-lib protbuf
winner my:util anything-config browse-url rx anything-match-plugin xml
url url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf url-util mailcap grep compile tramp tramp-compat
tramp-loaddefs trampver shell pcomplete comint ansi-color ring
format-spec dired-x dired-aux dired ffap url-parse auth-source eieio
eieio-core cl-generic password-cache url-vars thingatpt anything
anaphora auto-compile byte-opt packed use-package diminish bytecomp
byte-compile cl-extra seq cconv bind-key easy-mmode finder-inf edmacro
kmacro advice info easymenu cask cl-macs cask-bootstrap package-build
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045
ietf-drums json lisp-mnt shut-up epl git commander f dash s
ucs-normalize package epg-config saveplace jka-compr Profile-XING-theme
which-func imenu paren savehist avoid gnus gnus-ems nnheader gnus-util
mail-utils mm-util help-fns mail-prsvr wid-edit autorevert filenotify
autoinsert cus-start cus-load cl gv cl-loaddefs pcase cl-lib time-date
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel x-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 cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
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 make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

Memory information:
((conses 16 1273838 390958)
 (symbols 48 75609 0)
 (miscs 40 815 780)
 (strings 32 339429 163732)
 (string-bytes 1 12804142)
 (vectors 16 98299)
 (vector-slots 8 2537837 139652)
 (floats 8 2087 642)
 (intervals 56 1938 313)
 (buffers 976 29)
 (heap 1024 117970 24587))

bug#20223: 25.0.50; key-chord.el crashes Emacs

[Eli Zaretskii]

> From: Jan Tatarik <jan.tatarik@gmail.com>
> Date: Sun, 29 Mar 2015 12:01:42 +0200
> 
> This can be reproduced from emacs -Q, with just key-chord.el
> loaded. Happens with the latest key-chord available from MELPA
> (key-chord-20140929.2246.el). Tried with current emacs master.
> 
> (package-install-file "/PATH/TO/key-chord.el")
> (key-chord-define-global "vv" ctl-x-r-map)
> (key-chord-mode 1)
> 
> Pressing [vv b] will execute bookmark-jump, as expected.
> 
> But pressing the key-chord with a key that is not defined in the target
> map will crash Emacs immediately. Try [vv q], for instance.
> 
> Only the fact that matters is the key not defined in the map. The actual
> key-chord (tried vv, pf, others) nor the map affect the outcome.

This happens because key-chord triggers a situation where the value of
't' in the snippet below (from read_key_sequence) is not in sync with
the value of 'this_command_key_count':

      /* Record what part of this_command_keys is the current key sequence.  */
      this_single_command_key_start = this_command_key_count - t;

If 't' is greater than this_command_key_count, then
this_single_command_key_start will be assigned a negative value.  In
this scenario, the result is -2 (t is 4, while this_command_key_count
is 2).  Then this-single-command-keys will produce a vector with bogus
elements.  And when a key sequence is undefined, we invoke
'undefined', which does this:

  (message "%s is undefined" (key-description (this-single-command-keys)))

So we are trying to display bogus data, with predictable results.

The same problem happens if you type "v v b", but we escape narrowly
because this key sequence is bound to a command, so we don't try to
access the result of this-single-command-keys.

I can fix this with the simple patch shown below.  It looks like
papering over the problem, and perhaps it is.  But I couldn't find any
obvious place where this_command_key_count should have been
incremented, but wasn't.  We avoid incrementing it in this scenario
because the value of 'reread' variable is 'true', since we have events
in unread-post-input-method-events after invoking the "input-method"
provided by key-chord.

HTH

--- src/keyboard.c~	2015-03-08 08:16:29 +0200
+++ src/keyboard.c	2015-03-29 15:45:46 +0300
@@ -9591,6 +9591,8 @@ read_key_sequence (Lisp_Object *keybuf,
 
       /* Record what part of this_command_keys is the current key sequence.  */
       this_single_command_key_start = this_command_key_count - t;
+      if (this_single_command_key_start < 0)
+	this_single_command_key_start = 0;
 
       /* Look for this sequence in input-decode-map.
 	 Scan from indec.end until we find a bound suffix.  */

bug#20223: 25.0.50; key-chord.el crashes Emacs

[Jan Tatarik]

On Mon, Mar 30 2015, Eli Zaretskii wrote:

> I can fix this with the simple patch shown below.  It looks like
> papering over the problem, and perhaps it is.  But I couldn't find any
> obvious place where this_command_key_count should have been
> incremented, but wasn't.  We avoid incrementing it in this scenario
> because the value of 'reread' variable is 'true', since we have events
> in unread-post-input-method-events after invoking the "input-method"
> provided by key-chord.

That works, thanks.

I know nothing of the emacs internals, so I cannot judge whether this is
the appropriate way to fix the issue.

bug#20223: 25.0.50; key-chord.el crashes Emacs

[Stefan Monnier]

> +      if (this_single_command_key_start < 0)
> +	this_single_command_key_start = 0;

I guess it's OK to use such a paper-over, but please add comment
that explains the case it tries to address.


        Stefan

bug#20223: 25.0.50; key-chord.el crashes Emacs

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Jan Tatarik <jan.tatarik@gmail.com>,  20223@debbugs.gnu.org
> Date: Mon, 30 Mar 2015 17:01:15 -0400
> 
> > +      if (this_single_command_key_start < 0)
> > +	this_single_command_key_start = 0;
> 
> I guess it's OK to use such a paper-over, but please add comment
> that explains the case it tries to address.

Done.