fix(reflect): zero len slice case (#54)

also:

* ci: fix not running ./tests

.github/workflows/pr-check.yml

@@ -4,7 +4,7 @@ on: [ pull_request ]
 
 jobs:
   compliant:
-    runs-on: [ self-hosted, X64 ]
+    runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
 
@@ -17,20 +17,13 @@ jobs:
         uses: crate-ci/typos@master
 
   staticcheck:
-    runs-on: [ self-hosted, X64 ]
+    runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Set up Go
-        uses: actions/setup-go@v3
-        with:
-          go-version: "1.22"
-
-      - uses: actions/cache@v3
+        uses: actions/setup-go@v5
         with:
-          path: ~/go/pkg/mod
-          key: reviewdog-${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
-          restore-keys: |
-            reviewdog-${{ runner.os }}-go-
+          go-version: stable
 
       - uses: reviewdog/action-staticcheck@v1
         with:
@@ -39,23 +32,24 @@ jobs:
           reporter: github-pr-review
           # Report all results.
           filter_mode: nofilter
-          # Exit with 1 when it find at least one finding.
+          # Exit with 1 when it finds at least one finding.
           fail_on_error: true
           # Set staticcheck flags
+          # -ST1006 for fixing jit code receiver names like `self`
           staticcheck_flags: -checks=inherit,-SA1029, -ST1006
 
   lint:
-    runs-on: [ self-hosted, X64 ]
+    runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Set up Go
-        uses: actions/setup-go@v3
+        uses: actions/setup-go@v5
         with:
-          go-version: "1.22"
+          go-version: stable
 
       - name: Golangci Lint
         # https://golangci-lint.run/
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@v6
         with:
           version: latest
           only-new-issues: true

.github/workflows/tests.yml

@@ -3,22 +3,42 @@ name: Tests
 on: [ push, pull_request ]
 
 jobs:
-  unit-benchmark-test:
+  compatibility-test-amd64:
     strategy:
       matrix:
         go: [ "1.17", "1.18", "1.19", "1.20", "1.21", "1.22" ]
         os: [ X64 ]
     runs-on: ${{ matrix.os }}
     steps:
-      - uses: actions/checkout@v3
-
+      - uses: actions/checkout@v4
       - name: Set up Go
-        uses: actions/setup-go@v3
+        uses: actions/setup-go@v5
         with:
           go-version: ${{ matrix.go }}
+          cache: false # don't use cache for self-hosted runners
+      - name: Test ./...
+        run: go test -race ./...
+      - name: Test ./tests
+        run: cd tests && go test -race
+      - name: Test Benchmark
+        run: go test -bench=. -benchmem -run=none ./... -benchtime=100ms
 
-      - name: Unit Test
-        run: go test -race -covermode=atomic -coverprofile=coverage.out ./...
-
-      - name: Benchmark
-        run: go test -bench=. -benchmem -run=none ./...
+  compatibility-test-arm64:
+    strategy:
+      matrix:
+        go: [ "1.17", "1.18", "1.19", "1.20", "1.21", "1.22" ]
+        os: [ ARM64 ]
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ matrix.go }}
+          cache: false # don't use cache for self-hosted runners
+      - name: Test ./... # only tests reflect pkg, JIT pkgs is not supported
+        run: go test -race ./internal/reflect
+      - name: Test ./tests
+        run: cd tests && go test -race
+      - name: Test Benchmark
+        run: go test -bench=. -benchmem -run=none ./internal/reflect -benchtime=100ms

internal/reflect/decoder.go

@@ -27,6 +27,18 @@ import (
 	"github.com/cloudwego/frugal/internal/binary/defs"
 )
 
+var (
+	zerob   = make([]byte, 0)
+	zerostr = ""
+
+	// for slice, Data should points to zerobase var in `runtime`
+	// so that it can represent as []type{} instead of []type(nil)
+	zeroSliceHeader = *(*sliceHeader)(unsafe.Pointer(&zerob))
+
+	// for string, all fields should be zero
+	zeroStrHeader = *(*stringHeader)(unsafe.Pointer(&zerostr))
+)
+
 const maxDepthLimit = 1023
 
 var decoderPool = sync.Pool{
@@ -169,17 +181,22 @@ func (d *tDecoder) decodeType(t *tType, b []byte, p unsafe.Pointer, maxdepth int
 		l := int(binary.BigEndian.Uint32(b))
 		i += 4
 		if l == 0 {
+			if t.Tag == defs.T_binary {
+				*(*sliceHeader)(p) = zeroSliceHeader
+			} else {
+				*(*stringHeader)(p) = zeroStrHeader
+			}
 			return i, nil
 		}
 		x := d.Malloc(l, 1, 0)
 		if t.Tag == defs.T_binary {
 			h := (*sliceHeader)(p)
-			h.Data = x
+			h.Data = uintptr(x)
 			h.Len = l
 			h.Cap = l
 		} else { //  convert to str
 			h := (*stringHeader)(p)
-			h.Data = x
+			h.Data = uintptr(x)
 			h.Len = l
 		}
 		copyn(x, b[i:], l)
@@ -193,7 +210,7 @@ func (d *tDecoder) decodeType(t *tType, b []byte, p unsafe.Pointer, maxdepth int
 		// check types
 		kt := t.K
 		vt := t.V
-		if t0 != kt.T || t1 != vt.T {
+		if t0 != kt.WT || t1 != vt.WT {
 			return 0, errors.New("type mismatch")
 		}
 
@@ -218,11 +235,11 @@ func (d *tDecoder) decodeType(t *tType, b []byte, p unsafe.Pointer, maxdepth int
 		// instead of
 		// kp = new(type), decode(b, kp)
 		var sliceK unsafe.Pointer
-		if kt.IsPointer {
+		if kt.IsPointer && l > 0 {
 			sliceK = d.Malloc(l*kt.V.Size, kt.V.Align, kt.V.MallocAbiType)
 		}
 		var sliceV unsafe.Pointer
-		if vt.IsPointer {
+		if vt.IsPointer && l > 0 {
 			sliceV = d.Malloc(l*vt.V.Size, vt.V.Align, vt.V.MallocAbiType)
 		}
 
@@ -271,20 +288,21 @@ func (d *tDecoder) decodeType(t *tType, b []byte, p unsafe.Pointer, maxdepth int
 
 		// check types
 		et := t.V
-		if et.T != tp {
+		if et.WT != tp {
 			return 0, errors.New("type mismatch")
 		}
 
 		// decode list
 		h := (*sliceHeader)(p) // update the slice field
-		h.Data = unsafe.Pointer(nil)
+		h.Data = 0
 		h.Len = l
 		h.Cap = l
 		if l <= 0 {
+			*(*sliceHeader)(p) = zeroSliceHeader
 			return i, nil
 		}
 		x := d.Malloc(l*et.Size, et.Align, et.MallocAbiType) // malloc for slice. make([]Type, l, l)
-		h.Data = x
+		h.Data = uintptr(x)
 
 		// pre-allocate space for elements if they're pointers
 		// like

internal/reflect/decoder_test.go

@@ -89,9 +89,17 @@ func TestDecode(t *testing.T) {
 			test:   func(t *testing.T, p1 *TestTypes) { assert.Equal(t, vFloat64, p1.Double) },
 		},
 		{
-			name:   "case_string",
-			update: func(p0 *TestTypes) { p0.String_ = "str" },
-			test:   func(t *testing.T, p1 *TestTypes) { assert.Equal(t, "str", p1.String_) },
+			name:   "case_string_binary",
+			update: func(p0 *TestTypes) { p0.String_ = "str"; p0.Binary = []byte{1} },
+			test: func(t *testing.T, p1 *TestTypes) {
+				assert.Equal(t, "str", p1.String_)
+				assert.Equal(t, []byte{1}, p1.Binary)
+			},
+		},
+		{
+			name:   "case_zero_len_binary",
+			update: func(p0 *TestTypes) { p0.Binary = []byte{} },
+			test:   func(t *testing.T, p1 *TestTypes) { assert.Equal(t, []byte{}, p1.Binary) },
 		},
 		{
 			name:   "case_enum",
@@ -136,6 +144,19 @@ func TestDecode(t *testing.T) {
 				assert.Equal(t, []*Msg{{}, {Type: vInt32}}, p1.L2)
 			},
 		},
+		{
+			name: "case_zero_len_list",
+			update: func(p0 *TestTypes) {
+				p0.L0 = []int32{}
+				p0.L1 = []string{}
+				p0.L2 = []*Msg{}
+			},
+			test: func(t *testing.T, p1 *TestTypes) {
+				assert.Equal(t, []int32{}, p1.L0)
+				assert.Equal(t, []string{}, p1.L1)
+				assert.Equal(t, []*Msg{}, p1.L2)
+			},
+		},
 		{
 			name: "case_set",
 			update: func(p0 *TestTypes) {

internal/reflect/encoder.go

@@ -145,7 +145,7 @@ func (e *tEncoder) encodeContainerType(t *tType, b []byte, p unsafe.Pointer) (in
 		}
 		binary.BigEndian.PutUint32(b[1:], uint32(h.Len))
 		i := listHeaderLen
-		vp := h.Data
+		vp := h.UnsafePointer()
 		// list elements
 		for j := 0; j < h.Len; j++ {
 			if j != 0 {

internal/reflect/hack.go

@@ -221,19 +221,31 @@ func rtTypePtr(rt reflect.Type) uintptr {
 	return (*iface)(unsafe.Pointer(&rt)).data
 }
 
-// same as reflect.StringHeader with Data type is unsafe.Pointer
+// same as reflect.StringHeader
 type stringHeader struct {
-	Data unsafe.Pointer
+	Data uintptr
 	Len  int
 }
 
-// same as reflect.SliceHeader with Data type is unsafe.Pointer
+// UnsafePointer ... for passing checkptr
+// `p := unsafe.Pointer(h.Data)` is NOT allowed when testing with -race
+func (h *stringHeader) UnsafePointer() unsafe.Pointer {
+	return *(*unsafe.Pointer)(unsafe.Pointer(h))
+}
+
+// same as reflect.SliceHeader
 type sliceHeader struct {
-	Data unsafe.Pointer
+	Data uintptr
 	Len  int
 	Cap  int
 }
 
+// UnsafePointer ... for passing checkptr
+// `p := unsafe.Pointer(h.Data)` is NOT allowed when testing with -race
+func (h *sliceHeader) UnsafePointer() unsafe.Pointer {
+	return *(*unsafe.Pointer)(unsafe.Pointer(h))
+}
+
 //go:linkname mallocgc runtime.mallocgc
 func mallocgc(size uintptr, typ unsafe.Pointer, needzero bool) unsafe.Pointer
 

internal/reflect/ttype.go

@@ -310,7 +310,10 @@ func (t *tType) encodedListSize(p unsafe.Pointer) (int, error) {
 		return listHeaderLen + (h.Len * vt.FixedSize), nil
 	}
 	ret := listHeaderLen
-	vp := unsafe.Pointer(h.Data)
+	if h.Len == 0 {
+		return ret, nil
+	}
+	vp := h.UnsafePointer()
 	for i := 0; i < h.Len; i++ {
 		if i != 0 {
 			vp = unsafe.Add(vp, vt.Size) //  move to next element

internal/reflect/unknownfields.go

@@ -17,6 +17,7 @@
 package reflect
 
 import (
+	"runtime"
 	"sync"
 	"unsafe"
 )
@@ -47,16 +48,18 @@ func (p *unknownFields) Size() int {
 
 func (p *unknownFields) Copy(b []byte) []byte {
 	sz := p.Size()
+	data := mallocgc(uintptr(sz), nil, false) //  without zeroing
 	ret := []byte{}
 	h := (*sliceHeader)(unsafe.Pointer(&ret))
-	h.Data = mallocgc(uintptr(sz), nil, false) // without zeroing
+	h.Data = uintptr(data)
 	h.Len = sz
 	h.Cap = sz
 	off := 0
 	for _, x := range p.offs {
 		copy(ret[off:], b[x.off:x.off+x.sz])
 		off += x.sz
 	}
+	runtime.KeepAlive(data)
 	return ret
 }
 

internal/reflect/utils.go

@@ -19,6 +19,7 @@ package reflect
 import (
 	"fmt"
 	"reflect"
+	"runtime"
 	"sync"
 	"unsafe"
 )
@@ -28,10 +29,11 @@ import (
 func copyn(dst unsafe.Pointer, src []byte, n int) {
 	var b []byte
 	hdr := (*sliceHeader)(unsafe.Pointer(&b))
-	hdr.Data = dst
+	hdr.Data = uintptr(dst)
 	hdr.Cap = n
 	hdr.Len = n
 	copy(b, src)
+	runtime.KeepAlive(dst)
 }
 
 // only be used when NewRequiredFieldNotSetException

tests/deep_nested_test.go

@@ -17,7 +17,6 @@
 package tests
 
 import (
-	"runtime"
 	"testing"
 
 	"github.com/cloudwego/frugal"
@@ -95,19 +94,7 @@ var (
 )
 
 func TestDeepNested(t *testing.T) {
-	exit := make(chan int)
-	go func() {
-		for {
-			select {
-			case <-exit:
-				t.Logf("exit gc loop")
-			default:
-				runtime.GC()
-			}
-		}
-	}()
 	for i := 0; i < 100; i++ {
 		frugal.EncodedSize(deepNested)
 	}
-	exit <- 0
 }