bitmap unmarshalling and testing bug fixes

When unmarshalling ops, we weren't adding a meaningful OpN to them,
resulting in misleading reports from `pilosa inspect`. Also, we were
mistakenly reporting things as "mapped" when they were actually
using their internal storage (as with small array containers).

Add the "sanity check" to `pilosa inspect` so that errors like the
above get noticed more easily and corrected. Also, to make that work,
have roaring.InspectBinary actually put containers in the bitmap
it creates rather than just creating info entries for them.

ctl/inspect.go

@@ -29,6 +29,7 @@ import (
 	"syscall"
 	"text/tabwriter"
 	"time"
+	"unsafe"
 
 	"github.com/gogo/protobuf/proto"
 	"github.com/pilosa/pilosa/v2"
@@ -360,16 +361,24 @@ func (cmd *InspectCommand) InspectFile(f *os.File, fi os.FileInfo) error {
 			fmt.Fprintf(cmd.Stderr, "inspect command: munmap failed: %v", err)
 		}
 	}()
+	mappedFrom := uintptr(unsafe.Pointer(&data[0]))
+	mappedTo := mappedFrom + uintptr(len(data))
 	// Attach the mmap file to the bitmap.
 	t := time.Now()
 	fmt.Fprintf(cmd.Stderr, "inspecting bitmap...")
 	var info roaring.BitmapInfo
-	_, _, err = roaring.InspectBinary(data, true, &info)
+	bitmap, _, err := roaring.InspectBinary(data, true, &info)
 	fmt.Fprintf(cmd.Stderr, " (%s)\n", time.Since(t))
 	cmd.DisplayInfo(info)
 	if err != nil {
 		return errors.Wrap(err, "inspecting")
 	}
+	mappedIn, mappedOut, unmappedIn, errs, err := bitmap.SanityCheckMapping(mappedFrom, mappedTo)
+	if err != nil {
+		fmt.Fprintf(cmd.Stderr, "sanity check: %d mapped in, %d mapped out, %d unmapped in, %d errors\n",
+			mappedIn, mappedOut, unmappedIn, errs)
+		fmt.Fprintf(cmd.Stderr, "last error: %v\n", err)
+	}
 	return nil
 }
 

roaring/roaring.go

@@ -5957,7 +5957,7 @@ func (op *op) UnmarshalBinary(data []byte) error {
 
 	switch op.typ {
 	case opTypeAdd, opTypeRemove:
-		// nothing to do, just being not-default
+		op.opN = 1
 	case opTypeAddBatch, opTypeRemoveBatch:
 		// This ensures that in doing 13+op.value*8, the max int won't be exceeded and a wrap around case
 		// (resulting in a negative value) won't occur in the slice indexing while writing
@@ -5968,8 +5968,9 @@ func (op *op) UnmarshalBinary(data []byte) error {
 			return fmt.Errorf("op data truncated - expected %d, got %d", 13+op.value*8, len(data))
 		}
 		_, _ = h.Write(data[13 : 13+op.value*8])
-		op.values = make([]uint64, op.value)
-		for i := uint64(0); i < op.value; i++ {
+		op.opN = int(op.value)
+		op.values = make([]uint64, op.opN)
+		for i := range op.values {
 			start := 13 + i*8
 			op.values[i] = binary.LittleEndian.Uint64(data[start : start+8])
 		}

roaring/unmarshal_binary.go

@@ -58,7 +58,10 @@ func (b *Bitmap) UnmarshalBinary(data []byte) (err error) {
 		default:
 			panic("invalid container type")
 		}
-		newC.setMapped(true)
+		// If we're using the iterator's pointer, we're "mapped". But
+		// for instance, small arrays may use their own data structures,
+		// which is fine.
+		newC.setMapped(newC.pointer == itrPointer)
 		if !b.preferMapping {
 			newC = newC.unmapOrClone()
 		}
@@ -150,10 +153,14 @@ func InspectBinary(data []byte, mapped bool, info *BitmapInfo) (b *Bitmap, mappe
 		default:
 			panic("invalid container type")
 		}
-		newC.setMapped(true)
+		// If our pointer isn't itrPointer, we aren't actually mapped.
+		newC.setMapped(newC.pointer == itrPointer)
 		if !mapped {
 			newC = newC.unmapOrClone()
 		}
+		// Pristine means this is the original object read in from
+		// roaring data, even if it's not mapped, which this is for
+		// now.
 		newC.flags |= flagPristine
 		if newC.flags&flagMapped != 0 {
 			mappedAny = true
@@ -169,6 +176,7 @@ func InspectBinary(data []byte, mapped bool, info *BitmapInfo) (b *Bitmap, mappe
 		})
 		info.ContainerCount++
 		info.BitCount += uint64(newC.n)
+		b.Containers.Put(itrKey, newC)
 		itrKey, itrCType, itrN, itrLen, itrPointer, itrErr = itr.Next()
 	}
 	// note: if we get a non-EOF err, it's possible that we made SOME