diff options
Diffstat (limited to 'tools/go_marshal')
-rw-r--r-- | tools/go_marshal/README.md | 15 | ||||
-rw-r--r-- | tools/go_marshal/defs.bzl | 9 | ||||
-rw-r--r-- | tools/go_marshal/gomarshal/generator.go | 83 | ||||
-rw-r--r-- | tools/go_marshal/gomarshal/generator_interfaces.go | 4 | ||||
-rw-r--r-- | tools/go_marshal/gomarshal/generator_interfaces_array_newtype.go | 12 | ||||
-rw-r--r-- | tools/go_marshal/gomarshal/generator_interfaces_primitive_newtype.go | 20 | ||||
-rw-r--r-- | tools/go_marshal/gomarshal/generator_interfaces_struct.go | 51 | ||||
-rw-r--r-- | tools/go_marshal/gomarshal/util.go | 20 | ||||
-rw-r--r-- | tools/go_marshal/main.go | 11 | ||||
-rw-r--r-- | tools/go_marshal/test/escape/escape.go | 26 | ||||
-rw-r--r-- | tools/go_marshal/test/marshal_test.go | 114 | ||||
-rw-r--r-- | tools/go_marshal/test/test.go | 24 |
12 files changed, 234 insertions, 155 deletions
diff --git a/tools/go_marshal/README.md b/tools/go_marshal/README.md index 75e5c7888..d8045c295 100644 --- a/tools/go_marshal/README.md +++ b/tools/go_marshal/README.md @@ -113,3 +113,18 @@ The following are some guidelines for modifying the `go_marshal` tool: - No runtime reflection in the code generated for the marshallable interface. The entire point of the tool is to avoid runtime reflection. The generated tests may use reflection. + +## Debugging + +To enable debugging output from the go-marshal tool, use one of the following +options, depending on how go-marshal is being invoked: + +- Pass `--define gomarshal=verbose` to the bazel command. Note that this can + generate a lot of output depending on what's being compiled, as this will + enable debugging for all packages built by the command. + +- Set `marshal_debug = True` on the top-level `go_library` BUILD rule. + +- Set `debug = True` on the `go_marshal` BUILD rule. + +- Pass `-debug` to the go-marshal tool invocation. diff --git a/tools/go_marshal/defs.bzl b/tools/go_marshal/defs.bzl index ba98f3599..f44f83eab 100644 --- a/tools/go_marshal/defs.bzl +++ b/tools/go_marshal/defs.bzl @@ -4,11 +4,13 @@ def _go_marshal_impl(ctx): """Execute the go_marshal tool.""" output = ctx.outputs.lib output_test = ctx.outputs.test + output_test_unconditional = ctx.outputs.test_unconditional # Run the marshal command. args = ["-output=%s" % output.path] - args += ["-pkg=%s" % ctx.attr.package] - args += ["-output_test=%s" % output_test.path] + args.append("-pkg=%s" % ctx.attr.package) + args.append("-output_test=%s" % output_test.path) + args.append("-output_test_unconditional=%s" % output_test_unconditional.path) if ctx.attr.debug: args += ["-debug"] @@ -18,7 +20,7 @@ def _go_marshal_impl(ctx): args += [f.path for f in src.files.to_list()] ctx.actions.run( inputs = ctx.files.srcs, - outputs = [output, output_test], + outputs = [output, output_test, output_test_unconditional], mnemonic = "GoMarshal", progress_message = "go_marshal: %s" % ctx.label, arguments = args, @@ -48,6 +50,7 @@ go_marshal = rule( outputs = { "lib": "%{name}_unsafe.go", "test": "%{name}_test.go", + "test_unconditional": "%{name}_unconditional_test.go", }, ) diff --git a/tools/go_marshal/gomarshal/generator.go b/tools/go_marshal/gomarshal/generator.go index fd7cce8fa..56fbcb5d2 100644 --- a/tools/go_marshal/gomarshal/generator.go +++ b/tools/go_marshal/gomarshal/generator.go @@ -38,8 +38,8 @@ import ( // All recievers are single letters, so we don't allow import aliases to be a // single letter. var badIdents = []string{ - "addr", "blk", "buf", "dst", "dsts", "count", "err", "hdr", "idx", "inner", - "length", "limit", "ptr", "size", "src", "srcs", "task", "val", + "addr", "blk", "buf", "cc", "dst", "dsts", "count", "err", "hdr", "idx", + "inner", "length", "limit", "ptr", "size", "src", "srcs", "val", // All single-letter identifiers. } @@ -68,6 +68,8 @@ type Generator struct { output *os.File // Output file to write generated tests. outputTest *os.File + // Output file to write unconditionally generated tests. + outputTestUC *os.File // Package name for the generated file. pkg string // Set of extra packages to import in the generated file. @@ -75,7 +77,7 @@ type Generator struct { } // NewGenerator creates a new code Generator. -func NewGenerator(srcs []string, out, outTest, pkg string, imports []string) (*Generator, error) { +func NewGenerator(srcs []string, out, outTest, outTestUnconditional, pkg string, imports []string) (*Generator, error) { f, err := os.OpenFile(out, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) if err != nil { return nil, fmt.Errorf("Couldn't open output file %q: %v", out, err) @@ -84,12 +86,17 @@ func NewGenerator(srcs []string, out, outTest, pkg string, imports []string) (*G if err != nil { return nil, fmt.Errorf("Couldn't open test output file %q: %v", out, err) } + fTestUC, err := os.OpenFile(outTestUnconditional, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return nil, fmt.Errorf("Couldn't open unconditional test output file %q: %v", out, err) + } g := Generator{ - inputs: srcs, - output: f, - outputTest: fTest, - pkg: pkg, - imports: newImportTable(), + inputs: srcs, + output: f, + outputTest: fTest, + outputTestUC: fTestUC, + pkg: pkg, + imports: newImportTable(), } for _, i := range imports { // All imports on the extra imports list are unconditionally marked as @@ -454,6 +461,46 @@ func (g *Generator) Run() error { // source file. func (g *Generator) writeTests(ts []*testGenerator) error { var b sourceBuffer + + // Write the unconditional test file. This file is always compiled, + // regardless of what build tags were specified on the original input + // files. We use this file to guarantee we never end up with an empty test + // file, as that causes the build to fail with "no tests/benchmarks/examples + // found". + // + // There's no easy way to determine ahead of time if we'll end up with an + // empty build file since build constraints can arbitrarily cause some of + // the original types to be not defined. We also have no way to tell bazel + // to omit the entire test suite since the output files are already defined + // before go-marshal is called. + b.emit("// Automatically generated marshal tests. See tools/go_marshal.\n\n") + b.emit("package %s\n\n", g.pkg) + b.emit("func Example() {\n") + b.inIndent(func() { + b.emit("// This example is intentionally empty, and ensures this package contains at\n") + b.emit("// least one testable entity. go-marshal is forced to emit a test package if the\n") + b.emit("// input package is marked marshallable, but emitting no testable entities \n") + b.emit("// results in a build failure.\n") + }) + b.emit("}\n") + if err := b.write(g.outputTestUC); err != nil { + return err + } + + // Now generate the real test file that contains the real types we + // processed. These need to be conditionally compiled according to the build + // tags, as the original types may not be defined under all build + // configurations. + + b.reset() + b.emit("// Automatically generated marshal tests. See tools/go_marshal.\n\n") + + // Emit build tags. + if t := tags.Aggregate(g.inputs); len(t) > 0 { + b.emit(strings.Join(t.Lines(), "\n")) + b.emit("\n\n") + } + b.emit("package %s\n\n", g.pkg) if err := b.write(g.outputTest); err != nil { return err @@ -470,26 +517,6 @@ func (g *Generator) writeTests(ts []*testGenerator) error { } // Write test functions. - - // If we didn't generate any Marshallable implementations, we can't just - // emit an empty test file, since that causes the build to fail with "no - // tests/benchmarks/examples found". Unfortunately we can't signal bazel to - // omit the entire package since the outputs are already defined before - // go-marshal is called. If we'd otherwise emit an empty test suite, emit an - // empty example instead. - if len(ts) == 0 { - b.reset() - b.emit("func Example() {\n") - b.inIndent(func() { - b.emit("// This example is intentionally empty to ensure this file contains at least\n") - b.emit("// one testable entity. go-marshal is forced to emit a test file if a package\n") - b.emit("// is marked marshallable, but emitting a test file with no entities results\n") - b.emit("// in a build failure.\n") - }) - b.emit("}\n") - return b.write(g.outputTest) - } - for _, t := range ts { if err := t.write(g.outputTest); err != nil { return err diff --git a/tools/go_marshal/gomarshal/generator_interfaces.go b/tools/go_marshal/gomarshal/generator_interfaces.go index cf76b5241..36447b86b 100644 --- a/tools/go_marshal/gomarshal/generator_interfaces.go +++ b/tools/go_marshal/gomarshal/generator_interfaces.go @@ -43,8 +43,8 @@ type interfaceGenerator struct { // of t's interfaces. ms map[string]struct{} - // as records embedded fields in t that are potentially not packed. The key - // is the accessor for the field. + // as records fields in t that are potentially not packed. The key is the + // accessor for the field. as map[string]struct{} } diff --git a/tools/go_marshal/gomarshal/generator_interfaces_array_newtype.go b/tools/go_marshal/gomarshal/generator_interfaces_array_newtype.go index 72ef03a22..7525b52da 100644 --- a/tools/go_marshal/gomarshal/generator_interfaces_array_newtype.go +++ b/tools/go_marshal/gomarshal/generator_interfaces_array_newtype.go @@ -102,11 +102,11 @@ func (g *interfaceGenerator) emitMarshallableForArrayNewtype(n *ast.Ident, a *as g.emit("// CopyOutN implements marshal.Marshallable.CopyOutN.\n") g.emit("//go:nosplit\n") - g.emit("func (%s *%s) CopyOutN(task marshal.Task, addr usermem.Addr, limit int) (int, error) {\n", g.r, g.typeName()) + g.emit("func (%s *%s) CopyOutN(cc marshal.CopyContext, addr usermem.Addr, limit int) (int, error) {\n", g.r, g.typeName()) g.inIndent(func() { g.emitCastToByteSlice(g.r, "buf", fmt.Sprintf("%s.SizeBytes()", g.r)) - g.emit("length, err := task.CopyOutBytes(addr, buf[:limit]) // escapes: okay.\n") + g.emit("length, err := cc.CopyOutBytes(addr, buf[:limit]) // escapes: okay.\n") g.emitKeepAlive(g.r) g.emit("return length, err\n") }) @@ -114,19 +114,19 @@ func (g *interfaceGenerator) emitMarshallableForArrayNewtype(n *ast.Ident, a *as g.emit("// CopyOut implements marshal.Marshallable.CopyOut.\n") g.emit("//go:nosplit\n") - g.emit("func (%s *%s) CopyOut(task marshal.Task, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) + g.emit("func (%s *%s) CopyOut(cc marshal.CopyContext, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) g.inIndent(func() { - g.emit("return %s.CopyOutN(task, addr, %s.SizeBytes())\n", g.r, g.r) + g.emit("return %s.CopyOutN(cc, addr, %s.SizeBytes())\n", g.r, g.r) }) g.emit("}\n\n") g.emit("// CopyIn implements marshal.Marshallable.CopyIn.\n") g.emit("//go:nosplit\n") - g.emit("func (%s *%s) CopyIn(task marshal.Task, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) + g.emit("func (%s *%s) CopyIn(cc marshal.CopyContext, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) g.inIndent(func() { g.emitCastToByteSlice(g.r, "buf", fmt.Sprintf("%s.SizeBytes()", g.r)) - g.emit("length, err := task.CopyInBytes(addr, buf) // escapes: okay.\n") + g.emit("length, err := cc.CopyInBytes(addr, buf) // escapes: okay.\n") g.emitKeepAlive(g.r) g.emit("return length, err\n") }) diff --git a/tools/go_marshal/gomarshal/generator_interfaces_primitive_newtype.go b/tools/go_marshal/gomarshal/generator_interfaces_primitive_newtype.go index 39f654ea8..7edaf666c 100644 --- a/tools/go_marshal/gomarshal/generator_interfaces_primitive_newtype.go +++ b/tools/go_marshal/gomarshal/generator_interfaces_primitive_newtype.go @@ -154,11 +154,11 @@ func (g *interfaceGenerator) emitMarshallableForPrimitiveNewtype(nt *ast.Ident) g.emit("// CopyOutN implements marshal.Marshallable.CopyOutN.\n") g.emit("//go:nosplit\n") - g.emit("func (%s *%s) CopyOutN(task marshal.Task, addr usermem.Addr, limit int) (int, error) {\n", g.r, g.typeName()) + g.emit("func (%s *%s) CopyOutN(cc marshal.CopyContext, addr usermem.Addr, limit int) (int, error) {\n", g.r, g.typeName()) g.inIndent(func() { g.emitCastToByteSlice(g.r, "buf", fmt.Sprintf("%s.SizeBytes()", g.r)) - g.emit("length, err := task.CopyOutBytes(addr, buf[:limit]) // escapes: okay.\n") + g.emit("length, err := cc.CopyOutBytes(addr, buf[:limit]) // escapes: okay.\n") g.emitKeepAlive(g.r) g.emit("return length, err\n") }) @@ -166,19 +166,19 @@ func (g *interfaceGenerator) emitMarshallableForPrimitiveNewtype(nt *ast.Ident) g.emit("// CopyOut implements marshal.Marshallable.CopyOut.\n") g.emit("//go:nosplit\n") - g.emit("func (%s *%s) CopyOut(task marshal.Task, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) + g.emit("func (%s *%s) CopyOut(cc marshal.CopyContext, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) g.inIndent(func() { - g.emit("return %s.CopyOutN(task, addr, %s.SizeBytes())\n", g.r, g.r) + g.emit("return %s.CopyOutN(cc, addr, %s.SizeBytes())\n", g.r, g.r) }) g.emit("}\n\n") g.emit("// CopyIn implements marshal.Marshallable.CopyIn.\n") g.emit("//go:nosplit\n") - g.emit("func (%s *%s) CopyIn(task marshal.Task, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) + g.emit("func (%s *%s) CopyIn(cc marshal.CopyContext, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) g.inIndent(func() { g.emitCastToByteSlice(g.r, "buf", fmt.Sprintf("%s.SizeBytes()", g.r)) - g.emit("length, err := task.CopyInBytes(addr, buf) // escapes: okay.\n") + g.emit("length, err := cc.CopyInBytes(addr, buf) // escapes: okay.\n") g.emitKeepAlive(g.r) g.emit("return length, err\n") }) @@ -211,7 +211,7 @@ func (g *interfaceGenerator) emitMarshallableSliceForPrimitiveNewtype(nt *ast.Id g.emit("// Copy%sIn copies in a slice of %s objects from the task's memory.\n", slice.ident, eltType) g.emit("//go:nosplit\n") - g.emit("func Copy%sIn(task marshal.Task, addr usermem.Addr, dst []%s) (int, error) {\n", slice.ident, eltType) + g.emit("func Copy%sIn(cc marshal.CopyContext, addr usermem.Addr, dst []%s) (int, error) {\n", slice.ident, eltType) g.inIndent(func() { g.emit("count := len(dst)\n") g.emit("if count == 0 {\n") @@ -223,7 +223,7 @@ func (g *interfaceGenerator) emitMarshallableSliceForPrimitiveNewtype(nt *ast.Id g.emitCastSliceToByteSlice("&dst", "buf", "size * count") - g.emit("length, err := task.CopyInBytes(addr, buf) // escapes: okay.\n") + g.emit("length, err := cc.CopyInBytes(addr, buf) // escapes: okay.\n") g.emitKeepAlive("dst") g.emit("return length, err\n") }) @@ -231,7 +231,7 @@ func (g *interfaceGenerator) emitMarshallableSliceForPrimitiveNewtype(nt *ast.Id g.emit("// Copy%sOut copies a slice of %s objects to the task's memory.\n", slice.ident, eltType) g.emit("//go:nosplit\n") - g.emit("func Copy%sOut(task marshal.Task, addr usermem.Addr, src []%s) (int, error) {\n", slice.ident, eltType) + g.emit("func Copy%sOut(cc marshal.CopyContext, addr usermem.Addr, src []%s) (int, error) {\n", slice.ident, eltType) g.inIndent(func() { g.emit("count := len(src)\n") g.emit("if count == 0 {\n") @@ -243,7 +243,7 @@ func (g *interfaceGenerator) emitMarshallableSliceForPrimitiveNewtype(nt *ast.Id g.emitCastSliceToByteSlice("&src", "buf", "size * count") - g.emit("length, err := task.CopyOutBytes(addr, buf) // escapes: okay.\n") + g.emit("length, err := cc.CopyOutBytes(addr, buf) // escapes: okay.\n") g.emitKeepAlive("src") g.emit("return length, err\n") }) diff --git a/tools/go_marshal/gomarshal/generator_interfaces_struct.go b/tools/go_marshal/gomarshal/generator_interfaces_struct.go index 44fbb425c..fe76d3785 100644 --- a/tools/go_marshal/gomarshal/generator_interfaces_struct.go +++ b/tools/go_marshal/gomarshal/generator_interfaces_struct.go @@ -20,6 +20,7 @@ package gomarshal import ( "fmt" "go/ast" + "sort" "strings" ) @@ -40,6 +41,8 @@ func (g *interfaceGenerator) areFieldsPackedExpression() (string, bool) { for accessor, _ := range g.as { cs = append(cs, fmt.Sprintf("%s.Packed()", accessor)) } + // Sort expressions for determinstic build outputs. + sort.Strings(cs) return strings.Join(cs, " && "), true } @@ -48,12 +51,6 @@ func (g *interfaceGenerator) areFieldsPackedExpression() (string, bool) { // later. func (g *interfaceGenerator) validateStruct(ts *ast.TypeSpec, st *ast.StructType) { forEachStructField(st, func(f *ast.Field) { - if len(f.Names) == 0 { - g.abortAt(f.Pos(), "Cannot marshal structs with embedded fields, give the field a name; use '_' for anonymous fields such as padding fields") - } - }) - - forEachStructField(st, func(f *ast.Field) { fieldDispatcher{ primitive: func(_, t *ast.Ident) { g.validatePrimitiveNewtype(t) @@ -98,7 +95,7 @@ func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) { var dynamicSizeTerms []string forEachStructField(st, fieldDispatcher{ - primitive: func(n, t *ast.Ident) { + primitive: func(_, t *ast.Ident) { if size, dynamic := g.scalarSize(t); !dynamic { primitiveSize += size } else { @@ -106,13 +103,13 @@ func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) { dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("(*%s)(nil).SizeBytes()", t.Name)) } }, - selector: func(n, tX, tSel *ast.Ident) { + 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(n *ast.Ident, a *ast.ArrayType, t *ast.Ident) { + 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)) @@ -323,13 +320,13 @@ func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) { g.emit("//go:nosplit\n") g.recordUsedImport("marshal") g.recordUsedImport("usermem") - g.emit("func (%s *%s) CopyOutN(task marshal.Task, addr usermem.Addr, limit int) (int, error) {\n", g.r, g.typeName()) + g.emit("func (%s *%s) CopyOutN(cc marshal.CopyContext, addr usermem.Addr, limit int) (int, error) {\n", g.r, g.typeName()) g.inIndent(func() { fallback := func() { g.emit("// Type %s doesn't have a packed layout in memory, fall back to MarshalBytes.\n", g.typeName()) - g.emit("buf := task.CopyScratchBuffer(%s.SizeBytes()) // escapes: okay.\n", g.r) + g.emit("buf := cc.CopyScratchBuffer(%s.SizeBytes()) // escapes: okay.\n", g.r) g.emit("%s.MarshalBytes(buf) // escapes: fallback.\n", g.r) - g.emit("return task.CopyOutBytes(addr, buf[:limit]) // escapes: okay.\n") + g.emit("return cc.CopyOutBytes(addr, buf[:limit]) // escapes: okay.\n") } if thisPacked { g.recordUsedImport("reflect") @@ -343,7 +340,7 @@ func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) { // Fast serialization. g.emitCastToByteSlice(g.r, "buf", fmt.Sprintf("%s.SizeBytes()", g.r)) - g.emit("length, err := task.CopyOutBytes(addr, buf[:limit]) // escapes: okay.\n") + g.emit("length, err := cc.CopyOutBytes(addr, buf[:limit]) // escapes: okay.\n") g.emitKeepAlive(g.r) g.emit("return length, err\n") } else { @@ -356,9 +353,9 @@ func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) { g.emit("//go:nosplit\n") g.recordUsedImport("marshal") g.recordUsedImport("usermem") - g.emit("func (%s *%s) CopyOut(task marshal.Task, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) + g.emit("func (%s *%s) CopyOut(cc marshal.CopyContext, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) g.inIndent(func() { - g.emit("return %s.CopyOutN(task, addr, %s.SizeBytes())\n", g.r, g.r) + g.emit("return %s.CopyOutN(cc, addr, %s.SizeBytes())\n", g.r, g.r) }) g.emit("}\n\n") @@ -366,12 +363,12 @@ func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) { g.emit("//go:nosplit\n") g.recordUsedImport("marshal") g.recordUsedImport("usermem") - g.emit("func (%s *%s) CopyIn(task marshal.Task, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) + g.emit("func (%s *%s) CopyIn(cc marshal.CopyContext, addr usermem.Addr) (int, error) {\n", g.r, g.typeName()) g.inIndent(func() { fallback := func() { g.emit("// Type %s doesn't have a packed layout in memory, fall back to UnmarshalBytes.\n", g.typeName()) - g.emit("buf := task.CopyScratchBuffer(%s.SizeBytes()) // escapes: okay.\n", g.r) - g.emit("length, err := task.CopyInBytes(addr, buf) // escapes: okay.\n") + g.emit("buf := cc.CopyScratchBuffer(%s.SizeBytes()) // escapes: okay.\n", g.r) + g.emit("length, err := cc.CopyInBytes(addr, buf) // escapes: okay.\n") g.emit("// Unmarshal unconditionally. If we had a short copy-in, this results in a\n") g.emit("// partially unmarshalled struct.\n") g.emit("%s.UnmarshalBytes(buf) // escapes: fallback.\n", g.r) @@ -389,7 +386,7 @@ func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) { // Fast deserialization. g.emitCastToByteSlice(g.r, "buf", fmt.Sprintf("%s.SizeBytes()", g.r)) - g.emit("length, err := task.CopyInBytes(addr, buf) // escapes: okay.\n") + g.emit("length, err := cc.CopyInBytes(addr, buf) // escapes: okay.\n") g.emitKeepAlive(g.r) g.emit("return length, err\n") } else { @@ -442,7 +439,7 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, g.recordUsedImport("usermem") g.emit("// Copy%sIn copies in a slice of %s objects from the task's memory.\n", slice.ident, g.typeName()) - g.emit("func Copy%sIn(task marshal.Task, addr usermem.Addr, dst []%s) (int, error) {\n", slice.ident, g.typeName()) + g.emit("func Copy%sIn(cc marshal.CopyContext, addr usermem.Addr, dst []%s) (int, error) {\n", slice.ident, g.typeName()) g.inIndent(func() { g.emit("count := len(dst)\n") g.emit("if count == 0 {\n") @@ -454,8 +451,8 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, fallback := func() { g.emit("// Type %s doesn't have a packed layout in memory, fall back to UnmarshalBytes.\n", g.typeName()) - g.emit("buf := task.CopyScratchBuffer(size * count)\n") - g.emit("length, err := task.CopyInBytes(addr, buf)\n\n") + g.emit("buf := cc.CopyScratchBuffer(size * count)\n") + g.emit("length, err := cc.CopyInBytes(addr, buf)\n\n") g.emit("// Unmarshal as much as possible, even on error. First handle full objects.\n") g.emit("limit := length/size\n") @@ -489,7 +486,7 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, // Fast deserialization. g.emitCastSliceToByteSlice("&dst", "buf", "size * count") - g.emit("length, err := task.CopyInBytes(addr, buf)\n") + g.emit("length, err := cc.CopyInBytes(addr, buf)\n") g.emitKeepAlive("dst") g.emit("return length, err\n") } else { @@ -499,7 +496,7 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, g.emit("}\n\n") g.emit("// Copy%sOut copies a slice of %s objects to the task's memory.\n", slice.ident, g.typeName()) - g.emit("func Copy%sOut(task marshal.Task, addr usermem.Addr, src []%s) (int, error) {\n", slice.ident, g.typeName()) + g.emit("func Copy%sOut(cc marshal.CopyContext, addr usermem.Addr, src []%s) (int, error) {\n", slice.ident, g.typeName()) g.inIndent(func() { g.emit("count := len(src)\n") g.emit("if count == 0 {\n") @@ -511,13 +508,13 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, fallback := func() { g.emit("// Type %s doesn't have a packed layout in memory, fall back to MarshalBytes.\n", g.typeName()) - g.emit("buf := task.CopyScratchBuffer(size * count)\n") + g.emit("buf := cc.CopyScratchBuffer(size * count)\n") g.emit("for idx := 0; idx < count; idx++ {\n") g.inIndent(func() { g.emit("src[idx].MarshalBytes(buf[size*idx:size*(idx+1)])\n") }) g.emit("}\n") - g.emit("return task.CopyOutBytes(addr, buf)\n") + g.emit("return cc.CopyOutBytes(addr, buf)\n") } if thisPacked { g.recordUsedImport("reflect") @@ -531,7 +528,7 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, // Fast serialization. g.emitCastSliceToByteSlice("&src", "buf", "size * count") - g.emit("length, err := task.CopyOutBytes(addr, buf)\n") + g.emit("length, err := cc.CopyOutBytes(addr, buf)\n") g.emitKeepAlive("src") g.emit("return length, err\n") } else { diff --git a/tools/go_marshal/gomarshal/util.go b/tools/go_marshal/gomarshal/util.go index d94314302..6a42691cd 100644 --- a/tools/go_marshal/gomarshal/util.go +++ b/tools/go_marshal/gomarshal/util.go @@ -79,7 +79,7 @@ type fieldDispatcher struct { } // Precondition: All dispatch callbacks that will be invoked must be -// provided. Embedded fields are not allowed, len(f.Names) >= 1. +// provided. func (fd fieldDispatcher) dispatch(f *ast.Field) { // Each field declaration may actually be multiple declarations of the same // type. For example, consider: @@ -88,12 +88,24 @@ func (fd fieldDispatcher) dispatch(f *ast.Field) { // x, y, z int // } // - // We invoke the call-backs once per such instance. Embedded fields are not - // allowed, and results in a panic. + // We invoke the call-backs once per such instance. + + // Handle embedded fields. Embedded fields have no names, but can be + // referenced by the type name. if len(f.Names) < 1 { - panic("Precondition not met: attempted to dispatch on embedded field") + switch v := f.Type.(type) { + case *ast.Ident: + fd.primitive(v, v) + case *ast.SelectorExpr: + fd.selector(v.Sel, v.X.(*ast.Ident), v.Sel) + default: + // Note: Arrays can't be embedded, which is handled here. + panic(fmt.Sprintf("Attempted to dispatch on embedded field of unsupported kind: %#v", f.Type)) + } + return } + // Non-embedded field. for _, name := range f.Names { switch v := f.Type.(type) { case *ast.Ident: diff --git a/tools/go_marshal/main.go b/tools/go_marshal/main.go index f74be5c29..6e4a3e8c4 100644 --- a/tools/go_marshal/main.go +++ b/tools/go_marshal/main.go @@ -31,10 +31,11 @@ import ( ) var ( - pkg = flag.String("pkg", "", "output package") - output = flag.String("output", "", "output file") - outputTest = flag.String("output_test", "", "output file for tests") - imports = flag.String("imports", "", "comma-separated list of extra packages to import in generated code") + pkg = flag.String("pkg", "", "output package") + output = flag.String("output", "", "output file") + outputTest = flag.String("output_test", "", "output file for tests") + outputTestUnconditional = flag.String("output_test_unconditional", "", "output file for unconditional tests") + imports = flag.String("imports", "", "comma-separated list of extra packages to import in generated code") ) func main() { @@ -61,7 +62,7 @@ func main() { // as an import. extraImports = strings.Split(*imports, ",") } - g, err := gomarshal.NewGenerator(flag.Args(), *output, *outputTest, *pkg, extraImports) + g, err := gomarshal.NewGenerator(flag.Args(), *output, *outputTest, *outputTestUnconditional, *pkg, extraImports) if err != nil { panic(err) } diff --git a/tools/go_marshal/test/escape/escape.go b/tools/go_marshal/test/escape/escape.go index ff23d87d1..7f62b0a2b 100644 --- a/tools/go_marshal/test/escape/escape.go +++ b/tools/go_marshal/test/escape/escape.go @@ -20,29 +20,29 @@ import ( "gvisor.dev/gvisor/tools/go_marshal/test" ) -// dummyTask implements marshal.Task. -type dummyTask struct { +// dummyCopyContext implements marshal.CopyContext. +type dummyCopyContext struct { } -func (*dummyTask) CopyScratchBuffer(size int) []byte { +func (*dummyCopyContext) CopyScratchBuffer(size int) []byte { return make([]byte, size) } -func (*dummyTask) CopyOutBytes(addr usermem.Addr, b []byte) (int, error) { +func (*dummyCopyContext) CopyOutBytes(addr usermem.Addr, b []byte) (int, error) { return len(b), nil } -func (*dummyTask) CopyInBytes(addr usermem.Addr, b []byte) (int, error) { +func (*dummyCopyContext) CopyInBytes(addr usermem.Addr, b []byte) (int, error) { return len(b), nil } -func (t *dummyTask) MarshalBytes(addr usermem.Addr, marshallable marshal.Marshallable) { +func (t *dummyCopyContext) MarshalBytes(addr usermem.Addr, marshallable marshal.Marshallable) { buf := t.CopyScratchBuffer(marshallable.SizeBytes()) marshallable.MarshalBytes(buf) t.CopyOutBytes(addr, buf) } -func (t *dummyTask) MarshalUnsafe(addr usermem.Addr, marshallable marshal.Marshallable) { +func (t *dummyCopyContext) MarshalUnsafe(addr usermem.Addr, marshallable marshal.Marshallable) { buf := t.CopyScratchBuffer(marshallable.SizeBytes()) marshallable.MarshalUnsafe(buf) t.CopyOutBytes(addr, buf) @@ -50,14 +50,14 @@ func (t *dummyTask) MarshalUnsafe(addr usermem.Addr, marshallable marshal.Marsha // +checkescape:all //go:nosplit -func doCopyIn(t *dummyTask) { +func doCopyIn(t *dummyCopyContext) { var stat test.Stat stat.CopyIn(t, usermem.Addr(0xf000ba12)) } // +checkescape:all //go:nosplit -func doCopyOut(t *dummyTask) { +func doCopyOut(t *dummyCopyContext) { var stat test.Stat stat.CopyOut(t, usermem.Addr(0xf000ba12)) } @@ -65,7 +65,7 @@ func doCopyOut(t *dummyTask) { // +mustescape:builtin // +mustescape:stack //go:nosplit -func doMarshalBytesDirect(t *dummyTask) { +func doMarshalBytesDirect(t *dummyCopyContext) { var stat test.Stat buf := t.CopyScratchBuffer(stat.SizeBytes()) stat.MarshalBytes(buf) @@ -75,7 +75,7 @@ func doMarshalBytesDirect(t *dummyTask) { // +mustescape:builtin // +mustescape:stack //go:nosplit -func doMarshalUnsafeDirect(t *dummyTask) { +func doMarshalUnsafeDirect(t *dummyCopyContext) { var stat test.Stat buf := t.CopyScratchBuffer(stat.SizeBytes()) stat.MarshalUnsafe(buf) @@ -85,7 +85,7 @@ func doMarshalUnsafeDirect(t *dummyTask) { // +mustescape:local,heap // +mustescape:stack //go:nosplit -func doMarshalBytesViaMarshallable(t *dummyTask) { +func doMarshalBytesViaMarshallable(t *dummyCopyContext) { var stat test.Stat t.MarshalBytes(usermem.Addr(0xf000ba12), &stat) } @@ -93,7 +93,7 @@ func doMarshalBytesViaMarshallable(t *dummyTask) { // +mustescape:local,heap // +mustescape:stack //go:nosplit -func doMarshalUnsafeViaMarshallable(t *dummyTask) { +func doMarshalUnsafeViaMarshallable(t *dummyCopyContext) { var stat test.Stat t.MarshalUnsafe(usermem.Addr(0xf000ba12), &stat) } diff --git a/tools/go_marshal/test/marshal_test.go b/tools/go_marshal/test/marshal_test.go index 7c3481ac8..a00f9a684 100644 --- a/tools/go_marshal/test/marshal_test.go +++ b/tools/go_marshal/test/marshal_test.go @@ -36,13 +36,13 @@ import ( var simulatedErr error = syserror.EFAULT -// mockTask implements marshal.Task. -type mockTask struct { +// mockCopyContext implements marshal.CopyContext. +type mockCopyContext struct { taskMem usermem.BytesIO } // populate fills the task memory with the contents of val. -func (t *mockTask) populate(val interface{}) { +func (t *mockCopyContext) populate(val interface{}) { var buf bytes.Buffer // Use binary.Write so we aren't testing go-marshal against its own // potentially buggy implementation. @@ -52,7 +52,7 @@ func (t *mockTask) populate(val interface{}) { t.taskMem.Bytes = buf.Bytes() } -func (t *mockTask) setLimit(n int) { +func (t *mockCopyContext) setLimit(n int) { if len(t.taskMem.Bytes) < n { grown := make([]byte, n) copy(grown, t.taskMem.Bytes) @@ -62,22 +62,22 @@ func (t *mockTask) setLimit(n int) { t.taskMem.Bytes = t.taskMem.Bytes[:n] } -// CopyScratchBuffer implements marshal.Task.CopyScratchBuffer. -func (t *mockTask) CopyScratchBuffer(size int) []byte { +// CopyScratchBuffer implements marshal.CopyContext.CopyScratchBuffer. +func (t *mockCopyContext) CopyScratchBuffer(size int) []byte { return make([]byte, size) } -// CopyOutBytes implements marshal.Task.CopyOutBytes. The implementation +// CopyOutBytes implements marshal.CopyContext.CopyOutBytes. The implementation // completely ignores the target address and stores a copy of b in its // internally buffer, overriding any previous contents. -func (t *mockTask) CopyOutBytes(_ usermem.Addr, b []byte) (int, error) { +func (t *mockCopyContext) CopyOutBytes(_ usermem.Addr, b []byte) (int, error) { return t.taskMem.CopyOut(nil, 0, b, usermem.IOOpts{}) } -// CopyInBytes implements marshal.Task.CopyInBytes. The implementation +// CopyInBytes implements marshal.CopyContext.CopyInBytes. The implementation // completely ignores the source address and always fills b from the begining of // its internal buffer. -func (t *mockTask) CopyInBytes(_ usermem.Addr, b []byte) (int, error) { +func (t *mockCopyContext) CopyInBytes(_ usermem.Addr, b []byte) (int, error) { return t.taskMem.CopyIn(nil, 0, b, usermem.IOOpts{}) } @@ -171,11 +171,11 @@ func compareMemory(t *testing.T, expected, actual []byte, n int) { // dst. The task signals an error at limit bytes during copy-in, which should // result in a truncated unmarshalling. func limitedCopyIn(t *testing.T, src, dst marshal.Marshallable, limit int) { - var task mockTask - task.populate(src) - task.setLimit(limit) + var cc mockCopyContext + cc.populate(src) + cc.setLimit(limit) - n, err := dst.CopyIn(&task, usermem.Addr(0)) + n, err := dst.CopyIn(&cc, usermem.Addr(0)) if n != limit { t.Errorf("CopyIn copied unexpected number of bytes, expected %d, got %d", limit, n) } @@ -202,10 +202,10 @@ func limitedCopyIn(t *testing.T, src, dst marshal.Marshallable, limit int) { // limitedCopyOut marshals src to task memory. The task signals an error at // limit bytes during copy-out, which should result in a truncated marshalling. func limitedCopyOut(t *testing.T, src marshal.Marshallable, limit int) { - var task mockTask - task.setLimit(limit) + var cc mockCopyContext + cc.setLimit(limit) - n, err := src.CopyOut(&task, usermem.Addr(0)) + n, err := src.CopyOut(&cc, usermem.Addr(0)) if n != limit { t.Errorf("CopyOut copied unexpected number of bytes, expected %d, got %d", limit, n) } @@ -215,7 +215,7 @@ func limitedCopyOut(t *testing.T, src marshal.Marshallable, limit int) { expectedMem := unsafeMemory(src) defer runtime.KeepAlive(src) - actualMem := task.taskMem.Bytes + actualMem := cc.taskMem.Bytes compareMemory(t, expectedMem, actualMem, n) } @@ -223,10 +223,10 @@ func limitedCopyOut(t *testing.T, src marshal.Marshallable, limit int) { // copyOutN marshals src to task memory, requesting the marshalling to be // limited to limit bytes. func copyOutN(t *testing.T, src marshal.Marshallable, limit int) { - var task mockTask - task.setLimit(limit) + var cc mockCopyContext + cc.setLimit(limit) - n, err := src.CopyOutN(&task, usermem.Addr(0), limit) + n, err := src.CopyOutN(&cc, usermem.Addr(0), limit) if err != nil { t.Errorf("CopyOut returned unexpected error: %v", err) } @@ -236,7 +236,7 @@ func copyOutN(t *testing.T, src marshal.Marshallable, limit int) { expectedMem := unsafeMemory(src) defer runtime.KeepAlive(src) - actualMem := task.taskMem.Bytes + actualMem := cc.taskMem.Bytes t.Logf("Expected: %v + %v\n", expectedMem[:n], expectedMem[n:]) t.Logf("Actual : %v + %v\n", actualMem[:n], actualMem[n:]) @@ -303,20 +303,20 @@ func TestLimitedMarshalling(t *testing.T) { func TestLimitedSliceMarshalling(t *testing.T) { types := []struct { arrayPtrType reflect.Type - copySliceIn func(task marshal.Task, addr usermem.Addr, dstSlice interface{}) (int, error) - copySliceOut func(task marshal.Task, addr usermem.Addr, srcSlice interface{}) (int, error) + copySliceIn func(cc marshal.CopyContext, addr usermem.Addr, dstSlice interface{}) (int, error) + copySliceOut func(cc marshal.CopyContext, addr usermem.Addr, srcSlice interface{}) (int, error) unsafeMemory func(arrPtr interface{}) []byte }{ // Packed types. { reflect.TypeOf((*[20]test.Stat)(nil)), - func(task marshal.Task, addr usermem.Addr, dst interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, dst interface{}) (int, error) { slice := dst.(*[20]test.Stat)[:] - return test.CopyStatSliceIn(task, addr, slice) + return test.CopyStatSliceIn(cc, addr, slice) }, - func(task marshal.Task, addr usermem.Addr, src interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, src interface{}) (int, error) { slice := src.(*[20]test.Stat)[:] - return test.CopyStatSliceOut(task, addr, slice) + return test.CopyStatSliceOut(cc, addr, slice) }, func(a interface{}) []byte { slice := a.(*[20]test.Stat)[:] @@ -325,13 +325,13 @@ func TestLimitedSliceMarshalling(t *testing.T) { }, { reflect.TypeOf((*[1]test.Stat)(nil)), - func(task marshal.Task, addr usermem.Addr, dst interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, dst interface{}) (int, error) { slice := dst.(*[1]test.Stat)[:] - return test.CopyStatSliceIn(task, addr, slice) + return test.CopyStatSliceIn(cc, addr, slice) }, - func(task marshal.Task, addr usermem.Addr, src interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, src interface{}) (int, error) { slice := src.(*[1]test.Stat)[:] - return test.CopyStatSliceOut(task, addr, slice) + return test.CopyStatSliceOut(cc, addr, slice) }, func(a interface{}) []byte { slice := a.(*[1]test.Stat)[:] @@ -340,13 +340,13 @@ func TestLimitedSliceMarshalling(t *testing.T) { }, { reflect.TypeOf((*[5]test.SignalSetAlias)(nil)), - func(task marshal.Task, addr usermem.Addr, dst interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, dst interface{}) (int, error) { slice := dst.(*[5]test.SignalSetAlias)[:] - return test.CopySignalSetAliasSliceIn(task, addr, slice) + return test.CopySignalSetAliasSliceIn(cc, addr, slice) }, - func(task marshal.Task, addr usermem.Addr, src interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, src interface{}) (int, error) { slice := src.(*[5]test.SignalSetAlias)[:] - return test.CopySignalSetAliasSliceOut(task, addr, slice) + return test.CopySignalSetAliasSliceOut(cc, addr, slice) }, func(a interface{}) []byte { slice := a.(*[5]test.SignalSetAlias)[:] @@ -356,13 +356,13 @@ func TestLimitedSliceMarshalling(t *testing.T) { // Non-packed types. { reflect.TypeOf((*[20]test.Type1)(nil)), - func(task marshal.Task, addr usermem.Addr, dst interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, dst interface{}) (int, error) { slice := dst.(*[20]test.Type1)[:] - return test.CopyType1SliceIn(task, addr, slice) + return test.CopyType1SliceIn(cc, addr, slice) }, - func(task marshal.Task, addr usermem.Addr, src interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, src interface{}) (int, error) { slice := src.(*[20]test.Type1)[:] - return test.CopyType1SliceOut(task, addr, slice) + return test.CopyType1SliceOut(cc, addr, slice) }, func(a interface{}) []byte { slice := a.(*[20]test.Type1)[:] @@ -371,13 +371,13 @@ func TestLimitedSliceMarshalling(t *testing.T) { }, { reflect.TypeOf((*[1]test.Type1)(nil)), - func(task marshal.Task, addr usermem.Addr, dst interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, dst interface{}) (int, error) { slice := dst.(*[1]test.Type1)[:] - return test.CopyType1SliceIn(task, addr, slice) + return test.CopyType1SliceIn(cc, addr, slice) }, - func(task marshal.Task, addr usermem.Addr, src interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, src interface{}) (int, error) { slice := src.(*[1]test.Type1)[:] - return test.CopyType1SliceOut(task, addr, slice) + return test.CopyType1SliceOut(cc, addr, slice) }, func(a interface{}) []byte { slice := a.(*[1]test.Type1)[:] @@ -386,13 +386,13 @@ func TestLimitedSliceMarshalling(t *testing.T) { }, { reflect.TypeOf((*[7]test.Type8)(nil)), - func(task marshal.Task, addr usermem.Addr, dst interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, dst interface{}) (int, error) { slice := dst.(*[7]test.Type8)[:] - return test.CopyType8SliceIn(task, addr, slice) + return test.CopyType8SliceIn(cc, addr, slice) }, - func(task marshal.Task, addr usermem.Addr, src interface{}) (int, error) { + func(cc marshal.CopyContext, addr usermem.Addr, src interface{}) (int, error) { slice := src.(*[7]test.Type8)[:] - return test.CopyType8SliceOut(task, addr, slice) + return test.CopyType8SliceOut(cc, addr, slice) }, func(a interface{}) []byte { slice := a.(*[7]test.Type8)[:] @@ -439,11 +439,11 @@ func TestLimitedSliceMarshalling(t *testing.T) { limit += elem.SizeBytes() / 2 analysis.RandomizeValue(expected) - var task mockTask - task.populate(expected) - task.setLimit(limit) + var cc mockCopyContext + cc.populate(expected) + cc.setLimit(limit) - n, err := tt.copySliceIn(&task, usermem.Addr(0), actual) + n, err := tt.copySliceIn(&cc, usermem.Addr(0), actual) if n != limit { t.Errorf("CopyIn copied unexpected number of bytes, expected %d, got %d", limit, n) } @@ -493,11 +493,11 @@ func TestLimitedSliceMarshalling(t *testing.T) { limit += elem.SizeBytes() / 2 analysis.RandomizeValue(expected) - var task mockTask - task.populate(expected) - task.setLimit(limit) + var cc mockCopyContext + cc.populate(expected) + cc.setLimit(limit) - n, err := tt.copySliceOut(&task, usermem.Addr(0), expected) + n, err := tt.copySliceOut(&cc, usermem.Addr(0), expected) if n != limit { t.Errorf("CopyIn copied unexpected number of bytes, expected %d, got %d", limit, n) } @@ -507,7 +507,7 @@ func TestLimitedSliceMarshalling(t *testing.T) { expectedMem := tt.unsafeMemory(expected) defer runtime.KeepAlive(expected) - actualMem := task.taskMem.Bytes + actualMem := cc.taskMem.Bytes compareMemory(t, expectedMem, actualMem, n) }) diff --git a/tools/go_marshal/test/test.go b/tools/go_marshal/test/test.go index f75ca1b7f..d9e9f341b 100644 --- a/tools/go_marshal/test/test.go +++ b/tools/go_marshal/test/test.go @@ -174,3 +174,27 @@ type Type9 struct { x int64 y [sizeA]int32 } + +// Type10Embed is a test data type which is be embedded into another type. +// +// +marshal +type Type10Embed struct { + x int64 +} + +// Type10 is a test data type which contains an embedded struct. +// +// +marshal +type Type10 struct { + Type10Embed + y int64 +} + +// Type11 is a test data type which contains an embedded struct from an external +// package. +// +// +marshal +type Type11 struct { + ex.External + y int64 +} |