From: nobu.nakada@...
Date: 2003-06-27T19:35:48+09:00
Subject: [ruby-dev:20477] Re: 原因不明のSEGV

なかだです。

At Fri, 27 Jun 2003 16:23:29 +0900,
Yukihiro Matsumoto wrote:
> また〆切と戦っています。

じゃあ+1した理由を今思い出してもらうのは無理?

> |> あら、このパッチでこちらの再現はなくなったのですが、、
> |
> |やはり+1は余分のようですね。
> 
> なんか全容が理解できていないですが、[ruby-dev:20465]のパッチ
> で問題が解決したのでしょうか。ならコミットしてください。

rb_gc_mark_locations()がendの次までマークしようとするので、
thread_tの持つスタックのコピーの末尾がちょうどデータセグメント
の末尾と一致するときにSEGVになります。そもそもこのコピーは
malloc()したものなので、マージンは必要ないはずです。

"safety margin"というのが、Init_stack()で指定されたVALUEまで含
めるためということであれば、rb_gc_stack_startを変更するときかそ
れをマークするためにrb_gc_mark_locations()を呼ぶ側で何とかする
べきじゃないかと思います。

ついでにstack_growup_p()をconfigureに追い出してみたり。


Index: configure.in
===================================================================
RCS file: /cvs/ruby/src/ruby/configure.in,v
retrieving revision 1.176
diff -u -2 -p -r1.176 configure.in
--- configure.in	23 Jun 2003 19:54:41 -0000	1.176
+++ configure.in	27 Jun 2003 09:48:36 -0000
@@ -617,4 +617,28 @@ if test "$rb_cv_need_io_flush_before_see
 fi
 
+case "$target_cpu" in
+m68*|i?86|sparc) rb_cv_stack_grow_dir=-1;;
+esac
+AC_CACHE_CHECK(stack growing direction, rb_cv_stack_grow_dir,
+  [AC_TRY_RUN([
+/* recurse to get rid of inlining */
+static int
+stack_growup_p(addr, n)
+    volatile int *addr, n;
+{
+    volatile int end;
+    if (n > 0)
+	return *addr = stack_growup_p(addr, n - 1);
+    else
+	return (&end > addr);
+}
+int main()
+{
+    int x;
+    return stack_growup_p(&x, 10);
+}
+], rb_cv_stack_grow_dir=-1, rb_cv_stack_grow_dir=+1, rb_cv_stack_grow_dir=0)])
+AC_DEFINE_UNQUOTED(STACK_GROW_DIRECTION, $rb_cv_stack_grow_dir)
+
 dnl default value for $KANJI
 DEFAULT_KCODE="KCODE_NONE"
Index: eval.c
===================================================================
RCS file: /cvs/ruby/src/ruby/eval.c,v
retrieving revision 1.469
diff -u -2 -p -r1.469 eval.c
--- eval.c	24 Jun 2003 08:59:34 -0000	1.469
+++ eval.c	27 Jun 2003 08:22:57 -0000
@@ -8113,6 +8113,5 @@ rb_thread_save_context(th)
     len = ruby_stack_length(&pos);
     th->stk_len = 0;
-    th->stk_pos = (rb_gc_stack_start<pos)?rb_gc_stack_start
-				         :rb_gc_stack_start - len;
+    th->stk_pos = pos;
     if (len > th->stk_max) {
 	REALLOC_N(th->stk_ptr, VALUE, len);
@@ -8212,4 +8211,9 @@ rb_thread_restore_context(th, exit)
     if (!th->stk_ptr) rb_bug("unsaved context");
 
+#if STACK_GROW_DIRECTION < 0
+    if (&v > th->stk_pos) stack_extend(th, exit);
+#elif STACK_GROW_DIRECTION > 0
+    if (&v < th->stk_pos + th->stk_len) stack_extend(th, exit);
+#else
     if (&v < rb_gc_stack_start) {
 	/* Stack grows downward */
@@ -8220,4 +8224,5 @@ rb_thread_restore_context(th, exit)
 	if (&v < th->stk_pos + th->stk_len) stack_extend(th, exit);
     }
+#endif
 
     rb_trap_immediate = 0;	/* inhibit interrupts from here */
Index: gc.c
===================================================================
RCS file: /cvs/ruby/src/ruby/gc.c,v
retrieving revision 1.137
diff -u -2 -p -r1.137 gc.c
--- gc.c	16 Jun 2003 07:14:36 -0000	1.137
+++ gc.c	27 Jun 2003 09:47:08 -0000
@@ -377,8 +377,28 @@ static unsigned int STACK_LEVEL_MAX = 65
 #if defined(sparc) || defined(__sparc__)
 # define STACK_LENGTH  (rb_gc_stack_start - STACK_END + 0x80)
+#elif STACK_GROW_DIRECTION < 0
+# define STACK_LENGTH  (rb_gc_stack_start - STACK_END)
+#elif STACK_GROW_DIRECTION > 0
+# define STACK_LENGTH  (STACK_END - rb_gc_stack_start)
 #else
 # define STACK_LENGTH  ((STACK_END < rb_gc_stack_start) ? rb_gc_stack_start - STACK_END\
                                            : STACK_END - rb_gc_stack_start)
 #endif
+#if STACK_GROW_DIRECTION > 0
+# define STACK_UPPER(x, a, b) a
+#elif STACK_GROW_DIRECTION < 0
+# define STACK_UPPER(x, a, b) b
+#else
+static int
+stack_growup_p(addr)
+    VALUE *addr;
+{
+    SET_STACK_END;
+
+    if (STACK_END > addr) return Qtrue;
+    return Qfalse;
+}
+# define STACK_UPPER(x, a, b) (stack_growup_p(x) ? a : b)
+#endif
 
 #define GC_WARTER_MARK 512
@@ -394,5 +414,5 @@ ruby_stack_length(p)
 {
     SET_STACK_END;
-    if (p) *p = STACK_END;
+    if (p) *p = STACK_UPPER(STACK_END, rb_gc_stack_start, STACK_END);
     return STACK_LENGTH;
 }
@@ -545,5 +565,5 @@ rb_gc_mark_locations(start, end)
 	end = tmp;
     }
-    n = end - start + 1;
+    n = end - start;
     mark_locations_array(start,n);
 }
@@ -1228,8 +1248,17 @@ rb_gc()
     setjmp(save_regs_gc_mark);
     mark_locations_array((VALUE*)save_regs_gc_mark, sizeof(save_regs_gc_mark) / sizeof(VALUE *));
+#if STACK_GROW_DIRECTION < 0
+    rb_gc_mark_locations((VALUE*)STACK_END, rb_gc_stack_start);
+#elif STACK_GROW_DIRECTION > 0
     rb_gc_mark_locations(rb_gc_stack_start, (VALUE*)STACK_END);
+#else
+    if ((VALUE*)STACK_END < rb_gc_stack_start)
+	rb_gc_mark_locations((VALUE*)STACK_END, rb_gc_stack_start);
+    else
+	rb_gc_mark_locations(rb_gc_stack_start, (VALUE*)STACK_END);
+#endif
 #if defined(__human68k__) || defined(__mc68000__)
-    rb_gc_mark_locations((VALUE*)((char*)rb_gc_stack_start + 2),
-			 (VALUE*)((char*)STACK_END + 2));
+    rb_gc_mark_locations((VALUE*)((char*)STACK_END + 2),
+			 (VALUE*)((char*)rb_gc_stack_start + 2));
 #endif
     rb_gc_mark_threads();
@@ -1269,16 +1298,4 @@ rb_gc_start()
 }
 
-#if !defined(__human68k__)
-static int
-stack_growup_p(addr)
-    VALUE *addr;
-{
-    SET_STACK_END;
-
-    if (STACK_END > addr) return Qtrue;
-    return Qfalse;
-}
-#endif
-
 void
 Init_stack(addr)
@@ -1291,12 +1308,8 @@ Init_stack(addr)
     if (!addr) addr = (VALUE *)&addr;
     if (rb_gc_stack_start) {
-	if (stack_growup_p(addr)) {
-	    if (rb_gc_stack_start > addr)
-		rb_gc_stack_start = addr;
-	}
-	else {
-	    if (rb_gc_stack_start < addr)
-		rb_gc_stack_start = addr;
-	}
+	if (STACK_UPPER(&addr,
+			rb_gc_stack_start > --addr,
+			rb_gc_stack_start < ++addr))
+	    rb_gc_stack_start = addr;
 	return;
     }


-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦