From: ts <decoux@...>
Date: 2004-09-15T18:59:04+09:00
Subject: Re: [PATCH] dir.c --- Dir.chdir error handling

>>>>> "Y" == Yukihiro Matsumoto <matz@ruby-lang.org> writes:

Y> Interesting.  I'm happy to see the bug fixed.  But I'm not sure what
Y> was the problem, and how this "fix" changed the situation.  Can you
Y> (or anybody) explain for me?

 Well, this bug is really, really special and you must look at the
 assembler to understand it.

 What you must see is that is_pointer_to_heap() is inlined

    static inline int
    is_pointer_to_heap(ptr)
        void *ptr;
    {
        register RVALUE *p = RANY(ptr);
        register RVALUE *heap_org;
        register long i;
    
        if (p < lomem || p > himem) return Qfalse;
    
        /* check if p looks like a pointer */
        for (i=0; i < heaps_used; i++) {
            heap_org = heaps[i];
            if (heap_org <= p && p < heap_org + heaps_limits[i] &&
                ((((char*)p)-((char*)heap_org))%sizeof(RVALUE)) == 0)
                return Qtrue;
        }
        return Qfalse;
    }


 when mark_locations_array() is called with frame->argv, the address of
 frame->argv is in the stack of the mark_locations_array()

 If you look at the assembler this give :

   * frame->argv is stored in a register %edi

   * it store frame->argv[0] in %ecx and it will make the test with this
     register
 
   * it move heaps in %esi, then in (%ebp - 24) (to make the test) but
     the problem is that (%ebp - 24) is precisely the address of
     frame->argv (it's in the stack of mark_locations_array()). This means
     that it store heaps in frame->argv[0]

   * it make the test with %ecx and find that it has a VALUE

   * it will call rb_gc_mark() but rather than use %ecx it will retrieve
     the content of %edi : this mean that it retrieve heaps

 Now in rb_gc_mark() you have

    if (rb_special_const_p(ptr)) return; /* special const not marked */
    if (obj->as.basic.flags == 0) return;       /* free cell */
    if (obj->as.basic.flags & FL_MARK) return;  /* already marked */ 
    obj->as.basic.flags |= FL_MARK;

 when it do the last instruction, it modify heaps[0] and this is why ruby
 crash in the GC phase.

 The modification with `tmp' is to make in sort that it call rb_gc_mark()
 with %ecx to avoid that it retrieve the content of %edi which was
 modified.

 The patch was just to adjust frame->argv with ADJ()



Guy Decoux