Skip to content

Commit 936b01c

Browse files
author
Achille
authored
fix issue 84 (#85)
* fix issue 84 * unify bugs tests * more fixes * cleanup the test suite * run tests in parallel
1 parent 1c5cefe commit 936b01c

File tree

16 files changed

+181
-113
lines changed

16 files changed

+181
-113
lines changed

‎.circleci/config.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ jobs:
66
steps:
77
- checkout
88
- run: go mod download
9-
- run: make test
9+
- run: make test -j8
1010

1111
benchmark:
1212
docker:

‎Makefile

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,25 @@ go-fuzz-build := ${GOPATH}/bin/go-fuzz-build
1010
go-fuzz-corpus := ${GOPATH}/src/github.com/dvyukov/go-fuzz-corpus
1111
go-fuzz-dep := ${GOPATH}/src/github.com/dvyukov/go-fuzz/go-fuzz-dep
1212

13-
test:
14-
go test -v -cover ./ascii
15-
go test -v -cover ./json
16-
go test -v -cover ./proto
17-
go test -v -cover -tags go1.15 ./json
18-
go test -v -cover ./iso8601
19-
go run ./json/bugs/issue11/main.go
20-
go run ./json/bugs/issue18/main.go
13+
test: test-ascii test-json test-json-bugs test-json-1.17 test-proto test-iso8601
14+
15+
test-ascii:
16+
go test -cover -race ./ascii
17+
18+
test-json:
19+
go test -cover -race ./json
20+
21+
test-json-bugs:
22+
go test -cover -race ./json/bugs/...
23+
24+
test-json-1.17:
25+
go test -cover -race -tags go1.17 ./json
26+
27+
test-proto:
28+
go test -cover -race ./proto
29+
30+
test-iso8601:
31+
go test -cover -race ./iso8601
2132

2233
$(benchstat):
2334
GO111MODULE=off go get -u golang.org/x/perf/cmd/benchstat

‎ascii/ascii.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,18 @@ const (
3535
hasMoreConstR32 = hasMoreConstL32 * 128
3636
)
3737

38-
//go:nosplit
3938
func hasLess64(x, n uint64) bool {
4039
return ((x - (hasLessConstL64 * n)) & ^x & hasLessConstR64) != 0
4140
}
4241

43-
//go:nosplit
4442
func hasLess32(x, n uint32) bool {
4543
return ((x - (hasLessConstL32 * n)) & ^x & hasLessConstR32) != 0
4644
}
4745

48-
//go:nosplit
4946
func hasMore64(x, n uint64) bool {
5047
return (((x + (hasMoreConstL64 * (127 - n))) | x) & hasMoreConstR64) != 0
5148
}
5249

53-
//go:nosplit
5450
func hasMore32(x, n uint32) bool {
5551
return (((x + (hasMoreConstL32 * (127 - n))) | x) & hasMoreConstR32) != 0
5652
}

‎ascii/equal_fold.go

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,32 +35,41 @@ func EqualFoldString(a, b string) bool {
3535
n := uintptr(len(a))
3636
p := *(*unsafe.Pointer)(unsafe.Pointer(&a))
3737
q := *(*unsafe.Pointer)(unsafe.Pointer(&b))
38-
// If there is more than 32 bytes to copy, use the AVX optimized version,
39-
// otherwise the overhead of the function call tends to be greater than
40-
// looping 2 or 3 times over 8 bytes.
41-
if n >= 32 && asm.equalFoldAVX2 != nil {
42-
if asm.equalFoldAVX2((*byte)(p), (*byte)(q), n) == 0 {
43-
return false
38+
// Pre-check to avoid the other tests that would all evaluate to false.
39+
// For very small strings, this helps reduce the processing overhead.
40+
if n >= 8 {
41+
// If there is more than 32 bytes to copy, use the AVX optimized version,
42+
// otherwise the overhead of the function call tends to be greater than
43+
// looping 2 or 3 times over 8 bytes.
44+
if n > 32 && asm.equalFoldAVX2 != nil {
45+
if asm.equalFoldAVX2((*byte)(p), (*byte)(q), n) == 0 {
46+
return false
47+
}
48+
k := (n / 16) * 16
49+
p = unsafe.Pointer(uintptr(p) + k)
50+
q = unsafe.Pointer(uintptr(q) + k)
51+
n -= k
4452
}
45-
k := (n / 16) * 16
46-
p = unsafe.Pointer(uintptr(p) + k)
47-
q = unsafe.Pointer(uintptr(q) + k)
48-
n -= k
49-
}
5053

51-
for n >= 8 {
52-
const mask = 0xDFDFDFDFDFDFDFDF
54+
for n > 8 {
55+
const mask = 0xDFDFDFDFDFDFDFDF
5356

54-
if (*(*uint64)(p) & mask) != (*(*uint64)(q) & mask) {
55-
return false
57+
if (*(*uint64)(p) & mask) != (*(*uint64)(q) & mask) {
58+
return false
59+
}
60+
61+
p = unsafe.Pointer(uintptr(p) + 8)
62+
q = unsafe.Pointer(uintptr(q) + 8)
63+
n -= 8
5664
}
5765

58-
p = unsafe.Pointer(uintptr(p) + 8)
59-
q = unsafe.Pointer(uintptr(q) + 8)
60-
n -= 8
66+
if n == 8 {
67+
const mask = 0xDFDFDFDFDFDFDFDF
68+
return (*(*uint64)(p) & mask) == (*(*uint64)(q) & mask)
69+
}
6170
}
6271

63-
if n >= 4 {
72+
if n > 4 {
6473
const mask = 0xDFDFDFDF
6574

6675
if (*(*uint32)(p) & mask) != (*(*uint32)(q) & mask) {
@@ -73,6 +82,8 @@ func EqualFoldString(a, b string) bool {
7382
}
7483

7584
switch n {
85+
case 4:
86+
return (*(*uint32)(p) & 0xDFDFDFDF) == (*(*uint32)(q) & 0xDFDFDFDF)
7687
case 3:
7788
x := uint32(*(*uint16)(p)) | uint32(*(*uint8)(unsafe.Pointer(uintptr(p) + 2)))<<16
7889
y := uint32(*(*uint16)(q)) | uint32(*(*uint8)(unsafe.Pointer(uintptr(q) + 2)))<<16

‎ascii/valid.go

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,30 @@ func ValidString(s string) bool {
2323
p := *(*unsafe.Pointer)(unsafe.Pointer(&s))
2424
n := uintptr(len(s))
2525

26-
if n >= 32 && asm.validAVX2 != nil {
27-
if asm.validAVX2((*byte)(p), n) == 0 {
28-
return false
26+
if n >= 8 {
27+
if n > 32 && asm.validAVX2 != nil {
28+
if asm.validAVX2((*byte)(p), n) == 0 {
29+
return false
30+
}
31+
k := (n / 16) * 16
32+
p = unsafe.Pointer(uintptr(p) + k)
33+
n -= k
2934
}
30-
k := (n / 16) * 16
31-
p = unsafe.Pointer(uintptr(p) + k)
32-
n -= k
33-
}
3435

35-
for n >= 8 {
36-
if (*(*uint64)(p) & 0x8080808080808080) != 0 {
37-
return false
36+
for n > 8 {
37+
if (*(*uint64)(p) & 0x8080808080808080) != 0 {
38+
return false
39+
}
40+
p = unsafe.Pointer(uintptr(p) + 8)
41+
n -= 8
42+
}
43+
44+
if n == 8 {
45+
return (*(*uint64)(p) & 0x8080808080808080) == 0
3846
}
39-
p = unsafe.Pointer(uintptr(p) + 8)
40-
n -= 8
4147
}
4248

43-
if n >= 4 {
49+
if n > 4 {
4450
if (*(*uint32)(p) & 0x80808080) != 0 {
4551
return false
4652
}
@@ -50,6 +56,8 @@ func ValidString(s string) bool {
5056

5157
var x uint32
5258
switch n {
59+
case 4:
60+
x = *(*uint32)(p)
5361
case 3:
5462
x = uint32(*(*uint16)(p)) | uint32(*(*uint8)(unsafe.Pointer(uintptr(p) + 2)))<<16
5563
case 2:

‎ascii/valid_print.go

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,33 @@ func ValidPrintString(s string) bool {
2323
p := *(*unsafe.Pointer)(unsafe.Pointer(&s))
2424
n := uintptr(len(s))
2525

26-
if n >= 32 && asm.validPrintAVX2 != nil {
27-
if asm.validPrintAVX2((*byte)(p), n) == 0 {
28-
return false
26+
if n >= 8 {
27+
if n > 32 && asm.validPrintAVX2 != nil {
28+
if asm.validPrintAVX2((*byte)(p), n) == 0 {
29+
return false
30+
}
31+
if (n % 16) == 0 {
32+
return true
33+
}
34+
k := (n / 16) * 16
35+
p = unsafe.Pointer(uintptr(p) + k)
36+
n -= k
2937
}
30-
k := (n / 16) * 16
31-
p = unsafe.Pointer(uintptr(p) + k)
32-
n -= k
33-
}
3438

35-
for n >= 8 {
36-
if hasLess64(*(*uint64)(p), 0x20) || hasMore64(*(*uint64)(p), 0x7e) {
37-
return false
39+
for n > 8 {
40+
if hasLess64(*(*uint64)(p), 0x20) || hasMore64(*(*uint64)(p), 0x7e) {
41+
return false
42+
}
43+
p = unsafe.Pointer(uintptr(p) + 8)
44+
n -= 8
45+
}
46+
47+
if n == 8 {
48+
return !(hasLess64(*(*uint64)(p), 0x20) || hasMore64(*(*uint64)(p), 0x7e))
3849
}
39-
p = unsafe.Pointer(uintptr(p) + 8)
40-
n -= 8
4150
}
4251

43-
if n >= 4 {
52+
if n > 4 {
4453
if hasLess32(*(*uint32)(p), 0x20) || hasMore32(*(*uint32)(p), 0x7e) {
4554
return false
4655
}
@@ -50,6 +59,8 @@ func ValidPrintString(s string) bool {
5059

5160
var x uint32
5261
switch n {
62+
case 4:
63+
x = 0x20202020 | *(*uint32)(p)
5364
case 3:
5465
x = 0x20000000 | uint32(*(*uint16)(p)) | uint32(*(*uint8)(unsafe.Pointer(uintptr(p) + 2)))<<16
5566
case 2:

‎internal/runtime_reflect/map.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ func Assign(typ, dst, src unsafe.Pointer) {
1616
typedmemmove(typ, dst, src)
1717
}
1818

19-
func MapAssign(t, m, k unsafe.Pointer) uintptr {
20-
return uintptr(mapassign(t, m, k))
19+
func MapAssign(t, m, k unsafe.Pointer) unsafe.Pointer {
20+
return mapassign(t, m, k)
2121
}
2222

23-
func MakeMap(t unsafe.Pointer, cap int) uintptr {
24-
return uintptr(makemap(t, cap))
23+
func MakeMap(t unsafe.Pointer, cap int) unsafe.Pointer {
24+
return makemap(t, cap)
2525
}
2626

2727
type MapIter struct{ hiter }
@@ -45,9 +45,9 @@ func (it *MapIter) HasNext() bool {
4545
return it.key != nil
4646
}
4747

48-
func (it *MapIter) Key() uintptr { return uintptr(it.key) }
48+
func (it *MapIter) Key() unsafe.Pointer { return it.key }
4949

50-
func (it *MapIter) Value() uintptr { return uintptr(it.value) }
50+
func (it *MapIter) Value() unsafe.Pointer { return it.value }
5151

5252
// copied from src/runtime/map.go, all pointer types replaced with
5353
// unsafe.Pointer.

‎internal/runtime_reflect/slice.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ func (s *Slice) SetLen(n int) {
2020
s.len = n
2121
}
2222

23-
func (s *Slice) Index(i int, elemSize uintptr) uintptr {
24-
return uintptr(s.data) + (uintptr(i) * elemSize)
23+
func (s *Slice) Index(i int, elemSize uintptr) unsafe.Pointer {
24+
return unsafe.Pointer(uintptr(s.data) + (uintptr(i) * elemSize))
2525
}
2626

2727
func MakeSlice(elemType unsafe.Pointer, len, cap int) Slice {

‎json/bugs/issue11/main.go

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,3 @@
11
package main
22

3-
import (
4-
"fmt"
5-
"log"
6-
7-
"github.com/segmentio/encoding/json"
8-
)
9-
10-
func main() {
11-
m := map[string]map[string]interface{}{
12-
"outerkey": {
13-
"innerkey": "innervalue",
14-
},
15-
}
16-
17-
b, err := json.Marshal(m)
18-
if err != nil {
19-
log.Fatal(err)
20-
}
21-
fmt.Println(string(b))
22-
}
3+
func main() {}

‎json/bugs/issue11/main_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"log"
6+
"testing"
7+
8+
"github.com/segmentio/encoding/json"
9+
)
10+
11+
func TestIssue11(t *testing.T) {
12+
m := map[string]map[string]interface{}{
13+
"outerkey": {
14+
"innerkey": "innervalue",
15+
},
16+
}
17+
18+
b, err := json.Marshal(m)
19+
if err != nil {
20+
log.Fatal(err)
21+
}
22+
fmt.Println(string(b))
23+
}

‎json/bugs/issue18/main.go

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,4 @@
11
package main
22

3-
import (
4-
"bytes"
5-
"fmt"
6-
7-
"github.com/segmentio/encoding/json"
8-
)
9-
103
func main() {
11-
b := []byte(`{
12-
"userId": "blah",
13-
}`)
14-
15-
d := json.NewDecoder(bytes.NewReader(b))
16-
17-
var a struct {
18-
UserId string `json:"userId"`
19-
}
20-
fmt.Println(d.Decode(&a))
21-
fmt.Println(a)
224
}

‎json/bugs/issue18/main_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package main
2+
3+
import (
4+
"bytes"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/segmentio/encoding/json"
9+
)
10+
11+
func TestIssue18(t *testing.T) {
12+
b := []byte(`{
13+
"userId": "blah",
14+
}`)
15+
16+
d := json.NewDecoder(bytes.NewReader(b))
17+
18+
var a struct {
19+
UserId string `json:"userId"`
20+
}
21+
fmt.Println(d.Decode(&a))
22+
fmt.Println(a)
23+
}

‎json/bugs/issue84/main.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package main
2+
3+
func main() {}

0 commit comments

Comments
 (0)