Re: [ ruby-Bugs-10527 ] segfault printing instruction sequence for iterator

Now 'a' shows up twice in the local table:

== disasm: <ISeq:<main>@<compiled>>=====================================
== 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 <main>, 0, <ic>
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<RARRAY_LEN(locals); 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<RARRAY_LEN(locals); 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