Skip to content

Commit 5de2e5b

Browse files
committed
cmd/vet: add vet runner script for core
This CL adds a script to run vet on std and cmd. There are a considerable number of false positives, mostly from legitimate but unusual assembly in the runtime and reflect packages. There are also a few false positives that need fixes. They are noted as such in the whitelists; they're not worth holding this CL for. The unsafe pointer check is disabled. The false positive rate is just too high to be worth it. There is no cmd/dist/test integration yet. The tentative plan is that we'll check the local platform during all.bash, and that there'll be a fast builder that checks all platforms (to cover platforms that can't exec). Fixes #11041 Change-Id: Iee4ed32b05447888368ed86088e3ed3771f84442 Reviewed-on: https://go-review.googlesource.com/27811 Reviewed-by: Rob Pike <[email protected]>
1 parent 4cf95fd commit 5de2e5b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+853
-0
lines changed

src/cmd/vet/all/main.go

+320
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,320 @@
1+
// Copyright 2016 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// +build ignore
6+
7+
// The vet/all command runs go vet on the standard library and commands.
8+
// It compares the output against a set of whitelists
9+
// maintained in the whitelist directory.
10+
package main
11+
12+
import (
13+
"bufio"
14+
"bytes"
15+
"flag"
16+
"fmt"
17+
"go/build"
18+
"internal/testenv"
19+
"log"
20+
"os"
21+
"os/exec"
22+
"path/filepath"
23+
"runtime"
24+
"strings"
25+
)
26+
27+
var (
28+
flagPlatforms = flag.String("p", "", "platform(s) to use e.g. linux/amd64,darwin/386")
29+
flagAll = flag.Bool("all", false, "run all platforms")
30+
flagNoLines = flag.Bool("n", false, "don't print line numbers")
31+
)
32+
33+
var cmdGoPath string
34+
35+
func main() {
36+
log.SetPrefix("vet/all: ")
37+
log.SetFlags(0)
38+
39+
var err error
40+
cmdGoPath, err = testenv.GoTool()
41+
if err != nil {
42+
log.Print("could not find cmd/go; skipping")
43+
// We're on a platform that can't run cmd/go.
44+
// We want this script to be able to run as part of all.bash,
45+
// so return cleanly rather than with exit code 1.
46+
return
47+
}
48+
49+
flag.Parse()
50+
switch {
51+
case *flagAll && *flagPlatforms != "":
52+
log.Print("-all and -p flags are incompatible")
53+
flag.Usage()
54+
os.Exit(2)
55+
case *flagPlatforms != "":
56+
vetPlatforms(parseFlagPlatforms())
57+
case *flagAll:
58+
vetPlatforms(allPlatforms())
59+
default:
60+
host := platform{os: build.Default.GOOS, arch: build.Default.GOARCH}
61+
host.vet()
62+
}
63+
}
64+
65+
func allPlatforms() []platform {
66+
var pp []platform
67+
cmd := exec.Command(cmdGoPath, "tool", "dist", "list")
68+
out, err := cmd.Output()
69+
if err != nil {
70+
log.Fatal(err)
71+
}
72+
lines := bytes.Split(out, []byte{'\n'})
73+
for _, line := range lines {
74+
if len(line) == 0 {
75+
continue
76+
}
77+
pp = append(pp, parsePlatform(string(line)))
78+
}
79+
return pp
80+
}
81+
82+
func parseFlagPlatforms() []platform {
83+
var pp []platform
84+
components := strings.Split(*flagPlatforms, ",")
85+
for _, c := range components {
86+
pp = append(pp, parsePlatform(c))
87+
}
88+
return pp
89+
}
90+
91+
func parsePlatform(s string) platform {
92+
vv := strings.Split(s, "/")
93+
if len(vv) != 2 {
94+
log.Fatalf("could not parse platform %s, must be of form goos/goarch", s)
95+
}
96+
return platform{os: vv[0], arch: vv[1]}
97+
}
98+
99+
type whitelist map[string]int
100+
101+
// load adds entries from the whitelist file, if present, for os/arch to w.
102+
func (w whitelist) load(goos string, goarch string) {
103+
// Look up whether goarch is a 32-bit or 64-bit architecture.
104+
archbits, ok := nbits[goarch]
105+
if !ok {
106+
log.Fatal("unknown bitwidth for arch %q", goarch)
107+
}
108+
109+
// Look up whether goarch has a shared arch suffix,
110+
// such as mips64x for mips64 and mips64le.
111+
archsuff := goarch
112+
if x, ok := archAsmX[goarch]; ok {
113+
archsuff = x
114+
}
115+
116+
// Load whitelists.
117+
filenames := []string{
118+
"all.txt",
119+
goos + ".txt",
120+
goarch + ".txt",
121+
goos + "_" + goarch + ".txt",
122+
fmt.Sprintf("%dbit.txt", archbits),
123+
}
124+
if goarch != archsuff {
125+
filenames = append(filenames,
126+
archsuff+".txt",
127+
goos+"_"+archsuff+".txt",
128+
)
129+
}
130+
131+
// We allow error message templates using GOOS and GOARCH.
132+
if goos == "android" {
133+
goos = "linux" // so many special cases :(
134+
}
135+
136+
// Read whitelists and do template substitution.
137+
replace := strings.NewReplacer("GOOS", goos, "GOARCH", goarch, "ARCHSUFF", archsuff)
138+
139+
for _, filename := range filenames {
140+
path := filepath.Join("whitelist", filename)
141+
f, err := os.Open(path)
142+
if err != nil {
143+
// Allow not-exist errors; not all combinations have whitelists.
144+
if os.IsNotExist(err) {
145+
continue
146+
}
147+
log.Fatal(err)
148+
}
149+
scan := bufio.NewScanner(f)
150+
for scan.Scan() {
151+
line := scan.Text()
152+
if len(line) == 0 || strings.HasPrefix(line, "//") {
153+
continue
154+
}
155+
w[replace.Replace(line)]++
156+
}
157+
if err := scan.Err(); err != nil {
158+
log.Fatal(err)
159+
}
160+
}
161+
}
162+
163+
type platform struct {
164+
os string
165+
arch string
166+
}
167+
168+
func (p platform) String() string {
169+
return p.os + "/" + p.arch
170+
}
171+
172+
// ignorePathPrefixes are file path prefixes that should be ignored wholesale.
173+
var ignorePathPrefixes = [...]string{
174+
// These testdata dirs have lots of intentionally broken/bad code for tests.
175+
"cmd/go/testdata/",
176+
"cmd/vet/testdata/",
177+
"go/printer/testdata/",
178+
// cmd/compile/internal/big is a vendored copy of math/big.
179+
// Ignore it so that we only have to deal with math/big issues once.
180+
"cmd/compile/internal/big/",
181+
}
182+
183+
func vetPlatforms(pp []platform) {
184+
for _, p := range pp {
185+
p.vet()
186+
}
187+
}
188+
189+
func (p platform) vet() {
190+
if p.arch == "s390x" {
191+
// TODO: reinstate when s390x gets vet support (issue 15454)
192+
return
193+
}
194+
fmt.Printf("go run main.go -p %s\n", p)
195+
196+
// Load whitelist(s).
197+
w := make(whitelist)
198+
w.load(p.os, p.arch)
199+
200+
env := append(os.Environ(), "GOOS="+p.os, "GOARCH="+p.arch)
201+
202+
// Do 'go install std' before running vet.
203+
// It is cheap when already installed.
204+
// Not installing leads to non-obvious failures due to inability to typecheck.
205+
// TODO: If go/loader ever makes it to the standard library, have vet use it,
206+
// at which point vet can work off source rather than compiled packages.
207+
cmd := exec.Command(cmdGoPath, "install", "std")
208+
cmd.Env = env
209+
out, err := cmd.CombinedOutput()
210+
if err != nil {
211+
log.Fatalf("failed to run GOOS=%s GOARCH=%s 'go install std': %v\n%s", p.os, p.arch, err, out)
212+
}
213+
214+
// 'go tool vet .' is considerably faster than 'go vet ./...'
215+
// TODO: The unsafeptr checks are disabled for now,
216+
// because there are so many false positives,
217+
// and no clear way to improve vet to eliminate large chunks of them.
218+
// And having them in the whitelists will just cause annoyance
219+
// and churn when working on the runtime.
220+
cmd = exec.Command(cmdGoPath, "tool", "vet", "-unsafeptr=false", ".")
221+
cmd.Dir = filepath.Join(runtime.GOROOT(), "src")
222+
cmd.Env = env
223+
stderr, err := cmd.StderrPipe()
224+
if err != nil {
225+
log.Fatal(err)
226+
}
227+
if err := cmd.Start(); err != nil {
228+
log.Fatal(err)
229+
}
230+
231+
// Process vet output.
232+
scan := bufio.NewScanner(stderr)
233+
NextLine:
234+
for scan.Scan() {
235+
line := scan.Text()
236+
if strings.HasPrefix(line, "vet: ") {
237+
// Typecheck failure: Malformed syntax or multiple packages or the like.
238+
// This will yield nicer error messages elsewhere, so ignore them here.
239+
continue
240+
}
241+
242+
fields := strings.SplitN(line, ":", 3)
243+
var file, lineno, msg string
244+
switch len(fields) {
245+
case 2:
246+
// vet message with no line number
247+
file, msg = fields[0], fields[1]
248+
case 3:
249+
file, lineno, msg = fields[0], fields[1], fields[2]
250+
default:
251+
log.Fatalf("could not parse vet output line:\n%s", line)
252+
}
253+
msg = strings.TrimSpace(msg)
254+
255+
for _, ignore := range ignorePathPrefixes {
256+
if strings.HasPrefix(file, filepath.FromSlash(ignore)) {
257+
continue NextLine
258+
}
259+
}
260+
261+
key := file + ": " + msg
262+
if w[key] == 0 {
263+
// Vet error with no match in the whitelist. Print it.
264+
if *flagNoLines {
265+
fmt.Printf("%s: %s\n", file, msg)
266+
} else {
267+
fmt.Printf("%s:%s: %s\n", file, lineno, msg)
268+
}
269+
continue
270+
}
271+
w[key]--
272+
}
273+
if scan.Err() != nil {
274+
log.Fatalf("failed to scan vet output: %v", scan.Err())
275+
}
276+
err = cmd.Wait()
277+
// We expect vet to fail.
278+
// Make sure it has failed appropriately, though (for example, not a PathError).
279+
if _, ok := err.(*exec.ExitError); !ok {
280+
log.Fatalf("unexpected go vet execution failure: %v", err)
281+
}
282+
printedHeader := false
283+
if len(w) > 0 {
284+
for k, v := range w {
285+
if v != 0 {
286+
if !printedHeader {
287+
fmt.Println("unmatched whitelist entries:")
288+
printedHeader = true
289+
}
290+
for i := 0; i < v; i++ {
291+
fmt.Println(k)
292+
}
293+
}
294+
}
295+
}
296+
}
297+
298+
// nbits maps from architecture names to the number of bits in a pointer.
299+
// TODO: figure out a clean way to avoid get this info rather than listing it here yet again.
300+
var nbits = map[string]int{
301+
"386": 32,
302+
"amd64": 64,
303+
"amd64p32": 32,
304+
"arm": 32,
305+
"arm64": 64,
306+
"mips64": 64,
307+
"mips64le": 64,
308+
"ppc64": 64,
309+
"ppc64le": 64,
310+
}
311+
312+
// archAsmX maps architectures to the suffix usually used for their assembly files,
313+
// if different than the arch name itself.
314+
var archAsmX = map[string]string{
315+
"android": "linux",
316+
"mips64": "mips64x",
317+
"mips64le": "mips64x",
318+
"ppc64": "ppc64x",
319+
"ppc64le": "ppc64x",
320+
}

src/cmd/vet/all/whitelist/386.txt

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// 386-specific vet whitelist. See readme.txt for details.
2+
3+
runtime/asm_ARCHSUFF.s: [GOARCH] cannot check cross-package assembly function: Compare is in package bytes
4+
5+
// reflect trampolines intentionally omit arg size. Same for morestack.
6+
reflect/asm_386.s: [386] makeFuncStub: use of 4(SP) points beyond argument frame
7+
reflect/asm_386.s: [386] methodValueCall: use of 4(SP) points beyond argument frame
8+
runtime/asm_386.s: [386] morestack: use of 4(SP) points beyond argument frame
9+
runtime/asm_386.s: [386] morestack: use of 8(SP) points beyond argument frame
10+
runtime/asm_386.s: [386] morestack: use of 4(SP) points beyond argument frame
11+
12+
// Intentionally missing declarations. These are special assembly routines.
13+
runtime/asm_386.s: [386] ldt0setup: function ldt0setup missing Go declaration
14+
runtime/asm_386.s: [386] emptyfunc: function emptyfunc missing Go declaration
15+
runtime/asm_386.s: [386] aeshashbody: function aeshashbody missing Go declaration
16+
runtime/asm_386.s: [386] memeqbody: function memeqbody missing Go declaration
17+
runtime/asm_386.s: [386] cmpbody: function cmpbody missing Go declaration
18+
runtime/asm_386.s: [386] addmoduledata: function addmoduledata missing Go declaration
19+
runtime/duff_386.s: [386] duffzero: function duffzero missing Go declaration
20+
runtime/duff_386.s: [386] duffcopy: function duffcopy missing Go declaration
21+
22+
runtime/asm_386.s: [386] uint32tofloat64: function uint32tofloat64 missing Go declaration
23+
runtime/asm_386.s: [386] float64touint32: function float64touint32 missing Go declaration
24+
25+
runtime/asm_386.s: [386] stackcheck: function stackcheck missing Go declaration
26+
27+
// Clearer using FP than SP, but that requires named offsets.
28+
runtime/asm_386.s: [386] rt0_go: unknown variable argc
29+
runtime/asm_386.s: [386] rt0_go: unknown variable argv

src/cmd/vet/all/whitelist/64bit.txt

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// 64-bit-specific vet whitelist. See readme.txt for details.
2+
3+
// False positives.
4+
5+
// Clever const tricks outwit the "large shift" check.
6+
runtime/hashmap.go: hash might be too small for shift of 56
7+
runtime/hashmap.go: hash might be too small for shift of 56
8+
runtime/hashmap.go: hash might be too small for shift of 56
9+
runtime/hashmap.go: hash might be too small for shift of 56
10+
runtime/hashmap.go: hash might be too small for shift of 56
11+
runtime/hashmap.go: hash might be too small for shift of 56
12+
runtime/hashmap_fast.go: hash might be too small for shift of 56
13+
runtime/hashmap_fast.go: hash might be too small for shift of 56

0 commit comments

Comments
 (0)