proto: avoid pointer arithmetic with invalidField (#529)

In the case of a tag of zero, we perform some pointer arithmetic
where we add -1 to the pointer. Even though we do not end up dereferencing
the pointer, this still causes issues with the Go GC, which expects that
every Go pointer is a valid pointer.

This causes crashes on Go1.7 and below because the function arguments
are considered live throughout the entire function causing the GC to
scan this invalid pointer. On Go1.8 and above, this is not an issue
since the argument is not considered live and is not scanned.

To be on the safer side, perform pointer arithmetic that is effectively
a noop, so that we don't cause a valid pointer to become invalid.

proto/pointer_reflect.go

@@ -57,6 +57,9 @@ func toField(f *reflect.StructField) field {
 // invalidField is an invalid field identifier.
 var invalidField = field(nil)
 
+// zeroField is a noop when calling pointer.offset.
+var zeroField = field([]int{})
+
 // IsValid reports whether the field identifier is valid.
 func (f field) IsValid() bool { return f != nil }
 

proto/pointer_unsafe.go

@@ -55,6 +55,9 @@ func toField(f *reflect.StructField) field {
 // invalidField is an invalid field identifier.
 const invalidField = ^field(0)
 
+// zeroField is a noop when calling pointer.offset.
+const zeroField = field(0)
+
 // IsValid reports whether the field identifier is valid.
 func (f field) IsValid() bool {
 	return f != invalidField
@@ -102,6 +105,13 @@ func valToPointer(v reflect.Value) pointer {
 // offset converts from a pointer to a structure to a pointer to
 // one of its fields.
 func (p pointer) offset(f field) pointer {
+	// For safety, we should panic if !f.IsValid, however calling panic causes
+	// this to no longer be inlineable, which is a serious performance cost.
+	/*
+		if !f.IsValid() {
+			panic("invalid field")
+		}
+	*/
 	return pointer{p: unsafe.Pointer(uintptr(p.p) + uintptr(f))}
 }
 

proto/table_unmarshal.go

@@ -460,7 +460,7 @@ func (u *unmarshalInfo) computeUnmarshalInfo() {
 	// when decoding a buffer of all zeros. Without this code, we
 	// would decode and skip an all-zero buffer of even length.
 	// [0 0] is [tag=0/wiretype=varint varint-encoded-0].
-	u.setTag(0, invalidField, func(b []byte, f pointer, w int) ([]byte, error) {
+	u.setTag(0, zeroField, func(b []byte, f pointer, w int) ([]byte, error) {
 		return nil, fmt.Errorf("proto: %s: illegal tag 0 (wire type %d)", t, w)
 	}, 0)