From 7aabe2e113a04e686f56d4639ff64a2862fbbe02 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 14 Oct 2013 14:08:23 -0400 Subject: [PATCH] go.tools/ssa: build a separate Function for each init() func. Before, we would concatenate all the init() blocks together, resulting in incorrect treatment of a recovered panic in one init block: the implicit return would cause the subsequent ones to be skipped. The result is simpler, and closer to what gc does. The additional functions are visible in the call graph, so some tests required updating. R=gri CC=crawshaw, golang-dev https://golang.org/cl/14671044 --- importer/source_test.go | 5 +- oracle/testdata/src/main/calls.go | 7 +- oracle/testdata/src/main/calls.golden | 7 +- ssa/builder.go | 127 ++++++++++---------------- ssa/create.go | 6 +- ssa/interp/testdata/coverage.go | 23 ++--- ssa/source.go | 11 +++ ssa/ssa.go | 3 +- 8 files changed, 80 insertions(+), 109 deletions(-) diff --git a/importer/source_test.go b/importer/source_test.go index 161d8584..791b05bf 100644 --- a/importer/source_test.go +++ b/importer/source_test.go @@ -197,6 +197,7 @@ func TestPathEnclosingInterval_Paths(t *testing.T) { // -------- Tests of source.go ----------------------------------------- +// TODO(adonovan): move this test into ssa. func TestEnclosingFunction(t *testing.T) { tests := []struct { input string // the input file @@ -227,12 +228,12 @@ func TestEnclosingFunction(t *testing.T) { // No code for constants: {"package main; const a = 500", "500", "(none)"}, // Explicit init() - {"package main; func init() { println(600) }", "600", "main.init"}, + {"package main; func init() { println(600) }", "600", "main.init$1"}, // Multiple explicit init functions: {`package main func init() { println("foo") } func init() { println(800) }`, - "800", "main.init"}, + "800", "main.init$2"}, // init() containing FuncLit. {`package main func init() { println(func(){print(900)}) }`, diff --git a/oracle/testdata/src/main/calls.go b/oracle/testdata/src/main/calls.go index aa7e3ef9..93974aee 100644 --- a/oracle/testdata/src/main/calls.go +++ b/oracle/testdata/src/main/calls.go @@ -78,9 +78,10 @@ func deadcode() { // This code belongs to init. var global = 123 // @callers callers-global "global" -// init may be called by other packages' inits, or in this case, the -// root of the callgraph. +// The package initializer may be called by other packages' inits, or +// in this case, the root of the callgraph. The source-level init functions +// are in turn called by it. func init() { - // @callers callers-init "^" + // @callstack callstack-init "^" } diff --git a/oracle/testdata/src/main/calls.golden b/oracle/testdata/src/main/calls.golden index 8ba18e8d..20194370 100644 --- a/oracle/testdata/src/main/calls.golden +++ b/oracle/testdata/src/main/calls.golden @@ -96,7 +96,8 @@ main.deadcode is unreachable in this analysis scope main.init is called from these 1 sites: the root of the call graph --------- @callers callers-init -------- -main.init is called from these 1 sites: -the root of the call graph +-------- @callstack callstack-init -------- +Found a call path from root to main.init$1 +main.init$1 +static function call from main.init diff --git a/ssa/builder.go b/ssa/builder.go index ff544a68..de59c295 100644 --- a/ssa/builder.go +++ b/ssa/builder.go @@ -2065,24 +2065,6 @@ start: fn.emit(&v) case *ast.ReturnStmt: - if fn == fn.Pkg.init { - // A "return" within an init block is treated - // like a "goto" to the next init block. We - // use the outermost BREAK target for this purpose. - var block *BasicBlock - for t := fn.targets; t != nil; t = t.tail { - if t._break != nil { - block = t._break - } - } - // Run function calls deferred in this init - // block when explicitly returning from it. - fn.emit(new(RunDefers)) - emitJump(fn, block) - fn.currentBlock = fn.newBasicBlock("unreachable") - return - } - var results []Value if len(s.Results) == 1 && fn.Signature.Results().Len() > 1 { // Return of one expression in a multi-valued function. @@ -2243,61 +2225,43 @@ func (b *builder) buildFunction(fn *Function) { fn.finishBody() } -// buildInit emits to init any initialization code needed for -// declaration decl, causing SSA-building of any functions or methods -// it references transitively. -// -func (b *builder) buildInit(init *Function, decl ast.Decl) { - switch decl := decl.(type) { - case *ast.GenDecl: - if decl.Tok == token.VAR { - for _, spec := range decl.Specs { - b.globalValueSpec(init, spec.(*ast.ValueSpec), nil, nil) - } - } - - case *ast.FuncDecl: - if decl.Recv == nil && decl.Name.Name == "init" { - // init() block - if init.Prog.mode&LogSource != 0 { - fmt.Fprintln(os.Stderr, "build init block @", - init.Prog.Fset.Position(decl.Pos())) - } - - // A return statement within an init block is - // treated like a "goto" to the the next init - // block, which we stuff in the outermost - // break label. - next := init.newBasicBlock("init.next") - init.targets = &targets{ - tail: init.targets, - _break: next, - } - b.stmt(init, decl.Body) - // Run function calls deferred in this init - // block when falling off the end of the block. - init.emit(new(RunDefers)) - emitJump(init, next) - init.targets = init.targets.tail - init.currentBlock = next - } - } -} - // buildFuncDecl builds SSA code for the function or method declared // by decl in package pkg. // func (b *builder) buildFuncDecl(pkg *Package, decl *ast.FuncDecl) { id := decl.Name if isBlankIdent(id) { - // TODO(gri): workaround for missing object, - // see e.g. $GOROOT/test/blank.go:27. - return + return // discard } + var fn *Function if decl.Recv == nil && id.Name == "init" { - return // init() block: already done in pass 1 + if pkg.Prog.mode&LogSource != 0 { + fmt.Fprintln(os.Stderr, "build init func @", + pkg.Prog.Fset.Position(decl.Pos())) + } + + pkg.ninit++ + fn = &Function{ + name: fmt.Sprintf("init$%d", pkg.ninit), + Signature: new(types.Signature), + pos: decl.Name.NamePos, + Pkg: pkg, + Prog: pkg.Prog, + syntax: &funcSyntax{ + functype: decl.Type, + recvField: decl.Recv, + body: decl.Body, + }, + } + + var v Call + v.Call.Value = fn + v.setType(types.NewTuple()) + pkg.init.emit(&v) + } else { + fn = pkg.values[pkg.objectOf(id)].(*Function) } - b.buildFunction(pkg.values[pkg.objectOf(id)].(*Function)) + b.buildFunction(fn) } // BuildAll calls Package.Build() for each package in prog. @@ -2368,14 +2332,27 @@ func (p *Package) Build() { nTo1Vars: make(map[*ast.ValueSpec]bool), } - // Pass 1: visit the package's var decls and init funcs in - // source order, causing init() code to be generated in - // topological order. We visit package-level vars - // transitively through functions and methods, building them - // as we go. + // Pass 1: visit the package's var decls and in source order, + // causing init() code to be generated in topological order. + // We visit package-level vars transitively through functions + // and methods, building them as we go. for _, file := range p.info.Files { for _, decl := range file.Decls { - b.buildInit(init, decl) + if decl, ok := decl.(*ast.GenDecl); ok && decl.Tok == token.VAR { + for _, spec := range decl.Specs { + b.globalValueSpec(init, spec.(*ast.ValueSpec), nil, nil) + } + } + } + } + + // Pass 2: build all package-level functions, init functions + // and methods in source order, including unreachable/blank ones. + for _, file := range p.info.Files { + for _, decl := range file.Decls { + if decl, ok := decl.(*ast.FuncDecl); ok { + b.buildFuncDecl(p, decl) + } } } @@ -2386,16 +2363,6 @@ func (p *Package) Build() { init.emit(new(Return)) init.finishBody() - // Pass 2: build all remaining package-level functions and - // methods in source order, including unreachable/blank ones. - for _, file := range p.info.Files { - for _, decl := range file.Decls { - if decl, ok := decl.(*ast.FuncDecl); ok { - b.buildFuncDecl(p, decl) - } - } - } - p.info = nil // We no longer need ASTs or go/types deductions. } diff --git a/ssa/create.go b/ssa/create.go index fb6de474..1b14629c 100644 --- a/ssa/create.go +++ b/ssa/create.go @@ -162,11 +162,7 @@ func membersFromDecl(pkg *Package, decl ast.Decl) { case *ast.FuncDecl: id := decl.Name if decl.Recv == nil && id.Name == "init" { - if !pkg.init.pos.IsValid() { - pkg.init.pos = decl.Name.Pos() - pkg.init.Synthetic = "" - } - return // init blocks aren't functions + return // no object } if !isBlankIdent(id) { memberFromObject(pkg, pkg.objectOf(id), decl) diff --git a/ssa/interp/testdata/coverage.go b/ssa/interp/testdata/coverage.go index 56c30315..a78b834f 100644 --- a/ssa/interp/testdata/coverage.go +++ b/ssa/interp/testdata/coverage.go @@ -379,15 +379,14 @@ func init() { // An I->I type-assert fails iff the value is nil. func init() { - // TODO(adonovan): temporarily disabled; see comment at bottom of file. - // defer func() { - // r := fmt.Sprint(recover()) - // if r != "interface conversion: interface is nil, not main.I" { - // panic("I->I type assertion succeeed for nil value") - // } - // }() - // var x I - // _ = x.(I) + defer func() { + r := fmt.Sprint(recover()) + if r != "interface conversion: interface is nil, not main.I" { + panic("I->I type assertion succeeed for nil value") + } + }() + var x I + _ = x.(I) } ////////////////////////////////////////////////////////////////////// @@ -536,9 +535,3 @@ func init() { }() _ = i == j // interface comparison recurses on types } - -// TODO(adonovan): fix: the interpreter doesn't correctly implement -// defer/recover in an init function concatenated from many parts: the -// first recover causes the entire init() to return, not jump to the -// next part. This will be fixed in a follow-up CL. Until then, -// beware: adding new init() functions here will have no effect! diff --git a/ssa/source.go b/ssa/source.go index 8f539128..fd690722 100644 --- a/ssa/source.go +++ b/ssa/source.go @@ -86,6 +86,17 @@ func findEnclosingPackageLevelFunction(pkg *Package, path []ast.Node) *Function case *ast.FuncDecl: if decl.Recv == nil && decl.Name.Name == "init" { // Explicit init() function. + for _, b := range pkg.init.Blocks { + for _, instr := range b.Instrs { + if instr, ok := instr.(*Call); ok { + if callee, ok := instr.Call.Value.(*Function); ok && callee.Pkg == pkg && callee.Pos() == decl.Name.NamePos { + return callee + } + } + } + } + // Hack: return non-nil when SSA is not yet + // built so that HasEnclosingFunction works. return pkg.init } // Declared function/method. diff --git a/ssa/ssa.go b/ssa/ssa.go index 6100f214..ad81ee8e 100644 --- a/ssa/ssa.go +++ b/ssa/ssa.go @@ -44,12 +44,13 @@ type Package struct { Object *types.Package // the type checker's package object for this package Members map[string]Member // all package members keyed by name values map[types.Object]Value // package-level vars & funcs (incl. methods), keyed by object - init *Function // Func("init"); the package's (concatenated) init function + init *Function // Func("init"); the package's init function debug bool // include full debug info in this package. // The following fields are set transiently, then cleared // after building. started int32 // atomically tested and set at start of build phase + ninit int32 // number of init functions info *importer.PackageInfo // package ASTs and type information }