[Chicken-hackers] [PATCH] (hopefully) fix the massive random Salmonella breakage

[Peter Bex]

Hi all,

As you're probably aware, we've been seeing a lot of seemingly random
breakage on the Salmonella boxes.  Take for example today's report:
http://salmonella-linux-x86-64.call-cc.org/master-debugbuild/gcc/linux/x86-64/2015/09/30/salmonella-report/

Several eggs fail on installation or test with this message:
[panic] out of memory - heap full while resizing - execution terminated

I've been doing some digging and I'm not 100% sure, but it looks like
these problems have been triggered (but not _caused_) by the argvector
re-use patch (d04f591d / a33fc6209).  It looks like in some cases,
argvectors are "allocated" on the temporary stack.  This stack is
supposed to be used for setting aside live objects before performing
a GC, so that they get copied to the new heap, and so the argvector
won't get clobbered by the longjmp.

Currently, the argvector is *retained* on the temporary stack when
calling the trampoline function, but the temporary stack pointer is
reset to the bottom of the stack.  That means, the memory is seen as
"free".  This is wrong: if something _else_ will be allocated on the
temporary stack (like for example the save() calls in
C_allocate_vector()), that means the argvector data gets clobbered.
This may or may not be a problem, depending on whether the argvector
will be re-used.  Note that the argvector re-use patch itself only
_increases_ the number of places where an argvector may be re-used;
in the original argvector patches there are also some situations in
which this may happen (for example in C_apply and C_apply_values).

The attached patch tries to ensure that argvectors are always
stack-allocated.  After a GC, the argvector is moved from the
temporary stack to the C stack.  After that, the temporary stack is
reset as usual.

I also noticed that the C_make_structure() function can be simplified
quite a bit by just using the save_and_reclaim() function and jumping
back to itself if memory is insufficient.  There's no real reason
to have a second make_structure_2() function around.  I think the same
may be possible for C_allocate_vector()/allocate_vector_2(), but the
function is a bit too tricky and large for me to grok and refactor.
Hopefully, removing the _2 function will result in a bit less stack
usage.  But if nothing else, it makes the code more readble IMHO.

I'd appreciate it if someone could take a look at this ASAP, so we can
apply it and check whether it fixes Salmonella.  For the remainder of
this week, I'll have plenty time to dig in further if this doesn't
solve it, but next week I'll get back to work and will have less time.

Cheers,
Peter

0001-Avoid-allocating-argvectors-on-the-temporary-stack.chicken-5.patch
Description: Text Data

0002-Simplify-C_make_structure-by-using-standard-save_and.chicken-5.patch
Description: Text Data

0001-Avoid-allocating-argvectors-on-the-temporary-stack.master.patch
Description: Text Data

0002-Simplify-C_make_structure-by-using-standard-save_and.master.patch
Description: Text Data

signature.asc
Description: Digital signature