[fidl][golang] Start Union Tags at 1 instead of 0
This doesn't affect the wire format, only the representation of FIDL
unions and only in Go. It will help detect two kinds of errors:
1. Uninitialized unions were previously of the first member, now they
are errors when marshalling:
union Union1 {
int64 a;
int64 b;
};
u1 := Union1{} // This was previously of type "a" with value 0, now it is an error whenmarshalled.
2. Not using setters properly will be caught as errors when
marshalling:
u1 := Union1{}
u1.a = 12345 // This is wrong, it should have been u1.SetA(12345)
Both new and old encodings are modified and interoperable.
TESTED=fx run-test go_fidl_test
Change-Id: Ida12a5b8eb7cbc2a00f0661028381adfe3a0a2c6
diff --git a/src/syscall/zx/fidl/bindingstest/impl.go b/src/syscall/zx/fidl/bindingstest/impl.go
index 6175c70..71fcbe5 100644
--- a/src/syscall/zx/fidl/bindingstest/impl.go
+++ b/src/syscall/zx/fidl/bindingstest/impl.go
@@ -586,10 +586,11 @@
type Union1Tag uint32
const (
- Union1A Union1Tag = iota
- Union1B Union1Tag = iota
- Union1C Union1Tag = iota
- Union1D Union1Tag = iota
+ _ Union1Tag = iota
+ Union1A
+ Union1B
+ Union1C
+ Union1D
)
// Union1 is a FIDL union.
diff --git a/src/syscall/zx/fidl/encoding.go b/src/syscall/zx/fidl/encoding.go
index bba6e0d..4645128 100644
--- a/src/syscall/zx/fidl/encoding.go
+++ b/src/syscall/zx/fidl/encoding.go
@@ -364,13 +364,12 @@
// Expects the Type t and Value v to refer to a golang struct value, not a
// pointer. The alignment field is used to align the union's field.
func (e *encoder) marshalUnion(t reflect.Type, v reflect.Value, alignment int) error {
- kind := v.Field(0).Uint()
-
- // Index into the fields of the struct, adding 1 for the tag.
- fieldIndex := int(kind) + 1
- if fieldIndex >= t.NumField() {
- return newValueError(ErrInvalidUnionTag, kind)
+ fieldIndex := int(v.Field(0).Uint()) // Union tag values start at 1.
+ if fieldIndex < 1 || fieldIndex >= t.NumField() {
+ return newValueError(ErrInvalidUnionTag, fieldIndex)
}
+
+ kind := uint64(fieldIndex - 1) // Wire format tags start at 0.
// Save the head for proper padding.
head := e.head
e.writeUint(kind, 4)
@@ -748,7 +747,7 @@
if fieldIndex >= t.NumField() {
return ErrInvalidUnionTag
}
- v.Field(0).SetUint(kind)
+ v.Field(0).SetUint(kind + 1)
f := t.Field(fieldIndex)
var n nestedTypeData
diff --git a/src/syscall/zx/fidl/encoding_new.go b/src/syscall/zx/fidl/encoding_new.go
index 0d47a71..4bbe04e 100644
--- a/src/syscall/zx/fidl/encoding_new.go
+++ b/src/syscall/zx/fidl/encoding_new.go
@@ -417,7 +417,7 @@
func (m mUnion) marshal(v reflect.Value, out *encoder) error {
// Kind.
- kind := int(v.Field(0).Uint())
+ kind := int(v.Field(0).Uint()) - 1
if kind < 0 || len(m.fields) <= kind {
return newValueError(ErrInvalidUnionTag, kind)
}
@@ -449,7 +449,7 @@
if kind < 0 || len(m.fields) <= kind {
return newValueError(ErrInvalidUnionTag, kind)
}
- v.Field(0).SetUint(uint64(kind))
+ v.Field(0).SetUint(uint64(kind) + 1)
// Re-align to the union's alignement before writing its field.
in.head = align(in.head, m.alignment)
diff --git a/src/syscall/zx/fidl/errors.go b/src/syscall/zx/fidl/errors.go
index 1529e7f..85d58f1 100644
--- a/src/syscall/zx/fidl/errors.go
+++ b/src/syscall/zx/fidl/errors.go
@@ -107,6 +107,8 @@
return "message too small to have FIDL header"
case ErrStructIsNotPayload:
return "golang struct type must implement Payload"
+ case ErrInvalidUnionTag:
+ return "union tag out of bounds"
default:
return "unknown error code"
}
diff --git a/src/syscall/zx/fidl/fidl_test/encoding_new_test.go b/src/syscall/zx/fidl/fidl_test/encoding_new_test.go
index 9ce083a..9e49dfb 100644
--- a/src/syscall/zx/fidl/fidl_test/encoding_new_test.go
+++ b/src/syscall/zx/fidl/fidl_test/encoding_new_test.go
@@ -301,6 +301,7 @@
}
func (c checker) check(t *testing.T, input Message, expectSize int, output Message) {
+ t.Helper()
var respb [zx.ChannelMaxMessageBytes]byte
var resph [zx.ChannelMaxMessageHandles]zx.Handle
nb, nh, err := c.marshalFunc.fn(input, respb[:], resph[:])
@@ -362,6 +363,10 @@
C: []int32{99},
D: &v1, // limit is 2, provided is 3
}, ErrVectorTooLong},
+ {"union1-A-uninitialized", &TestUnion1{
+ A: Union1{}, // Intentionally don't set any members of the union.
+ B: nil,
+ }, ErrInvalidUnionTag},
}
for _, ex := range cases {
t.Run(ex.name, func(t *testing.T) {
diff --git a/src/syscall/zx/fidl/fidl_test/encoding_test.go b/src/syscall/zx/fidl/fidl_test/encoding_test.go
index 96d1f59..097ce20 100644
--- a/src/syscall/zx/fidl/fidl_test/encoding_test.go
+++ b/src/syscall/zx/fidl/fidl_test/encoding_test.go
@@ -16,6 +16,7 @@
)
func testIdentity(t *testing.T, input Payload, expectSize int, output Payload) {
+ t.Helper()
var respb [zx.ChannelMaxMessageBytes]byte
var resph [zx.ChannelMaxMessageHandles]zx.Handle
nb, nh, err := Marshal(input, respb[:], resph[:])
@@ -152,6 +153,20 @@
B: []*Union1{&u1, nil, nil},
}, 120, &TestUnion2{})
})
+ t.Run("Uninitialized unions", func(t *testing.T) {
+ u1 := Union1{}
+ // Intentionally don't set any members of the union.
+ tu1 := TestUnion1{
+ A: u1,
+ B: nil,
+ }
+ var respb [zx.ChannelMaxMessageBytes]byte
+ var resph [zx.ChannelMaxMessageHandles]zx.Handle
+ _, _, err := Marshal(&tu1, respb[:], resph[:])
+ if err == nil {
+ t.Errorf("got %v when marshalling invalid union but expected ErrInvalidUnionTag", err)
+ }
+ })
t.Run("Handles", func(t *testing.T) {
vmo, err := zx.NewVMO(10, 0)
if err != nil {
diff --git a/src/syscall/zx/io/impl.go b/src/syscall/zx/io/impl.go
index fed9159..2c649b8 100644
--- a/src/syscall/zx/io/impl.go
+++ b/src/syscall/zx/io/impl.go
@@ -297,12 +297,13 @@
type NodeInfoTag uint32
const (
- NodeInfoService NodeInfoTag = iota
- NodeInfoFile NodeInfoTag = iota
- NodeInfoDirectory NodeInfoTag = iota
- NodeInfoPipe NodeInfoTag = iota
- NodeInfoVmofile NodeInfoTag = iota
- NodeInfoDevice NodeInfoTag = iota
+ _ NodeInfoTag = iota
+ NodeInfoService
+ NodeInfoFile
+ NodeInfoDirectory
+ NodeInfoPipe
+ NodeInfoVmofile
+ NodeInfoDevice
)
// NodeInfo is a FIDL union.
diff --git a/src/syscall/zx/net/impl.go b/src/syscall/zx/net/impl.go
index cd0bcfc..b9fd6d5 100644
--- a/src/syscall/zx/net/impl.go
+++ b/src/syscall/zx/net/impl.go
@@ -294,8 +294,9 @@
type IpAddressTag uint32
const (
- IpAddressIpv4 IpAddressTag = iota
- IpAddressIpv6 IpAddressTag = iota
+ _ IpAddressTag = iota
+ IpAddressIpv4
+ IpAddressIpv6
)
// IpAddress is a FIDL union.
