Subject: Re: 25.0.50; [PATCH] SIGSEGV when misusing (backtrace-frame) from custom debugger
From: Paul Eggert <[email-masked]>
To: Pip Cet <[email-masked]>
Cc: [email-masked]
Date: Mon, 16 Nov 2015 23:16:46 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

Thanks for the bug report in <http://bugs.gnu.org/21245>. Good eye, noticing 
those dangling pointers -- these could cause problems in obscure circumstances 
even without custom debuggers.

I installed the attached patch into the emacs-25 branch, and (if I understand 
things correctly) it should address the issues raised in the bug report so I'll 
close the bug report for now; if I'm wrong and this patch doesn't fix the bugs 
we can always reopen it.
From 1e38ec75fad15952217c4c1fdf6262c960888577 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Mon, 16 Nov 2015 23:08:54 -0800
Subject: [PATCH] eval_sub followed dangling pointer when debugging
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Pip Cet (Bug#21245).
This bug could occur in eval_sub if the C compiler reused
storage associated with the ‘argvals’ local after ‘argvals’
went out of scope, and if the Elisp debugger stopped on Elisp
function exit and accessed ‘argvals’.  It could also occur if
a variadic function was called with so many arguments (over
2048 args on x86-64) that SAFE_ALLOCA_LISP called malloc, then
SAFE_FREE freed the arguments, then the memory manager used
the storage for other purposes, then the debugger accessed the
arguments.
* src/eval.c (eval_sub): Declare ‘argvals’ at top level of
function body.	Simplify local decls.
When allocating args via SAFE_ALLOCA, call
debugger before invoking SAFE_FREE, as the debugger needs
access to the args.
(eval_sub, apply_lambda): Rework to avoid need for
set_backtrace_debug_on_exit hack.  This is cleaner,
and should work better with buggy custom debuggers.
---
 src/eval.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index d460048..3ee07a7 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2014,6 +2014,10 @@ eval_sub (Lisp_Object form)
   Lisp_Object funcar;
   ptrdiff_t count;
 
+  /* Declare here, as this array may be accessed by call_debugger near
+     the end of this function.  See Bug#21245.  */
+  Lisp_Object argvals[8];
+
   if (SYMBOLP (form))
     {
       /* Look up its binding in the lexical environment.
@@ -2066,13 +2070,8 @@ eval_sub (Lisp_Object form)
 
   if (SUBRP (fun))
     {
-      Lisp_Object numargs;
-      Lisp_Object argvals[8];
-      Lisp_Object args_left;
-      register int i, maxargs;
-
-      args_left = original_args;
-      numargs = Flength (args_left);
+      Lisp_Object args_left = original_args;
+      Lisp_Object numargs = Flength (args_left);
 
       check_cons_list ();
 
@@ -2101,11 +2100,20 @@ eval_sub (Lisp_Object form)
 	  set_backtrace_args (specpdl + count, vals, XINT (numargs));
 
 	  val = (XSUBR (fun)->function.aMANY) (XINT (numargs), vals);
+
+	  check_cons_list ();
+	  lisp_eval_depth--;
+	  /* Do the debug-on-exit now, while VALS still exists.  */
+	  if (backtrace_debug_on_exit (specpdl + count))
+	    val = call_debugger (list2 (Qexit, val));
 	  SAFE_FREE ();
+	  specpdl_ptr--;
+	  return val;
 	}
       else
 	{
-	  maxargs = XSUBR (fun)->max_args;
+	  int i, maxargs = XSUBR (fun)->max_args;
+
 	  for (i = 0; i < maxargs; i++)
 	    {
 	      argvals[i] = eval_sub (Fcar (args_left));
@@ -2165,7 +2173,7 @@ eval_sub (Lisp_Object form)
 	}
     }
   else if (COMPILEDP (fun))
-    val = apply_lambda (fun, original_args, count);
+    return apply_lambda (fun, original_args, count);
   else
     {
       if (NILP (fun))
@@ -2195,7 +2203,7 @@ eval_sub (Lisp_Object form)
 	}
       else if (EQ (funcar, Qlambda)
 	       || EQ (funcar, Qclosure))
-	val = apply_lambda (fun, original_args, count);
+	return apply_lambda (fun, original_args, count);
       else
 	xsignal1 (Qinvalid_function, original_fun);
     }
@@ -2750,14 +2758,13 @@ apply_lambda (Lisp_Object fun, Lisp_Object args, ptrdiff_t count)
   set_backtrace_args (specpdl + count, arg_vector, i);
   tem = funcall_lambda (fun, numargs, arg_vector);
 
+  check_cons_list ();
+  lisp_eval_depth--;
   /* Do the debug-on-exit now, while arg_vector still exists.  */
   if (backtrace_debug_on_exit (specpdl + count))
-    {
-      /* Don't do it again when we return to eval.  */
-      set_backtrace_debug_on_exit (specpdl + count, false);
-      tem = call_debugger (list2 (Qexit, tem));
-    }
+    tem = call_debugger (list2 (Qexit, tem));
   SAFE_FREE ();
+  specpdl_ptr--;
   return tem;
 }
 
-- 
2.1.0