From cfa4633c3d206aa2f9abdaac60d053162244ee6d Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Mon, 8 Feb 2021 18:03:29 -0800 Subject: [go-marshal] Add dynamic tag in go_marshal. This makes it easier to implement dynamically sized types in go-marshal. You really only need to implement MarshalBytes, UnmarshalBytes and SizeBytes to implement the entire interface. By using the `dynamic` tag, the autogenerator will generate the rest of the methods for us. This change also simplifies how KernelIPTGetEntries implements Marshallable using the newly added utility. PiperOrigin-RevId: 356397114 --- pkg/abi/linux/BUILD | 1 - pkg/abi/linux/netfilter.go | 73 +---- pkg/abi/linux/netfilter_ipv6.go | 67 +--- tools/go_marshal/README.md | 12 + tools/go_marshal/gomarshal/generator.go | 31 +- .../gomarshal/generator_interfaces_struct.go | 339 +++++++++++---------- tools/go_marshal/gomarshal/generator_tests.go | 12 +- tools/go_marshal/test/BUILD | 6 +- tools/go_marshal/test/marshal_test.go | 19 ++ tools/go_marshal/test/test.go | 35 +++ 10 files changed, 293 insertions(+), 302 deletions(-) diff --git a/pkg/abi/linux/BUILD b/pkg/abi/linux/BUILD index 8fa61d6f7..ecaeb11ac 100644 --- a/pkg/abi/linux/BUILD +++ b/pkg/abi/linux/BUILD @@ -80,7 +80,6 @@ go_library( "//pkg/bits", "//pkg/marshal", "//pkg/marshal/primitive", - "//pkg/usermem", ], ) diff --git a/pkg/abi/linux/netfilter.go b/pkg/abi/linux/netfilter.go index b521144d9..378f1baf3 100644 --- a/pkg/abi/linux/netfilter.go +++ b/pkg/abi/linux/netfilter.go @@ -15,11 +15,8 @@ package linux import ( - "io" - "gvisor.dev/gvisor/pkg/marshal" "gvisor.dev/gvisor/pkg/marshal/primitive" - "gvisor.dev/gvisor/pkg/usermem" ) // This file contains structures required to support netfilter, specifically @@ -129,8 +126,8 @@ type IPTEntry struct { const SizeOfIPTEntry = 112 // KernelIPTEntry is identical to IPTEntry, but includes the Elems field. -// KernelIPTEntry itself is not Marshallable but it implements some methods of -// marshal.Marshallable that help in other implementations of Marshallable. +// +// +marshal dynamic type KernelIPTEntry struct { Entry IPTEntry @@ -158,6 +155,8 @@ func (ke *KernelIPTEntry) UnmarshalBytes(src []byte) { ke.Elems.UnmarshalBytes(src[ke.Entry.SizeBytes():]) } +var _ marshal.Marshallable = (*KernelIPTEntry)(nil) + // IPTIP contains information for matching a packet's IP header. // It corresponds to struct ipt_ip in // include/uapi/linux/netfilter_ipv4/ip_tables.h. @@ -411,8 +410,9 @@ type IPTGetEntries struct { const SizeOfIPTGetEntries = 40 // KernelIPTGetEntries is identical to IPTGetEntries, but includes the -// Entrytable field. This has been manually made marshal.Marshallable since it -// is dynamically sized. +// Entrytable field. +// +// +marshal dynamic type KernelIPTGetEntries struct { IPTGetEntries Entrytable []KernelIPTEntry @@ -447,65 +447,6 @@ func (ke *KernelIPTGetEntries) UnmarshalBytes(src []byte) { } } -// Packed implements marshal.Marshallable.Packed. -func (ke *KernelIPTGetEntries) Packed() bool { - // KernelIPTGetEntries isn't packed because the ke.Entrytable contains an - // indirection to the actual data we want to marshal (the slice data - // pointer), and the memory for KernelIPTGetEntries contains the slice - // header which we don't want to marshal. - return false -} - -// MarshalUnsafe implements marshal.Marshallable.MarshalUnsafe. -func (ke *KernelIPTGetEntries) MarshalUnsafe(dst []byte) { - // Fall back to safe Marshal because the type in not packed. - ke.MarshalBytes(dst) -} - -// UnmarshalUnsafe implements marshal.Marshallable.UnmarshalUnsafe. -func (ke *KernelIPTGetEntries) UnmarshalUnsafe(src []byte) { - // Fall back to safe Unmarshal because the type in not packed. - ke.UnmarshalBytes(src) -} - -// CopyIn implements marshal.Marshallable.CopyIn. -func (ke *KernelIPTGetEntries) CopyIn(cc marshal.CopyContext, addr usermem.Addr) (int, error) { - buf := cc.CopyScratchBuffer(ke.SizeBytes()) // escapes: okay. - length, err := cc.CopyInBytes(addr, buf) // escapes: okay. - // Unmarshal unconditionally. If we had a short copy-in, this results in a - // partially unmarshalled struct. - ke.UnmarshalBytes(buf) // escapes: fallback. - return length, err -} - -// CopyOut implements marshal.Marshallable.CopyOut. -func (ke *KernelIPTGetEntries) CopyOut(cc marshal.CopyContext, addr usermem.Addr) (int, error) { - // Type KernelIPTGetEntries doesn't have a packed layout in memory, fall - // back to MarshalBytes. - return cc.CopyOutBytes(addr, ke.marshalAll(cc)) -} - -// CopyOutN implements marshal.Marshallable.CopyOutN. -func (ke *KernelIPTGetEntries) CopyOutN(cc marshal.CopyContext, addr usermem.Addr, limit int) (int, error) { - // Type KernelIPTGetEntries doesn't have a packed layout in memory, fall - // back to MarshalBytes. - return cc.CopyOutBytes(addr, ke.marshalAll(cc)[:limit]) -} - -func (ke *KernelIPTGetEntries) marshalAll(cc marshal.CopyContext) []byte { - buf := cc.CopyScratchBuffer(ke.SizeBytes()) - ke.MarshalBytes(buf) - return buf -} - -// WriteTo implements io.WriterTo.WriteTo. -func (ke *KernelIPTGetEntries) WriteTo(w io.Writer) (int64, error) { - buf := make([]byte, ke.SizeBytes()) - ke.MarshalBytes(buf) - length, err := w.Write(buf) - return int64(length), err -} - var _ marshal.Marshallable = (*KernelIPTGetEntries)(nil) // IPTReplace is the argument for the IPT_SO_SET_REPLACE sockopt. It diff --git a/pkg/abi/linux/netfilter_ipv6.go b/pkg/abi/linux/netfilter_ipv6.go index bcb57642e..b953e62dc 100644 --- a/pkg/abi/linux/netfilter_ipv6.go +++ b/pkg/abi/linux/netfilter_ipv6.go @@ -15,11 +15,8 @@ package linux import ( - "io" - "gvisor.dev/gvisor/pkg/marshal" "gvisor.dev/gvisor/pkg/marshal/primitive" - "gvisor.dev/gvisor/pkg/usermem" ) // This file contains structures required to support IPv6 netfilter and @@ -70,8 +67,9 @@ type IP6TReplace struct { const SizeOfIP6TReplace = 96 // KernelIP6TGetEntries is identical to IP6TGetEntries, but includes the -// Entrytable field. This has been manually made marshal.Marshallable since it -// is dynamically sized. +// Entrytable field. +// +// +marshal dynamic type KernelIP6TGetEntries struct { IPTGetEntries Entrytable []KernelIP6TEntry @@ -106,65 +104,6 @@ func (ke *KernelIP6TGetEntries) UnmarshalBytes(src []byte) { } } -// Packed implements marshal.Marshallable.Packed. -func (ke *KernelIP6TGetEntries) Packed() bool { - // KernelIP6TGetEntries isn't packed because the ke.Entrytable contains - // an indirection to the actual data we want to marshal (the slice data - // pointer), and the memory for KernelIP6TGetEntries contains the slice - // header which we don't want to marshal. - return false -} - -// MarshalUnsafe implements marshal.Marshallable.MarshalUnsafe. -func (ke *KernelIP6TGetEntries) MarshalUnsafe(dst []byte) { - // Fall back to safe Marshal because the type in not packed. - ke.MarshalBytes(dst) -} - -// UnmarshalUnsafe implements marshal.Marshallable.UnmarshalUnsafe. -func (ke *KernelIP6TGetEntries) UnmarshalUnsafe(src []byte) { - // Fall back to safe Unmarshal because the type in not packed. - ke.UnmarshalBytes(src) -} - -// CopyIn implements marshal.Marshallable.CopyIn. -func (ke *KernelIP6TGetEntries) CopyIn(cc marshal.CopyContext, addr usermem.Addr) (int, error) { - buf := cc.CopyScratchBuffer(ke.SizeBytes()) // escapes: okay. - length, err := cc.CopyInBytes(addr, buf) // escapes: okay. - // Unmarshal unconditionally. If we had a short copy-in, this results - // in a partially unmarshalled struct. - ke.UnmarshalBytes(buf) // escapes: fallback. - return length, err -} - -// CopyOut implements marshal.Marshallable.CopyOut. -func (ke *KernelIP6TGetEntries) CopyOut(cc marshal.CopyContext, addr usermem.Addr) (int, error) { - // Type KernelIP6TGetEntries doesn't have a packed layout in memory, - // fall back to MarshalBytes. - return cc.CopyOutBytes(addr, ke.marshalAll(cc)) -} - -// CopyOutN implements marshal.Marshallable.CopyOutN. -func (ke *KernelIP6TGetEntries) CopyOutN(cc marshal.CopyContext, addr usermem.Addr, limit int) (int, error) { - // Type KernelIP6TGetEntries doesn't have a packed layout in memory, fall - // back to MarshalBytes. - return cc.CopyOutBytes(addr, ke.marshalAll(cc)[:limit]) -} - -func (ke *KernelIP6TGetEntries) marshalAll(cc marshal.CopyContext) []byte { - buf := cc.CopyScratchBuffer(ke.SizeBytes()) - ke.MarshalBytes(buf) - return buf -} - -// WriteTo implements io.WriterTo.WriteTo. -func (ke *KernelIP6TGetEntries) WriteTo(w io.Writer) (int64, error) { - buf := make([]byte, ke.SizeBytes()) - ke.MarshalBytes(buf) - length, err := w.Write(buf) - return int64(length), err -} - var _ marshal.Marshallable = (*KernelIP6TGetEntries)(nil) // IP6TEntry is an iptables rule. It corresponds to struct ip6t_entry in diff --git a/tools/go_marshal/README.md b/tools/go_marshal/README.md index d8045c295..eddba0c21 100644 --- a/tools/go_marshal/README.md +++ b/tools/go_marshal/README.md @@ -98,6 +98,18 @@ for embedded structs that are not aligned. Because of this, it's generally best to avoid using `marshal:"unaligned"` and insert explicit padding fields instead. +## Working with dynamically sized structs + +While `go_marshal` seamlessly supports statically sized structs (which most ABI +structs are), it can also used for other uses cases where marshalling is +required. There is some provision to partially support dynamically sized structs +that may not be ABI structs. A user can define a dynamic struct and define +`SizeBytes()`, `MarshalBytes(dst)` and `UnmarshalBytes(src)` for it. Then user +can then add a comment above the struct like `// +marshal dynamic` while will +make `go_marshal` autogenerate the remaining methods required to complete the +`Marshallable` interface. This feature is currently only available for structs +and can not be used alongside the Slice API. + ## Modifying the `go_marshal` Tool The following are some guidelines for modifying the `go_marshal` tool: diff --git a/tools/go_marshal/gomarshal/generator.go b/tools/go_marshal/gomarshal/generator.go index fa642c88a..abd6f69ea 100644 --- a/tools/go_marshal/gomarshal/generator.go +++ b/tools/go_marshal/gomarshal/generator.go @@ -211,9 +211,10 @@ type sliceAPI struct { // marshallableType carries information about a type marked with the '+marshal' // directive. type marshallableType struct { - spec *ast.TypeSpec - slice *sliceAPI - recv string + spec *ast.TypeSpec + slice *sliceAPI + recv string + dynamic bool } func newMarshallableType(fset *token.FileSet, tagLine *ast.Comment, spec *ast.TypeSpec) *marshallableType { @@ -247,6 +248,9 @@ func newMarshallableType(fset *token.FileSet, tagLine *ast.Comment, spec *ast.Ty sa.inner = true } + continue + } else if tag == "dynamic" { + mt.dynamic = true continue } @@ -379,23 +383,38 @@ func (g *Generator) generateOne(t *marshallableType, fset *token.FileSet) *inter i := newInterfaceGenerator(t.spec, t.recv, fset) switch ty := t.spec.Type.(type) { case *ast.StructType: + if t.dynamic { + // Don't validate because this type is dynamically sized and probably + // contains some funky slices which the validation does not allow. + i.emitMarshallableForStruct(ty, t.dynamic) + if t.slice != nil { + abortAt(fset.Position(t.slice.comment.Slash), "Slice API is not supported for dynamic types because it assumes that each slice element is statically sized.") + } + break + } i.validateStruct(t.spec, ty) - i.emitMarshallableForStruct(ty) + i.emitMarshallableForStruct(ty, t.dynamic) if t.slice != nil { i.emitMarshallableSliceForStruct(ty, t.slice) } case *ast.Ident: i.validatePrimitiveNewtype(ty) + if t.dynamic { + abortAt(fset.Position(t.slice.comment.Slash), "Primitive type marked as '+marshal dynamic', but primitive types can not be dynamic.") + } i.emitMarshallableForPrimitiveNewtype(ty) if t.slice != nil { i.emitMarshallableSliceForPrimitiveNewtype(ty, t.slice) } case *ast.ArrayType: i.validateArrayNewtype(t.spec.Name, ty) + if t.dynamic { + abortAt(fset.Position(t.slice.comment.Slash), "Marking array types as `dynamic` is currently not supported.") + } // After validate, we can safely call arrayLen. i.emitMarshallableForArrayNewtype(t.spec.Name, ty, ty.Elt.(*ast.Ident)) if t.slice != nil { - abortAt(fset.Position(t.slice.comment.Slash), fmt.Sprintf("Array type marked as '+marshal slice:...', but this is not supported. Perhaps fold one of the dimensions?")) + abortAt(fset.Position(t.slice.comment.Slash), "Array type marked as '+marshal slice:...', but this is not supported. Perhaps fold one of the dimensions?") } default: // This should've been filtered out by collectMarshallabeTypes. @@ -408,7 +427,7 @@ func (g *Generator) generateOne(t *marshallableType, fset *token.FileSet) *inter // implementations type t. func (g *Generator) generateOneTestSuite(t *marshallableType) *testGenerator { i := newTestGenerator(t.spec, t.recv) - i.emitTests(t.slice) + i.emitTests(t.slice, t.dynamic) return i } diff --git a/tools/go_marshal/gomarshal/generator_interfaces_struct.go b/tools/go_marshal/gomarshal/generator_interfaces_struct.go index 5f6306b8f..f98e41ed7 100644 --- a/tools/go_marshal/gomarshal/generator_interfaces_struct.go +++ b/tools/go_marshal/gomarshal/generator_interfaces_struct.go @@ -69,7 +69,11 @@ func (g *interfaceGenerator) validateStruct(ts *ast.TypeSpec, st *ast.StructType }) } -func (g *interfaceGenerator) isStructPacked(st *ast.StructType) bool { +func (g *interfaceGenerator) isStructPacked(st *ast.StructType, isDynamic bool) bool { + if isDynamic { + // Dynamic types are not packed because a slice header might be present. + return false + } packed := true forEachStructField(st, func(f *ast.Field) { if f.Tag != nil { @@ -85,165 +89,17 @@ func (g *interfaceGenerator) isStructPacked(st *ast.StructType) bool { return packed } -func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) { - thisPacked := g.isStructPacked(st) - - g.emit("// SizeBytes implements marshal.Marshallable.SizeBytes.\n") - g.emit("func (%s *%s) SizeBytes() int {\n", g.r, g.typeName()) - g.inIndent(func() { - primitiveSize := 0 - var dynamicSizeTerms []string - - forEachStructField(st, fieldDispatcher{ - primitive: func(_, t *ast.Ident) { - if size, dynamic := g.scalarSize(t); !dynamic { - primitiveSize += size - } else { - g.recordUsedMarshallable(t.Name) - dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("(*%s)(nil).SizeBytes()", t.Name)) - } - }, - selector: func(_, tX, tSel *ast.Ident) { - tName := fmt.Sprintf("%s.%s", tX.Name, tSel.Name) - g.recordUsedImport(tX.Name) - g.recordUsedMarshallable(tName) - dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("(*%s)(nil).SizeBytes()", tName)) - }, - array: func(_ *ast.Ident, a *ast.ArrayType, t *ast.Ident) { - lenExpr := g.arrayLenExpr(a) - if size, dynamic := g.scalarSize(t); !dynamic { - dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("%d*%s", size, lenExpr)) - } else { - g.recordUsedMarshallable(t.Name) - dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("(*%s)(nil).SizeBytes()*%s", t.Name, lenExpr)) - } - }, - }.dispatch) - g.emit("return %d", primitiveSize) - if len(dynamicSizeTerms) > 0 { - g.incIndent() - } - { - for _, d := range dynamicSizeTerms { - g.emitNoIndent(" +\n") - g.emit(d) - } - } - if len(dynamicSizeTerms) > 0 { - g.decIndent() - } - }) - g.emit("\n}\n\n") +func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType, isDynamic bool) { + thisPacked := g.isStructPacked(st, isDynamic) - g.emit("// MarshalBytes implements marshal.Marshallable.MarshalBytes.\n") - g.emit("func (%s *%s) MarshalBytes(dst []byte) {\n", g.r, g.typeName()) - g.inIndent(func() { - forEachStructField(st, fieldDispatcher{ - primitive: func(n, t *ast.Ident) { - if n.Name == "_" { - g.emit("// Padding: dst[:sizeof(%s)] ~= %s(0)\n", t.Name, t.Name) - if len, dynamic := g.scalarSize(t); !dynamic { - g.shift("dst", len) - } else { - // We can't use shiftDynamic here because we don't have - // an instance of the dynamic type we can reference here - // (since the version in this struct is anonymous). Use - // a typed nil pointer to call SizeBytes() instead. - g.emit("dst = dst[(*%s)(nil).SizeBytes():]\n", t.Name) - } - return - } - g.marshalScalar(g.fieldAccessor(n), t.Name, "dst") - }, - selector: func(n, tX, tSel *ast.Ident) { - if n.Name == "_" { - g.emit("// Padding: dst[:sizeof(%s)] ~= %s(0)\n", tX.Name, tSel.Name) - g.emit("dst = dst[(*%s.%s)(nil).SizeBytes():]\n", tX.Name, tSel.Name) - return - } - g.marshalScalar(g.fieldAccessor(n), fmt.Sprintf("%s.%s", tX.Name, tSel.Name), "dst") - }, - array: func(n *ast.Ident, a *ast.ArrayType, t *ast.Ident) { - lenExpr := g.arrayLenExpr(a) - if n.Name == "_" { - g.emit("// Padding: dst[:sizeof(%s)*%s] ~= [%s]%s{0}\n", t.Name, lenExpr, lenExpr, t.Name) - if size, dynamic := g.scalarSize(t); !dynamic { - g.emit("dst = dst[%d*(%s):]\n", size, lenExpr) - } else { - // We can't use shiftDynamic here because we don't have - // an instance of the dynamic type we can reference here - // (since the version in this struct is anonymous). Use - // a typed nil pointer to call SizeBytes() instead. - g.emit("dst = dst[(*%s)(nil).SizeBytes()*(%s):]\n", t.Name, lenExpr) - } - return - } - - g.emit("for idx := 0; idx < %s; idx++ {\n", lenExpr) - g.inIndent(func() { - g.marshalScalar(fmt.Sprintf("%s[idx]", g.fieldAccessor(n)), t.Name, "dst") - }) - g.emit("}\n") - }, - }.dispatch) - }) - g.emit("}\n\n") - - g.emit("// UnmarshalBytes implements marshal.Marshallable.UnmarshalBytes.\n") - g.emit("func (%s *%s) UnmarshalBytes(src []byte) {\n", g.r, g.typeName()) - g.inIndent(func() { - forEachStructField(st, fieldDispatcher{ - primitive: func(n, t *ast.Ident) { - if n.Name == "_" { - g.emit("// Padding: var _ %s ~= src[:sizeof(%s)]\n", t.Name, t.Name) - if len, dynamic := g.scalarSize(t); !dynamic { - g.shift("src", len) - } else { - // We don't have an instance of the dynamic type we can - // reference here (since the version in this struct is - // anonymous). Use a typed nil pointer to call - // SizeBytes() instead. - g.shiftDynamic("src", fmt.Sprintf("(*%s)(nil)", t.Name)) - g.recordPotentiallyNonPackedField(fmt.Sprintf("(*%s)(nil)", t.Name)) - } - return - } - g.unmarshalScalar(g.fieldAccessor(n), t.Name, "src") - }, - selector: func(n, tX, tSel *ast.Ident) { - if n.Name == "_" { - g.emit("// Padding: %s ~= src[:sizeof(%s.%s)]\n", g.fieldAccessor(n), tX.Name, tSel.Name) - g.emit("src = src[(*%s.%s)(nil).SizeBytes():]\n", tX.Name, tSel.Name) - g.recordPotentiallyNonPackedField(fmt.Sprintf("(*%s.%s)(nil)", tX.Name, tSel.Name)) - return - } - g.unmarshalScalar(g.fieldAccessor(n), fmt.Sprintf("%s.%s", tX.Name, tSel.Name), "src") - }, - array: func(n *ast.Ident, a *ast.ArrayType, t *ast.Ident) { - lenExpr := g.arrayLenExpr(a) - if n.Name == "_" { - g.emit("// Padding: ~ copy([%s]%s(%s), src[:sizeof(%s)*%s])\n", lenExpr, t.Name, g.fieldAccessor(n), t.Name, lenExpr) - if size, dynamic := g.scalarSize(t); !dynamic { - g.emit("src = src[%d*(%s):]\n", size, lenExpr) - } else { - // We can't use shiftDynamic here because we don't have - // an instance of the dynamic type we can referece here - // (since the version in this struct is anonymous). Use - // a typed nil pointer to call SizeBytes() instead. - g.emit("src = src[(*%s)(nil).SizeBytes()*(%s):]\n", t.Name, lenExpr) - } - return - } - - g.emit("for idx := 0; idx < %s; idx++ {\n", lenExpr) - g.inIndent(func() { - g.unmarshalScalar(fmt.Sprintf("%s[idx]", g.fieldAccessor(n)), t.Name, "src") - }) - g.emit("}\n") - }, - }.dispatch) - }) - g.emit("}\n\n") + // Dynamic types are supposed to manually implement SizeBytes, MarshalBytes + // and UnmarshalBytes. The rest of the methos are autogenerated and depend on + // the implementation of these three. + if !isDynamic { + g.emitSizeBytesForStruct(st) + g.emitMarshalBytesForStruct(st) + g.emitUnmarshalBytesForStruct(st) + } g.emit("// Packed implements marshal.Marshallable.Packed.\n") g.emit("//go:nosplit\n") @@ -428,8 +284,171 @@ func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) { g.emit("}\n\n") } +func (g *interfaceGenerator) emitSizeBytesForStruct(st *ast.StructType) { + g.emit("// SizeBytes implements marshal.Marshallable.SizeBytes.\n") + g.emit("func (%s *%s) SizeBytes() int {\n", g.r, g.typeName()) + g.inIndent(func() { + primitiveSize := 0 + var dynamicSizeTerms []string + + forEachStructField(st, fieldDispatcher{ + primitive: func(_, t *ast.Ident) { + if size, dynamic := g.scalarSize(t); !dynamic { + primitiveSize += size + } else { + g.recordUsedMarshallable(t.Name) + dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("(*%s)(nil).SizeBytes()", t.Name)) + } + }, + selector: func(_, tX, tSel *ast.Ident) { + tName := fmt.Sprintf("%s.%s", tX.Name, tSel.Name) + g.recordUsedImport(tX.Name) + g.recordUsedMarshallable(tName) + dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("(*%s)(nil).SizeBytes()", tName)) + }, + array: func(_ *ast.Ident, a *ast.ArrayType, t *ast.Ident) { + lenExpr := g.arrayLenExpr(a) + if size, dynamic := g.scalarSize(t); !dynamic { + dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("%d*%s", size, lenExpr)) + } else { + g.recordUsedMarshallable(t.Name) + dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("(*%s)(nil).SizeBytes()*%s", t.Name, lenExpr)) + } + }, + }.dispatch) + g.emit("return %d", primitiveSize) + if len(dynamicSizeTerms) > 0 { + g.incIndent() + } + { + for _, d := range dynamicSizeTerms { + g.emitNoIndent(" +\n") + g.emit(d) + } + } + if len(dynamicSizeTerms) > 0 { + g.decIndent() + } + }) + g.emit("\n}\n\n") +} + +func (g *interfaceGenerator) emitMarshalBytesForStruct(st *ast.StructType) { + g.emit("// MarshalBytes implements marshal.Marshallable.MarshalBytes.\n") + g.emit("func (%s *%s) MarshalBytes(dst []byte) {\n", g.r, g.typeName()) + g.inIndent(func() { + forEachStructField(st, fieldDispatcher{ + primitive: func(n, t *ast.Ident) { + if n.Name == "_" { + g.emit("// Padding: dst[:sizeof(%s)] ~= %s(0)\n", t.Name, t.Name) + if len, dynamic := g.scalarSize(t); !dynamic { + g.shift("dst", len) + } else { + // We can't use shiftDynamic here because we don't have + // an instance of the dynamic type we can reference here + // (since the version in this struct is anonymous). Use + // a typed nil pointer to call SizeBytes() instead. + g.emit("dst = dst[(*%s)(nil).SizeBytes():]\n", t.Name) + } + return + } + g.marshalScalar(g.fieldAccessor(n), t.Name, "dst") + }, + selector: func(n, tX, tSel *ast.Ident) { + if n.Name == "_" { + g.emit("// Padding: dst[:sizeof(%s)] ~= %s(0)\n", tX.Name, tSel.Name) + g.emit("dst = dst[(*%s.%s)(nil).SizeBytes():]\n", tX.Name, tSel.Name) + return + } + g.marshalScalar(g.fieldAccessor(n), fmt.Sprintf("%s.%s", tX.Name, tSel.Name), "dst") + }, + array: func(n *ast.Ident, a *ast.ArrayType, t *ast.Ident) { + lenExpr := g.arrayLenExpr(a) + if n.Name == "_" { + g.emit("// Padding: dst[:sizeof(%s)*%s] ~= [%s]%s{0}\n", t.Name, lenExpr, lenExpr, t.Name) + if size, dynamic := g.scalarSize(t); !dynamic { + g.emit("dst = dst[%d*(%s):]\n", size, lenExpr) + } else { + // We can't use shiftDynamic here because we don't have + // an instance of the dynamic type we can reference here + // (since the version in this struct is anonymous). Use + // a typed nil pointer to call SizeBytes() instead. + g.emit("dst = dst[(*%s)(nil).SizeBytes()*(%s):]\n", t.Name, lenExpr) + } + return + } + + g.emit("for idx := 0; idx < %s; idx++ {\n", lenExpr) + g.inIndent(func() { + g.marshalScalar(fmt.Sprintf("%s[idx]", g.fieldAccessor(n)), t.Name, "dst") + }) + g.emit("}\n") + }, + }.dispatch) + }) + g.emit("}\n\n") +} + +func (g *interfaceGenerator) emitUnmarshalBytesForStruct(st *ast.StructType) { + g.emit("// UnmarshalBytes implements marshal.Marshallable.UnmarshalBytes.\n") + g.emit("func (%s *%s) UnmarshalBytes(src []byte) {\n", g.r, g.typeName()) + g.inIndent(func() { + forEachStructField(st, fieldDispatcher{ + primitive: func(n, t *ast.Ident) { + if n.Name == "_" { + g.emit("// Padding: var _ %s ~= src[:sizeof(%s)]\n", t.Name, t.Name) + if len, dynamic := g.scalarSize(t); !dynamic { + g.shift("src", len) + } else { + // We don't have an instance of the dynamic type we can + // reference here (since the version in this struct is + // anonymous). Use a typed nil pointer to call + // SizeBytes() instead. + g.shiftDynamic("src", fmt.Sprintf("(*%s)(nil)", t.Name)) + g.recordPotentiallyNonPackedField(fmt.Sprintf("(*%s)(nil)", t.Name)) + } + return + } + g.unmarshalScalar(g.fieldAccessor(n), t.Name, "src") + }, + selector: func(n, tX, tSel *ast.Ident) { + if n.Name == "_" { + g.emit("// Padding: %s ~= src[:sizeof(%s.%s)]\n", g.fieldAccessor(n), tX.Name, tSel.Name) + g.emit("src = src[(*%s.%s)(nil).SizeBytes():]\n", tX.Name, tSel.Name) + g.recordPotentiallyNonPackedField(fmt.Sprintf("(*%s.%s)(nil)", tX.Name, tSel.Name)) + return + } + g.unmarshalScalar(g.fieldAccessor(n), fmt.Sprintf("%s.%s", tX.Name, tSel.Name), "src") + }, + array: func(n *ast.Ident, a *ast.ArrayType, t *ast.Ident) { + lenExpr := g.arrayLenExpr(a) + if n.Name == "_" { + g.emit("// Padding: ~ copy([%s]%s(%s), src[:sizeof(%s)*%s])\n", lenExpr, t.Name, g.fieldAccessor(n), t.Name, lenExpr) + if size, dynamic := g.scalarSize(t); !dynamic { + g.emit("src = src[%d*(%s):]\n", size, lenExpr) + } else { + // We can't use shiftDynamic here because we don't have + // an instance of the dynamic type we can referece here + // (since the version in this struct is anonymous). Use + // a typed nil pointer to call SizeBytes() instead. + g.emit("src = src[(*%s)(nil).SizeBytes()*(%s):]\n", t.Name, lenExpr) + } + return + } + + g.emit("for idx := 0; idx < %s; idx++ {\n", lenExpr) + g.inIndent(func() { + g.unmarshalScalar(fmt.Sprintf("%s[idx]", g.fieldAccessor(n)), t.Name, "src") + }) + g.emit("}\n") + }, + }.dispatch) + }) + g.emit("}\n\n") +} + func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, slice *sliceAPI) { - thisPacked := g.isStructPacked(st) + thisPacked := g.isStructPacked(st, false /* isDynamic */) if slice.inner { abortAt(g.f.Position(slice.comment.Slash), fmt.Sprintf("The ':inner' argument to '+marshal slice:%s:inner' is only applicable to newtypes on primitives. Remove it from this struct declaration.", slice.ident)) diff --git a/tools/go_marshal/gomarshal/generator_tests.go b/tools/go_marshal/gomarshal/generator_tests.go index 6cf00843f..ca3e15c16 100644 --- a/tools/go_marshal/gomarshal/generator_tests.go +++ b/tools/go_marshal/gomarshal/generator_tests.go @@ -216,12 +216,16 @@ func (g *testGenerator) emitTestSizeBytesOnTypedNilPtr() { }) } -func (g *testGenerator) emitTests(slice *sliceAPI) { +func (g *testGenerator) emitTests(slice *sliceAPI, isDynamic bool) { g.emitTestNonZeroSize() g.emitTestSuspectAlignment() - g.emitTestMarshalUnmarshalPreservesData() - g.emitTestWriteToUnmarshalPreservesData() - g.emitTestSizeBytesOnTypedNilPtr() + if !isDynamic { + // Do not test these for dynamic structs because they violate some + // assumptions that these tests make. + g.emitTestMarshalUnmarshalPreservesData() + g.emitTestWriteToUnmarshalPreservesData() + g.emitTestSizeBytesOnTypedNilPtr() + } if slice != nil { g.emitTestMarshalUnmarshalSlicePreservesData(slice) diff --git a/tools/go_marshal/test/BUILD b/tools/go_marshal/test/BUILD index 4b27773c2..cb2d4e6e3 100644 --- a/tools/go_marshal/test/BUILD +++ b/tools/go_marshal/test/BUILD @@ -26,7 +26,10 @@ go_library( srcs = ["test.go"], marshal = True, visibility = ["//tools/go_marshal/test:__subpackages__"], - deps = ["//tools/go_marshal/test/external"], + deps = [ + "//pkg/marshal/primitive", + "//tools/go_marshal/test/external", + ], ) go_test( @@ -36,6 +39,7 @@ go_test( deps = [ ":test", "//pkg/marshal", + "//pkg/marshal/primitive", "//pkg/syserror", "//pkg/usermem", "//tools/go_marshal/analysis", diff --git a/tools/go_marshal/test/marshal_test.go b/tools/go_marshal/test/marshal_test.go index a00f9a684..b0091dc64 100644 --- a/tools/go_marshal/test/marshal_test.go +++ b/tools/go_marshal/test/marshal_test.go @@ -28,6 +28,7 @@ import ( "github.com/google/go-cmp/cmp" "gvisor.dev/gvisor/pkg/marshal" + "gvisor.dev/gvisor/pkg/marshal/primitive" "gvisor.dev/gvisor/pkg/syserror" "gvisor.dev/gvisor/pkg/usermem" "gvisor.dev/gvisor/tools/go_marshal/analysis" @@ -513,3 +514,21 @@ func TestLimitedSliceMarshalling(t *testing.T) { }) } } + +func TestDynamicType(t *testing.T) { + t12 := test.Type12Dynamic{ + X: 32, + Y: []primitive.Int64{5, 6, 7}, + } + + var m marshal.Marshallable + m = &t12 // Ensure that all methods were generated. + b := make([]byte, m.SizeBytes()) + m.MarshalBytes(b) + + var res test.Type12Dynamic + res.UnmarshalBytes(b) + if !reflect.DeepEqual(t12, res) { + t.Errorf("dynamic type is not same after marshalling and unmarshalling: before = %+v, after = %+v", t12, res) + } +} diff --git a/tools/go_marshal/test/test.go b/tools/go_marshal/test/test.go index e7e3ed74a..b8eb989d9 100644 --- a/tools/go_marshal/test/test.go +++ b/tools/go_marshal/test/test.go @@ -16,6 +16,8 @@ package test import ( + "gvisor.dev/gvisor/pkg/marshal/primitive" + // We're intentionally using a package name alias here even though it's not // necessary to test the code generator's ability to handle package aliases. ex "gvisor.dev/gvisor/tools/go_marshal/test/external" @@ -198,3 +200,36 @@ type Type11 struct { ex.External y int64 } + +// Type12Dynamic is a dynamically sized struct which depends on the autogenerator +// to generate some Marshallable methods for it. +// +// +marshal dynamic +type Type12Dynamic struct { + X primitive.Int64 + Y []primitive.Int64 +} + +// SizeBytes implements marshal.Marshallable.SizeBytes. +func (t *Type12Dynamic) SizeBytes() int { + return (len(t.Y) * 8) + t.X.SizeBytes() +} + +// MarshalBytes implements marshal.Marshallable.MarshalBytes. +func (t *Type12Dynamic) MarshalBytes(dst []byte) { + t.X.MarshalBytes(dst) + dst = dst[t.X.SizeBytes():] + for i, x := range t.Y { + x.MarshalBytes(dst[i*8 : (i+1)*8]) + } +} + +// UnmarshalBytes implements marshal.Marshallable.UnmarshalBytes. +func (t *Type12Dynamic) UnmarshalBytes(src []byte) { + t.X.UnmarshalBytes(src) + for i := t.X.SizeBytes(); i < len(src); i += 8 { + var x primitive.Int64 + x.UnmarshalBytes(src[i:]) + t.Y = append(t.Y, x) + } +} -- cgit v1.2.3