Bug #8484: Restoring conditions through the ruby method call during VM processing

• Status: Closed
• Priority: Normal
• Assignee: ko1 (Koichi Sasada)
• Target version: -
• ruby -v: ruby 2.1.0dev (2013-06-03 trunk 41049) [x86_64-freebsd9.1]
• Tags: core

[naruse (Yui NARUSE)]

r41041 で、ブロック呼び出し前の VM 内の処理中に Ruby のメソッドが何らかの理由で呼ばれると argv が壊れるという問題を直しました。
具体的には、

y = Object.new
def y.s(a)
  yield(a)
end
m = Object.new
def m.method_missing(*a)
  super
end
assert_equal [m, nil], y.s(m){|a,b|[a,b]}

のようなコードの場合、

• y.s に m と block が渡される
• y.s で yield(a) が呼ばれる
• argvが設定される
• vm_invoke_block 内の VALUE * const rsp = GET_SP() - ci->argc; SET_SP(rsp); で argv の先頭に sp が設定される
• vm_yield_setup_args
• vm_yield_setup_block_args
• y.s のブロックパラメータは2つなのに、引数は一つなので、a.to_ary が呼ばれる (rb_check_array_type(arg0))
• method_missing が呼ばれる
• super が呼ばれる (このへんで vm _push_frame が呼ばれる)
• vm_push_frameのinitialize local variablesのところでargvにnilが代入されて破壊される

で、これ自体は argv を対比しておいて戻せばよいです。
また、SET_SP(rsp)のあたりは、ささださん曰く「 argv に書き込みがない、という前提でそこは作ってるんだよね」だそうな。
しかし、「そんな前提わかるかっ」ですし、
methodの方は SAVE_RESTORE_CI(tmp = rb_check_convert_type(ary, T_ARRAY, "Array", "to_a"), ci); ともうちょっとわかりやすく書いてあるので、
「頂いた問題点を元に、一度全部総点検が必要そうです」とのことですので、お願いします。

[hsbt (Hiroshi SHIBATA)]

• Target version changed from 2.1.0 to 2.2.0

[nobu (Nobuyoshi Nakada)]

• Description updated (diff)

[ko1 (Koichi Sasada)]

• Backport deleted (1.9.3: UNKNOWN, 2.0.0: UNKNOWN)

まだ考え直してない。この間も tailcall で同じような問題があった。

[naruse (Yui NARUSE)]

• Target version deleted (2.2.0)

[jeremyevans0 (Jeremy Evans)]

• Status changed from Assigned to Closed

Google Translate isn't helpful in this case, but I don't think this is a bug in any released version of Ruby. Basically, code was removed in 767c502252daf751a2efbd0acc5766cd5492e9fb, but then it was found to be necessary and restored in 284d7463924b313f12da8de5184af3407c3612ac (with more comments added in dd87e463106ade202e3b04e7703a91e438c027d6 and c120db4547592a0b96b14d1fcd372968442032b2). The code to restore argv[0] still exists in args_check_block_arg0.

The translation seems to indicate a review was needed of the then current code to determine whether writing to argv is required, or whether it could be eliminated. However, there are definitely other cases where argv is modified in the current implementation (e.g. args_kw_argv_to_hash, setup_parameters_complex, args_extend), so I don't think we could eliminate it.