From: Yusuke ENDOH Date: 2007-12-29T15:28:34+09:00 Subject: [ruby-dev:32888] Re: emptstack dumps core 遠藤です。 07/12/29 に Yusuke ENDOH さんは書きました: > とりあえず emptstack の使用をあきらめる応急処置のパッチです。 > ただし性能劣化はそれなりにある予感がします (調べてません) 。 (snip) このパッチにはバグがありました。このパッチだと以下で落ちます。 def m p yield end m do x = begin ensure next 1 break 2 end end あんまりちゃんと追ってないですが、NODE_BREAK の throw の後に if(poped) ADD_INSN(pop) が足りないようです。元のパッチに 加えて以下が必要です。 --- compile.c 2007-12-29 15:09:52.000000000 +0900 +++ compile.c 2007-12-29 15:10:04.000000000 +0900 @@ -2951,6 +2951,9 @@ COMPILE(ret, "break val (block)", node->nd_stts); ADD_INSN1(ret, nd_line(node), throw, INT2FIX(level | 0x02) /* TAG_BREAK */ ); + if (poped) { + ADD_INSN(ret, nd_line(node), pop); + } } else if (iseq->type == ISEQ_TYPE_EVAL) { break_in_eval: compile.c の NODE_BREAK 、NODE_NEXT 、NODE_REDO のあたりにバグが 入りやすいのは、このあたりのコードがつぎはぎだらけになっていて、 視認性が悪いのが原因の一つではないかと思いました (責任転嫁) 。 これらのコンパイル方法は (throw のオペランド (= level) 以外)は ほぼ対称的になるのが自然だと思います。なのでコードの見た目もなるべく 対称的にするパッチを書いてみました。 Index: compile.c =================================================================== --- compile.c (revision 14780) +++ compile.c (working copy) @@ -2920,93 +2920,51 @@ break; } case NODE_BREAK:{ - unsigned long level = 0; - - if (iseq->compile_data->redo_label != 0) { - /* while/until */ -#if 0 - add_ensure_iseq(ret, iseq); - COMPILE_(ret, "break val (while/until)", node->nd_stts, - iseq->compile_data->loopval_popped); - ADD_INSNL(ret, nd_line(node), jump, - iseq->compile_data->end_label); - - if (poped) { - ADD_INSN(ret, nd_line(node), pop); - } -#else - level = 0x8000 | 0x4000; - COMPILE(ret, "break val (while/until)", node->nd_stts); - ADD_INSN1(ret, nd_line(node), throw, - INT2FIX(level | 0x02) /* TAG_BREAK */ ); - - if (poped) { - ADD_INSN(ret, nd_line(node), pop); - } -#endif - } - else if (iseq->type == ISEQ_TYPE_BLOCK) { - break_by_insn: - /* escape from block */ - COMPILE(ret, "break val (block)", node->nd_stts); - ADD_INSN1(ret, nd_line(node), throw, - INT2FIX(level | 0x02) /* TAG_BREAK */ ); - } - else if (iseq->type == ISEQ_TYPE_EVAL) { + if (iseq->compile_data->redo_label == 0 && iseq->type == ISEQ_TYPE_EVAL) { break_in_eval: COMPILE_ERROR((ERROR_ARGS "Can't escape from eval with break")); } else { - rb_iseq_t *ip = iseq->parent_iseq; + unsigned long level = 0; + rb_iseq_t *ip = iseq; while (ip) { - level++; if (ip->compile_data->redo_label != 0) { - level = 0x8000; - if (ip->compile_data->loopval_popped == 0) { - /* need value */ - level |= 0x4000; - } - goto break_by_insn; + level = 0x8000 | 0x4000; + break; } else if (ip->type == ISEQ_TYPE_BLOCK) { level <<= 16; - goto break_by_insn; + break; } else if (ip->type == ISEQ_TYPE_EVAL) { goto break_in_eval; } + level++; ip = ip->parent_iseq; } - COMPILE_ERROR((ERROR_ARGS "Invalid break")); + if (ip != 0) { + COMPILE(ret, "break val", node->nd_stts); + ADD_INSN1(ret, nd_line(node), throw, + INT2FIX(level | 0x02) /* TAG_BREAK */ ); + if (poped) { + ADD_INSN(ret, nd_line(node), pop); + } + } + else { + COMPILE_ERROR((ERROR_ARGS "Invalid break")); + } } break; } case NODE_NEXT:{ - unsigned long level = 0; - int pop_after_throw = 0; - if (iseq->compile_data->redo_label != 0) { - /* next in while loop */ - debugs("next in while\n"); - pop_after_throw = poped; - goto next_by_throw; - } - else if (iseq->compile_data->end_label) { - debugs("next in block\n"); - ADD_INSN (ret, nd_line(node), emptstack); - COMPILE(ret, "next val", node->nd_stts); - add_ensure_iseq(ret, iseq); - ADD_INSNL(ret, nd_line(node), jump, - iseq->compile_data->end_label); - } - else if (iseq->type == ISEQ_TYPE_EVAL) { + if (iseq->compile_data->redo_label == 0 && iseq->type == ISEQ_TYPE_EVAL) { next_in_eval: COMPILE_ERROR((ERROR_ARGS "Can't escape from eval with next")); } else { - rb_iseq_t *ip; - next_by_throw: - ip = iseq; + unsigned long level = 0; + rb_iseq_t *ip = iseq; while (ip) { level = 0x8000; if (ip->compile_data->redo_label != 0) { @@ -3026,7 +2984,7 @@ COMPILE(ret, "next val", node->nd_stts); ADD_INSN1(ret, nd_line(node), throw, INT2FIX(level | 0x03) /* TAG_NEXT */ ); - if (pop_after_throw) { + if (poped) { ADD_INSN(ret, nd_line(node), pop); } } @@ -3037,40 +2995,13 @@ break; } case NODE_REDO:{ - int pop_after_throw = 0; - if (iseq->compile_data->redo_label) { - debugs("redo in while"); -#if 1 - pop_after_throw = poped; - goto redo_by_throw; -#else - add_ensure_iseq(ret, iseq); - ADD_INSNL(ret, nd_line(node), jump, - iseq->compile_data->redo_label); - if (!poped) { /* for stack consistency */ - ADD_INSN(ret, nd_line(node), putnil); - } -#endif - } - else if (iseq->type == ISEQ_TYPE_EVAL) { + if (iseq->compile_data->redo_label == 0 && iseq->type == ISEQ_TYPE_EVAL) { redo_in_eval: COMPILE_ERROR((ERROR_ARGS "Can't escape from eval with redo")); } - else if (iseq->compile_data->start_label) { - ADD_INSN (ret, nd_line(node), emptstack); - add_ensure_iseq(ret, iseq); - ADD_INSNL(ret, nd_line(node), jump, - iseq->compile_data->start_label); - if (!poped) { /* for stack consistency */ - ADD_INSN(ret, nd_line(node), putnil); - } - } else { - rb_iseq_t *ip; - unsigned long level; - redo_by_throw: - level = 0x8000 | 0x4000; - ip = iseq; + rb_iseq_t *ip = iseq; + unsigned long level = 0x8000 | 0x4000; while (ip) { if (ip->compile_data->redo_label != 0) { break; @@ -3088,7 +3019,7 @@ ADD_INSN1(ret, nd_line(node), throw, INT2FIX(level | 0x05) /* TAG_REDO */ ); - if (pop_after_throw) { + if (poped) { ADD_INSN(ret, nd_line(node), pop); } } @@ -3808,22 +3739,12 @@ break; } else { - if (is->type == ISEQ_TYPE_METHOD) { - ADD_INSN(ret, nd_line(node), emptstack); - } - COMPILE(ret, "return nd_stts (return val)", node->nd_stts); - if (is->type == ISEQ_TYPE_METHOD) { - add_ensure_iseq(ret, iseq); - ADD_INSN(ret, nd_line(node), leave); + ADD_INSN1(ret, nd_line(node), throw, INT2FIX(0x01) /* TAG_RETURN */ ); + if (poped) { + ADD_INSN(ret, nd_line(node), pop); } - else { - ADD_INSN1(ret, nd_line(node), throw, INT2FIX(0x01) /* TAG_RETURN */ ); - if (poped) { - ADD_INSN(ret, nd_line(node), pop); - } - } break; } } -- Yusuke ENDOH