copyArray# bug

[Roman Leshchinskiy]

Simon Marlow wrote:
> On 06/10/2012 22:41, Roman Leshchinskiy wrote:
>> I've been chasing a segfault in the dev version of vector and I think I
>> finally traced it to a bug in the implementation of copyArray# and
>> copyMutableArray#. More specifically, I think emitSetCards in
>> StgCmmPrim.hs (and CgPrimOp.hs) will sometimes fail to mark the last
>> card as dirty because in the current implementation, the number of cards
>> to mark is computed solely from the number of copied elements while it
>> really depends on which cards the first and the last elements belong to.
>> That is, the number of elements to copy might be less than the number of
>> elements per card but the copied range might still span two cards.
>>
>> The attached patch fixes this (and the segfault in vector) and also
>> makes copyArray# return immediately if the number of elements to copy is
>> 0. Could someone who is familiar with the code please review it and tell
>> me if it looks sensible. If it does, I'll make the same modification to
>> CgPrimOp.hs (which has exactly the same code) and commit. Unfortunately,
>> I have no idea how to write a testcase for this since the bug is only
>> triggered in very specific circumstances.
>>
>> It seems that all released versions of GHC that implement
>> copyArray#/copyMutableArray# have this problem. At least, vector's
>> testsuite now segfaults with all of them in roughly the same place after
>> recent modifications I've made (which involve calling copyArray# a lot).
>> If I'm right then I would suggest not to use copyArray# and
>> copyMutableArray# for GHC < 7.8.
>
> Nice catch!
>
> Just to make sure I'm understanding: the conditional you added is not
> just an optimisation, it is required because otherwise the memset() call
> will attempt to mark a single card. (this was the bug I "fixed" last
> time I touched this code, but I think I might have inadverdently
> introduced the bug you just fixed)

Yes, that's exactly right. I'll add a comment.

> Please go ahead and commit.  Note that CgPrimOp is scheduled for
> demolition very shortly, but the bug will need to be fixed there in the
> 7.6 branch.

Will do. Thanks for taking a look!

Roman