Bug #4855

rb_context_t::saved_thread::machine_stack_(start|end) should be cleared

Added by Tomoyuki Chikanaga almost 4 years ago. Updated almost 4 years ago.

[ruby-dev:43680]
Status:Closed
Priority:Normal
Assignee:Tomoyuki Chikanaga
ruby -v:ruby 1.9.3dev (2011-06-08 trunk 31957) [x86_64-darwin10.7.0] Backport:

Description

近永と申します。英語で説明するのが難しいのでruby-devで失礼します。

#4827 の調査の途上で再現スクリプトを valgrind つきで実行すると以下のように
不正な領域を machine stack として mark しようとしているようなメッセージが出ました。
(ただし FIBER_USE_NATIVE=0 にした場合です)。

==27771== Invalid read of size 4
==27771== at 0x809AC1E: mark_locations_array (gc.c:1373)
==27771== by 0x809ACED: gc_mark_locations (gc.c:1389)
==27771== by 0x809D018: rb_gc_mark_machine_stack (gc.c:2498)
==27771== by 0x819684E: rb_thread_mark (vm.c:1700)
==27771== by 0x819FC05: cont_mark (cont.c:141)
==27771== by 0x819FE09: fiber_mark (cont.c:268)
==27771== by 0x809B6FF: gc_mark_children (gc.c:1813)
==27771== by 0x809B27C: gc_mark (gc.c:1605)
==27771== by 0x809B2B6: rb_gc_mark (gc.c:1611)
==27771== by 0x8196790: rb_thread_mark (vm.c:1690)
==27771== by 0x809B6FF: gc_mark_children (gc.c:1813)
==27771== by 0x809B27C: gc_mark (gc.c:1605)
==27771== by 0x809B2B6: rb_gc_mark (gc.c:1611)
==27771== by 0x819620E: vm_mark_each_thread_func (vm.c:1493)
==27771== by 0x8136FD9: st_foreach (st.c:747)
==27771== by 0x819627B: rb_vm_mark (vm.c:1516)
==27771== by 0x809B6FF: gc_mark_children (gc.c:1813)
==27771== by 0x809B27C: gc_mark (gc.c:1605)
==27771== by 0x809B2B6: rb_gc_mark (gc.c:1611)
==27771== by 0x809CC23: gc_marks (gc.c:2423)
==27771== by 0x809CE98: garbage_collect (gc.c:2474)
==27771== by 0x8099C08: garbage_collect_with_gvl (gc.c:689)
==27771== by 0x8099CB6: vm_malloc_prepare (gc.c:719)
==27771== by 0x8099CE9: vm_xmalloc (gc.c:751)
==27771== by 0x8099EBB: ruby_xmalloc2 (gc.c:831)
==27771== by 0x81A005C: cont_save_machine_stack (cont.c:350)
==27771== by 0x81A0E70: fiber_store (cont.c:1187)
==27771== by 0x81A10D5: fiber_switch (cont.c:1277)
==27771== by 0x81A113A: rb_fiber_transfer (cont.c:1292)
==27771== by 0x81A11E4: rb_fiber_yield (cont.c:1311)
==27771== by 0x81A1293: rb_fiber_s_yield (cont.c:1389)
==27771== by 0x8185238: call_cfunc (vm_insnhelper.c:317)
==27771== by 0x8185B7C: vm_call_cfunc (vm_insnhelper.c:404)
==27771== by 0x8186091: vm_call_method (vm_insnhelper.c:526)
==27771== by 0x818A6C7: vm_exec_core (insns.def:1012)
==27771== by 0x8195705: vm_exec (vm.c:1163)
==27771== by 0x8194404: invoke_block_from_c (vm.c:574)
==27771== by 0x81945BE: rb_vm_invoke_proc (vm.c:620)
==27771== by 0x81A0C93: rb_fiber_start (cont.c:1121)
==27771== by 0x808AF19: ruby_exec_internal (eval.c:213)
==27771== by 0x808AFF9: ruby_exec_node (eval.c:260)
==27771== by 0x808AFD3: ruby_run_node (eval.c:253)
==27771== by 0x805B1CD: main (main.c:38)
==27771== Address 0xbed8aa48 is not stack'd, malloc'd or (recently) free'd

これは rb_context_t::saved_thread に thread の情報を格納した後
すぐに machine_stack_(start|end) を 0 クリアせず、そのまま vm stack の確保のために GC が走る
可能性があり、その時に saved_thread->machine_stack_end に保存されているアドレスが
現在のスタック末尾よりも伸びている場合に余分な領域まで mark してしまうようです。
一度は実際にそこまで machine stack が伸長していたわけなので、不正メモリアクセスエラーになる
危険はないかもしれませんが、無駄な mark を省くために修正したいと思います。

そこで saved_thread にコピーする時は常にすぐ machine_stack_(start|end) をクリアするようにしました。
また _ia64 では machine_register_stack(start|end) というメンバもありこれも同様にしてみました。
ただ IA64 環境が使えないので動作は未確認です。

saved_thread.machine_stack_(start|end) の用途については cont.c を読んで以下のように
理解しているのでこれで良いと思っていますが、かなり込み入っているので
ささださんや現在の Fiber 実装された芝さんにチェックしていただきたいと思いパッチ投稿します。

  • FIBER_USE_NATIVE=0 の時(setjmp/longjmp 利用時)
    saved_thread.machine_stack_(start|end) は利用しない。常に 0 で良い。

  • FIBER_USE_NATIVE=1 の時
    saved_thread.machine_stack_end は利用しない。常に 0 で良い。
    saved_thread.machine_stack_start は Thread の実行中の Fiber でない間は
    ucontext_t::uc_stack のスタックを指しておく。
    saved_thread に格納した後で 0 クリアしても fiber_setcontext() で改めて
    保存されるので大丈夫。

fiber_machine_stack_clear.patch Magnifier (1.95 KB) Tomoyuki Chikanaga, 06/08/2011 11:58 PM

Associated revisions

Revision 32084
Added by Tomoyuki Chikanaga almost 4 years ago

  • ChangeLog: add ML and ticket link. [Bug #4855]

Revision 32084
Added by Tomoyuki Chikanaga almost 4 years ago

  • ChangeLog: add ML and ticket link. [Bug #4855]

History

#1 Updated by Tomoyuki Chikanaga almost 4 years ago

  • Status changed from Open to Assigned

別途メールで芝さんに確認して頂けました。問題なさそうなのでこれでコミットします。

#2 Updated by Tomoyuki Chikanaga almost 4 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r32084.
Tomoyuki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • ChangeLog: add ML and ticket link. [Bug #4855]

Also available in: Atom PDF