Guard the in-place operations against self-application

api_test.go

@@ -581,3 +581,68 @@ func TestIteratorReadRanges(t *testing.T) {
 		t.Error("expected zero ranges for an empty buffer")
 	}
 }
+
+// The in-place C routines assert that their operands are distinct; applying a
+// bitmap to itself used to abort the process.
+func TestSelfAliasedOperations(t *testing.T) {
+	fresh := func() *Bitmap { return New(1, 2, 3, 100000) }
+
+	rb := fresh()
+	rb.Xor(rb)
+	if !rb.IsEmpty() {
+		t.Errorf("x XOR x should be empty, got %v", rb.ToArray())
+	}
+
+	rb = fresh()
+	rb.AndNot(rb)
+	if !rb.IsEmpty() {
+		t.Errorf("x ANDNOT x should be empty, got %v", rb.ToArray())
+	}
+
+	rb = fresh()
+	rb.And(rb)
+	if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) {
+		t.Errorf("x AND x should be x, got %v", rb.ToArray())
+	}
+
+	rb = fresh()
+	rb.Or(rb)
+	if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) {
+		t.Errorf("x OR x should be x, got %v", rb.ToArray())
+	}
+
+	rb = fresh()
+	rb.LazyOrInplace(rb, false)
+	rb.RepairAfterLazy()
+	if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) {
+		t.Errorf("lazy x OR x should be x, got %v", rb.ToArray())
+	}
+
+	rb = fresh()
+	rb.LazyXorInplace(rb)
+	rb.RepairAfterLazy()
+	if !rb.IsEmpty() {
+		t.Errorf("lazy x XOR x should be empty, got %v", rb.ToArray())
+	}
+
+	rb = fresh()
+	if !rb.Assign(rb) {
+		t.Error("self-assignment should succeed")
+	}
+	if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) {
+		t.Errorf("self-assignment should be a no-op, got %v", rb.ToArray())
+	}
+}
+
+func TestFromRangeEmpty(t *testing.T) {
+	for _, c := range []struct{ min, max uint64 }{{5, 5}, {10, 3}, {0, 0}} {
+		rb := FromRange(c.min, c.max, 1)
+		if !rb.IsEmpty() {
+			t.Errorf("FromRange(%d, %d, 1) should be empty, got %v", c.min, c.max, rb.ToArray())
+		}
+	}
+	// The 32-bit range saturates at 2^32, so this is not an empty range.
+	if got := FromRange(0xFFFFFFFE, 1<<40, 1).Cardinality(); got != 2 {
+		t.Errorf("expected 2 values, got %d", got)
+	}
+}

gocroaring.go

@@ -314,12 +314,20 @@ func NewWithCapacity(capacity uint32) *Bitmap {
 }
 
 // FromRange creates a bitmap containing min, min+step, min+2*step... up to but
-// not including max. The step must be strictly positive.
+// not including max. An empty range yields an empty bitmap. The step must be
+// strictly positive.
 // This function may panic if the allocation failed.
 func FromRange(min, max uint64, step uint32) *Bitmap {
 	if step == 0 {
 		panic("gocroaring: FromRange requires a strictly positive step")
 	}
+	// C returns a null pointer for an empty range, which is not an error.
+	if max > 0x100000000 {
+		max = 0x100000000
+	}
+	if max <= min {
+		return New()
+	}
 	return wrap(C.roaring_bitmap_from_range(C.uint64_t(min), C.uint64_t(max), C.uint32_t(step)))
 }
 
@@ -333,6 +341,9 @@ func (rb *Bitmap) Clone() *Bitmap {
 
 // Assign copies x2 over rb, returning false if the copy failed.
 func (rb *Bitmap) Assign(x2 *Bitmap) bool {
+	if rb.aliases(x2) {
+		return true // already a copy of itself
+	}
 	answer := bool(C.roaring_bitmap_overwrite(rb.cpointer, x2.cpointer))
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
@@ -732,9 +743,19 @@ func (rb *Bitmap) SetCopyOnWrite(cow bool) {
 // In-place set operations
 ////////////////////////////////////////////////////////////////////////////////
 
+// aliases reports whether the two wrappers refer to the same C bitmap. Several
+// of the in-place C routines assert that their operands are distinct, so we
+// answer the aliased cases ourselves rather than let the C library abort.
+func (rb *Bitmap) aliases(x2 *Bitmap) bool {
+	return rb == x2 || rb.cpointer == x2.cpointer
+}
+
 // And computes the intersection between two bitmaps and stores the result in
 // the current bitmap.
 func (rb *Bitmap) And(x2 *Bitmap) {
+	if rb.aliases(x2) {
+		return // x AND x is x
+	}
 	C.roaring_bitmap_and_inplace(rb.cpointer, x2.cpointer)
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
@@ -743,6 +764,10 @@ func (rb *Bitmap) And(x2 *Bitmap) {
 // Xor computes the symmetric difference between two bitmaps and stores the
 // result in the current bitmap.
 func (rb *Bitmap) Xor(x2 *Bitmap) {
+	if rb.aliases(x2) {
+		rb.Clear() // x XOR x is empty
+		return
+	}
 	C.roaring_bitmap_xor_inplace(rb.cpointer, x2.cpointer)
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
@@ -751,6 +776,9 @@ func (rb *Bitmap) Xor(x2 *Bitmap) {
 // Or computes the union between two bitmaps and stores the result in the
 // current bitmap.
 func (rb *Bitmap) Or(x2 *Bitmap) {
+	if rb.aliases(x2) {
+		return // x OR x is x
+	}
 	C.roaring_bitmap_or_inplace(rb.cpointer, x2.cpointer)
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
@@ -759,6 +787,10 @@ func (rb *Bitmap) Or(x2 *Bitmap) {
 // AndNot computes the difference between two bitmaps and stores the result in
 // the current bitmap.
 func (rb *Bitmap) AndNot(x2 *Bitmap) {
+	if rb.aliases(x2) {
+		rb.Clear() // x ANDNOT x is empty
+		return
+	}
 	C.roaring_bitmap_andnot_inplace(rb.cpointer, x2.cpointer)
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
@@ -768,15 +800,21 @@ func (rb *Bitmap) AndNot(x2 *Bitmap) {
 // invalid state until RepairAfterLazy is called. Set bitsetconversion to true
 // to eagerly convert containers to bitsets when it might help.
 func (rb *Bitmap) LazyOrInplace(x2 *Bitmap, bitsetconversion bool) {
+	if rb.aliases(x2) {
+		return // x OR x is x
+	}
 	C.roaring_bitmap_lazy_or_inplace(rb.cpointer, x2.cpointer, C.bool(bitsetconversion))
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
 }
 
 // LazyXorInplace computes the symmetric difference with x2 in place, leaving
-// the bitmap in an invalid state until RepairAfterLazy is called. The two
-// bitmaps must be distinct.
+// the bitmap in an invalid state until RepairAfterLazy is called.
 func (rb *Bitmap) LazyXorInplace(x2 *Bitmap) {
+	if rb.aliases(x2) {
+		rb.Clear() // x XOR x is empty
+		return
+	}
 	C.roaring_bitmap_lazy_xor_inplace(rb.cpointer, x2.cpointer)
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)

gocroaring64.go

@@ -240,18 +240,28 @@ func New64(x ...uint64) *Bitmap64 {
 }
 
 // FromRange64 creates a bitmap containing min, min+step, min+2*step... up to
-// but not including max. The step must be strictly positive.
+// but not including max. An empty range yields an empty bitmap. The step must
+// be strictly positive.
 // This function may panic if the allocation failed.
 func FromRange64(min, max, step uint64) *Bitmap64 {
 	if step == 0 {
 		panic("gocroaring: FromRange64 requires a strictly positive step")
 	}
+	// C returns a null pointer for an empty range, which is not an error.
+	if max <= min {
+		return New64()
+	}
 	return wrap64(C.roaring64_bitmap_from_range(C.uint64_t(min), C.uint64_t(max), C.uint64_t(step)))
 }
 
 // MoveFrom32 builds a 64-bit bitmap by moving the containers out of a 32-bit
 // bitmap. This avoids copying the container data, but it leaves the source
 // bitmap empty.
+//
+// The containers are taken without regard for copy-on-write sharing, so do not
+// use it on a bitmap whose containers are shared with another one (that is, a
+// bitmap that has copy-on-write enabled and has been cloned). Clone first if
+// you need the source intact.
 // This function may panic if the allocation failed.
 func MoveFrom32(from *Bitmap) *Bitmap64 {
 	b := wrap64(C.roaring64_bitmap_move_from_roaring32(from.cpointer))
@@ -269,6 +279,9 @@ func (rb *Bitmap64) Clone() *Bitmap64 {
 
 // Assign copies x2 over rb.
 func (rb *Bitmap64) Assign(x2 *Bitmap64) {
+	if rb.aliases(x2) {
+		return // already a copy of itself
+	}
 	C.roaring64_bitmap_overwrite(rb.cpointer, x2.cpointer)
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
@@ -628,9 +641,19 @@ func (rb *Bitmap64) InternalValidate() error {
 // In-place set operations
 ////////////////////////////////////////////////////////////////////////////////
 
+// aliases reports whether the two wrappers refer to the same C bitmap. Several
+// of the in-place C routines assert that their operands are distinct, so we
+// answer the aliased cases ourselves rather than let the C library abort.
+func (rb *Bitmap64) aliases(x2 *Bitmap64) bool {
+	return rb == x2 || rb.cpointer == x2.cpointer
+}
+
 // And computes the intersection between two bitmaps and stores the result in
 // the current bitmap.
 func (rb *Bitmap64) And(x2 *Bitmap64) {
+	if rb.aliases(x2) {
+		return // x AND x is x
+	}
 	C.roaring64_bitmap_and_inplace(rb.cpointer, x2.cpointer)
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
@@ -639,6 +662,10 @@ func (rb *Bitmap64) And(x2 *Bitmap64) {
 // Xor computes the symmetric difference between two bitmaps and stores the
 // result in the current bitmap.
 func (rb *Bitmap64) Xor(x2 *Bitmap64) {
+	if rb.aliases(x2) {
+		rb.Clear() // x XOR x is empty
+		return
+	}
 	C.roaring64_bitmap_xor_inplace(rb.cpointer, x2.cpointer)
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
@@ -647,6 +674,9 @@ func (rb *Bitmap64) Xor(x2 *Bitmap64) {
 // Or computes the union between two bitmaps and stores the result in the
 // current bitmap.
 func (rb *Bitmap64) Or(x2 *Bitmap64) {
+	if rb.aliases(x2) {
+		return // x OR x is x
+	}
 	C.roaring64_bitmap_or_inplace(rb.cpointer, x2.cpointer)
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
@@ -655,6 +685,10 @@ func (rb *Bitmap64) Or(x2 *Bitmap64) {
 // AndNot computes the difference between two bitmaps and stores the result in
 // the current bitmap.
 func (rb *Bitmap64) AndNot(x2 *Bitmap64) {
+	if rb.aliases(x2) {
+		rb.Clear() // x ANDNOT x is empty
+		return
+	}
 	C.roaring64_bitmap_andnot_inplace(rb.cpointer, x2.cpointer)
 	runtime.KeepAlive(rb)
 	runtime.KeepAlive(x2)
@@ -905,8 +939,9 @@ func AlignedBuffer64(size int) []byte {
 // ReadFrozenView64 reads a frozen serialized version of the bitmap, as written
 // by Bitmap64.WriteFrozen. The result is immutable: attempting to mutate it
 // will fail catastrophically. The buffer must be aligned on a
-// Frozen64Alignment boundary (see AlignedBuffer64). A reference to the buffer
-// is retained for the lifetime of the view.
+// Frozen64Alignment boundary (see AlignedBuffer64) and its length must be
+// exactly the length that was written. A reference to the buffer is retained
+// for the lifetime of the view.
 func ReadFrozenView64(b []byte) (*Bitmap64, error) {
 	if len(b) == 0 {
 		return nil, ErrEmptyBuffer

gocroaring64_test.go

@@ -590,3 +590,46 @@ func TestIterator64ReadRanges(t *testing.T) {
 		t.Error("expected zero ranges for an empty buffer")
 	}
 }
+
+func TestSelfAliasedOperations64(t *testing.T) {
+	fresh := func() *Bitmap64 { return New64(1, 2, big, big+1) }
+
+	rb := fresh()
+	rb.Xor(rb)
+	if !rb.IsEmpty() {
+		t.Errorf("x XOR x should be empty, got %v", rb.ToArray())
+	}
+
+	rb = fresh()
+	rb.AndNot(rb)
+	if !rb.IsEmpty() {
+		t.Errorf("x ANDNOT x should be empty, got %v", rb.ToArray())
+	}
+
+	rb = fresh()
+	rb.And(rb)
+	if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) {
+		t.Errorf("x AND x should be x, got %v", rb.ToArray())
+	}
+
+	rb = fresh()
+	rb.Or(rb)
+	if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) {
+		t.Errorf("x OR x should be x, got %v", rb.ToArray())
+	}
+
+	rb = fresh()
+	rb.Assign(rb)
+	if !reflect.DeepEqual(rb.ToArray(), fresh().ToArray()) {
+		t.Errorf("self-assignment should be a no-op, got %v", rb.ToArray())
+	}
+}
+
+func TestFromRange64Empty(t *testing.T) {
+	for _, c := range []struct{ min, max uint64 }{{5, 5}, {10, 3}, {big, big}} {
+		rb := FromRange64(c.min, c.max, 1)
+		if !rb.IsEmpty() {
+			t.Errorf("FromRange64(%d, %d, 1) should be empty, got %v", c.min, c.max, rb.ToArray())
+		}
+	}
+}