The bug: we use null pointers to represent uninitialized boxes. These can occur in the unused side of a sum type, for example the value Nothing : Maybe (List Float) will have a null pointer for the List Float side. We check whether the pointer is null before doing a copy. Not only would it be bad to dereference the null pointer, but allocating the memory for the result would also fail because we'd be determining the allocation size based on the garbage value in the "size" part of the List Float. Previously in the null case we just didn't do any copy, but actually we need to go further than that to maintain the invariant that "garbage size implies null pointer". We need to explicitly put a null pointer in the destination because the current value might be non-null. Note that this is not because it might contain uninitialized memory -- the initialization we do at allocation time handles that -- but because it might have had a legitimate non-null pointer stored there, such as if a Ref (Maybe (List Float)) previously contained the Just case.
In implementing the fix, it ended up being easier to just clean up the way we handle boxed memory generally. A few changes:
Move the logic for null-intializing pointer-containing memory to Imp.hs which is where it belongs, (since that's where the rest of the box memory management happens), rather than in ImpToLLVM.hs.
Codegen the deep copy rather than calling out to a C++ runtime function. The deep copies are rare enough that I'm not worried about code blowup and this lets us keep all the logic in one place.
Add an explicit NullPtr case to our pointer literals. We generally want to avoid compiling code that contains pointer literals but the null pointer is a special case.
Actually do the freeing of inaccessible boxes! This is a TODO we've had since we first had dependent pairs. The memory leak probably isn't too bad in practice because nested boxes are rare, but it's good to fix it even so!
The bug: we use null pointers to represent uninitialized boxes. These can occur in the unused side of a sum type, for example the value
Nothing : Maybe (List Float)will have a null pointer for theList Floatside. We check whether the pointer is null before doing a copy. Not only would it be bad to dereference the null pointer, but allocating the memory for the result would also fail because we'd be determining the allocation size based on the garbage value in the "size" part of theList Float. Previously in the null case we just didn't do any copy, but actually we need to go further than that to maintain the invariant that "garbage size implies null pointer". We need to explicitly put a null pointer in the destination because the current value might be non-null. Note that this is not because it might contain uninitialized memory -- the initialization we do at allocation time handles that -- but because it might have had a legitimate non-null pointer stored there, such as if aRef (Maybe (List Float))previously contained theJustcase.In implementing the fix, it ended up being easier to just clean up the way we handle boxed memory generally. A few changes:
NullPtrcase to our pointer literals. We generally want to avoid compiling code that contains pointer literals but the null pointer is a special case.