cmd/vet: check argument types in printf formats

Fixes #4404.

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

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
 

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") {

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,

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.

src/cmd/vet/print_unsafe.go

+// 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)
+}