From: Rob Pike Date: Sat, 23 Feb 2013 23:08:36 +0000 (-0800) Subject: cmd/vet: check argument types in printf formats X-Git-Tag: go1.1rc2~879 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=72b6daa3ba7a5842c07724cb49f41f178e1af778;p=gostls13.git cmd/vet: check argument types in printf formats Fixes #4404. R=gri, rsc CC=golang-dev https://golang.org/cl/7378061 --- diff --git a/src/cmd/vet/Makefile b/src/cmd/vet/Makefile index 2cdf96261f..c0e3169989 100644 --- a/src/cmd/vet/Makefile +++ b/src/cmd/vet/Makefile @@ -3,6 +3,6 @@ # license that can be found in the LICENSE file. test testshort: - go build + go build -tags unsafe ../../../test/errchk ./vet -printfuncs='Warn:1,Warnf:1' *.go diff --git a/src/cmd/vet/atomic.go b/src/cmd/vet/atomic.go index 9c7ae7dbfc..0abc6f5241 100644 --- a/src/cmd/vet/atomic.go +++ b/src/cmd/vet/atomic.go @@ -10,7 +10,7 @@ import ( "sync/atomic" ) -// checkAtomicAssignment walks the assignment statement checking for comomon +// checkAtomicAssignment walks the assignment statement checking for common // mistaken usage of atomic package, such as: x = atomic.AddUint64(&x, 1) func (f *File) checkAtomicAssignment(n *ast.AssignStmt) { if !vet("atomic") { diff --git a/src/cmd/vet/main.go b/src/cmd/vet/main.go index 0fe26f8725..a00b299ad4 100644 --- a/src/cmd/vet/main.go +++ b/src/cmd/vet/main.go @@ -160,7 +160,8 @@ func doPackageDir(directory string) { } type Package struct { - types map[ast.Expr]types.Type + types map[ast.Expr]types.Type + values map[ast.Expr]interface{} } // doPackage analyzes the single package constructed from the named files. @@ -188,8 +189,12 @@ func doPackage(names []string) { } pkg := new(Package) pkg.types = make(map[ast.Expr]types.Type) + pkg.values = make(map[ast.Expr]interface{}) exprFn := func(x ast.Expr, typ types.Type, val interface{}) { pkg.types[x] = typ + if val != nil { + pkg.values[x] = val + } } context := types.Context{ Expr: exprFn, diff --git a/src/cmd/vet/print.go b/src/cmd/vet/print.go index 007bb3f0f4..b164a9b588 100644 --- a/src/cmd/vet/print.go +++ b/src/cmd/vet/print.go @@ -77,8 +77,11 @@ func (f *File) literal(value ast.Expr) *ast.BasicLit { x, errX := strconv.Unquote(litX.Value) y, errY := strconv.Unquote(litY.Value) if errX == nil && errY == nil { - lit.Value = strconv.Quote(x + y) - return &lit + return &ast.BasicLit{ + ValuePos: lit.ValuePos, + Kind: lit.Kind, + Value: strconv.Quote(x + y), + } } } case *ast.Ident: @@ -104,13 +107,12 @@ func (f *File) literal(value ast.Expr) *ast.BasicLit { } // checkPrintf checks a call to a formatted print routine such as Printf. -// The skip argument records how many arguments to ignore; that is, -// call.Args[skip] is (well, should be) the format argument. -func (f *File) checkPrintf(call *ast.CallExpr, name string, skip int) { - if len(call.Args) <= skip { +// call.Args[formatIndex] is (well, should be) the format argument. +func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) { + if formatIndex >= len(call.Args) { return } - lit := f.literal(call.Args[skip]) + lit := f.literal(call.Args[formatIndex]) if lit == nil { if *verbose { f.Warn(call.Pos(), "can't check non-literal format in call to", name) @@ -122,60 +124,69 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string, skip int) { } format, err := strconv.Unquote(lit.Value) if err != nil { + // Shouldn't happen if parser returned no errors, but be safe. f.Badf(call.Pos(), "invalid quoted string literal") } + firstArg := formatIndex + 1 // Arguments are immediately after format string. if !strings.Contains(format, "%") { - if len(call.Args) > skip+1 { + if len(call.Args) > firstArg { f.Badf(call.Pos(), "no formatting directive in %s call", name) } return } // Hard part: check formats against args. - // Trivial but useful test: count. - numArgs := 0 + argNum := firstArg for i, w := 0, 0; i < len(format); i += w { w = 1 if format[i] == '%' { - nbytes, nargs := f.parsePrintfVerb(call, format[i:]) + verb, flags, nbytes, nargs := f.parsePrintfVerb(call, format[i:]) w = nbytes - numArgs += nargs + if verb == '%' { // "%%" does nothing interesting. + continue + } + // If we've run out of args, print after loop will pick that up. + if argNum+nargs <= len(call.Args) { + f.checkPrintfArg(call, verb, flags, argNum, nargs) + } + argNum += nargs } } - expect := len(call.Args) - (skip + 1) - // Don't be too strict on dotdotdot. - if call.Ellipsis.IsValid() && numArgs >= expect { + // TODO: Dotdotdot is hard. + if call.Ellipsis.IsValid() && argNum != len(call.Args) { return } - if numArgs != expect { - f.Badf(call.Pos(), "wrong number of args in %s call: %d needed but %d args", name, numArgs, expect) + if argNum != len(call.Args) { + expect := argNum - firstArg + numArgs := len(call.Args) - firstArg + f.Badf(call.Pos(), "wrong number of args for format in %s call: %d needed but %d args", name, expect, numArgs) } } -// parsePrintfVerb returns the number of bytes and number of arguments -// consumed by the Printf directive that begins s, including its percent sign -// and verb. -func (f *File) parsePrintfVerb(call *ast.CallExpr, s string) (nbytes, nargs int) { +// parsePrintfVerb returns the verb that begins the format string, along with its flags, +// the number of bytes to advance the format to step past the verb, and number of +// arguments it consumes. +func (f *File) parsePrintfVerb(call *ast.CallExpr, format string) (verb rune, flags []byte, nbytes, nargs int) { // There's guaranteed a percent sign. - flags := make([]byte, 0, 5) + flags = make([]byte, 0, 5) nbytes = 1 - end := len(s) + end := len(format) // There may be flags. FlagLoop: for nbytes < end { - switch s[nbytes] { + switch format[nbytes] { case '#', '0', '+', '-', ' ': - flags = append(flags, s[nbytes]) + flags = append(flags, format[nbytes]) nbytes++ default: break FlagLoop } } getNum := func() { - if nbytes < end && s[nbytes] == '*' { + if nbytes < end && format[nbytes] == '*' { nbytes++ nargs++ } else { - for nbytes < end && '0' <= s[nbytes] && s[nbytes] <= '9' { + for nbytes < end && '0' <= format[nbytes] && format[nbytes] <= '9' { nbytes++ } } @@ -183,24 +194,38 @@ FlagLoop: // There may be a width. getNum() // If there's a period, there may be a precision. - if nbytes < end && s[nbytes] == '.' { + if nbytes < end && format[nbytes] == '.' { flags = append(flags, '.') // Treat precision as a flag. nbytes++ getNum() } // Now a verb. - c, w := utf8.DecodeRuneInString(s[nbytes:]) + c, w := utf8.DecodeRuneInString(format[nbytes:]) nbytes += w + verb = c if c != '%' { nargs++ - f.checkPrintfVerb(call, c, flags) } return } +// printfArgType encodes the types of expressions a printf verb accepts. It is a bitmask. +type printfArgType int + +const ( + argBool printfArgType = 1 << iota + argInt + argRune + argString + argFloat + argPointer + anyType printfArgType = ^0 +) + type printVerb struct { verb rune flags string // known flags are all ASCII + typ printfArgType } // Common flag sets for printf verbs. @@ -219,36 +244,52 @@ var printVerbs = []printVerb{ // '+' is required sign for numbers, Go format for %v. // '#' is alternate format for several verbs. // ' ' is spacer for numbers - {'b', numFlag}, - {'c', "-"}, - {'d', numFlag}, - {'e', numFlag}, - {'E', numFlag}, - {'f', numFlag}, - {'F', numFlag}, - {'g', numFlag}, - {'G', numFlag}, - {'o', sharpNumFlag}, - {'p', "-#"}, - {'q', " -+.0#"}, - {'s', " -+.0"}, - {'t', "-"}, - {'T', "-"}, - {'U', "-#"}, - {'v', allFlags}, - {'x', sharpNumFlag}, - {'X', sharpNumFlag}, + {'b', numFlag, argInt}, + {'c', "-", argRune | argInt}, + {'d', numFlag, argInt}, + {'e', numFlag, argFloat}, + {'E', numFlag, argFloat}, + {'f', numFlag, argFloat}, + {'F', numFlag, argFloat}, + {'g', numFlag, argFloat}, + {'G', numFlag, argFloat}, + {'o', sharpNumFlag, argInt}, + {'p', "-#", argPointer}, + {'q', " -+.0#", argRune | argInt | argString}, + {'s', " -+.0", argString}, + {'t', "-", argBool}, + {'T', "-", anyType}, + {'U', "-#", argRune | argInt}, + {'v', allFlags, anyType}, + {'x', sharpNumFlag, argRune | argInt | argString}, + {'X', sharpNumFlag, argRune | argInt | argString}, } const printfVerbs = "bcdeEfFgGopqstTvxUX" -func (f *File) checkPrintfVerb(call *ast.CallExpr, verb rune, flags []byte) { +func (f *File) checkPrintfArg(call *ast.CallExpr, verb rune, flags []byte, argNum, nargs int) { // Linear scan is fast enough for a small list. for _, v := range printVerbs { if v.verb == verb { for _, flag := range flags { if !strings.ContainsRune(v.flags, rune(flag)) { f.Badf(call.Pos(), "unrecognized printf flag for verb %q: %q", verb, flag) + return + } + } + // Verb is good. If nargs>1, we have something like %.*s and all but the final + // arg must be integer. + for i := 0; i < nargs-1; i++ { + if !f.matchArgType(argInt, call.Args[argNum+i]) { + f.Badf(call.Pos(), "arg for * in printf format not of type int") + } + } + for _, v := range printVerbs { + if v.verb == verb { + if !f.matchArgType(v.typ, call.Args[argNum+nargs-1]) { + f.Badf(call.Pos(), "arg for printf verb %%%c of wrong type", verb) + } + break } } return @@ -257,15 +298,65 @@ func (f *File) checkPrintfVerb(call *ast.CallExpr, verb rune, flags []byte) { f.Badf(call.Pos(), "unrecognized printf verb %q", verb) } +func (f *File) matchArgType(t printfArgType, arg ast.Expr) bool { + if f.pkg == nil { + return true // Don't know; assume OK. + } + // TODO: for now, we can only test builtin types and untyped constants. + typ := f.pkg.types[arg] + if typ == nil { + return true + } + basic, ok := typ.(*types.Basic) + if !ok { + return true + } + switch basic.Kind { + case types.Bool: + return t&argBool != 0 + case types.Int, types.Int8, types.Int16, types.Int32, types.Int64: + fallthrough + case types.Uint, types.Uint8, types.Uint16, types.Uint32, types.Uint64, types.Uintptr: + return t&argInt != 0 + case types.Float32, types.Float64, types.Complex64, types.Complex128: + return t&argFloat != 0 + case types.String: + return t&argString != 0 + case types.UnsafePointer: + return t&argPointer != 0 + case types.UntypedBool: + return t&argBool != 0 + case types.UntypedComplex: + return t&argFloat != 0 + case types.UntypedFloat: + // If it's integral, we can use an int format. + switch f.pkg.values[arg].(type) { + case int, int8, int16, int32, int64: + return t&(argInt|argFloat) != 0 + case uint, uint8, uint16, uint32, uint64: + return t&(argInt|argFloat) != 0 + } + return t&argFloat != 0 + case types.UntypedInt: + return t&(argInt|argFloat) != 0 // You might say Printf("%g", 1234) + case types.UntypedRune: + return t&(argInt|argRune) != 0 + case types.UntypedString: + return t&argString != 0 + case types.UntypedNil: + return t&argPointer != 0 // TODO? + } + return false +} + // checkPrint checks a call to an unformatted print routine such as Println. -// The skip argument records how many arguments to ignore; that is, -// call.Args[skip] is the first argument to be printed. -func (f *File) checkPrint(call *ast.CallExpr, name string, skip int) { +// call.Args[firstArg] is the first argument to be printed. +func (f *File) checkPrint(call *ast.CallExpr, name string, firstArg int) { isLn := strings.HasSuffix(name, "ln") isF := strings.HasPrefix(name, "F") args := call.Args // check for Println(os.Stderr, ...) - if skip == 0 && !isF && len(args) > 0 { + if firstArg == 0 && !isF && len(args) > 0 { if sel, ok := args[0].(*ast.SelectorExpr); ok { if x, ok := sel.X.(*ast.Ident); ok { if x.Name == "os" && strings.HasPrefix(sel.Sel.Name, "Std") { @@ -274,7 +365,7 @@ func (f *File) checkPrint(call *ast.CallExpr, name string, skip int) { } } } - if len(args) <= skip { + if len(args) <= firstArg { // If we have a call to a method called Error that satisfies the Error interface, // then it's ok. Otherwise it's something like (*T).Error from the testing package // and we need to check it. @@ -284,13 +375,13 @@ func (f *File) checkPrint(call *ast.CallExpr, name string, skip int) { // If it's an Error call now, it's probably for printing errors. if !isLn { // Check the signature to be sure: there are niladic functions called "error". - if f.pkg == nil || skip != 0 || f.numArgsInSignature(call) != skip { + if f.pkg == nil || firstArg != 0 || f.numArgsInSignature(call) != firstArg { f.Badf(call.Pos(), "no args in %s call", name) } } return } - arg := args[skip] + arg := args[firstArg] if lit, ok := arg.(*ast.BasicLit); ok && lit.Kind == token.STRING { if strings.Contains(lit.Value, "%") { f.Badf(call.Pos(), "possible formatting directive in %s call", name) @@ -399,15 +490,66 @@ func (errorTest5) error() { // niladic; don't complain if no args (was bug) // This function never executes, but it serves as a simple test for the program. // Test with make test. func BadFunctionUsedInTests() { + var b bool + var i int + var r rune + var s string + var x float64 + var p *int + // Some good format/argtypes + fmt.Printf("") + fmt.Printf("%b %b", 3, i) + fmt.Printf("%c %c %c %c", 3, i, 'x', r) + fmt.Printf("%d %d", 3, i) + fmt.Printf("%e %e %e", 3, 3e9, x) + fmt.Printf("%E %E %E", 3, 3e9, x) + fmt.Printf("%f %f %f", 3, 3e9, x) + fmt.Printf("%F %F %F", 3, 3e9, x) + fmt.Printf("%g %g %g", 3, 3e9, x) + fmt.Printf("%G %G %G", 3, 3e9, x) + fmt.Printf("%o %o", 3, i) + fmt.Printf("%p %p", p, nil) + fmt.Printf("%q %q %q %q", 3, i, 'x', r) + fmt.Printf("%s %s", "hi", s) + fmt.Printf("%t %t", true, b) + fmt.Printf("%T %T", 3, i) + fmt.Printf("%U %U", 3, i) + fmt.Printf("%v %v", 3, i) + fmt.Printf("%x %x %x %x", 3, i, "hi", s) + fmt.Printf("%X %X %X %X", 3, i, "hi", s) + fmt.Printf("%.*s %d %g", 3, "hi", 23, 2.3) + // Some bad format/argTypes + fmt.Printf("%b", 2.3) // ERROR "arg for printf verb %b of wrong type" + fmt.Printf("%c", 2.3) // ERROR "arg for printf verb %c of wrong type" + fmt.Printf("%d", 2.3) // ERROR "arg for printf verb %d of wrong type" + fmt.Printf("%e", "hi") // ERROR "arg for printf verb %e of wrong type" + fmt.Printf("%E", true) // ERROR "arg for printf verb %E of wrong type" + fmt.Printf("%f", "hi") // ERROR "arg for printf verb %f of wrong type" + fmt.Printf("%F", 'x') // ERROR "arg for printf verb %F of wrong type" + fmt.Printf("%g", "hi") // ERROR "arg for printf verb %g of wrong type" + fmt.Printf("%G", i) // ERROR "arg for printf verb %G of wrong type" + fmt.Printf("%o", x) // ERROR "arg for printf verb %o of wrong type" + fmt.Printf("%p", 23) // ERROR "arg for printf verb %p of wrong type" + fmt.Printf("%q", x) // ERROR "arg for printf verb %q of wrong type" + fmt.Printf("%s", b) // ERROR "arg for printf verb %s of wrong type" + fmt.Printf("%t", 23) // ERROR "arg for printf verb %t of wrong type" + fmt.Printf("%U", x) // ERROR "arg for printf verb %U of wrong type" + fmt.Printf("%x", nil) // ERROR "arg for printf verb %x of wrong type" + fmt.Printf("%X", 2.3) // ERROR "arg for printf verb %X of wrong type" + fmt.Printf("%.*s %d %g", 3, "hi", 23, 'x') // ERROR "arg for printf verb %g of wrong type" + // TODO fmt.Println() // not an error fmt.Println("%s", "hi") // ERROR "possible formatting directive in Println call" - fmt.Printf("%s", "hi", 3) // ERROR "wrong number of args in Printf call" - fmt.Printf("%"+("s"), "hi", 3) // ERROR "wrong number of args in Printf call" + fmt.Printf("%s", "hi", 3) // ERROR "wrong number of args for format in Printf call" + fmt.Printf("%"+("s"), "hi", 3) // ERROR "wrong number of args for format in Printf call" fmt.Printf("%s%%%d", "hi", 3) // correct fmt.Printf("%08s", "woo") // correct fmt.Printf("% 8s", "woo") // correct fmt.Printf("%.*d", 3, 3) // correct - fmt.Printf("%.*d", 3, 3, 3) // ERROR "wrong number of args in Printf call" + fmt.Printf("%.*d", 3, 3, 3) // ERROR "wrong number of args for format in Printf call" + fmt.Printf("%.*d", "hi", 3) // ERROR "arg for \* in printf format not of type int" + fmt.Printf("%.*d", i, 3) // correct + fmt.Printf("%.*d", s, 3) // ERROR "arg for \* in printf format not of type int" fmt.Printf("%q %q", multi()...) // ok fmt.Printf("%#q", `blah`) // ok printf("now is the time", "buddy") // ERROR "no formatting directive" @@ -415,10 +557,10 @@ func BadFunctionUsedInTests() { Printf("hi") // ok const format = "%s %s\n" Printf(format, "hi", "there") - Printf(format, "hi") // ERROR "wrong number of args in Printf call" + Printf(format, "hi") // ERROR "wrong number of args for format in Printf call" f := new(File) f.Warn(0, "%s", "hello", 3) // ERROR "possible formatting directive in Warn call" - f.Warnf(0, "%s", "hello", 3) // ERROR "wrong number of args in Warnf call" + f.Warnf(0, "%s", "hello", 3) // ERROR "wrong number of args for format in Warnf call" f.Warnf(0, "%r", "hello") // ERROR "unrecognized printf verb" f.Warnf(0, "%#s", "hello") // ERROR "unrecognized printf flag" // Something that satisfies the error interface. diff --git a/src/cmd/vet/print_unsafe.go b/src/cmd/vet/print_unsafe.go new file mode 100644 index 0000000000..1446b927dc --- /dev/null +++ b/src/cmd/vet/print_unsafe.go @@ -0,0 +1,19 @@ +// Copyright 2013 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build unsafe + +// This file contains a special test for the printf-checker that tests unsafe.Pointer. + +package main + +import ( + "fmt" + "unsafe" // just for test case printing unsafe.Pointer +) + +func UnsafePointerPrintfTest() { + var up *unsafe.Pointer + fmt.Printf("%p", up) +}