Re: MPS: Bytecode stack

["Gerd Möllmann" <gerd.moellmann@gmail.com>]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> 6 maj 2024 kl. 11.58 skrev Gerd Möllmann <gerd.moellmann@gmail.com>:
>
>> I think I fixed, or better said worked around, something important today
>> in scan_bc which can lead to objects being "lost". Might be interesting
>> to see if some errors magically disappear.
>
> I'm not sure what you are trying to fix. Dodging a data race?
>
> To see if I understood what your problem is, and how MPS works, consider the thread state
>
> Lisp_Object *a;
> int top;
>
> where `a` is a heap-allocated array of some size, but only indices [0..top] are actually used so the GC doesn't need to look at the rest. The mutator thread does:
>
> 1  x = a[top];
> 2  top = top - 1;
>
> What does the GC thread see? Even if it reads a (signal-atomic)
> snapshot of the mutator's register and stack, does this atomicity
> extend to `top` which isn't even a Lisp_Object? Is the mutator
> suspended during this scan?
>
> Furthermore, the compiler is allowed to hoist the store in line 2 past
> the load in line 1. If that's the only problem, it may just be a
> matter of adding a compiler barrier (acquire).

It would be easiest if you'd look at scan_bc in igc.c on the branch
That explains it better als prosa:

static mps_res_t
scan_bc (mps_ss_t ss, void *start, void *end, void *closure)
{
  MPS_SCAN_BEGIN (ss)
  {x
    struct igc_thread_list *t = closure;
    struct bc_thread_state *bc = &t->d.ts->bc;
    igc_assert (start == (void *) bc->stack);
    igc_assert (end == (void *) bc->stack_end);
    /* FIXME: AFAIU the current top frame starts at bc->fp->next_stack
       and has a maximum length that is given by the bytecode being
       executed (COMPILED_STACK_DEPTH). So, we need to scan upto
       bc->fo->next_stack + that max depth to be safe.  Since I don't
       have that number ATM, I'm using an arbitrary estimate for
       now.

       This must be changed to something better. Note that Mattias said
       the bc stack marking will be changed in the future.  */
    const size_t HORRIBLE_ESTIMATE = 1024;
    char *scan_end = bc_next_frame (bc->fp);
    scan_end += HORRIBLE_ESTIMATE;
    end = min (end, (void *) scan_end);
    if (end > start)
      IGC_FIX_CALL (ss, scan_ambig (ss, start, end, NULL));
  }
  MPS_SCAN_END (ss);
  return MPS_RES_OK;
}