unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: "Linas Vepstas" <linasvepstas@gmail.com>
To: bug-guile@gnu.org, guile-devel <guile-devel@gnu.org>
Subject: [PATCH] fix for Re: crash in gc with upside-down stack
Date: Thu, 13 Nov 2008 15:50:07 -0600	[thread overview]
Message-ID: <3ae3aa420811131350v12ea6a6dj46f7cc59750d8469@mail.gmail.com> (raw)

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

Patch below; I'm also attaching the same patch, in case
gmail is scrambling this thing :-/  Also, I've long had a
generic assignment on file with the FSF.

--linas

The patch below fixes a crash during garbage collection, where, during
the mark-stack phase, the top and bottom of the stack are found to be
in backwards order, typically because scm_with_guile() was called when
the stack is much shorter than when a thread was first guilified. That
is, the stack base pointer is stale, and can be inverted from the stack
top. If GC runs due to activity in some other thread, the stale base
pointer leads to the crash (as base-top is approximately 2^32 or 2^64).

A typical symptom of this bug, on a 32-bit system, is:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xf5333b90 (LWP 20587)]
0xf7711ce3 in scm_mark_locations (x=0xf5333110, n=4294966782) at gc-mark.c:435
435           SCM obj = * (SCM *) &x[m];
Current language:  auto; currently c
(gdb) bt
#0  0xf7711ce3 in scm_mark_locations (x=0xf5333110, n=4294966782) at
gc-mark.c:435
#1  0xf7766a12 in scm_threads_mark_stacks () at threads.c:1375

Notice that 4294966782 == fffffdfe == -202

Please apply in time for guile-1.8.6!

Signed-off-by: Linas Vepstas <linasvepstas@gmail.com>

---
 libguile/threads.c |   19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Index: guile-1.8.5/libguile/threads.c
===================================================================
--- guile-1.8.5.orig/libguile/threads.c	2008-11-13 15:17:12.000000000 -0600
+++ guile-1.8.5/libguile/threads.c	2008-11-13 15:32:07.000000000 -0600
@@ -577,9 +577,24 @@ scm_i_init_thread_for_guile (SCM_STACKIT
       /* This thread is already guilified but not in guile mode, just
 	 resume it.
 	
-	 XXX - base might be lower than when this thread was first
-	 guilified.
+         A user call to scm_with_guile() will lead us to here. This
+         could happen anywhere on the stack, and in particular, the
+         stack can be *much* shorter than what it was when this thread
+         was first guilified. This will typically happen in
+         on_thread_exit(), where the stack is *always* shorter than
+         when the thread was first guilified. If the GC happens to
+         get triggered due to some other thread, we'd end up with
+         t->top "upside-down" w.r.t. t->base, which will result in
+         chaos in scm_threads_mark_stacks() when top-base=2^32 or 2^64.
+         Thus, reset the base, if needed.
        */
+#if SCM_STACK_GROWS_UP
+      if (base < t->base)
+         t->base = base;
+#else
+      if (base > t->base)
+         t->base = base;
+#endif
       scm_enter_guile ((scm_t_guile_ticket) t);
       return 1;
     }

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: stack-backwards.patch --]
[-- Type: text/x-diff; name=stack-backwards.patch, Size: 2572 bytes --]

The patch below fixes a crash during garbage collection, where, during
the mark-stack phase, the top and bottom of the stack are found to be 
in backwards order, typically because scm_with_guile() was called when
the stack is much shorter than when a thread was first guilified. That
is, the stack base pointer is stale, and can be inverted from the stack
top. If GC runs due to activity in some other thread, the stale base
pointer leads to the crash (as base-top is approximately 2^32 or 2^64).

A typical symptom of this bug, on a 32-bit system, is:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xf5333b90 (LWP 20587)]
0xf7711ce3 in scm_mark_locations (x=0xf5333110, n=4294966782) at gc-mark.c:435
435           SCM obj = * (SCM *) &x[m];
Current language:  auto; currently c
(gdb) bt
#0  0xf7711ce3 in scm_mark_locations (x=0xf5333110, n=4294966782) at gc-mark.c:435
#1  0xf7766a12 in scm_threads_mark_stacks () at threads.c:1375

Notice that 4294966782 == fffffdfe == -202

Please apply in time for guile-1.8.6!

Signed-off-by: Linas Vepstas <linasvepstas@gmail.com>

---
 libguile/threads.c |   19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Index: guile-1.8.5/libguile/threads.c
===================================================================
--- guile-1.8.5.orig/libguile/threads.c	2008-11-13 15:17:12.000000000 -0600
+++ guile-1.8.5/libguile/threads.c	2008-11-13 15:32:07.000000000 -0600
@@ -577,9 +577,24 @@ scm_i_init_thread_for_guile (SCM_STACKIT
       /* This thread is already guilified but not in guile mode, just
 	 resume it.
 	 
-	 XXX - base might be lower than when this thread was first
-	 guilified.
+         A user call to scm_with_guile() will lead us to here. This
+         could happen anywhere on the stack, and in particular, the
+         stack can be *much* shorter than what it was when this thread
+         was first guilified. This will typically happen in
+         on_thread_exit(), where the stack is *always* shorter than
+         when the thread was first guilified. If the GC happens to
+         get triggered due to some other thread, we'd end up with
+         t->top "upside-down" w.r.t. t->base, which will result in
+         chaos in scm_threads_mark_stacks() when top-base=2^32 or 2^64.
+         Thus, reset the base, if needed.
        */
+#if SCM_STACK_GROWS_UP
+      if (base < t->base)
+         t->base = base;
+#else
+      if (base > t->base)
+         t->base = base;
+#endif
       scm_enter_guile ((scm_t_guile_ticket) t);
       return 1;
     }

             reply	other threads:[~2008-11-13 21:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-13 21:50 Linas Vepstas [this message]
2008-11-13 23:40 ` [PATCH] fix for Re: crash in gc with upside-down stack Ludovic Courtès
2008-11-16  0:17   ` Neil Jerram

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/guile/

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

  git send-email \
    --in-reply-to=3ae3aa420811131350v12ea6a6dj46f7cc59750d8469@mail.gmail.com \
    --to=linasvepstas@gmail.com \
    --cc=bug-guile@gnu.org \
    --cc=guile-devel@gnu.org \
    /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.
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).