Replying to both of your comments here.

Almost all uses of this pattern actually replace w rather than append to
it. I haven't tested, but I think it's fine in all of these cases to use
gc or gc3. The exceptions which do modify w are in cp.c and vgauss.c,
although it's possible I missed some.

tpopnotw isn't possible--the stack is stored in an array, so ignoring w
would just leave it dangling beyond the end of the stack. Your first
suggestion still works, except that derec, rather than ra, makes objects
non-recursive.

I've pushed my changes so far to the unbox branch refcount:
https://github.com/iocane/unbox/commits/refcount

Failing tests are

- Because 7!:0 is no longer accurate
    core/g202.ijs
    core/g202b.ijs
    core/g7x.ijs
    core/glocale.ijs

- Problem with in-place sparse amends
    sparse/gsp530i.ijs
    sparse/gsp530l.ijs
    sparse/gsp530n.ijs

- Miscellaneous
    core/g13x.ijs
    core/g5x2.ijs
    core/gq.ijs

Presumably the last two groups are use-after-frees. Standard tools don't
work to identify these because of J's pool allocation, and making J use
mallocs instead of pool allocations causes bugs for reasons that are
probably even harder to track down. I have tried to configure Valgrind
to recognize pool allocations (using the requests described at
http://www.network-theory.co.uk/docs/valgrind/valgrind_57.html), but it
seems to only recognize that certain reads/writes are invalid, not the
free that caused them to be invalid. Useless.

Marshall

On Sat, May 21, 2016 at 04:29:03PM -0400, Henry Rich wrote:
>   store the current stack top in variable old.
>   for(...) {
>      add something to boxed array w.
>     call gc(w,old), putting w in non-recursive form.
>   }
> 
> What I was suggesting is:
> 
>    store the current stack top in variable old.
>    ra(w);
>    for(...) {
>      add something to boxed array w.
>      ra(something);  // putting something in non-recursive form.
>      tpop(old);
>   }
>    tpush(w);
> 
> Now I suggest
> 
>    store the current stack top in variable old.
>    for(...) {
>      add something to boxed array w.
>      ra(something);  // putting something in non-recursive form?
>      tpopnotw(old,w);
>   }
> 
> Where tpopnotw is like tpop but simply ignores stack entries that match w.
> 
> If this idea is sound, I think you might as well do it, because
> 
> * you will have to inspect each place gc() is used anyway
> * assuming that the current code is there because the free was needed,
>    this preserves that behavior.  It will be hard to be confident that
>    this is not needed (you'd have to deploy and wait for user complaints -
>    I wouldn't want to take that chance)
> * No usecount will be modified or block freed that doesn't need to be
> * it is necessary to increment the usecount of (something)
> 
> Henry
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> On 5/20/2016 10:06 AM, Marshall Lochbaum wrote:
> >> >A number of spots in the J source follow this pattern, or similar:
> >> >   store the current stack top in variable old.
> >> >   for(...) {
> >> >     add something to boxed array w.
> >> >     call gc(w,old), putting w in non-recursive form.
> >> >   }
> >> >
> >> >(A particular example is cp.c, lines 91--97, which performs (v^:l x)
> >> >where l is an integer list).
> 
> ----------------------------------------------------------------------
> For information about J forums see http://www.jsoftware.com/forums.htm
----------------------------------------------------------------------
For information about J forums see http://www.jsoftware.com/forums.htm

Reply via email to