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>

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)

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},
+}))