bug#24673: 25.1.50; Evaluating '((closure))' aborts Emacs

bug#24673: 25.1.50; Evaluating '((closure))' aborts Emacs

[Andreas Politz]

emacs -Q

M-x ((closure)) RET

=> eval.c:2845: Emacs fatal error: assertion failed: CONSP (fun)

The function takes the (XCAR (XCDR '((closure)))), which is not a good
idea, since XCDR returns Qnil, while XCAR expects a cons. 


static Lisp_Object
funcall_lambda (Lisp_Object fun, ptrdiff_t nargs,
		register Lisp_Object *arg_vector)
{
  ....        
  if (CONSP (fun))
    {
      if (EQ (XCAR (fun), Qclosure))
	{
	  fun = XCDR (fun);	/* Drop `closure'.  */
	  lexenv = XCAR (fun);                 <============ 2845
	  CHECK_LIST_CONS (fun, fun);
	}
        ...
    }
  else ...
}

-ap

bug#24673: 25.1.50; Evaluating '((closure))' aborts Emacs

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 296 bytes --]

Andreas Politz <politza@hochschule-trier.de> schrieb am Mi., 12. Okt. 2016
um 12:28 Uhr:

>
>
> emacs -Q
>
>
>
> M-x ((closure)) RET
>
>
>
> => eval.c:2845: Emacs fatal error: assertion failed: CONSP (fun)
>
>
>
Thanks for reporting, if nobody complains I'll push the attached patch to
emacs-25.

[-- Attachment #1.2: Type: text/html, Size: 711 bytes --]

[-- Attachment #2: 0001-Fix-crash-in-evaluating-functions.txt --]
[-- Type: text/plain, Size: 2316 bytes --]

From e8a7ffad370d6bbbf7c7bc29179a7b196cfc3415 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Wed, 12 Oct 2016 22:48:32 +0200
Subject: [PATCH] Fix crash in evaluating functions

See Bug#24673

* src/eval.c (funcall_lambda): Fix crash for bogus functions such
as ((closure)).

* test/automated/eval-tests.el (eval-tests--bug24673): Add test.
---
 src/eval.c                   |  2 +-
 test/automated/eval-tests.el | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 test/automated/eval-tests.el

diff --git a/src/eval.c b/src/eval.c
index fe6460d..aa08cc9 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2821,7 +2821,7 @@ funcall_lambda (Lisp_Object fun, ptrdiff_t nargs,
       if (EQ (XCAR (fun), Qclosure))
 	{
 	  fun = XCDR (fun);	/* Drop `closure'.  */
-	  lexenv = XCAR (fun);
+	  lexenv = CAR (fun);
 	  CHECK_LIST_CONS (fun, fun);
 	}
       else
diff --git a/test/automated/eval-tests.el b/test/automated/eval-tests.el
new file mode 100644
index 0000000..1455cf3
--- /dev/null
+++ b/test/automated/eval-tests.el
@@ -0,0 +1,35 @@
+;;; eval-tests.el --- unit tests for src/eval.c      -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2016 Free Software Foundation, Inc.
+
+;; Author: Philipp Stephani <p.stephani2@gmail.com>
+
+;; 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/>.
+
+;;; Commentary:
+
+;; Unit tests for src/eval.c.
+
+;;; Code:
+
+(require 'ert)
+
+(ert-deftest eval-tests--bug24673 ()
+  "Checks that Bug#24673 has been fixed."
+  ;; This should not crash.
+  (should-error (eval '((closure)))) :type 'wrong-type-argument)
+
+;;; eval-tests.el ends here
-- 
2.8.0.rc3.226.g39d4020

bug#24673: 25.1.50; Evaluating '((closure))' aborts Emacs

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed, 12 Oct 2016 20:52:12 +0000
> 
>  emacs -Q
> 
>  M-x ((closure)) RET
> 
>  => eval.c:2845: Emacs fatal error: assertion failed: CONSP (fun)
> 
> Thanks for reporting, if nobody complains I'll push the attached patch to emacs-25.

I'm okay with the eval.c patch being pushed to emacs-25, but please
don't push the test there, push it to master instead (and to the
appropriate directory), as merging tests from emacs-25 frequently
produces problems due to the changes in directory structure.

Thanks.

bug#24673: 25.1.50; Evaluating '((closure))' aborts Emacs

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am Do., 13. Okt. 2016 um 07:51 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
>
> > Date: Wed, 12 Oct 2016 20:52:12 +0000
>
> >
>
> >  emacs -Q
>
> >
>
> >  M-x ((closure)) RET
>
> >
>
> >  => eval.c:2845: Emacs fatal error: assertion failed: CONSP (fun)
>
> >
>
> > Thanks for reporting, if nobody complains I'll push the attached patch
> to emacs-25.
>
>
>
> I'm okay with the eval.c patch being pushed to emacs-25, but please
>
> don't push the test there, push it to master instead (and to the
>
> appropriate directory), as merging tests from emacs-25 frequently
>
> produces problems due to the changes in directory structure.
>
>
>
OK, pushed a slight variant that signals invalid-function instead of
wrong-type-argument.

[-- Attachment #2: Type: text/html, Size: 1489 bytes --]

bug#24673: 25.1.50; Evaluating '((closure))' aborts Emacs

[Robert Cochran]

Andreas Politz <politza@hochschule-trier.de> writes:

> M-x ((closure)) RET

This is probably a brain-o, but `M-x ((closure))` prompts a "[No match]"
response on my machine. Perhaps you meant `M-:`?

If that's the case, then doing `M-: ((closure))` causes a SIGSEGV on
master's 6d6c93f4 with a non-debug build.

HTH,
-- 
~Robert Cochran

GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26  AD2D E5C3 EB36 4886 8871