Re: [PATCH] dir.c --- Dir.chdir error handling

>>>>> "Y" == Yukihiro Matsumoto <[e-mail]> writes:

Y> Can you show us both?

 Well, this is with the old 1.8.0 and with the example that I've given
 previously (Thread.new &thread_block).

 It's with gdb

svg% gdb ruby
[...]
(gdb) b rb_eval
Breakpoint 1 at 0x80555e0: file eval.c, line 2430.
(gdb) r b.rb

Breakpoint 1, rb_eval (self=1074436704, n=0x0) at eval.c:2430
2430        NODE * volatile node = n;
(gdb) p heaps
$1 = (struct RVALUE **) 0x80e6898
(gdb) p heaps[0]
$2 = (struct RVALUE *) 0x4007a008
(gdb) dis 1
(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x080a0ab6 in st_foreach (table=0x4, func=0x8069474 <mark_entry>, arg=0)
    at st.c:492
492         for(i = 0; i < table->num_bins; i++) {
(gdb) p heaps
$3 = (struct RVALUE **) 0x80e6898
(gdb) p heaps[0]
$4 = (struct RVALUE *) 0x4007a048
(gdb) 

 It was able to modify heaps[0], this is why it crash. we just need to
 find where

Breakpoint 1, rb_eval (self=1074436704, n=0x0) at eval.c:2430
2430        NODE * volatile node = n;
(gdb) p heaps[0]
$5 = (struct RVALUE *) 0x4007a008
(gdb) watch heaps[0]
Hardware watchpoint 2: heaps[0]
(gdb) dis 1
(gdb) c
Continuing.
Hardware watchpoint 2: heaps[0]

Old value = (struct RVALUE *) 0x4007a008
New value = (struct RVALUE *) 0x4007a048
rb_gc_mark (ptr=135162008) at gc.c:630
630         CHECK_STACK(ret);
(gdb) l
625         if (rb_special_const_p(ptr)) return; /* special const not marked */
626         if (obj->as.basic.flags == 0) return;       /* free cell */
627         if (obj->as.basic.flags & FL_MARK) return;  /* already marked */ 
628         obj->as.basic.flags |= FL_MARK;
629
630         CHECK_STACK(ret);
631         if (ret) {
632             if (!mark_stack_overflow) {
633                 if (mark_stack_ptr - mark_stack < MARK_STACK_MAX) {
634                     *mark_stack_ptr = ptr;
(gdb) 

 This is the line 628 which modify heaps[0]

(gdb) bt
#0  rb_gc_mark (ptr=135162008) at gc.c:630
#1  0x080693ee in mark_locations_array (x=0xbfff43d0, n=0) at gc.c:556
#2  0x0806a191 in rb_gc_mark_frame (frame=0x845ed98) at gc.c:1160
#3  0x080611a2 in thread_mark (th=0x816fea8) at eval.c:8087

[...]

(gdb) printf "%d\n", heaps
135162008
(gdb) info reg ebp
ebp            0xbfff43a8       0xbfff43a8
(gdb) 

 It call rb_gc_mark() with heaps and it came from mark_locations_array()
 (compare the value of frame->argv (0xbfff43d0) with %ebp : it's important)

 When it call mark_locations_array(), in reality frame->argv will be at
 %ebp - 24 

 if you make the test is_pointer_to_heap(heaps) you'll see that it give
 false, but it was able to call rb_gc_mark(heaps)

 Now to see the problem, you must look at the assembler of
 mark_locations_array() (there is only the relevant part) to
 understand how it can call rb_gc_mark() with heaps

 ------------------------------------------------------------ 
mark_locations_array:

#  it store frame->argv in %edi

        movl    8(%ebp), %edi

# it store frame->argv[0] in %ecx, and now it will make the test with %ecx

        movl    (%edi), %ecx

# is_pointer_to_heap() is inlined
# it move heaps in %esi, then in (%ebp - 24) but this is the same
# address than frame->argv

        movl    heaps, %esi
        movl    heaps_limits, %eax
        movl    %esi, -24(%ebp)
        movl    %eax, -16(%ebp)

# it make the test and find that the value in %ecx is a valid value
# and it call rb_gc_mark()

.L166:
        subl    $12, %esp

# it retrieve the content of the address stored in %edi, and push it
# on the stack, but this content was modified, and in reality it push
# heaps on the stack

        pushl   (%edi)

# finally it call rb_gc_mark()

        call    rb_gc_mark

 ------------------------------------------------------------ 

 and you have the bug


 Now if you add `tmp' it will do exactly the same *except* at the end where
 it do


L166:
	subl	$12, %esp
	pushl	%ecx
.LCFI105:
	call	rb_gc_mark

 it call rb_gc_mark() with the right value




Guy Decoux