masak / bel

An interpreter for Bel, Paul Graham's Lisp language
GNU General Public License v3.0
26 stars 1 forks source link

(len 'z) goes into an infinite loop #428

Open masak opened 2 years ago

masak commented 2 years ago

Instead it should error with cdr-on-atom, because the implementation of len should mirror the one in bel.bel:

(def len (xs)
  (if (no xs) 0 (inc:len:cdr xs)))

And cdr on the symbol z dies with an error.

The fastfunc, however, ignores this. Might be worth an extra check to see if similar fastfuncs (that also loop down a list in the same way) are similarly susceptible.

Needs a test.

masak commented 2 years ago

The fastfunc, however, ignores this. Might be worth an extra check to see if similar fastfuncs (that also loop down a list in the same way) are similarly susceptible.

all, some, where__some, reduce, append, snoc, mem, where__mem, find, where__find, hug, keep, rem, get, where__get, put, rev, snap, udrop (on both the input lists!), pairwise, foldl, foldr, fuse, split (in the acc parameter, subtle!), i+, i-, i*, i/, i^, r+, r-, r*, r/, sr+, sr-, srinv, sr*, sr/, srrecip ...

At this point I stopped listing the offending fastfuncs and went looking at the $bel->cdr method. It calls prim_cdr, which does do the expected error checking. So... I notice that I am confused.

masak commented 2 years ago

Ok, for some reason the error handler ends up in a loop where it gets called repeatedly.

masak commented 2 years ago

Ok, I bisected the hang, and the culprit is 7932c720a5bdd3d6fe6f3edcca62fac467193faa, "Introduce an 'async call' mechanism" from about this time last year. I kind of suspected it would be that one.

It's a wonder there were no tests that triggered this problem. Clearly we have something of a coverage issue there.

masak commented 8 months ago

I have another short example/symptom of this problem:

> (set x '(a . b))
(a . b)
> (list x)
((a . b))
> (append x nil)
Error: car-on-atom

The error at the end is what it should reply with. (I checked by re-implementing append on the REPL.) Instead, it just goes into an infinite loop.

masak commented 8 months ago

Here's the thing. The err function creates an async call:

return make_async_call(
    $err,
    [$message_symbol],
    sub {
        my ($result) = @_; 
        return $result;
    },  
);  

That err function is indeed called from prim_cdr when we discover that 'z is not a pair:

elsif (!is_pair($object)) {
    return $self->{err}->("cdr-on-atom");
}

Which then is called from fastfunc_len, like this:

$length = 0;
while (!is_nil($xs)) {
    $xs = $bel->cdr($xs);
    ++$length;
}   

Well, that explains it. The async call ends up in $xs.

The solution involves something like CPS-transforming the calls to primitives. The problem is that that is no small task. There are 521 calls to car or cdr in these fastfuncs, and some of them in map calls.