]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/vet/all: use x/tools/go/analysis/cmd/vet not cmd/vet
authorAlan Donovan <adonovan@google.com>
Mon, 12 Nov 2018 20:48:46 +0000 (15:48 -0500)
committerAlan Donovan <adonovan@google.com>
Thu, 15 Nov 2018 18:46:33 +0000 (18:46 +0000)
cmd/vet/all applies vet to all packages in the standard tree.
It is run for every configuration using this command:
  GO_BUILDER_NAME=misc-vetall go tool dist test
by the misc-vetall builder (see chart at build.golang.org).

Ideally we would switch to 'go vet', but it effectively does a partial
build. This means that its analysis has accurate type information, so
it reports slightly fewer spurious diagnostics. However, it is more
than twice as slow.

Instead, cmd/vet/all builds and runs
golang.org/x/tools/go/analysis/cmd/vet, which uses x/tools/go/packages
to load the entire std lib from source. It takes about 4min to run all
OS/ARCH pairs. An important consequence is that golang.org/x/tools
must be on your $GOPATH to run cmd/vet/all. The test has been
temporarily modified to warn and skip if this is not the case.

This is a preparatory step for switching to the new
cmd/vet based on vet-lite.

Whitelist changes:
- The two "deadcode" diagnostics removed from the whitelist were due
  to if-conditions that could now be proven false.
- The asmdecl warnings are now printed with the log.Printf prefix,
  so they are discarded by the parser and needn't be whitelisted.

Change-Id: I6486508b0de2cd947c897523af086a408cbaf4a8
Reviewed-on: https://go-review.googlesource.com/c/149097
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

15 files changed:
src/cmd/vet/all/main.go
src/cmd/vet/all/whitelist/386.txt
src/cmd/vet/all/whitelist/all.txt
src/cmd/vet/all/whitelist/amd64.txt
src/cmd/vet/all/whitelist/arm.txt
src/cmd/vet/all/whitelist/arm64.txt
src/cmd/vet/all/whitelist/mipsx.txt
src/cmd/vet/all/whitelist/nacl_386.txt
src/cmd/vet/all/whitelist/nacl_amd64p32.txt
src/cmd/vet/all/whitelist/nacl_arm.txt
src/cmd/vet/all/whitelist/ppc64x.txt
src/cmd/vet/all/whitelist/s390x.txt
src/cmd/vet/all/whitelist/wasm.txt
src/cmd/vet/all/whitelist/windows_386.txt
src/cmd/vet/all/whitelist/windows_amd64.txt

index 7e4a68101f045786501776336325b17d6b01a39f..71915ed9f12135e334aa9f8f75ee0de2b591461e 100644 (file)
@@ -7,6 +7,9 @@
 // The vet/all command runs go vet on the standard library and commands.
 // It compares the output against a set of whitelists
 // maintained in the whitelist directory.
+//
+// This program attempts to build packages from golang.org/x/tools,
+// which must be in your GOPATH.
 package main
 
 import (
@@ -18,6 +21,7 @@ import (
        "go/types"
        "internal/testenv"
        "io"
+       "io/ioutil"
        "log"
        "os"
        "os/exec"
@@ -217,13 +221,40 @@ func (p platform) vet() {
        w := make(whitelist)
        w.load(p.os, p.arch)
 
-       // 'go tool vet .' is considerably faster than 'go vet ./...'
+       tmpdir, err := ioutil.TempDir("", "cmd-vet-all")
+       if err != nil {
+               log.Fatal(err)
+       }
+       defer os.RemoveAll(tmpdir)
+
+       // Build the go/packages-based vet command from the x/tools
+       // repo. It is considerably faster than "go vet", which rebuilds
+       // the standard library.
+       vetTool := filepath.Join(tmpdir, "vet")
+       cmd := exec.Command(cmdGoPath, "build", "-o", vetTool, "golang.org/x/tools/go/analysis/cmd/vet")
+       cmd.Dir = filepath.Join(runtime.GOROOT(), "src")
+       cmd.Stderr = os.Stderr
+       cmd.Stdout = os.Stderr
+       if err := cmd.Run(); err != nil {
+               if _, err := build.Default.Import("golang.org/x/tools/go/analysis/cmd/vet", "", 0); err != nil {
+                       fmt.Printf("skipping because golang.org/x/tools is not in GOPATH")
+                       return
+               }
+               log.Fatal(err)
+       }
+
        // TODO: The unsafeptr checks are disabled for now,
        // because there are so many false positives,
        // and no clear way to improve vet to eliminate large chunks of them.
        // And having them in the whitelists will just cause annoyance
        // and churn when working on the runtime.
-       cmd := exec.Command(cmdGoPath, "tool", "vet", "-unsafeptr=false", "-source", ".")
+       cmd = exec.Command(vetTool,
+               "-unsafeptr=0",
+               "-nilness=0", // expensive, uses SSA
+               "std",
+               "cmd/...",
+               "cmd/compile/internal/gc/testdata",
+       )
        cmd.Dir = filepath.Join(runtime.GOROOT(), "src")
        cmd.Env = append(os.Environ(), "GOOS="+p.os, "GOARCH="+p.arch, "CGO_ENABLED=0")
        stderr, err := cmd.StderrPipe()
@@ -243,6 +274,9 @@ NextLine:
                if strings.HasPrefix(line, "vet: ") {
                        // Typecheck failure: Malformed syntax or multiple packages or the like.
                        // This will yield nicer error messages elsewhere, so ignore them here.
+
+                       // This includes warnings from asmdecl of the form:
+                       //   "vet: foo.s:16: [amd64] cannot check cross-package assembly function"
                        continue
                }
 
@@ -254,22 +288,48 @@ NextLine:
                        io.Copy(os.Stderr, stderr)
                        break
                }
+               if strings.HasPrefix(line, "# ") {
+                       // 'go vet' prefixes the output of each vet invocation by a comment:
+                       //    # [package]
+                       continue
+               }
 
-               fields := strings.SplitN(line, ":", 3)
+               // Parse line.
+               // Assume the part before the first ": "
+               // is the "file:line:col: " information.
+               // TODO(adonovan): parse vet -json output.
                var file, lineno, msg string
-               switch len(fields) {
-               case 2:
-                       // vet message with no line number
-                       file, msg = fields[0], fields[1]
-               case 3:
-                       file, lineno, msg = fields[0], fields[1], fields[2]
-               default:
+               if i := strings.Index(line, ": "); i >= 0 {
+                       msg = line[i+len(": "):]
+
+                       words := strings.Split(line[:i], ":")
+                       switch len(words) {
+                       case 3:
+                               _ = words[2] // ignore column
+                               fallthrough
+                       case 2:
+                               lineno = words[1]
+                               fallthrough
+                       case 1:
+                               file = words[0]
+
+                               // Make the file name relative to GOROOT/src.
+                               if rel, err := filepath.Rel(cmd.Dir, file); err == nil {
+                                       file = rel
+                               }
+                       default:
+                               // error: too many columns
+                       }
+               }
+               if file == "" {
                        if !parseFailed {
                                parseFailed = true
                                fmt.Fprintf(os.Stderr, "failed to parse %s vet output:\n", p)
                        }
                        fmt.Fprintln(os.Stderr, line)
+                       continue
                }
+
                msg = strings.TrimSpace(msg)
 
                for _, ignore := range ignorePathPrefixes {
index 3dbb340cbd6d1910cee4be7b9f1a10ccb13c4855..f791a265701751e715ed97a2262b7819d8a06d58 100644 (file)
@@ -1,7 +1,5 @@
 // 386-specific vet whitelist. See readme.txt for details.
 
-internal/bytealg/compare_386.s: [386] cannot check cross-package assembly function: cmpstring is in package runtime
-
 // startup code uses non-standard calling convention and intentionally
 // omits args.
 runtime/asm_386.s: [386] rt0_go: use of 4(SP) points beyond argument frame
index 38caac3c3b356595408f26e79fc422df3788a463..c73516392f6bf1f67f1de3b75242901dcf6f7a89 100644 (file)
@@ -9,10 +9,6 @@ go/types/scope.go: method WriteTo(w io.Writer, n int, recurse bool) should have
 
 // False positives.
 
-// Nothing much to do about cross-package assembly. Unfortunate.
-internal/bytealg/equal_ARCHSUFF.s: [GOARCH] cannot check cross-package assembly function: memequal is in package runtime
-internal/bytealg/equal_ARCHSUFF.s: [GOARCH] cannot check cross-package assembly function: memequal_varlen is in package runtime
-
 // The write barrier is called directly by the compiler, so no Go def
 runtime/asm_ARCHSUFF.s: [GOARCH] gcWriteBarrier: function gcWriteBarrier missing Go declaration
 
@@ -22,8 +18,6 @@ encoding/json/decode_test.go: struct field m has json tag but is not exported
 encoding/json/decode_test.go: struct field m2 has json tag but is not exported
 encoding/json/decode_test.go: struct field s has json tag but is not exported
 encoding/json/tagkey_test.go: struct field tag `:"BadFormat"` not compatible with reflect.StructTag.Get: bad syntax for struct tag key
-runtime/testdata/testprog/deadlock.go: unreachable code
-runtime/testdata/testprog/deadlock.go: unreachable code
 
 // Compiler tests that make sure even vet-failing code adheres to the spec.
 cmd/compile/internal/gc/testdata/arithConst_test.go: a (64 bits) too small for shift of 4294967296
@@ -68,3 +62,7 @@ cmd/link/link_test.go: struct field tag "\n\tLondon. Michaelmas term lately over
 cmd/link/link_test.go: struct field tag "\n\tIt was grand to see how the wind awoke, and bent the trees, and drove the rain before it like a cloud of smoke; and to hear the solemn thunder, and to see the lightning; and while thinking with awe of the tremendous powers by which our little lives are encompassed, to consider how beneficent they are, and how upon the smallest flower and leaf there was already a freshness poured from all this seeming rage, which seemed to make creation new again." not compatible with reflect.StructTag.Get: bad syntax for struct tag key
 cmd/link/link_test.go: struct field tag "\n\tJarndyce and Jarndyce drones on. This scarecrow of a suit has, over the course of time, become so complicated, that no man alive knows what it means. The parties to it understand it least; but it has been observed that no two Chancery lawyers can talk about it for five minutes, without coming to a total disagreement as to all the premises. Innumerable children have been born into the cause; innumerable young people have married into it; innumerable old people have died out of it. Scores of persons have deliriously found themselves made parties in Jarndyce and Jarndyce, without knowing how or why; whole families have inherited legendary hatreds with the suit. The little plaintiff or defendant, who was promised a new rocking-horse when Jarndyce and Jarndyce should be settled, has grown up, possessed himself of a real horse, and trotted away into the other world. Fair wards of court have faded into mothers and grandmothers; a long procession of Chancellors has come in and gone out; the legion of bills in the suit have been transformed into mere bills of mortality; there are not three Jarndyces left upon the earth perhaps, since old Tom Jarndyce in despair blew his brains out at a coffee-house in Chancery Lane; but Jarndyce and Jarndyce still drags its dreary length before the Court, perennially hopeless." not compatible with reflect.StructTag.Get: bad syntax for struct tag key
 cmd/link/link_test.go: struct field tag "\n\tThe one great principle of the English law is, to make business for itself. There is no other principle distinctly, certainly, and consistently maintained through all its narrow turnings. Viewed by this light it becomes a coherent scheme, and not the monstrous maze the laity are apt to think it. Let them but once clearly perceive that its grand principle is to make business for itself at their expense, and surely they will cease to grumble." not compatible with reflect.StructTag.Get: bad syntax for struct tag key
+
+// Tests of Decode(nil) trigger legitimate diagnostics.
+encoding/gob/encoder_test.go: call of Decode passes non-pointer
+encoding/gob/encoder_test.go: call of Decode passes non-pointer
index 94f782aa2fd2a70179fdbb9eb92f00ac7e30c7ec..020241f615a50f41ddf1eef454dc4b9d7cfedc57 100644 (file)
@@ -2,9 +2,6 @@
 
 // False positives.
 
-// Nothing much to do about cross-package assembly. Unfortunate.
-internal/bytealg/compare_amd64.s: [amd64] cannot check cross-package assembly function: cmpstring is in package runtime
-
 // reflect trampolines intentionally omit arg size. Same for morestack.
 runtime/asm_amd64.s: [amd64] morestack: use of 8(SP) points beyond argument frame
 runtime/asm_amd64.s: [amd64] morestack: use of 16(SP) points beyond argument frame
index 5dc2766e10d3011935eb79487f7790d85629dd61..81a1f1831ea8f3f37a592da3e8a8a57ef3b0c52d 100644 (file)
@@ -1,7 +1,5 @@
 // arm-specific vet whitelist. See readme.txt for details.
 
-internal/bytealg/compare_arm.s: [arm] cannot check cross-package assembly function: cmpstring is in package runtime
-
 // Intentionally missing declarations.
 runtime/asm_arm.s: [arm] emptyfunc: function emptyfunc missing Go declaration
 runtime/asm_arm.s: [arm] armPublicationBarrier: function armPublicationBarrier missing Go declaration
index 72528c5145a3799daba6f86547ed649263a596fa..5a0af626f6aea4fca7984db7f52dd67a54ae84b1 100644 (file)
@@ -1,7 +1,5 @@
 // arm64-specific vet whitelist. See readme.txt for details.
 
-internal/bytealg/compare_arm64.s: [arm64] cannot check cross-package assembly function: cmpstring is in package runtime
-
 // Intentionally missing declarations.
 runtime/asm_arm64.s: [arm64] addmoduledata: function addmoduledata missing Go declaration
 runtime/duff_arm64.s: [arm64] duffzero: function duffzero missing Go declaration
index bd53e9acdf853c4f7e72f356946bda5edaa1fca6..1451a86e28c62992b084141ff9745ba0a7f8f847 100644 (file)
@@ -1,7 +1,5 @@
 // mips/mipsle-specific vet whitelist. See readme.txt for details.
 
-internal/bytealg/compare_mipsx.s: [GOARCH] cannot check cross-package assembly function: cmpstring is in package runtime
-
 runtime/tls_mipsx.s: [GOARCH] save_g: function save_g missing Go declaration
 runtime/tls_mipsx.s: [GOARCH] load_g: function load_g missing Go declaration
 runtime/sys_linux_mipsx.s: [GOARCH] clone: 12(R29) should be mp+8(FP)
index 68bba518ac6739ce997e04a64157cdc0bbc7d7a4..c4b03e40936fe82b531c52663906f0627b6fd3a2 100644 (file)
@@ -1,7 +1,5 @@
 // nacl/386-specific vet whitelist. See readme.txt for details.
 
-runtime/sys_nacl_386.s: [386] cannot check cross-package assembly function: naclWrite is in package syscall
-runtime/sys_nacl_386.s: [386] cannot check cross-package assembly function: now is in package syscall
 runtime/sys_nacl_386.s: [386] nacl_clock_gettime: function nacl_clock_gettime missing Go declaration
 runtime/sys_nacl_386.s: [386] setldt: function setldt missing Go declaration
 runtime/sys_nacl_386.s: [386] sigtramp: use of 20(SP) points beyond argument frame
index 5625e3c55d0b46e7c7519e4f2338f4d709c830d1..9661f57b23d9c20ce548f40ce6070d2ebb2aa059 100644 (file)
@@ -1,7 +1,5 @@
 // nacl/amd64p32-specific vet whitelist. See readme.txt for details.
 
-internal/bytealg/compare_amd64p32.s: [amd64p32] cannot check cross-package assembly function: cmpstring is in package runtime
-
 // reflect trampolines intentionally omit arg size. Same for morestack.
 runtime/asm_amd64p32.s: [amd64p32] morestack: use of 8(SP) points beyond argument frame
 runtime/asm_amd64p32.s: [amd64p32] morestack: use of 16(SP) points beyond argument frame
@@ -13,8 +11,6 @@ runtime/sys_nacl_amd64p32.s: [amd64p32] sigtramp: unknown variable ctxt
 runtime/sys_nacl_amd64p32.s: [amd64p32] sigtramp: unknown variable ctxt
 runtime/sys_nacl_amd64p32.s: [amd64p32] sigtramp: unknown variable ctxt
 runtime/sys_nacl_amd64p32.s: [amd64p32] nacl_sysinfo: function nacl_sysinfo missing Go declaration
-runtime/sys_nacl_amd64p32.s: [amd64p32] cannot check cross-package assembly function: naclWrite is in package syscall
-runtime/sys_nacl_amd64p32.s: [amd64p32] cannot check cross-package assembly function: now is in package syscall
 runtime/sys_nacl_amd64p32.s: [amd64p32] nacl_clock_gettime: function nacl_clock_gettime missing Go declaration
 runtime/sys_nacl_amd64p32.s: [amd64p32] settls: function settls missing Go declaration
 
index cc0fcbab7f2fe9202fb6f716563931f383acf1b9..dde0092570234f68025c6476cfe5600f77f542aa 100644 (file)
@@ -1,8 +1,6 @@
 // nacl/arm-specific vet whitelist. See readme.txt for details.
 
 runtime/asm_arm.s: [arm] sigreturn: function sigreturn missing Go declaration
-runtime/sys_nacl_arm.s: [arm] cannot check cross-package assembly function: naclWrite is in package syscall
-runtime/sys_nacl_arm.s: [arm] cannot check cross-package assembly function: now is in package syscall
 runtime/sys_nacl_arm.s: [arm] nacl_clock_gettime: function nacl_clock_gettime missing Go declaration
 runtime/sys_nacl_arm.s: [arm] nacl_sysinfo: function nacl_sysinfo missing Go declaration
 runtime/sys_nacl_arm.s: [arm] read_tls_fallback: function read_tls_fallback missing Go declaration
index 39f8c0da31adbed881791e6004077f63e5dac739..730a753afc2e0d4a97eb51b4d4f0a840cd995109 100644 (file)
@@ -1,7 +1,5 @@
 // ppc64-specific vet whitelist. See readme.txt for details.
 
-internal/bytealg/compare_ppc64x.s: [GOARCH] cannot check cross-package assembly function: cmpstring is in package runtime
-
 runtime/asm_ppc64x.s: [GOARCH] reginit: function reginit missing Go declaration
 runtime/asm_ppc64x.s: [GOARCH] goexit: use of 24(R1) points beyond argument frame
 runtime/asm_ppc64x.s: [GOARCH] addmoduledata: function addmoduledata missing Go declaration
index 4b8424203821250f0a5125377f6dcc83c48a75d6..55cf44a5198bc6d74ad1322e02c51227d9a41f41 100644 (file)
@@ -1,13 +1,12 @@
-internal/bytealg/compare_s390x.s: [s390x] cannot check cross-package assembly function: cmpstring is in package runtime
 runtime/asm_s390x.s: [s390x] addmoduledata: function addmoduledata missing Go declaration
 runtime/memclr_s390x.s: [s390x] memclr_s390x_exrl_xc: function memclr_s390x_exrl_xc missing Go declaration
 runtime/memmove_s390x.s: [s390x] memmove_s390x_exrl_mvc: function memmove_s390x_exrl_mvc missing Go declaration
 runtime/tls_s390x.s: [s390x] save_g: function save_g missing Go declaration
 runtime/tls_s390x.s: [s390x] load_g: function load_g missing Go declaration
-internal/cpu/cpu_s390x.s: [s390x] stfle: invalid MOVD of ret+0(FP); cpu.facilityList is 32-byte value
-internal/cpu/cpu_s390x.s: [s390x] kmQuery: invalid MOVD of ret+0(FP); cpu.queryResult is 16-byte value
-internal/cpu/cpu_s390x.s: [s390x] kmcQuery: invalid MOVD of ret+0(FP); cpu.queryResult is 16-byte value
-internal/cpu/cpu_s390x.s: [s390x] kmctrQuery: invalid MOVD of ret+0(FP); cpu.queryResult is 16-byte value
-internal/cpu/cpu_s390x.s: [s390x] kmaQuery: invalid MOVD of ret+0(FP); cpu.queryResult is 16-byte value
-internal/cpu/cpu_s390x.s: [s390x] kimdQuery: invalid MOVD of ret+0(FP); cpu.queryResult is 16-byte value
-internal/cpu/cpu_s390x.s: [s390x] klmdQuery: invalid MOVD of ret+0(FP); cpu.queryResult is 16-byte value
+internal/cpu/cpu_s390x.s: [s390x] stfle: invalid MOVD of ret+0(FP); internal/cpu.facilityList is 32-byte value
+internal/cpu/cpu_s390x.s: [s390x] kmQuery: invalid MOVD of ret+0(FP); internal/cpu.queryResult is 16-byte value
+internal/cpu/cpu_s390x.s: [s390x] kmcQuery: invalid MOVD of ret+0(FP); internal/cpu.queryResult is 16-byte value
+internal/cpu/cpu_s390x.s: [s390x] kmctrQuery: invalid MOVD of ret+0(FP); internal/cpu.queryResult is 16-byte value
+internal/cpu/cpu_s390x.s: [s390x] kmaQuery: invalid MOVD of ret+0(FP); internal/cpu.queryResult is 16-byte value
+internal/cpu/cpu_s390x.s: [s390x] kimdQuery: invalid MOVD of ret+0(FP); internal/cpu.queryResult is 16-byte value
+internal/cpu/cpu_s390x.s: [s390x] klmdQuery: invalid MOVD of ret+0(FP); internal/cpu.queryResult is 16-byte value
index d066e5b76f8f021902c6903bdfb62dc8e3fe619f..a3f8c291bf9e2326aa4aa692e7c9d22be3a6a5af 100644 (file)
@@ -2,9 +2,6 @@
 
 // False positives.
 
-// Nothing much to do about cross-package assembly. Unfortunate.
-internal/bytealg/compare_wasm.s: [wasm] cannot check cross-package assembly function: cmpstring is in package runtime
-
 // morestack intentionally omits arg size.
 runtime/asm_wasm.s: [wasm] morestack: use of 8(SP) points beyond argument frame
 runtime/asm_wasm.s: [wasm] morestack: use of 16(SP) points beyond argument frame
index d910022ef655eadb2449762365b988a13ec87ce8..87b3b24d7f70d3c9fea16c13e970753c2f6c5c0d 100644 (file)
@@ -6,4 +6,3 @@ runtime/sys_windows_386.s: [386] setldt: function setldt missing Go declaration
 runtime/sys_windows_386.s: [386] callbackasm1+0: function callbackasm1+0 missing Go declaration
 runtime/sys_windows_386.s: [386] tstart: function tstart missing Go declaration
 runtime/sys_windows_386.s: [386] tstart_stdcall: RET without writing to 4-byte ret+4(FP)
-runtime/sys_windows_386.s: [386] cannot check cross-package assembly function: now is in package time
index 676e6baf71b79aa56436f7975c6d54aa5361e665..daa23e73a15fddc87e537bab47880299b4ee6c18 100644 (file)
@@ -5,4 +5,3 @@ runtime/sys_windows_amd64.s: [amd64] ctrlhandler: RET without writing to 4-byte
 runtime/sys_windows_amd64.s: [amd64] callbackasm1: function callbackasm1 missing Go declaration
 runtime/sys_windows_amd64.s: [amd64] tstart_stdcall: RET without writing to 4-byte ret+8(FP)
 runtime/sys_windows_amd64.s: [amd64] settls: function settls missing Go declaration
-runtime/sys_windows_amd64.s: [amd64] cannot check cross-package assembly function: now is in package time