GNU bug report logs - #23949
25.0.95; Regression in handling error caused by (string-match-p "." nil)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 23949 in the body.
You can then email your comments to 23949 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Mon, 11 Jul 2016 20:12:10 +0000

[Message part 1 (text/plain, inline)]

As I do not understand the true nature of this bug, the title of this
debbugs could be confusing. Please read the below in entirety.

Related discussion is also on emacs-devel:
https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00519.html

Evaluating the below in emacs 24.5 emacs -Q results in the below error
backtrace as expected:

=====
Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  string-match("." nil nil)
  string-match-p("." nil)
  (progn (require (quote package)) (if (member (quote ("melpa" . "
http://melpa.org/packages/")) package-archives) package-archives (setq
package-archives (append package-archives (list (quote ("melpa" . "
http://melpa.org/packages/")))))) (package-initialize)
(package-refresh-contents) (package-install (quote projectile)) (require
(quote projectile)) (projectile-global-mode) (string-match-p "." nil))
  eval((progn (require (quote package)) (if (member (quote ("melpa" . "
http://melpa.org/packages/")) package-archives) package-archives (setq
package-archives (append package-archives (list (quote ("melpa" . "
http://melpa.org/packages/")))))) (package-initialize)
(package-refresh-contents) (package-install (quote projectile)) (require
(quote projectile)) (projectile-global-mode) (string-match-p "." nil)) nil)
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp nil nil)
  command-execute(eval-last-sexp)
=====

Minimal code to reproduce the issue:
=====
(progn
  (require 'package)
  (add-to-list 'package-archives '("melpa" . "http://melpa.org/packages/")
t)
  (package-initialize)
  (package-refresh-contents)
  (package-install 'projectile)

  (require 'projectile)
  (projectile-global-mode)
  (string-match-p "." nil))
=====

But evaluating the same on emacs-25 build results in:

=====
Entering debugger...
help-function-arglist: End of file during parsing
=====

Also evaluating the above after M-x toggle-debug-on-error does not give a
backtrace on the "help-function-arglist: End of file during parsing" error.

This is a regression from how the actual error (wrong-type-argument stringp
nil) backtrace showed up correctly on emacs 24.5 but is giving confusing
and difficult to debug error on emacs-25.

emacs-25 build info:

In GNU Emacs 25.0.95.11 (x86_64-unknown-linux-gnu, GTK+ Version 2.24.23)
 of 2016-06-29 built on ulcf41.cld.analog.com
Repository revision: 6192b6c3a4374b2cb6e02ca865e1899a04a7f7dc
Windowing system distributor 'The X.Org Foundation', version 11.0.60900000
System Description: Red Hat Enterprise Linux Workstation release 6.6
(Santiago)

Configured using:
 'configure --with-modules
 --prefix=/home/kmodi/usr_local/apps/6/emacs/emacs-25
 'CPPFLAGS=-fgnu89-inline -I/home/kmodi/usr_local/6/include
 -I/usr/include/freetype2 -I/usr/include' 'CFLAGS=-ggdb3 -O0'
 'CXXFLAGS=-ggdb3 -O0' 'LDFLAGS=-L/home/kmodi/usr_local/6/lib
 -L/home/kmodi/usr_local/6/lib64 -ggdb3'
 PKG_CONFIG_PATH=/home/kmodi/usr_local/6/lib/pkgconfig:/home/kmodi/usr_local/6/lib64/pkgconfig:/cad/adi/apps/gnu/linux/x86_64/6/lib/pkgconfig:/cad/adi/apps/gnu/linux/x86_64/6/lib64/pkgconfig:/usr/lib/pkgconfig:/usr/lib64/pkgconfig:/usr/share/pkgconfig:/lib/pkgconfig:/lib64/pkgconfig'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK2 X11 MODULES

-- 

-- 
Kaushal Modi

[Message part 2 (text/html, inline)]

Message #8 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: 23949 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Tue, 12 Jul 2016 12:29:25 +0000

[Message part 1 (text/plain, inline)]

I evaluated the below after:

> gdb ./emacs
gdb> r -Q

====
(defmacro emacs-q-template (pkgs &rest body)
  "Install packages in PKGS list and evaluate BODY.

Example usage:

1. Launch 'emacs -Q'.
2. Copy this macro definition to its scratch buffer and evaluate it.
3. Evaluate a minimum working example using this macro as below:
     (emacs-q-template '(projectile)
       (projectile-global-mode)) "
  (declare (indent 1) (debug t))
  `(progn
     (require 'package)
     (setq package-user-dir (concat (getenv "HOME") "/.emacs.d/elpa_test/"))
     (add-to-list 'package-archives '("melpa" . "http://melpa.org/packages/")
t)
     (package-initialize)
     (package-refresh-contents)

     (dolist (pkg ,pkgs)
       (package-install pkg)
       (require pkg))

     ,@body))

(emacs-q-template '(projectile)
  (projectile-global-mode)
  (string-match-p "." nil))
=====

Below is the backtrace at the point where I get the unexpected end-of-file
error

(gdb) c
Continuing.
[New Thread 0x7fffec504700 (LWP 11972)]
[Thread 0x7fffec504700 (LWP 11972) exited]

Breakpoint 3, Fsignal (error_symbol=19104, data=0) at eval.c:1472
1472        = (NILP (error_symbol) ? Fcar (data) : error_symbol);
(gdb) p error_symbol
$15 = 19104
(gdb) xsymbol
$16 = (struct Lisp_Symbol *) 0xcae060 <lispsym+19104>
"end-of-file"
(gdb) bt
#0  Fsignal (error_symbol=19104, data=0) at eval.c:1472
#1  0x00000000005f4a06 in xsignal (error_symbol=19104, data=0) at
eval.c:1578
#2  0x00000000005f4a33 in xsignal0 (error_symbol=19104) at eval.c:1587
#3  0x0000000000623284 in end_of_file_error () at lread.c:1738
#4  0x0000000000624dcd in read1 (readcharfun=57797668, pch=0x7fffffff353c,
first_in_list=false) at lread.c:2553
#5  0x00000000006278e3 in read_list (flag=false, readcharfun=57797668) at
lread.c:3659
#6  0x0000000000624dfc in read1 (readcharfun=57797668, pch=0x7fffffff38ac,
first_in_list=false) at lread.c:2558
#7  0x00000000006241b0 in read0 (readcharfun=57797668) at lread.c:2139
#8  0x0000000000624112 in read_internal_start (stream=57797668, start=0,
end=0) at lread.c:2112
#9  0x0000000000623fab in Fread_from_string (string=57797668, start=0,
end=0) at lread.c:2075
#10 0x00000000005f7627 in Ffuncall (nargs=2, args=0x7fffffff3a28) at
eval.c:2708
#11 0x000000000063df57 in exec_byte_code (bytestr=10617580,
vector=10617613, maxdepth=34, args_template=0, nargs=0, args=0x0) at
bytecode.c:880
#12 0x00000000005f817a in funcall_lambda (fun=10617485, nargs=2,
arg_vector=0xa2030d <pure+1081741>) at eval.c:2929
#13 0x00000000005f783a in Ffuncall (nargs=3, args=0x7fffffff3f80) at
eval.c:2750
#14 0x000000000063df57 in exec_byte_code (bytestr=40406500,
vector=35307789, maxdepth=18, args_template=1030, nargs=1,
args=0x7fffffff44c8) at bytecode.c:880
#15 0x00000000005f7e3e in funcall_lambda (fun=35307829, nargs=1,
arg_vector=0x7fffffff44c0) at eval.c:2863
#16 0x00000000005f783a in Ffuncall (nargs=2, args=0x7fffffff44b8) at
eval.c:2750
#17 0x000000000063df57 in exec_byte_code (bytestr=41942932,
vector=35797685, maxdepth=66, args_template=1030, nargs=1,
args=0x7fffffff4a78) at bytecode.c:880
#18 0x00000000005f7e3e in funcall_lambda (fun=26917541, nargs=1,
arg_vector=0x7fffffff4a70) at eval.c:2863
#19 0x00000000005f783a in Ffuncall (nargs=2, args=0x7fffffff4a68) at
eval.c:2750
#20 0x000000000063df57 in exec_byte_code (bytestr=32991236,
vector=36233277, maxdepth=50, args_template=2058, nargs=2,
args=0x7fffffff4fd0) at bytecode.c:880
#21 0x00000000005f7e3e in funcall_lambda (fun=36233437, nargs=2,
arg_vector=0x7fffffff4fc0) at eval.c:2863
#22 0x00000000005f783a in Ffuncall (nargs=3, args=0x7fffffff4fb8) at
eval.c:2750
#23 0x000000000063df57 in exec_byte_code (bytestr=29885572,
vector=35430989, maxdepth=26, args_template=2054, nargs=1,
args=0x7fffffff5508) at bytecode.c:880
#24 0x00000000005f7e3e in funcall_lambda (fun=35431157, nargs=1,
arg_vector=0x7fffffff5500) at eval.c:2863
#25 0x00000000005f783a in Ffuncall (nargs=2, args=0x7fffffff54f8) at
eval.c:2750
#26 0x000000000063df57 in exec_byte_code (bytestr=29509364,
vector=35472261, maxdepth=34, args_template=1026, nargs=0,
args=0x7fffffff5a50) at bytecode.c:880
#27 0x00000000005f7e3e in funcall_lambda (fun=37586485, nargs=0,
arg_vector=0x7fffffff5a50) at eval.c:2863
#28 0x00000000005f783a in Ffuncall (nargs=1, args=0x7fffffff5a48) at
eval.c:2750
#29 0x000000000063df57 in exec_byte_code (bytestr=29347988,
vector=37183445, maxdepth=18, args_template=2, nargs=0,
args=0x7fffffff6010) at bytecode.c:880
#30 0x00000000005f7e3e in funcall_lambda (fun=37183509, nargs=0,
arg_vector=0x7fffffff6010) at eval.c:2863
#31 0x00000000005f783a in Ffuncall (nargs=1, args=0x7fffffff6008) at
eval.c:2750
#32 0x00000000005f6a9b in funcall_nil (nargs=1, args=0x7fffffff6008) at
eval.c:2340
#33 0x00000000005f6e9f in run_hook_with_args (nargs=1, args=0x7fffffff6008,
funcall=0x5f6a78 <funcall_nil>) at eval.c:2517
#34 0x00000000005f6b22 in Frun_hook_with_args (nargs=1,
args=0x7fffffff6008) at eval.c:2382
#35 0x00000000005f6efb in run_hook (hook=28871488) at eval.c:2530
#36 0x00000000005f6adf in Frun_hooks (nargs=1, args=0x7fffffff6118) at
eval.c:2364
#37 0x00000000005f74ba in Ffuncall (nargs=2, args=0x7fffffff6110) at
eval.c:2681
#38 0x000000000063df57 in exec_byte_code (bytestr=9658076, vector=9658109,
maxdepth=22, args_template=514, nargs=1, args=0x7fffffff6640) at
bytecode.c:880
#39 0x00000000005f7e3e in funcall_lambda (fun=9658029, nargs=1,
arg_vector=0x7fffffff6640) at eval.c:2863
#40 0x00000000005f783a in Ffuncall (nargs=2, args=0x7fffffff6638) at
eval.c:2750
#41 0x000000000063df57 in exec_byte_code (bytestr=26545060,
vector=26696973, maxdepth=10, args_template=2, nargs=0,
args=0x7fffffff6c50) at bytecode.c:880
#42 0x00000000005f7e3e in funcall_lambda (fun=26697125, nargs=0,
arg_vector=0x7fffffff6c50) at eval.c:2863
#43 0x00000000005f783a in Ffuncall (nargs=1, args=0x7fffffff6c48) at
eval.c:2750
#44 0x000000000063df57 in exec_byte_code (bytestr=26480452,
vector=26654173, maxdepth=166, args_template=514, nargs=2,
args=0x7fffffff71a8) at bytecode.c:880
#45 0x00000000005f7e3e in funcall_lambda (fun=26654805, nargs=2,
arg_vector=0x7fffffff71a8) at eval.c:2863
#46 0x00000000005f783a in Ffuncall (nargs=3, args=0x7fffffff71a0) at
eval.c:2750
#47 0x00000000005f6a41 in Fapply (nargs=2, args=0x7fffffff7270) at
eval.c:2329
#48 0x00000000005f6f90 in apply1 (fn=16848, arg=56618787) at eval.c:2545
#49 0x00000000005f23b6 in call_debugger (arg=56618787) at eval.c:308
#50 0x00000000005f4e2b in maybe_call_debugger (conditions=9578427,
sig=51984, data=56618835) at eval.c:1723
#51 0x00000000005f48dd in Fsignal (error_symbol=51984, data=56618835) at
eval.c:1541
#52 0x00000000005f4a06 in xsignal (error_symbol=51984, data=56618835) at
eval.c:1578
#53 0x00000000005f4a9a in xsignal2 (error_symbol=51984, arg1=44400, arg2=0)
at eval.c:1599
#54 0x00000000005d7967 in wrong_type_argument (predicate=44400, value=0) at
data.c:151
#55 0x000000000055388b in CHECK_STRING (x=0) at lisp.h:2807
#56 0x00000000005b2278 in string_match_1 (regexp=55505044, string=0,
start=0, posix=false) at search.c:373
#57 0x00000000005b25e5 in Fstring_match (regexp=55505044, string=0,
start=0) at search.c:444
#58 0x00000000005f7627 in Ffuncall (nargs=4, args=0x7fffffff75c0) at
eval.c:2708
#59 0x000000000063df57 in exec_byte_code (bytestr=9684412, vector=9684445,
maxdepth=30, args_template=3082, nargs=2, args=0x7fffffff7a40) at
bytecode.c:880
#60 0x00000000005f7e3e in funcall_lambda (fun=9684365, nargs=2,
arg_vector=0x7fffffff7a30) at eval.c:2863
#61 0x00000000005f7bca in apply_lambda (fun=9684365, args=55521603,
count=15) at eval.c:2802
#62 0x00000000005f623e in eval_sub (form=55521587) at eval.c:2219
#63 0x00000000005f2647 in Fprogn (body=55521667) at eval.c:427
#64 0x00000000005f5c69 in eval_sub (form=55520739) at eval.c:2127
#65 0x00000000005f5786 in Feval (form=55520739, lexical=0) at eval.c:1996
#66 0x00000000005f75ec in Ffuncall (nargs=3, args=0x7fffffff7e28) at
eval.c:2704
#67 0x000000000063df57 in exec_byte_code (bytestr=11219292,
vector=11219325, maxdepth=22, args_template=1030, nargs=1,
args=0x7fffffff8360) at bytecode.c:880
#68 0x00000000005f7e3e in funcall_lambda (fun=11219245, nargs=1,
arg_vector=0x7fffffff8358) at eval.c:2863
#69 0x00000000005f783a in Ffuncall (nargs=2, args=0x7fffffff8350) at
eval.c:2750
#70 0x000000000063df57 in exec_byte_code (bytestr=11220028,
vector=11220061, maxdepth=18, args_template=1030, nargs=1,
args=0x7fffffff8968) at bytecode.c:880
#71 0x00000000005f7e3e in funcall_lambda (fun=11219973, nargs=1,
arg_vector=0x7fffffff8960) at eval.c:2863
#72 0x00000000005f783a in Ffuncall (nargs=2, args=0x7fffffff8958) at
eval.c:2750
#73 0x00000000005ef8fc in Ffuncall_interactively (nargs=2,
args=0x7fffffff8958) at callint.c:252
#74 0x00000000005f74ba in Ffuncall (nargs=3, args=0x7fffffff8950) at
eval.c:2681
#75 0x00000000005f1c82 in Fcall_interactively (function=4203120,
record_flag=0, keys=13589173) at callint.c:840
#76 0x00000000005f7627 in Ffuncall (nargs=4, args=0x7fffffff8c78) at
eval.c:2708
#77 0x000000000063df57 in exec_byte_code (bytestr=10517116,
vector=10517149, maxdepth=54, args_template=4102, nargs=1,
args=0x7fffffff91d0) at bytecode.c:880
#78 0x00000000005f7e3e in funcall_lambda (fun=10517069, nargs=1,
arg_vector=0x7fffffff91c8) at eval.c:2863
#79 0x00000000005f783a in Ffuncall (nargs=2, args=0x7fffffff91c0) at
eval.c:2750
#80 0x00000000005f6fe2 in call1 (fn=15072, arg1=4203120) at eval.c:2560
#81 0x000000000055b646 in command_loop_1 () at keyboard.c:1479
#82 0x00000000005f424a in internal_condition_case (bfun=0x55ae8d
<command_loop_1>, handlers=19392, hfun=0x55a677 <cmd_error>) at eval.c:1310
#83 0x000000000055ab95 in command_loop_2 (ignore=0) at keyboard.c:1107
#84 0x00000000005f3b6a in internal_catch (tag=46560, func=0x55ab6c
<command_loop_2>, arg=0) at eval.c:1075
#85 0x000000000055ab35 in command_loop () at keyboard.c:1086
#86 0x000000000055a246 in recursive_edit_1 () at keyboard.c:692
#87 0x000000000055a3d9 in Frecursive_edit () at keyboard.c:763
#88 0x000000000055825a in main (argc=2, argv=0x7fffffff9688) at emacs.c:1656

Lisp Backtrace:
"read-from-string" (0xffff3a30)
"help-function-arglist" (0xffff3f88)
"ad-arglist" (0xffff44c0)
"ad-make-advised-definition" (0xffff4a70)
"ad-activate-advised-definition" (0xffff4fc0)
"ad-activate" (0xffff5500)
"projectile-mode" (0xffff5a50)
"projectile-global-mode-enable-in-buffers" (0xffff6010)
"run-hooks" (0xffff6118)
"run-mode-hooks" (0xffff6640)
"debugger-mode" (0xffff6c50)
"debug" (0xffff71a8)
"string-match" (0xffff75c8)
"string-match-p" (0xffff7a30)
"progn" (0xffff7c70)
"eval" (0xffff7e30)
"elisp--eval-last-sexp" (0xffff8358)
"eval-last-sexp" (0xffff8960)
"funcall-interactively" (0xffff8958)
"call-interactively" (0xffff8c80)
"command-execute" (0xffff91c8)
(gdb)
-- 

-- 
Kaushal Modi

[Message part 2 (text/html, inline)]

Message #11 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: 23949 <at> debbugs.gnu.org
Subject: Re: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Tue, 12 Jul 2016 16:14:09 +0300

> From: Kaushal Modi <kaushal.modi <at> gmail.com>
> Date: Tue, 12 Jul 2016 12:29:25 +0000
> 
> Lisp Backtrace:
> "read-from-string" (0xffff3a30)
> "help-function-arglist" (0xffff3f88)

This says it tried to read from a string.  What string was that?  And
why does help-function-arglist try to read from that string?

> #9 0x0000000000623fab in Fread_from_string (string=57797668, start=0, end=0) at lread.c:2075

In this frame, please show the value of 'string', like this:

 (gdb) frame 9
 (gdb) p string
 (gdb) xstring

Message #14 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 23949 <at> debbugs.gnu.org
Subject: Re: 25.0.95; Regression in handling error caused by (string-match-p
 "." nil)
Date: Tue, 12 Jul 2016 13:33:05 +0000

[Message part 1 (text/plain, inline)]

Hi Eli,

This is what I get (obviously odd-looking string):

(gdb) frame 9
#9  0x0000000000624397 in Fread_from_string (string=54138084, start=0,
end=0) at lread.c:2075
2075      ret = read_internal_start (string, start, end);
(gdb) p string
$15 = 54138084
(gdb) xstring
$16 = (struct Lisp_String *) 0x33a14e0
"(nilory is relative, it is com"

Looks like the 'nil' argument in '(string-match-p "." nil)' somehow did
this?

(BTW this debug session is on the 2f67f8a commit of master (very recent,
today). But the bug is present on emacs-25 too.)

On Tue, Jul 12, 2016 at 9:14 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> In this frame, please show the value of 'string', like this:
>
>  (gdb) frame 9
>  (gdb) p string
>  (gdb) xstring
>
-- 

-- 
Kaushal Modi

[Message part 2 (text/html, inline)]

Message #17 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 23949 <at> debbugs.gnu.org
Subject: Re: 25.0.95; Regression in handling error caused by (string-match-p
 "." nil)
Date: Tue, 12 Jul 2016 13:37:57 +0000

[Message part 1 (text/plain, inline)]

Some interesting discovery .. hope this helps debug this:

Searched the whole emacs source and my ~/.emacs.d for 'is relative, it is
com'. And the only place I found it was in lisp/progmodes/compile.el, line
2679:

=====
km²~/downloads/:git/emacs> ag 'is relative, it is com'
lisp/progmodes/compile.el
2679:If DIRECTORY is relative, it is combined with `default-directory'.
=====

Somehow "(nil" replaced "If DIRECT", the whole thing got lower-cased,
string after "com" in "combined" got truncated, and we got:

"(nilory is relative, it is com"




On Tue, Jul 12, 2016 at 9:33 AM Kaushal Modi <kaushal.modi <at> gmail.com> wrote:

> Hi Eli,
>
> This is what I get (obviously odd-looking string):
>
> (gdb) frame 9
> #9  0x0000000000624397 in Fread_from_string (string=54138084, start=0,
> end=0) at lread.c:2075
> 2075      ret = read_internal_start (string, start, end);
> (gdb) p string
> $15 = 54138084
> (gdb) xstring
> $16 = (struct Lisp_String *) 0x33a14e0
> "(nilory is relative, it is com"
>
> Looks like the 'nil' argument in '(string-match-p "." nil)' somehow did
> this?
>
> (BTW this debug session is on the 2f67f8a commit of master (very recent,
> today). But the bug is present on emacs-25 too.)
>
> On Tue, Jul 12, 2016 at 9:14 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
>> In this frame, please show the value of 'string', like this:
>>
>>  (gdb) frame 9
>>  (gdb) p string
>>  (gdb) xstring
>
> Kaushal Modi
>
-- 

-- 
Kaushal Modi

[Message part 2 (text/html, inline)]

Message #20 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: 23949 <at> debbugs.gnu.org
Subject: Re: 25.0.95; Regression in handling error caused by (string-match-p
 "." nil)
Date: Tue, 12 Jul 2016 17:01:33 +0300

> From: Kaushal Modi <kaushal.modi <at> gmail.com>
> Date: Tue, 12 Jul 2016 13:33:05 +0000
> Cc: 23949 <at> debbugs.gnu.org
> 
> (gdb) frame 9
> #9 0x0000000000624397 in Fread_from_string (string=54138084, start=0, end=0) at lread.c:2075
> 2075 ret = read_internal_start (string, start, end);
> (gdb) p string
> $15 = 54138084
> (gdb) xstring
> $16 = (struct Lisp_String *) 0x33a14e0
> "(nilory is relative, it is com"
> 
> Looks like the 'nil' argument in '(string-match-p "." nil)' somehow did this?

string-match-p just signals an error, because its 2nd arg must be a
string.  Look up the backtrace, and you will see that Emacs is trying
to signal an error:

  #47 0x00000000005f6a41 in Fapply (nargs=2, args=0x7fffffff7270) at
  eval.c:2329
  #48 0x00000000005f6f90 in apply1 (fn=16848, arg=56618787) at eval.c:2545
  #49 0x00000000005f23b6 in call_debugger (arg=56618787) at eval.c:308
  #50 0x00000000005f4e2b in maybe_call_debugger (conditions=9578427,
  sig=51984, data=56618835) at eval.c:1723
  #51 0x00000000005f48dd in Fsignal (error_symbol=51984, data=56618835) at
  eval.c:1541
  #52 0x00000000005f4a06 in xsignal (error_symbol=51984, data=56618835) at
  eval.c:1578
  #53 0x00000000005f4a9a in xsignal2 (error_symbol=51984, arg1=44400, arg2=0)
  at eval.c:1599
  #54 0x00000000005d7967 in wrong_type_argument (predicate=44400, value=0) at
  data.c:151
  #55 0x000000000055388b in CHECK_STRING (x=0) at lisp.h:2807
  #56 0x00000000005b2278 in string_match_1 (regexp=55505044, string=0,
  start=0, posix=false) at search.c:373
  #57 0x00000000005b25e5 in Fstring_match (regexp=55505044, string=0,
  start=0) at search.c:444

What happens next is that Emacs calls the debugger, and then your
advices kick in, starting at apply1.  And that's where the trouble
begins.

I asked why help-function-arglist is trying to read from a string, but
got no answer.

> (BTW this debug session is on the 2f67f8a commit of master (very recent, today). But the bug is present on
> emacs-25 too.)

Sorry, I see no bug yet, just a lot of ad-FOO stuff that tries to do
something silly during an error, when it should have moved out of the
way.  If there is a bug, its root cause hides inside
help-function-arglist, and the way it is called by the advices you (or
maybe it's Projectile?) have set up.  That's where we should be
looking, not in string-match-p, which did what it was supposed to do:
signaled an error when called with nil instead of a string.

Message #23 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: 23949 <at> debbugs.gnu.org
Subject: Re: 25.0.95; Regression in handling error caused by (string-match-p
 "." nil)
Date: Tue, 12 Jul 2016 17:03:34 +0300

> From: Kaushal Modi <kaushal.modi <at> gmail.com>
> Date: Tue, 12 Jul 2016 13:37:57 +0000
> Cc: 23949 <at> debbugs.gnu.org
> 
> Some interesting discovery .. hope this helps debug this:
> 
> Searched the whole emacs source and my ~/.emacs.d for 'is relative, it is com'. And the only place I found it
> was in lisp/progmodes/compile.el, line 2679:
> 
> =====
> km²~/downloads/:git/emacs> ag 'is relative, it is com' 
> lisp/progmodes/compile.el
> 2679:If DIRECTORY is relative, it is combined with `default-directory'.
> =====
> 
> Somehow "(nil" replaced "If DIRECT", the whole thing got lower-cased, string after "com" in "combined" got
> truncated, and we got:
> 
> "(nilory is relative, it is com"

It's probably just some random garbage left on the stack.

Once again, the root cause lies in help-function-arglist or its
immediate callers.  Why does all this get called when Emacs signals an
error?

Message #26 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> suse.de>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 23949 <at> debbugs.gnu.org
Subject: Re: bug#23949: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Tue, 12 Jul 2016 16:15:11 +0200

Kaushal Modi <kaushal.modi <at> gmail.com> writes:

> (gdb) frame 9
> #9  0x0000000000624397 in Fread_from_string (string=54138084, start=0,
> end=0) at lread.c:2075
> 2075      ret = read_internal_start (string, start, end);
> (gdb) p string
> $15 = 54138084
> (gdb) xstring

This is only valid if $ is a Lisp_String.  Use xtype to find out.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Message #29 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 schwab <at> suse.de
Cc: 23949 <at> debbugs.gnu.org
Subject: Re: 25.0.95; Regression in handling error caused by (string-match-p
 "." nil)
Date: Tue, 12 Jul 2016 18:35:02 +0000

[Message part 1 (text/plain, inline)]

On Tue, Jul 12, 2016 at 10:02 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> string-match-p just signals an error, because its 2nd arg must be a
> string.  Look up the backtrace, and you will see that Emacs is trying
> to signal an error:
>

Correct, but that error does not show up within emacs. All the user sees is:

Entering debugger...
help-function-arglist: End of file during parsing

In any case, I believe that that should not happen.

Also concerning is the fact that,

- (string-match "." nil) gives the expected error backtrace.
- But (string-match-p "." nil) gives the help-function-arglist error.

What happens next is that Emacs calls the debugger, and then your
> advices kick in, starting at apply1.  And that's where the trouble
> begins.
>

Correct. That's exactly what I do not understand.


> I asked why help-function-arglist is trying to read from a string, but
> got no answer.
>

Sorry, I missed answering that question in my earlier emails; I got too
busy with gdb.

I do not have answer for that question. I filed this bug report to learn
why this happens, and help in any way I can to fix it.

From the below:

> "help-function-arglist" (0xffff3f88)
> "ad-arglist" (0xffff44c0)
> "ad-make-advised-definition" (0xffff4a70)
> "ad-activate-advised-definition" (0xffff4fc0)
> "ad-activate" (0xffff5500)
> "projectile-mode" (0xffff5a50)

, the advices in the projectile package are causing this. I grepped the
projectile source, and it contains only 2 advices.

(1)
(defadvice compilation-find-file (around projectile-compilation-find-file)
  "Try to find a buffer for FILENAME, if we cannot find it,
fallback to the original function."
  (let ((filename (ad-get-arg 1)))
    (ad-set-arg 1
                (or
                 (if (file-exists-p (expand-file-name filename))
                     filename)
                 ;; Try to find the filename using projectile
                 (and (projectile-project-p)
                      (let ((root (projectile-project-root))
                            (dirs (cons ""
(projectile-current-project-dirs))))
                        (-when-let (full-filename (->> dirs
                                                       (--map
(expand-file-name filename (expand-file-name it root)))
                                                       (-filter
#'file-exists-p)
                                                       (-first-item)))
                          full-filename)))
                 ;; Fall back to the old argument
                 filename))
    ad-do-it))

(2)
(defadvice delete-file (before purge-from-projectile-cache (filename
&optional trash))
  (if (and projectile-enable-caching (projectile-project-p))
      (let* ((project-root (projectile-project-root))
             (true-filename (file-truename filename))
             (relative-filename (file-relative-name true-filename
project-root)))
        (if (projectile-file-cached-p relative-filename project-root)
            (projectile-purge-file-from-cache relative-filename)))))

As I started using advices only after emacs 24.4, I never learned the old
advice style. So I hoped someone experienced with these would help connect
the dots between invalid arg error for string-match-p and
help-function-arglist error. @Stefan?

Also, with the exact same projectile version, I do *not* get the
misleading help-function-arglist
error on emacs 24.5. So something probably changed in the way major mode
hooks are run in the debugger since then?

Sorry, I see no bug yet, just a lot of ad-FOO stuff that tries to do
> something silly during an error, when it should have moved out of the
> way.  If there is a bug, its root cause hides inside
> help-function-arglist,


Copying Stefan to help throw some light on this.


> and the way it is called by the advices you (or
> maybe it's Projectile?) have set up.


The minimal code I posted was run in emacs -Q. So the only effective
advices are the ones in Projectile; I have pasted the code for those 2
advices above for reference.


> That's where we should be
> looking, not in string-match-p, which did what it was supposed to do:
> signaled an error when called with nil instead of a string.
>

But then it would be interesting to know why (string-match "." nil) instead
does not cause the help-function-arglist error.

Here is the backtrace on evaluating

=====
(emacs-q-template '(projectile)
  (projectile-global-mode)
  (string-match-p "." nil))
=====
(Code for the emacs-q-template macro is in my previous email in this
thread.)

Backtrace:
=====
Debugger entered--Lisp error: (wrong-type-argument stringp nil)
string-match("." nil nil)
string-match-p("." nil)
(progn (require (quote package)) (setq package-user-dir (concat ...
eval((progn (require (quote package)) (setq package-user-dir (concat ...
eval-last-sexp-1(nil)
eval-last-sexp(nil)
call-interactively(eval-last-sexp nil nil)
command-execute(eval-last-sexp)
=====
(I do not get this backtrace in emacs 25.x; above is from emacs 24.5.)

And here is the backtrace for evaluating the same when using string-match
instead of string-match-p in emacs 24.5:

=====
(emacs-q-template '(projectile)
  (projectile-global-mode)
  (string-match "." nil))
=====

Backtrace:
=====

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  string-match("." nil)
  (progn (require (quote package)) (setq package-user-dir (concat ...
  eval((progn (require (quote package)) (setq package-user-dir (concat ...
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp nil nil)
  command-execute(eval-last-sexp)
=====

In the former case, it was

  string-match("." nil nil)

In the latter case, it was,

  string-match("." nil)

Do those 2 consecutive 'nil's somehow throw off the
debugger/ad-arglist/help-function-arglist
in emacs 25.x?




On Tue, Jul 12, 2016 at 10:15 AM Andreas Schwab <schwab <at> suse.de> wrote:

> > $15 = 54138084
> > (gdb) xstring
>
> This is only valid if $ is a Lisp_String.  Use xtype to find out.
>

 Hi Andreas,

Thanks for looking into this bug thread. I have close to 0 experience with
gdb. I typed 'xtype' for the same frame and this is what I got:

(gdb) xstring
$16 = (struct Lisp_String *) 0x33a14e0
"(nilory is relative, it is com"
(gdb) xtype
Argument to arithmetic operation not a number or boolean.
(gdb)

I do not know what to make out of that. Let me know if there are any other
gdb commands that I can use to give you more helpful debug info.

Thanks everyone.
-- 

-- 
Kaushal Modi

[Message part 2 (text/html, inline)]

Message #32 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: Andreas Schwab <schwab <at> suse.de>, Eli Zaretskii <eliz <at> gnu.org>,
 23949 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#23949: 25.0.95; Regression in handling error caused by
 (string-match-p "." nil)
Date: Tue, 12 Jul 2016 14:55:38 -0400

On Tue, Jul 12, 2016 at 2:35 PM, Kaushal Modi <kaushal.modi <at> gmail.com> wrote:
> (gdb) xstring
> $16 = (struct Lisp_String *) 0x33a14e0
> "(nilory is relative, it is com"
> (gdb) xtype
> Argument to arithmetic operation not a number or boolean.
> (gdb)
>
> I do not know what to make out of that.

I think you have to use xtype before xstring (otherwise it applies to
$16, the return value from xstring).

Message #35 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: Andreas Schwab <schwab <at> suse.de>, Eli Zaretskii <eliz <at> gnu.org>,
 23949 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#23949: 25.0.95; Regression in handling error caused by
 (string-match-p "." nil)
Date: Tue, 12 Jul 2016 19:00:20 +0000

[Message part 1 (text/plain, inline)]

Hi Noam,

Thanks. Now it says "Lisp_String".

(gdb) xstring
$18 = (struct Lisp_String *) 0x33a14e0
"(nilory is relative, it is com"
(gdb) p string
$19 = 54138084
(gdb) xtype
Lisp_String
(gdb)

On Tue, Jul 12, 2016 at 2:55 PM Noam Postavsky <
npostavs <at> users.sourceforge.net> wrote:

> I think you have to use xtype before xstring (otherwise it applies to
> $16, the return value from xstring).
>
-- 

-- 
Kaushal Modi

[Message part 2 (text/html, inline)]

Message #38 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: 25.0.95; Regression in handling error caused by (string-match-p
 "." nil)
Date: Tue, 12 Jul 2016 22:10:18 +0300

> From: Kaushal Modi <kaushal.modi <at> gmail.com>
> Date: Tue, 12 Jul 2016 18:35:02 +0000
> Cc: 23949 <at> debbugs.gnu.org
> 
> On Tue, Jul 12, 2016 at 10:15 AM Andreas Schwab <schwab <at> suse.de> wrote:
> 
>  > $15 = 54138084
>  > (gdb) xstring
> 
>  This is only valid if $ is a Lisp_String. Use xtype to find out.
> 
> Hi Andreas,
> 
> Thanks for looking into this bug thread. I have close to 0 experience with gdb. I typed 'xtype' for the same
> frame and this is what I got:
> 
> (gdb) xstring
> $16 = (struct Lisp_String *) 0x33a14e0
> "(nilory is relative, it is com"
> (gdb) xtype
> Argument to arithmetic operation not a number or boolean.
> (gdb)

No, the other way around:

 (gdb) p string
 (gdb) xtype

"xtype" works on the last value that was printed, it should do that on
the value produced by the p(rint) command.

Message #41 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#23949: 25.0.95; Regression in handling error caused by
 (string-match-p "." nil)
Date: Tue, 12 Jul 2016 22:12:10 +0300

> From: Kaushal Modi <kaushal.modi <at> gmail.com>
> Date: Tue, 12 Jul 2016 19:00:20 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>, 
> 	Andreas Schwab <schwab <at> suse.de>, 23949 <at> debbugs.gnu.org
> 
> Thanks. Now it says "Lisp_String".
> 
> (gdb) xstring
> $18 = (struct Lisp_String *) 0x33a14e0
> "(nilory is relative, it is com"
> (gdb) p string
> $19 = 54138084
> (gdb) xtype
> Lisp_String
> (gdb)

If it says "Lisp_String", then using "xstring" is correct.

Message #44 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: 25.0.95; Regression in handling error caused by (string-match-p
 "." nil)
Date: Tue, 12 Jul 2016 22:19:34 +0300

> From: Kaushal Modi <kaushal.modi <at> gmail.com>
> Date: Tue, 12 Jul 2016 18:35:02 +0000
> Cc: 23949 <at> debbugs.gnu.org
> 
>  string-match-p just signals an error, because its 2nd arg must be a
>  string. Look up the backtrace, and you will see that Emacs is trying
>  to signal an error:
> 
> Correct, but that error does not show up within emacs. All the user sees is:
> 
> Entering debugger...
> help-function-arglist: End of file during parsing

Because Emacs hits a second error while trying to show the backtrace
of the first one.

> In any case, I believe that that should not happen.

Indeed, it shouldn't, but the question is: what code is responsible
for that which shouldn't happen?  If some package or your own
customizations cause the debugger to call extra code, and that extra
code signals an error, then that extra code needs to be fixed, not
Emacs.

> Also concerning is the fact that,
> 
> - (string-match "." nil) gives the expected error backtrace.
> - But (string-match-p "." nil) gives the help-function-arglist error. 

Sorry, I fail to see the significance of this to the issue at hand.
They are two different functions, and we still don't know which
functions were advised and how.  Perhaps the advice will explain the
difference.  Or perhaps we understand the reason  for the difference
once we get to the bottom of investigating the problem.  Either way,
the efficient method of looking into this problem is to understand
what are those advices and where do they come from.

Message #47 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: 25.0.95; Regression in handling error caused by (string-match-p
 "." nil)
Date: Tue, 12 Jul 2016 19:29:45 +0000

[Message part 1 (text/plain, inline)]

On Tue, Jul 12, 2016 at 3:20 PM Eli Zaretskii <eliz <at> gnu.org> wrote:

> Indeed, it shouldn't, but the question is: what code is responsible
> for that which shouldn't happen?  If some package or your own
> customizations cause the debugger to call extra code, and that extra
> code signals an error, then that extra code needs to be fixed, not
> Emacs.
>

It should also be considered that the help-function-arglist error does not
happen on emacs 24.5, using the exact same projectile version.


> > Also concerning is the fact that,
> >
> > - (string-match "." nil) gives the expected error backtrace.
> > - But (string-match-p "." nil) gives the help-function-arglist error.
>
> Sorry, I fail to see the significance of this to the issue at hand.
>

I find the above to be important because both forms give the expected error
backtrace on emacs 24.5. But on emacs 25.x, string-match-p gives the
unintended help-function-arglist error without any backtrace, while
string-match gives the intended error backtrace. The same projectile
advices are in effect for all of these.


> They are two different functions, and we still don't know which
> functions were advised and how.


Projectile is advising the delete-file and compilation-find-file functions.
The full advice definitions were posted in my previous email.


> Perhaps the advice will explain the
> difference.  Or perhaps we understand the reason  for the difference
> once we get to the bottom of investigating the problem.  Either way,
> the efficient method of looking into this problem is to understand
> what are those advices and where do they come from.

-- 

-- 
Kaushal Modi

[Message part 2 (text/html, inline)]

Message #50 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: schwab <at> suse.de, Eli Zaretskii <eliz <at> gnu.org>, 23949 <at> debbugs.gnu.org
Subject: Re: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Tue, 12 Jul 2016 16:27:03 -0400

> Correct, but that error does not show up within emacs. All the user sees is:
> Entering debugger...
> help-function-arglist: End of file during parsing

Clearly, the problem is that string-match-p uses
"(let ((inhibit-changing-match-data t))", so the debugger is run with
inhibit-changing-match-data bound to t and that breaks lots of
Elisp code.

That's a general problem with the use dynamic binding to pass extra
parameters: you end up passing them not just to that one function but
also to all other functions called from that one.


        Stefan

Message #53 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: schwab <at> suse.de, Eli Zaretskii <eliz <at> gnu.org>, 23949 <at> debbugs.gnu.org
Subject: Re: 25.0.95; Regression in handling error caused by (string-match-p
 "." nil)
Date: Wed, 13 Jul 2016 13:10:46 +0000

[Message part 1 (text/plain, inline)]

On Tue, Jul 12, 2016 at 4:27 PM Stefan Monnier <monnier <at> iro.umontreal.ca>
wrote:

> Clearly, the problem is that string-match-p uses
> "(let ((inhibit-changing-match-data t))", so the debugger is run with
> inhibit-changing-match-data bound to t and that breaks lots of
> Elisp code.
>
> That's a general problem with the use dynamic binding to pass extra
> parameters: you end up passing them not just to that one function but
> also to all other functions called from that one.
>

Thanks Stefan.

So what is the way forward?

Fixing just string-match-p and string-match does not seem to the complete
solution, because I have seen just let-bound dynamic vars at many places.

As I posted in the emacs-devel thread (
https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00544.html ),
while this string-match-p issue causes a confusing seemingly unrelated
error, some other packages cause emacs to freeze up (check the drag-stuff
example in the above link). When I get a change I will add a minimum
working example for the drag-stuff package causing emacs freeze too.

Certainly there was a lower level change after emacs 24.5 that changed the
behavior of how the run-hooks behave or how the advices are executed in
general or when called within a debugger?
-- 

Kaushal Modi

[Message part 2 (text/html, inline)]

Message #56 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: schwab <at> suse.de, Eli Zaretskii <eliz <at> gnu.org>, 23949 <at> debbugs.gnu.org
Subject: Re: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Wed, 13 Jul 2016 09:59:33 -0400

>> That's a general problem with the use dynamic binding to pass extra
>> parameters: you end up passing them not just to that one function but
>> also to all other functions called from that one.
> So what is the way forward?

I think it's a fairly fundamental problem that's hard/impossible to "fix".

> Fixing just string-match-p and string-match does not seem to the complete
> solution, because I have seen just let-bound dynamic vars at many places.

A way to fix *some* of the problems is to use concurrency (not yet in
"master"), and make the debugger run in another thread so it's not
affected by the dynamic bindings of the code that signaled the error.

For the more general problem, the only "fix" is to try and remove those
uses of dynamic bindings and replace them with something else, which
seems like a very large undertaking and whose benefits are not
necessarily that clear (the problem being to decide what that "something
else" should be, and the fact that this "something else" will come with
its own problems).

In the case of string-match-p I think we should get rid of
inhibit-changing-match-data and implement string-match-p (and
looking-at-p) some other way.


        Stefan

Message #59 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, kaushal.modi <at> gmail.com
Subject: Re: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Wed, 13 Jul 2016 17:24:49 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  schwab <at> suse.de,  23949 <at> debbugs.gnu.org
> Date: Tue, 12 Jul 2016 16:27:03 -0400
> 
> Clearly, the problem is that string-match-p uses
> "(let ((inhibit-changing-match-data t))",

Since this is the only difference between string-match and
string-match-p, yes, that's pretty much obvious.  But saying that
doesn't yet point out the code which is affected by this binding in a
way that breaks popping the debugger.

> so the debugger is run with inhibit-changing-match-data bound to t
> and that breaks lots of Elisp code.

Then perhaps the solution is to avoid the effect of binding
inhibit-changing-match-data on the debugger.

> That's a general problem with the use dynamic binding to pass extra
> parameters: you end up passing them not just to that one function but
> also to all other functions called from that one.

This is a strange thing to hear, from you of all the people.
Dynamically binding variables around some expression is standard Emacs
Lisp programming technique, used all over the place.  The doc string
of this particular variable even says so.  How come it is suddenly a
problem?

I also think that the "breaks a lot of Elisp code" part is at least a
tad exaggerated.  We bind this particular variable in 2 functions that
are called from more than 300 different places in the Emacs sources,
so if doing so indeed breaks a lot of Lisp, we are in deep trouble,
which I don't think is the case.

Anyway, a cursory glance at help-function-arglist points to the
problematic code.  Compare this:

  (help-split-fundoc (documentation 'delete-file) nil)
    => ("(nil FILENAME &optional TRASH)" . "Delete file named FILENAME.  If it is a symlink, remove the symlink.
  If file has multiple names, it continues to exist with the other names.
  TRASH non-nil means to trash the file instead of deleting, provided
  ‘delete-by-moving-to-trash’ is non-nil.

  When called interactively, TRASH is t if no prefix argument is given.
  With a prefix argument, TRASH is nil.")

with this:

  (let ((inhibit-changing-match-data t))
    (help-split-fundoc (documentation 'delete-file) nil))
    => ("(nilnil" . "Delete fi")

The latter is clearly bogus.  Now, help-function-arglist calls
help-split-fundoc, and then reads from the string produced by the
latter:

          (let* ((doc (condition-case nil (documentation def) (error nil)))
                 (docargs (if doc (car (help-split-fundoc doc nil))))
                 (arglist (if docargs
                              (cdar (read-from-string (downcase docargs)))))

I hope the reason for EOF is now clear.  (I have no idea why it only
happens on master: the above bogus value shows on emacs-25 as well.)

Does the following variant of string-match-p look right?  Its intent
is to limit the effect of inhibit-changing-match-data to the call to
string-match only, leaving the error handling, if any is needed,
outside of that binding.

(defsubst string-match-p (regexp string &optional start)
  "\
Same as `string-match' except this function does not change the match data."
  (condition-case err
      (let ((inhibit-changing-match-data t))
	(string-match regexp string start))
    (error (signal (car err) (cdr err)))))

Message #62 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, kaushal.modi <at> gmail.com
Subject: Re: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Wed, 13 Jul 2016 10:48:15 -0400

> Dynamically binding variables around some expression is standard Emacs
> Lisp programming technique, used all over the place.

For sure.  But it's risky and does cause problems in lots of corner
cases (which luckily for us are very rarely triggered).

E.g. we often use re-search-forward or string-match without binding
case-fold-search explicitly, so we depend on the particular setting it
happens to have.  In some cases it truly doesn't matter, while in others
it happens to work because in 99% of the cases the code is run with the
right setting.

Same for completion-ignore-case, completion-regexp-list,
inhibit-read-only, ...

That's just part of life in Elisp.
And I'm not sure what a real "fix" would look like.  E.g. for
minibuffer-setup-hook, we have minibuffer-with-setup-hook but it's
pretty hackish.

Instead, we just live with it and fix the corner cases that we actually
bump into.

> I also think that the "breaks a lot of Elisp code" part is at least a
> tad exaggerated.

Binding inhibit-changing-match-data to t will pretty much break any
function that uses match-beginning or match-end.  I think that counts as
"a lot of Elisp code".  Of course, we currently don't have any code that
binds inhibit-changing-match-data to t around calls to those functions,
except when the calls happen via the debugger.  But it could rear its
ugly head in other cases, e.g. if/when we make it possible for the
regexp-engine to run Elisp code (there can be various reasons to do
that.  Such as to setup the syntax-table property lazily, or to allow
the regexp primitives to be expanded in Elisp [I've a long term desire
to do so for the zero-length primitives such as \< ]) or to pause and
run timers or process filters.

> (defsubst string-match-p (regexp string &optional start)
>   "\
> Same as `string-match' except this function does not change the match data."
>   (condition-case err
>       (let ((inhibit-changing-match-data t))
> 	(string-match regexp string start))
>     (error (signal (car err) (cdr err)))))

That will still cause the same problems when debug-on-signal is non-nil.


        Stefan

Message #65 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> suse.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 23949 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 kaushal.modi <at> gmail.com
Subject: Re: bug#23949: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Wed, 13 Jul 2016 17:03:22 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

> Does the following variant of string-match-p look right?  Its intent
> is to limit the effect of inhibit-changing-match-data to the call to
> string-match only, leaving the error handling, if any is needed,
> outside of that binding.
>
> (defsubst string-match-p (regexp string &optional start)
>   "\
> Same as `string-match' except this function does not change the match data."
>   (condition-case err
>       (let ((inhibit-changing-match-data t))
> 	(string-match regexp string start))
>     (error (signal (car err) (cdr err)))))

This optimizes for the rare case that string-match throws an error.
Better would be to bind inhibit-changing-match-data in call_debugger.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Message #68 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: 25.0.95; Regression in handling error caused by (string-match-p
 "." nil)
Date: Wed, 13 Jul 2016 18:03:07 +0300

> From: Kaushal Modi <kaushal.modi <at> gmail.com>
> Date: Wed, 13 Jul 2016 13:10:46 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, schwab <at> suse.de, 23949 <at> debbugs.gnu.org
> 
> So what is the way forward?

I just proposed one way forward.

> Certainly there was a lower level change after emacs 24.5 that changed the behavior of how the run-hooks
> behave or how the advices are executed in general or when called within a debugger? 

The underlying program with binding inhibit-changing-match-data when
help-function-arglist is called exists on the emacs-25 branch as
well.  It also exists in Emacs 24.5, except that there
help-split-fundoc signals an error, instead of returning bogus data:

  (let ((inhibit-changing-match-data t))
    (help-split-fundoc (documentation 'delete-file) nil))
    => error->Args out of range: 3004

Message #71 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, kaushal.modi <at> gmail.com
Subject: Re: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Wed, 13 Jul 2016 18:06:28 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  schwab <at> suse.de,  23949 <at> debbugs.gnu.org
> Date: Wed, 13 Jul 2016 09:59:33 -0400
> 
> In the case of string-match-p I think we should get rid of
> inhibit-changing-match-data and implement string-match-p (and
> looking-at-p) some other way.

Why?  The only problem with the current implementation is when
string-match signals an error.  We need to make sure the error is
handled not under the binding of inhibit-changing-match-data.

Message #74 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, kaushal.modi <at> gmail.com
Subject: Re: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Wed, 13 Jul 2016 18:14:39 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: kaushal.modi <at> gmail.com,  schwab <at> suse.de,  23949 <at> debbugs.gnu.org
> Date: Wed, 13 Jul 2016 10:48:15 -0400
> 
> > I also think that the "breaks a lot of Elisp code" part is at least a
> > tad exaggerated.
> 
> Binding inhibit-changing-match-data to t will pretty much break any
> function that uses match-beginning or match-end.

But those functions aren't supposed to run when string-match is
called.

> > (defsubst string-match-p (regexp string &optional start)
> >   "\
> > Same as `string-match' except this function does not change the match data."
> >   (condition-case err
> >       (let ((inhibit-changing-match-data t))
> > 	(string-match regexp string start))
> >     (error (signal (car err) (cdr err)))))
> 
> That will still cause the same problems when debug-on-signal is non-nil.

So you don't consider this an improvement that should be installed?

Message #77 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andreas Schwab <schwab <at> suse.de>
Cc: 23949 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, kaushal.modi <at> gmail.com
Subject: Re: bug#23949: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Wed, 13 Jul 2016 18:17:24 +0300

> From: Andreas Schwab <schwab <at> suse.de>
> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>,  23949 <at> debbugs.gnu.org,  kaushal.modi <at> gmail.com
> Date: Wed, 13 Jul 2016 17:03:22 +0200
> 
> > (defsubst string-match-p (regexp string &optional start)
> >   "\
> > Same as `string-match' except this function does not change the match data."
> >   (condition-case err
> >       (let ((inhibit-changing-match-data t))
> > 	(string-match regexp string start))
> >     (error (signal (car err) (cdr err)))))
> 
> This optimizes for the rare case that string-match throws an error.
> Better would be to bind inhibit-changing-match-data in call_debugger.

Fine, let's do that, then.

Thanks.

Message #80 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, kaushal.modi <at> gmail.com
Subject: Re: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Wed, 13 Jul 2016 12:00:40 -0400

>> > I also think that the "breaks a lot of Elisp code" part is at least a
>> > tad exaggerated.
>> Binding inhibit-changing-match-data to t will pretty much break any
>> function that uses match-beginning or match-end.
> But those functions aren't supposed to run when string-match is
> called.

Yet they are in bug#23949.

In practice, there's always a discrepancy between what is supposed to
happen and what can happen in the general case ;-)

>> > (defsubst string-match-p (regexp string &optional start)
>> >   "\
>> > Same as `string-match' except this function does not change the match data."
>> >   (condition-case err
>> >       (let ((inhibit-changing-match-data t))
>> > 	(string-match regexp string start))
>> >     (error (signal (car err) (cdr err)))))
>> That will still cause the same problems when debug-on-signal is non-nil.
> So you don't consider this an improvement that should be installed?

No.

A simpler and more robust solution would be
(save-match-data (string-match regexp string start))

Of course, with either solution, it means that string-match-p is even
worse in terms of efficiency, whereas the unsuspecting coder would
rightfully expect string-match-p to be (slightly) *more* efficient than
string-match.


        Stefan

Message #83 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, kaushal.modi <at> gmail.com
Subject: Re: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Wed, 13 Jul 2016 19:18:05 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: kaushal.modi <at> gmail.com,  schwab <at> suse.de,  23949 <at> debbugs.gnu.org
> Date: Wed, 13 Jul 2016 12:00:40 -0400
> 
> >> > I also think that the "breaks a lot of Elisp code" part is at least a
> >> > tad exaggerated.
> >> Binding inhibit-changing-match-data to t will pretty much break any
> >> function that uses match-beginning or match-end.
> > But those functions aren't supposed to run when string-match is
> > called.
> 
> Yet they are in bug#23949.

No, they aren't.  They run from the debugger.

> >> > (defsubst string-match-p (regexp string &optional start)
> >> >   "\
> >> > Same as `string-match' except this function does not change the match data."
> >> >   (condition-case err
> >> >       (let ((inhibit-changing-match-data t))
> >> > 	(string-match regexp string start))
> >> >     (error (signal (car err) (cdr err)))))
> >> That will still cause the same problems when debug-on-signal is non-nil.
> > So you don't consider this an improvement that should be installed?
> 
> No.

What about the suggestion made by Andreas?

> A simpler and more robust solution would be
> (save-match-data (string-match regexp string start))
> 
> Of course, with either solution, it means that string-match-p is even
> worse in terms of efficiency, whereas the unsuspecting coder would
> rightfully expect string-match-p to be (slightly) *more* efficient than
> string-match.

Exactly.  And we are punishing the innocent (the calls that don't
signal an error) for fear of the guilty (those that do).

Message #86 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: schwab <at> suse.de, 23949 <at> debbugs.gnu.org, kaushal.modi <at> gmail.com
Subject: Re: 25.0.95;
 Regression in handling error caused by (string-match-p "." nil)
Date: Wed, 13 Jul 2016 12:41:30 -0400

>> >> > I also think that the "breaks a lot of Elisp code" part is at least a
>> >> > tad exaggerated.
>> >> Binding inhibit-changing-match-data to t will pretty much break any
>> >> function that uses match-beginning or match-end.
>> > But those functions aren't supposed to run when string-match is
>> > called.
>> Yet they are in bug#23949.
> No, they aren't.  They run from the debugger.

I guess it depends what you call "run when string-match is called".

For me, I look at the C backtrace and see that Fstring_match calls
functions which end up calling the debugger which ends up running
the rest.

>> >> > (defsubst string-match-p (regexp string &optional start)
>> >> >   "\
>> >> > Same as `string-match' except this function does not change the match data."
>> >> >   (condition-case err
>> >> >       (let ((inhibit-changing-match-data t))
>> >> > 	(string-match regexp string start))
>> >> >     (error (signal (car err) (cdr err)))))
>> >> That will still cause the same problems when debug-on-signal is non-nil.
>> > So you don't consider this an improvement that should be installed?
>> No.
> What about the suggestion made by Andreas?

I think that would also fix this problem, yes.  It won't fix other
possible cases where the string-match could end up running Elisp code,
but AFAIK there aren't any currently.

> Exactly.  And we are punishing the innocent (the calls that don't
> signal an error) for fear of the guilty (those that do).

Right.  A more efficient solution would be to implement string-match-p
in C alongside string-match rather than on top of it, so we can get rid of
inhibit-changing-match-data (replaced by some other mechanism to
propagate this info where we need it).  But it's more work.


        Stefan

Message #97 received at 23949 <at> debbugs.gnu.org (full text, mbox):

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: 23949 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#23949 acknowledged by developer (Re: bug#24166: With --eval, 
 errors in string-match-p do not produce backtraces (but errors in
 string-match do?!))
Date: Tue, 09 Aug 2016 15:56:19 +0000

[Message part 1 (text/plain, inline)]

Hi Noam,

I appreciate the work you put on this. Thanks for fixing this.

On Sun, Aug 7, 2016 at 10:28 AM GNU bug Tracking System <
help-debbugs <at> gnu.org> wrote:

> This is an automatic notification regarding your bug report
> #23949: 25.0.95; Regression in handling error caused by (string-match-p
> "." nil),
> which was filed against the emacs package.
>
> Thank you for your report, which has now been closed.
> You can view the full report at
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23949
>
> If you require further information, please followup to
> 23949 <at> debbugs.gnu.org.
>
> debbugs.gnu.org maintainers
> (administrator, GNU bugs database)
>
-- 

Kaushal Modi

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.