From: Paul Brannan Date: 2007-05-18T05:05:24+09:00 Subject: Re: [ ruby-Bugs-10527 ] segfault printing instruction sequence for iterator Now 'a' shows up twice in the local table: == disasm: @>===================================== == catch table | catch type: retry st: 0002 ed: 0010 sp: 0000 cont: 0002 |------------------------------------------------------------------------ local table (size: 3, argc: 0 [opts: 0, rest: -1, post: 0, block: -1] c) [ 3] ? [ 2] a [ 1] a 0000 trace 1 0002 newarray 0 0004 send :each, 0, block in
, 0, 0010 leave And I'm seeing this warning from valgrind: ==30248== Thread 1: ==30248== Invalid read of size 4 ==30248== at 0x80D9C88: ruby_iseq_disasm (iseq.c:763) ==30248== by 0x80E2A41: call_cfunc (call_cfunc.ci:27) ==30248== by 0x80DF993: th_eval (insns.def:1279) ==30248== by 0x80E1E91: th_eval_body (vm.c:1620) ==30248== by 0x80E2625: rb_thread_eval (vm.c:1826) ==30248== by 0x80E48BD: yarvcore_eval_iseq (yarvcore.c:97) ==30248== by 0x80E498D: yarvcore_eval_parsed (yarvcore.c:129) ==30248== by 0x805968D: ruby_exec_internal (eval.c:210) ==30248== by 0x80596C4: ruby_exec (eval.c:223) ==30248== by 0x80596FE: ruby_run (eval.c:242) ==30248== by 0x8056B35: main (main.c:47) ==30248== Address 0x447FDA4 is 0 bytes after a block of size 4 alloc'd ==30248== at 0x4018651: malloc (vg_replace_malloc.c:149) ==30248== by 0x805F8E8: ruby_xmalloc (gc.c:238) ==30248== by 0x805F984: ruby_xmalloc2 (gc.c:258) ==30248== by 0x810AD0D: set_local_table (compile.c:976) ==30248== by 0x81098A2: rb_iseq_compile (compile.c:149) ==30248== by 0x80D8999: rb_iseq_new_with_bopt_and_opt (iseq.c:277) ==30248== by 0x80D89D1: rb_iseq_new_with_opt (iseq.c:287) ==30248== by 0x80D903A: iseq_s_compile (iseq.c:414) ==30248== by 0x80E2A2B: call_cfunc (call_cfunc.ci:24) ==30248== by 0x80DF993: th_eval (insns.def:1279) ==30248== by 0x80E1E91: th_eval_body (vm.c:1620) ==30248== by 0x80E2625: rb_thread_eval (vm.c:1826) for this program: i = VM::InstructionSequence.new('for a in []; end') puts i.disasm Line 763 of iseq.c is: const char *name = rb_id2name(tbl[i]); I think the loop is going off the end of the array. If I change the previous line back to use local_table_size instead of local_size, the warning goes away, but the indexes into the table are still wrong (or maybe just printed wrong). Paul On Fri, May 18, 2007 at 03:24:55AM +0900, Nobuyoshi Nakada wrote: > Hi, > > At Thu, 17 May 2007 23:39:54 +0900, > Paul Brannan wrote in [ruby-core:11196]: > > disasm seems to not like local_table being set to NULL when there are > > dynamic variables. > > Indeed. > > > Index: compile.c > =================================================================== > --- compile.c (revision 12289) > +++ compile.c (working copy) > @@ -4794,13 +4794,22 @@ iseq_build_from_ary(rb_iseq_t *iseq, VAL > } > > - iseq->local_size = opt + RARRAY_LEN(locals); > - iseq->local_table_size = iseq->local_size; > - iseq->local_table = (ID *)ALLOC_N(ID *, iseq->local_size); > - tbl = iseq->local_table + opt; > - > - for (i=0; i - tbl[i] = SYM2ID(RARRAY_PTR(locals)[i]); > + iseq->local_table_size = RARRAY_LEN(locals); > + iseq->local_size = opt + iseq->local_table_size; > + if (iseq->local_table_size) { > + iseq->local_table = (ID *)ALLOC_N(ID *, iseq->local_size); > + tbl = iseq->local_table + opt; > + > + if (opt) { > + iseq->local_table[0] = (ID)-1; > + } > + for (i=0; i + VALUE lv = RARRAY_PTR(locals)[i]; > + tbl[i] = FIXNUM_P(lv) ? FIX2INT(lv) : SYM2ID(lv); > + } > } > - > + else { > + iseq->local_table = NULL; > + } > + > /* args */ > if (FIXNUM_P(args)) { > Index: iseq.c > =================================================================== > --- iseq.c (revision 12289) > +++ iseq.c (working copy) > @@ -760,5 +760,5 @@ ruby_iseq_disasm(VALUE self) > rb_str_cat2(str, buff); > > - for (i = 0; i < iseqdat->local_table_size; i++) { > + for (i = 0; i < iseqdat->local_size; i++) { > const char *name = rb_id2name(tbl[i]); > char info[0x100]; > @@ -1141,5 +1141,5 @@ iseq_data_to_ary(rb_iseq_t *iseq) > ID lid = iseq->local_table[i]; > if (lid) { > - if (rb_id2str(lid)) rb_ary_push(locals, ID2SYM(lid)); > + rb_ary_push(locals, rb_id2str(lid) ? ID2SYM(lid) : INT2FIX(lid)); > } > else { > > > -- > Nobu Nakada