reflect: fix more issues with StructOf GC programs

First the insidious bug:

  var n uintptr
  for n := elemPtrs; n > 120; n -= 120 {
    prog = append(prog, 120)
    prog = append(prog, mask[:15]...)
    mask = mask[15:]
  }
  prog = append(prog, byte(n))
  prog = append(prog, mask[:(n+7)/8]...)

The := breaks this code, because the n after the loop is always 0!

We also do need to handle field padding correctly. In particular
the old padding code doesn't correctly handle fields that are not
a multiple of a pointer in size.

Fixes #30606.

Change-Id: Ifcab9494dc25c20116753c5d7e0145d6c2053ed8
Reviewed-on: https://go-review.googlesource.com/c/go/+/165860
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/reflect/type.go b/src/reflect/type.go
index 531417ea9314d0421e774cf85eb100a449272a0e..5c7ed243d56144589a138665d5ba7b182966211f 100644 (file)
--- a/src/reflect/type.go
+++ b/src/reflect/type.go
@@ -2674,43 +2674,48 @@ func StructOf(fields []StructField) Type {
                        }
                }
                prog := []byte{0, 0, 0, 0} // will be length of prog
+               var off uintptr
                for i, ft := range fs {
                        if i > lastPtrField {
                                // gcprog should not include anything for any field after
                                // the last field that contains pointer data
                                break
                        }
-                       // FIXME(sbinet) handle padding, fields smaller than a word
+                       if !ft.typ.pointers() {
+                               // Ignore pointerless fields.
+                               continue
+                       }
+                       // Pad to start of this field with zeros.
+                       if ft.offset() > off {
+                               n := (ft.offset() - off) / ptrSize
+                               prog = append(prog, 0x01, 0x00) // emit a 0 bit
+                               if n > 1 {
+                                       prog = append(prog, 0x81)      // repeat previous bit
+                                       prog = appendVarint(prog, n-1) // n-1 times
+                               }
+                               off = ft.offset()
+                       }
+
                        elemGC := (*[1 << 30]byte)(unsafe.Pointer(ft.typ.gcdata))[:]
                        elemPtrs := ft.typ.ptrdata / ptrSize
-                       switch {
-                       case ft.typ.kind&kindGCProg == 0 && ft.typ.ptrdata != 0:
+                       if ft.typ.kind&kindGCProg == 0 {
                                // Element is small with pointer mask; use as literal bits.
                                mask := elemGC
                                // Emit 120-bit chunks of full bytes (max is 127 but we avoid using partial bytes).
                                var n uintptr
-                               for n := elemPtrs; n > 120; n -= 120 {
+                               for n = elemPtrs; n > 120; n -= 120 {
                                        prog = append(prog, 120)
                                        prog = append(prog, mask[:15]...)
                                        mask = mask[15:]
                                }
                                prog = append(prog, byte(n))
                                prog = append(prog, mask[:(n+7)/8]...)
-                       case ft.typ.kind&kindGCProg != 0:
+                       } else {
                                // Element has GC program; emit one element.
                                elemProg := elemGC[4 : 4+*(*uint32)(unsafe.Pointer(&elemGC[0]))-1]
                                prog = append(prog, elemProg...)
                        }
-                       // Pad from ptrdata to size.
-                       elemWords := ft.typ.size / ptrSize
-                       if elemPtrs < elemWords {
-                               // Emit literal 0 bit, then repeat as needed.
-                               prog = append(prog, 0x01, 0x00)
-                               if elemPtrs+1 < elemWords {
-                                       prog = append(prog, 0x81)
-                                       prog = appendVarint(prog, elemWords-elemPtrs-1)
-                               }
-                       }
+                       off += ft.typ.ptrdata
                }
                prog = append(prog, 0)
                *(*uint32)(unsafe.Pointer(&prog[0])) = uint32(len(prog) - 4)

diff --git a/test/fixedbugs/issue30606b.go b/test/fixedbugs/issue30606b.go
new file mode 100644 (file)
index 0000000..2ce2804
--- /dev/null
+++ b/test/fixedbugs/issue30606b.go
@@ -0,0 +1,51 @@
+// run
+
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import "reflect"
+
+func main() {}
+
+func typ(x interface{}) reflect.Type { return reflect.ValueOf(x).Type() }
+
+var byteType = typ((byte)(0))
+var ptrType = typ((*byte)(nil))
+
+// Arrays of pointers. There are two size thresholds.
+// Bit masks are chunked in groups of 120 pointers.
+// Array types with >16384 pointers have a GC program instead of a bitmask.
+var smallPtrType = reflect.ArrayOf(100, ptrType)
+var mediumPtrType = reflect.ArrayOf(1000, ptrType)
+var bigPtrType = reflect.ArrayOf(16385, ptrType)
+
+var x0 = reflect.New(reflect.StructOf([]reflect.StructField{
+       {Name: "F1", Type: byteType},
+       {Name: "F2", Type: bigPtrType},
+}))
+var x1 = reflect.New(reflect.StructOf([]reflect.StructField{
+       {Name: "F1", Type: smallPtrType},
+       {Name: "F2", Type: bigPtrType},
+}))
+var x2 = reflect.New(reflect.StructOf([]reflect.StructField{
+       {Name: "F1", Type: mediumPtrType},
+       {Name: "F2", Type: bigPtrType},
+}))
+var x3 = reflect.New(reflect.StructOf([]reflect.StructField{
+       {Name: "F1", Type: ptrType},
+       {Name: "F2", Type: byteType},
+       {Name: "F3", Type: bigPtrType},
+}))
+var x4 = reflect.New(reflect.StructOf([]reflect.StructField{
+       {Name: "F1", Type: ptrType},
+       {Name: "F2", Type: smallPtrType},
+       {Name: "F3", Type: bigPtrType},
+}))
+var x5 = reflect.New(reflect.StructOf([]reflect.StructField{
+       {Name: "F1", Type: ptrType},
+       {Name: "F2", Type: mediumPtrType},
+       {Name: "F3", Type: bigPtrType},
+}))