From 06c41924236c0b3287b17ed251d87f6976f90e57 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 30 Sep 2013 12:39:54 -0400 Subject: [PATCH] go.tools/pointer: minor API simplifications. Details: - Warnings are reported as values in Result, not a callback in Config. - remove TODO to eliminate Print callback. It's better than the alternative. - remove unused Config.root field. - hang Result off analysis object (impl. detail) - reword TODO. R=crawshaw CC=golang-dev https://golang.org/cl/14128043 --- oracle/oracle.go | 21 ++++++--------------- pointer/TODO | 13 +++++-------- pointer/analysis.go | 22 +++++++--------------- pointer/api.go | 34 ++++++++++++++-------------------- pointer/gen.go | 2 +- pointer/pointer_test.go | 18 ++++++------------ 6 files changed, 39 insertions(+), 71 deletions(-) diff --git a/oracle/oracle.go b/oracle/oracle.go index 3c0a7510..6477f18c 100644 --- a/oracle/oracle.go +++ b/oracle/oracle.go @@ -99,12 +99,6 @@ type queryResult interface { display(printf printfFunc) } -type warning struct { - pos token.Pos - format string - args []interface{} -} - // A QueryPos represents the position provided as input to a query: // a textual extent in the program's source code, the AST node it // corresponds to, and the package to which it belongs. @@ -121,9 +115,9 @@ type Result struct { fset *token.FileSet // fprintf is a closure over the oracle's fileset and start/end position. fprintf func(w io.Writer, pos interface{}, format string, args ...interface{}) - q queryResult // the query-specific result - mode string // query mode - warnings []warning // pointer analysis warnings + q queryResult // the query-specific result + mode string // query mode + warnings []pointer.Warning // pointer analysis warnings } // Serial returns an instance of serial.Result, which implements the @@ -135,8 +129,8 @@ func (res *Result) Serial() *serial.Result { res.q.toSerial(resj, res.fset) for _, w := range res.warnings { resj.Warnings = append(resj.Warnings, serial.PTAWarning{ - Pos: res.fset.Position(w.pos).String(), - Message: fmt.Sprintf(w.format, w.args...), + Pos: res.fset.Position(w.Pos).String(), + Message: w.Message, }) } return resj @@ -302,9 +296,6 @@ func (o *Oracle) query(minfo *modeInfo, qpos *QueryPos) (*Result, error) { fset: o.prog.Fset, fprintf: o.fprintf, // captures o.prog, o.{start,end}Pos for later printing } - o.config.Warn = func(pos token.Pos, format string, args ...interface{}) { - res.warnings = append(res.warnings, warning{pos, format, args}) - } var err error res.q, err = minfo.impl(o, qpos) if err != nil { @@ -342,7 +333,7 @@ func (res *Result) WriteTo(out io.Writer) { if res.warnings != nil { fmt.Fprintln(out, "\nPointer analysis warnings:") for _, w := range res.warnings { - printf(w.pos, "warning: "+w.format, w.args...) + printf(w.Pos, "warning: "+w.Message) } } } diff --git a/pointer/TODO b/pointer/TODO index 10afd49a..d288c48f 100644 --- a/pointer/TODO +++ b/pointer/TODO @@ -16,13 +16,11 @@ CONSTRAINT GENERATION: A downside is that we can't keep the identity field of struct allocations that identifies the object. -PRESOLVER OPTIMISATIONS -- use HVN, HRU, LE, PE, HCD, LCD. - -SOLVER: -- use BDDs and/or sparse bitvectors for ptsets +OPTIMISATIONS +- pre-solver: PE and LE via HVN/HRU. +- solver: HCD, LCD. +- use sparse bitvectors for ptsets - use sparse bitvectors for graph edges -- use BDD-based relational composition for e.g. offset computations. - experiment with different worklist algorithms: priority queue (solver visit-time order) red-black tree (node id order) @@ -34,9 +32,8 @@ SOLVER: API: - Some optimisations (e.g. LE, PE) may change the API. Think about them sooner rather than later. -- Eliminate Print probe now that we can query specific ssa.Values. -Misc: +MISC: - os.Args should point to something; currently they don't. - Test on all platforms. Currently we assume these go/build tags: linux, amd64, !cgo. diff --git a/pointer/analysis.go b/pointer/analysis.go index 75933b88..7d14e814 100644 --- a/pointer/analysis.go +++ b/pointer/analysis.go @@ -183,7 +183,7 @@ type analysis struct { localval map[ssa.Value]nodeid // node for each local ssa.Value localobj map[ssa.Value]nodeid // maps v to sole member of pts(v), if singleton work worklist // solver's worklist - queries map[ssa.Value][]Pointer // same as Results.Queries + result *Result // results of the analysis // Reflection: hasher typemap.Hasher // cache of type hashes @@ -221,13 +221,7 @@ func (a *analysis) labelFor(id nodeid) *Label { } func (a *analysis) warnf(pos token.Pos, format string, args ...interface{}) { - if Warn := a.config.Warn; Warn != nil { - Warn(pos, format, args...) - } else { - fmt.Fprintf(os.Stderr, "%s: warning: ", a.prog.Fset.Position(pos)) - fmt.Fprintf(os.Stderr, format, args...) - fmt.Fprintln(os.Stderr) - } + a.result.Warnings = append(a.result.Warnings, Warning{pos, fmt.Sprintf(format, args...)}) } // Analyze runs the pointer analysis with the scope and options @@ -245,7 +239,9 @@ func Analyze(config *Config) *Result { intrinsics: make(map[*ssa.Function]intrinsic), probes: make(map[*ssa.CallCommon]nodeid), work: makeMapWorklist(), - queries: make(map[ssa.Value][]Pointer), + result: &Result{ + Queries: make(map[ssa.Value][]Pointer), + }, } if false { @@ -318,13 +314,9 @@ func Analyze(config *Config) *Result { } } - var callgraph *cgraph if a.config.BuildCallGraph { - callgraph = &cgraph{root, a.cgnodes} + a.result.CallGraph = &cgraph{root, a.cgnodes} } - return &Result{ - CallGraph: callgraph, - Queries: a.queries, - } + return a.result } diff --git a/pointer/api.go b/pointer/api.go index 06fb6146..583d4a16 100644 --- a/pointer/api.go +++ b/pointer/api.go @@ -14,12 +14,12 @@ import ( "code.google.com/p/go.tools/ssa" ) +// A Config formulates a pointer analysis problem for Analyze(). type Config struct { - // -------- Scope of the analysis -------- - - // Clients must provide the analysis with at least one package defining a main() function. - Mains []*ssa.Package // set of 'main' packages to analyze - root *ssa.Function // synthetic analysis root + // Mains contains the set of 'main' packages to analyze + // Clients must provide the analysis with at least one + // package defining a main() function. + Mains []*ssa.Package // Reflection determines whether to handle reflection // operators soundly, which is currently rather slow since it @@ -32,25 +32,15 @@ type Config struct { // If enabled, the graph will be available in Result.CallGraph. BuildCallGraph bool - // -------- Optional callbacks invoked by the analysis -------- - - // Warn is invoked for each warning encountered by the analysis, - // e.g. unknown external function, unsound use of unsafe.Pointer. - // pos may be zero if the position is not known. - Warn func(pos token.Pos, format string, args ...interface{}) - // Print is invoked during the analysis for each discovered - // call to the built-in print(x). + // call to the built-in print(x), providing a convenient way + // to identify arbitrary expressions of interest in the tests. // // Pointer p may be saved until the analysis is complete, at // which point its methods provide access to the analysis // (The result of callings its methods within the Print // callback is undefined.) p is nil if x is non-pointerlike. // - // TODO(adonovan): this was a stop-gap measure for identifing - // arbitrary expressions of interest in the tests. Now that - // ssa.ValueForExpr exists, we should use that instead. - // Print func(site *ssa.CallCommon, p Pointer) // The client populates Queries[v] for each ssa.Value v of @@ -73,9 +63,7 @@ type Config struct { // Queries map[ssa.Value]Indirect - // -------- Other configuration options -------- - - // If Log is non-nil, a log messages are written to it. + // If Log is non-nil, log messages are written to it. // Logging is extremely verbose. Log io.Writer } @@ -89,6 +77,11 @@ func (c *Config) prog() *ssa.Program { panic("empty scope") } +type Warning struct { + Pos token.Pos + Message string +} + // A Result contains the results of a pointer analysis. // // See Config for how to request the various Result components. @@ -96,6 +89,7 @@ func (c *Config) prog() *ssa.Program { type Result struct { CallGraph call.Graph // discovered call graph Queries map[ssa.Value][]Pointer // points-to sets for queried ssa.Values + Warnings []Warning // warnings of unsoundness } // A Pointer is an equivalence class of pointerlike values. diff --git a/pointer/gen.go b/pointer/gen.go index f62cd265..8dd52187 100644 --- a/pointer/gen.go +++ b/pointer/gen.go @@ -87,7 +87,7 @@ func (a *analysis) setValueNode(v ssa.Value, id nodeid, cgn *cgnode) { a.genLoad(cgn, tmp, v, 0, a.sizeof(v.Type())) id = tmp } - a.queries[v] = append(a.queries[v], ptr{a, cgn, id}) + a.result.Queries[v] = append(a.result.Queries[v], ptr{a, cgn, id}) } } diff --git a/pointer/pointer_test.go b/pointer/pointer_test.go index 7f4a500c..80f68ae8 100644 --- a/pointer/pointer_test.go +++ b/pointer/pointer_test.go @@ -285,7 +285,6 @@ func doOneInput(input, filename string) bool { } var probes []probe - var warnings []string var log bytes.Buffer // Run the analysis. @@ -297,11 +296,6 @@ func doOneInput(input, filename string) bool { Print: func(site *ssa.CallCommon, p pointer.Pointer) { probes = append(probes, probe{site, p}) }, - Warn: func(pos token.Pos, format string, args ...interface{}) { - msg := fmt.Sprintf(format, args...) - fmt.Printf("%s: warning: %s\n", prog.Fset.Position(pos), msg) - warnings = append(warnings, msg) - }, } result := pointer.Analyze(config) @@ -346,7 +340,7 @@ func doOneInput(input, filename string) bool { } case "warning": - if !checkWarningExpectation(prog, e, warnings) { + if !checkWarningExpectation(prog, e, result.Warnings) { ok = false } } @@ -493,7 +487,7 @@ func checkCallsExpectation(prog *ssa.Program, e *expectation, callgraph call.Gra return false } -func checkWarningExpectation(prog *ssa.Program, e *expectation, warnings []string) bool { +func checkWarningExpectation(prog *ssa.Program, e *expectation, warnings []pointer.Warning) bool { // TODO(adonovan): check the position part of the warning too? re, err := regexp.Compile(e.args[0]) if err != nil { @@ -506,15 +500,15 @@ func checkWarningExpectation(prog *ssa.Program, e *expectation, warnings []strin return false } - for _, warning := range warnings { - if re.MatchString(warning) { + for _, w := range warnings { + if re.MatchString(w.Message) { return true } } e.errorf("@warning %s expectation not satised; found these warnings though:", strconv.Quote(e.args[0])) - for _, warning := range warnings { - fmt.Println("\t", warning) + for _, w := range warnings { + fmt.Printf("%s: warning: %s\n", prog.Fset.Position(w.Pos), w.Message) } return false }