Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?

[felix . winkelmann]

>
> 1) Generated CPS functions will allocate what they need on the
>    stack for local variables up front, then do a C_stack_probe()
>    or a C_demand, but they will allocate an arbitrary amount of
>    words for the argvectors *WITHOUT ANY CHECK WHETHER IT WILL FIT*.
>
> 2) C_stack_probe() and C_demand() will check if the stack pointer exceeds
>    C_stack_limit and trigger a GC.  However, C_stack_overflow_check
>    will call C_stack_check1() which adds C_STACK_RESERVE.
>    That means that a C_demand() check might say "yeah, sure, it fits in
>    the stack", whereas the function that it calls will do a
>    C_stack_overflow_check which says "nope, it won't fit in the stack
>    minus the stack reserve".
>
> These two probably need to be fixed before we can do a release.
> The second was always broken but it might not have been such a big
> deal because we never allocated as much on the stack(?).  The first
> is new with argvectors.

Unless one operates with native threads, the stack can be considered
"large enough" (IIRC, it even grows on demand on modern systems,
up to certain limits, but that may be wrong)

>
> Like I said, I'm a bit ill so this may all be just a fever dream, but
> the proper solution eludes me.  A fix for 2) might involve collecting
> all the possible function calls a generated function will perform and
> then pre-allocating the av2 (but only when needed), and then checking
> whether that would cause the stack to overflow.

Do you mean 1) here? Since a CPS-procedure will call only a limited
amount of CPS functions (usually 1), this should be doable.

>
> A fix for 1), if I'm not mistaken, would be to swap the way the stack
> checks are done: the C_demand and C_stack_probe should observe the
> reserve, while C_stack_check1() should only look at the hard limit.

(2?) I'm not sure whether I'm following completely, but the reserve should
probably respected in any case.

>
> Besides, I think C_stack_check1 looks too complicated; why does it
> create the temporary *_sp variable?  Is that to avoid a pointer
> overflow/underflow?  Couldn't it just do a check like this:
>
> if ((C_byte *)C_stack_pointer < ((C_byte *)C_stack_limit - C_STACK_RESERVE)) 
> err;

I think this doesn't make much of a difference in terms of compilation.
The C compiler should take care of this.


felix