Re: [Chicken-hackers] Heap size calculation in rereclaim2

[Peter Bex]

On Wed, Oct 14, 2015 at 09:43:59AM +0200, Peter Bex wrote:
> On Tue, Oct 13, 2015 at 11:06:58PM +0200, address@hidden wrote:
> > > Am I correct in thinking that double_plus is misnamed and should really
> > > be called "relative_size" or something?  Felix: Do you remember the
> > > original meaning of these two parameters, how are they to be used?
> > 
> > I'm not sure. Double_plus is intended to enlarge the heap (by doubling it),
> > and add the required size. But it may very well be the case that by the 
> > subsequent
> > halving (for the two heap spaces), the "doubling" gets lost. Your 
> > description
> > of the bug seems plausible, but shouldn't this have caused problems before?
> 
> I think the reason it hasn't caused problems before is because it's
> kind of rare to allocate literals that are more than the current heap
> size.  And my guess is that most large literals are strings or blobs (or
> srfi-4 vectors which are internally blobs).  These do not trigger this
> problem due to the fact that they're allocated with malloc().

Well, here's a bunch of patches to fix this issue (#1221) as well as a
handful of memory-related issues.  I still haven't managed to pinpoint
the crashes we're seeing on Salmonella, but this is a separate issue
which seems to be fixed by one of the attached patches.

The other patches should be pretty self-explanatory, except perhaps for
patch 0002.  I noticed that these crashes we're seeing are always
happening due to data corruption in a reallocating GC.  The patch adds
a simple check:  When the object that we're currently mark()ing triggers
a reallocating GC because it's too big to fit the heap, we first check
whether its size makes some basic sense: if it's in the stack, it should
*never* be larger than the stack.  If it's in the heap, it should *never*
be larger than the heap.

By adding this check, we get an earlier opportunity to panic, which makes
the panic message make a little bit more sense, and might be helpful in
debugging, too.  It's not 100% reliable of course: a corrupted object does
not necessarily have to have an absurd size, but I still find the change
rather helpful.  It doesn't add any overhead to speak of, because it
only checks a single object when entering the reallocating GC.

Cheers,
Peter

0001-Fix-heap-allocation-size-calculations-in-toplevel.chicken-5.patch
Description: Text Data

0002-Try-to-detect-corrupted-data-before-performing-GC_RE.chicken-5.patch
Description: Text Data

0003-Make-memory-demand-for-rest-args-list-more-precise.chicken-5.patch
Description: Text Data

0004-Make-literal-size-calculation-for-symbols-more-preci.chicken-5.patch
Description: Text Data

0001-Fix-heap-allocation-size-calculations-in-toplevel.master.patch
Description: Text Data

0002-Try-to-detect-corrupted-data-before-performing-GC_RE.master.patch
Description: Text Data

0003-Make-memory-demand-for-rest-args-list-more-precise.master.patch
Description: Text Data

0004-Make-literal-size-calculation-for-symbols-more-preci.master.patch
Description: Text Data

signature.asc
Description: Digital signature