]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/vet: check argument types in printf formats
authorRob Pike <r@golang.org>
Sat, 23 Feb 2013 23:08:36 +0000 (15:08 -0800)
committerRob Pike <r@golang.org>
Sat, 23 Feb 2013 23:08:36 +0000 (15:08 -0800)
Fixes #4404.

R=gri, rsc
CC=golang-dev
https://golang.org/cl/7378061

src/cmd/vet/Makefile
src/cmd/vet/atomic.go
src/cmd/vet/main.go
src/cmd/vet/print.go
src/cmd/vet/print_unsafe.go [new file with mode: 0644]

index 2cdf96261fbb10b49ca0bfa990614a537681df44..c0e316998903a2fa5055b33ab47dde7e226cf76f 100644 (file)
@@ -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
 
index 9c7ae7dbfcb996738904f829278ac306e9604e1c..0abc6f524135cda1003b87ece21e160ccafc4375 100644 (file)
@@ -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") {
index 0fe26f87259e6197a0789162aac607b8b21ff920..a00b299ad4a06f9a4a674a7dfb38f02f51655425 100644 (file)
@@ -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,
index 007bb3f0f4f33e5b7c738f2bed58133c1d6ab3ad..b164a9b5884dd2442398771304afd8b0793659aa 100644 (file)
@@ -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 (file)
index 0000000..1446b92
--- /dev/null
@@ -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)
+}