maxBE128 implementation failing with similar values (#433)

* Create failing test to exercise maxBE128 bug * fix: JAE => JBE Co-authored-by: Achille Roussel <[email protected]>
jdhudson3 · Dec 6, 2022 · 813b06a · 813b06a
1 parent b37e188
commit 813b06a
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 1 deletion.
diff --git a/page_bounds_test.go b/page_bounds_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"math/rand"
+	"reflect"
 	"testing"
 
 	"github.com/segmentio/parquet-go/internal/quick"
@@ -168,6 +169,55 @@ func TestBoundsFloat64(t *testing.T) {
 	}
 }
 
+func TestBE128MinMaxSimilar(t *testing.T) {
+	var min [16]byte
+
+	// Test values:
+	//   [1 1 ... 1 1]
+	//   [0 1 ... 1 1]
+	//   ...
+	//   [0 0 ... 0 1]
+	//   [0 0 ... 0 0]
+	for i := 0; i < 17; i++ {
+		var max [16]byte
+		for j := i; j < 16; j++ {
+			max[j] = 1
+		}
+		testBE182MinMaxPerm(t, min, max)
+	}
+
+	// Test values:
+	//   [0 0 ... 0 0]
+	//   [1 0 ... 0 0]
+	//   ...
+	//   [1 1 ... 1 0]
+	//   [1 1 ... 1 1]
+	for i := 0; i < 17; i++ {
+		var max [16]byte
+		for j := 0; j < i; j++ {
+			max[j] = 1
+		}
+		testBE182MinMaxPerm(t, min, max)
+	}
+}
+
+func testBE182MinMaxPerm(t *testing.T, min, max [16]byte) {
+	testBE128MinMax(t, min[:], max[:], [][16]byte{min, max})
+	testBE128MinMax(t, min[:], max[:], [][16]byte{max, min})
+}
+
+func testBE128MinMax(t *testing.T, min, max []byte, data [][16]byte) {
+	bmin := minBE128(data)
+	if !reflect.DeepEqual(bmin, min[:]) {
+		t.Errorf("unexpected min value\nexpected %v\n     got %v", min, bmin)
+	}
+
+	bmax := maxBE128(data)
+	if !reflect.DeepEqual(bmax, max[:]) {
+		t.Errorf("unexpected max value\nexpected %v\n     got %v", max, bmax)
+	}
+}
+
 func TestBoundsBE128(t *testing.T) {
 	err := quick.Check(func(values [][16]byte) bool {
 		min := [16]byte{}

diff --git a/page_max_amd64.s b/page_max_amd64.s
@@ -577,7 +577,7 @@ loop:
     BSWAPQ R8
     BSWAPQ R9
     CMPQ R8, R9
-    JAE next
+    JBE next
 more:
     MOVQ AX, BX
 next: