From 4a6c0ab086eec189350f0424ffea5387b9a35638 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 9 Mar 2022 21:02:39 -0600 Subject: [PATCH 1/4] bitmapN not reliable et --- rbf/cursorx.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rbf/cursorx.go b/rbf/cursorx.go index 0a29f07f3..fafdbf512 100644 --- a/rbf/cursorx.go +++ b/rbf/cursorx.go @@ -177,7 +177,7 @@ func intoContainer(l leafCell, tx *Tx, replacing *roaring.Container, target []by case ContainerTypeBitmapPtr: _, bm, _ := tx.leafCellBitmap(toPgno(cpMaybe)) cloneMaybe := bm - c = roaring.RemakeContainerBitmapN(replacing, cloneMaybe, int32(l.BitN)) + c = roaring.RemakeContainerBitmap(replacing, cloneMaybe) case ContainerTypeBitmap: c = roaring.RemakeContainerBitmapN(replacing, toArray64(cpMaybe), int32(l.BitN)) case ContainerTypeRLE: @@ -216,9 +216,9 @@ func toContainer(l leafCell, tx *Tx) (c *roaring.Container) { case ContainerTypeBitmapPtr: _, bm, _ := tx.leafCellBitmap(toPgno(cpMaybe)) cloneMaybe := bm - c = roaring.NewContainerBitmap(l.BitN, cloneMaybe) + c = roaring.NewContainerBitmap(-1, cloneMaybe) case ContainerTypeBitmap: - c = roaring.NewContainerBitmap(l.BitN, toArray64(cpMaybe)) + c = roaring.NewContainerBitmap(-1, toArray64(cpMaybe)) case ContainerTypeRLE: c = roaring.NewContainerRun(toInterval16(cpMaybe)) } From 09a993295157d14efaecb73ca88a9a532c05fce4 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 10 Mar 2022 07:48:51 -0600 Subject: [PATCH 2/4] extra protection of bitmapToArray --- roaring/containers_test.go | 8 ++++++++ roaring/roaring.go | 41 ++++++++++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/roaring/containers_test.go b/roaring/containers_test.go index e5d9fc2c4..5e170d4d1 100644 --- a/roaring/containers_test.go +++ b/roaring/containers_test.go @@ -176,6 +176,14 @@ func TestSliceContainers(t *testing.T) { }) } +func TestContainersFB1247(t *testing.T) { + bm := [bitmapN]uint64{0xF} + co := NewContainerBitmap(1, bm[:]) + co = co.bitmapToArray() + //should not panic + +} + func genRun(r *rand.Rand) Interval16 { gen: dat := r.Uint32() diff --git a/roaring/roaring.go b/roaring/roaring.go index 9f2ef5039..afbaa6b42 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3567,21 +3567,40 @@ func (c *Container) bitmapToArray() *Container { return c } bitmap := c.bitmap() - n := int32(0) - array := make([]uint16, c.N()) - for i, word := range bitmap { - for word != 0 { - t := word & -word - if roaringParanoia { - if n >= c.N() { - panic("bitmap has more bits set than container.n") + // FB-1247 adding an extra check just in case c.N proves to be unreliable + // TODO prove this has to be reliable + makeArray := func(bm []uint64, ar []uint16) ([]uint16, bool, int32) { + n := int32(0) + for i, word := range bm { + for word != 0 { + t := word & -word + if roaringParanoia { + if n >= c.N() { + panic("bitmap has more bits set than container.n") + } } + if n == int32(len(ar)) { + return ar, true, n + } + ar[n] = uint16((i*64 + int(popcount(t-1)))) + n++ + word ^= t } - array[n] = uint16((i*64 + int(popcount(t-1)))) - n++ - word ^= t } + return ar, false, n + } + array, fail, n := makeArray(bitmap, make([]uint16, c.N())) + if fail { + // the onlyreason we are here is because N was incorrect + // so we force a recount of N and try again + c.bitmapRepair() // the onlyreason we are here is because N was incorrect + array, fail, n = makeArray(bitmap, make([]uint16, c.N())) + if fail { + //this should not be able to happen under any circumstance + panic("bitmapToArray failure") + } + } if roaringParanoia { if n != c.N() { From 52e9027c791ddbba393a585e11f9fcc100b0abc9 Mon Sep 17 00:00:00 2001 From: tgruben Date: Thu, 10 Mar 2022 08:26:07 -0600 Subject: [PATCH 3/4] Update roaring.go --- roaring/roaring.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index afbaa6b42..c09ae4673 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3594,7 +3594,7 @@ func (c *Container) bitmapToArray() *Container { if fail { // the onlyreason we are here is because N was incorrect // so we force a recount of N and try again - c.bitmapRepair() // the onlyreason we are here is because N was incorrect + c.bitmapRepair() array, fail, n = makeArray(bitmap, make([]uint16, c.N())) if fail { //this should not be able to happen under any circumstance From 57b8dcd43a4f0d2f1b31402bee578b91291c3f01 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 10 Mar 2022 09:03:07 -0600 Subject: [PATCH 4/4] go fmt --- roaring/roaring.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index c09ae4673..ffc0c3c3c 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3594,7 +3594,7 @@ func (c *Container) bitmapToArray() *Container { if fail { // the onlyreason we are here is because N was incorrect // so we force a recount of N and try again - c.bitmapRepair() + c.bitmapRepair() array, fail, n = makeArray(bitmap, make([]uint16, c.N())) if fail { //this should not be able to happen under any circumstance