unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: Steve Kemp <steve@steve.org.uk>
Cc: 27585-done@debbugs.gnu.org
Subject: bug#27585: segfault when evaluating a file containing only backticks
Date: Fri, 14 Jul 2017 05:09:34 -0700	[thread overview]
Message-ID: <070206be-9f8b-a324-0650-fd21b37a4132@cs.ucla.edu> (raw)
In-Reply-To: <1499235670.28433.1@ssh.steve.org.uk>

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

Thanks for reporting the bug. I reproduced the problem on Fedora 26 x86-64, 
fixed it in master by applying the attached patch, and am boldly marking the bug 
as fixed.

As Eli and Daniel mentioned, this area of Emacs cannot be 100% reliable and to 
some extent is indeed a "ticking time bomb". That being said, the problem in 
this particular case was that Emacs had a bad heuristic for guessing whether a 
segmentation violation address was due to stack overflow on GNU/Linux. This bad 
heuristic has been in place for years without anybody reporting it. It's good 
that we fixed this bug (though I hope "normal" users never notice the bug fix :-).

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Improve-stack-overflow-heuristic-on-GNU-Linux.patch --]
[-- Type: text/x-patch; name="0001-Improve-stack-overflow-heuristic-on-GNU-Linux.patch", Size: 5692 bytes --]

From 9dee1c884eb50ba282eb9dd2495c5269add25963 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 14 Jul 2017 04:54:05 -0700
Subject: [PATCH] Improve stack-overflow heuristic on GNU/Linux

Problem reported by Steve Kemp (Bug#27585).
* src/eval.c (near_C_stack_top): Remove.  All uses replaced
by current_thread->stack_top.
(record_in_backtrace): Set current_thread->stack_top.
This is for when the Lisp interpreter calls itself.
* src/lread.c (read1): Set current_thread->stack_top.
This is for recursive s-expression reads.
* src/print.c (print_object): Set current_thread->stack_top.
This is for recursive s-expression printing.
* src/thread.c (mark_one_thread): Get stack top first.
* src/thread.h (struct thread_state.stack_top): Now void *, not char *.
---
 src/eval.c   |  9 +--------
 src/lisp.h   |  1 -
 src/lread.c  |  1 +
 src/print.c  |  2 +-
 src/sysdep.c |  2 +-
 src/thread.c | 10 ++++++----
 src/thread.h | 10 ++++++++--
 7 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index 8f293c9..e590038 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -213,13 +213,6 @@ backtrace_next (union specbinding *pdl)
   return pdl;
 }
 
-/* Return a pointer to somewhere near the top of the C stack.  */
-void *
-near_C_stack_top (void)
-{
-  return backtrace_args (backtrace_top ());
-}
-
 void
 init_eval_once (void)
 {
@@ -2090,7 +2083,7 @@ record_in_backtrace (Lisp_Object function, Lisp_Object *args, ptrdiff_t nargs)
   specpdl_ptr->bt.kind = SPECPDL_BACKTRACE;
   specpdl_ptr->bt.debug_on_exit = false;
   specpdl_ptr->bt.function = function;
-  specpdl_ptr->bt.args = args;
+  current_thread->stack_top = specpdl_ptr->bt.args = args;
   specpdl_ptr->bt.nargs = nargs;
   grow_specpdl ();
 
diff --git a/src/lisp.h b/src/lisp.h
index f5cb6c7..1e8ef7a 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3874,7 +3874,6 @@ extern Lisp_Object vformat_string (const char *, va_list)
   ATTRIBUTE_FORMAT_PRINTF (1, 0);
 extern void un_autoload (Lisp_Object);
 extern Lisp_Object call_debugger (Lisp_Object arg);
-extern void *near_C_stack_top (void);
 extern void init_eval_once (void);
 extern Lisp_Object safe_call (ptrdiff_t, Lisp_Object, ...);
 extern Lisp_Object safe_call1 (Lisp_Object, Lisp_Object);
diff --git a/src/lread.c b/src/lread.c
index fe5de38..901e40b 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2676,6 +2676,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list)
   bool uninterned_symbol = false;
   bool multibyte;
   char stackbuf[MAX_ALLOCA];
+  current_thread->stack_top = stackbuf;
 
   *pch = 0;
 
diff --git a/src/print.c b/src/print.c
index b6ea3ff..12edf01 100644
--- a/src/print.c
+++ b/src/print.c
@@ -1748,7 +1748,7 @@ print_object (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag)
   char buf[max (sizeof "from..to..in " + 2 * INT_STRLEN_BOUND (EMACS_INT),
 		max (sizeof " . #" + INT_STRLEN_BOUND (printmax_t),
 		     40))];
-
+  current_thread->stack_top = buf;
   maybe_quit ();
 
   /* Detect circularities and truncate them.  */
diff --git a/src/sysdep.c b/src/sysdep.c
index b522367..db99f53 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -1772,7 +1772,7 @@ stack_overflow (siginfo_t *siginfo)
   /* The known top and bottom of the stack.  The actual stack may
      extend a bit beyond these boundaries.  */
   char *bot = stack_bottom;
-  char *top = near_C_stack_top ();
+  char *top = current_thread->stack_top;
 
   /* Log base 2 of the stack heuristic ratio.  This ratio is the size
      of the known stack divided by the size of the guard area past the
diff --git a/src/thread.c b/src/thread.c
index e378797..1f7ced3 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -595,14 +595,15 @@ thread_select (select_func *func, int max_fds, fd_set *rfds,
 static void
 mark_one_thread (struct thread_state *thread)
 {
-  struct handler *handler;
-  Lisp_Object tem;
+  /* Get the stack top now, in case mark_specpdl changes it.  */
+  void *stack_top = thread->stack_top;
 
   mark_specpdl (thread->m_specpdl, thread->m_specpdl_ptr);
 
-  mark_stack (thread->m_stack_bottom, thread->stack_top);
+  mark_stack (thread->m_stack_bottom, stack_top);
 
-  for (handler = thread->m_handlerlist; handler; handler = handler->next)
+  for (struct handler *handler = thread->m_handlerlist;
+       handler; handler = handler->next)
     {
       mark_object (handler->tag_or_ch);
       mark_object (handler->val);
@@ -610,6 +611,7 @@ mark_one_thread (struct thread_state *thread)
 
   if (thread->m_current_buffer)
     {
+      Lisp_Object tem;
       XSETBUFFER (tem, thread->m_current_buffer);
       mark_object (tem);
     }
diff --git a/src/thread.h b/src/thread.h
index 9e94de5..52b16f1 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -62,8 +62,14 @@ struct thread_state
   char *m_stack_bottom;
 #define stack_bottom (current_thread->m_stack_bottom)
 
-  /* An address near the top of the stack.  */
-  char *stack_top;
+  /* The address of an object near the C stack top, used to determine
+     which words need to be scanned by the garbage collector.  This is
+     also used to detect heuristically whether segmentation violation
+     address indicates stack overflow, as opposed to some internal
+     error in Emacs.  If the C function F calls G which calls H which
+     calls ... F, then at least one of the functions in the chain
+     should set this to the address of a local variable.  */
+  void *stack_top;
 
   struct catchtag *m_catchlist;
 #define catchlist (current_thread->m_catchlist)
-- 
2.7.4


  parent reply	other threads:[~2017-07-14 12:09 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-05  6:21 bug#27585: segfault when evaluating a file containing only backticks Steve Kemp
2017-07-05  7:51 ` Andreas Schwab
2017-07-05  8:26   ` Steve Kemp
2017-07-05 18:41 ` Eli Zaretskii
2017-07-05 18:55   ` Steve Kemp
2017-07-05 19:47     ` Eli Zaretskii
2017-07-06  3:46       ` Steve Kemp
2017-07-06 15:16         ` Eli Zaretskii
2017-07-06 15:33           ` Steve Kemp
2017-07-06 16:24             ` Eli Zaretskii
2017-07-06  6:46       ` Andreas Schwab
2017-07-06 15:19         ` Eli Zaretskii
2017-07-06 15:31           ` Andreas Schwab
2017-07-06 15:37             ` Eli Zaretskii
2017-07-06 15:41               ` Andreas Schwab
2017-07-06 15:52       ` Daniel Colascione
2017-07-06 16:19         ` Eli Zaretskii
2017-07-06 16:37           ` Daniel Colascione
2017-07-06 17:27             ` Eli Zaretskii
2017-07-06 15:48   ` Daniel Colascione
2017-07-14 12:09 ` Paul Eggert [this message]
2017-07-14 13:30   ` Eli Zaretskii
2017-07-15  5:03     ` Steve Kemp
2017-07-15  5:12       ` Paul Eggert
2017-07-15  7:15         ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=070206be-9f8b-a324-0650-fd21b37a4132@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=27585-done@debbugs.gnu.org \
    --cc=steve@steve.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).