From a87bb4aae21347ace31bb349e1a540ad6a7af741 Mon Sep 17 00:00:00 2001 From: Adin Scannell Date: Thu, 3 Feb 2022 23:43:51 -0800 Subject: [PATCH] Ensure that nogo is enabled. A recent change stopped using the correct file (the export data, not the archive) and checklocks started failing. Unfortunately, this was suppressed, since the filter command was not failing with findings. This change fixes that problem and adds a test to ensure that this cannot happen again. If nogo starts failing to identify problems, the sanity_test in nogo/sanity will also start to fail. This change also requires updating the WORKSPACE to the latest rules_go and Go version, in order to pick up the fixed go_tools. The latest rules_go in turn required an updated bazel, which in turn required a minor change in the coverdata implementation. Fixing the fact propagation brought forward a number of problems with caching for bazel workers. Its unclear whether this was a core worker issue or whether some caching was broken, but the situation was basically undebugable. Instead, the way facts are stored and loaded is optimized to be able to remove the use of workers altogether and ideally make nogo debuggable. PiperOrigin-RevId: 426327186 --- .buildkite/pipeline.yaml | 3 + Makefile | 7 +- WORKSPACE | 75 +++-- go.mod | 2 +- go.sum | 2 + images/default/Dockerfile | 2 +- nogo.yaml | 24 ++ pkg/coverage/coverage.go | 20 +- pkg/sentry/fsimpl/fuse/connection_test.go | 2 + pkg/sentry/fsimpl/fuse/fusefs.go | 6 +- tools/bazeldefs/BUILD | 8 +- tools/bazeldefs/go.bzl | 2 - tools/checkescape/BUILD | 1 + tools/checkescape/checkescape.go | 84 +++-- tools/checklocks/facts.go | 10 +- tools/defs.bzl | 3 +- tools/go_types_memoize.patch | 39 +++ tools/nogo/check/BUILD | 1 - tools/nogo/check/build.go | 7 +- tools/nogo/check/check.go | 325 +++++++++--------- tools/nogo/check/findings.go | 18 - tools/nogo/cli/BUILD | 1 - tools/nogo/cli/cli.go | 166 ++++----- tools/nogo/config/config.go | 11 - tools/nogo/defs.bzl | 105 +++--- tools/nogo/facts/facts.go | 245 ++++++++------ tools/nogo/flags/flags.go | 4 + tools/nogo/sanity/BUILD | 36 ++ tools/nogo/sanity/sanity.go | 26 ++ tools/nogo/sanity/sanity.yaml | 7 + tools/nogo/sanity/sanity_test.sh | 37 ++ tools/rules_go_sdk.patch | 31 ++ tools/worker/BUILD | 51 --- tools/worker/worker.go | 389 ---------------------- 34 files changed, 753 insertions(+), 997 deletions(-) create mode 100644 tools/go_types_memoize.patch create mode 100644 tools/nogo/sanity/BUILD create mode 100644 tools/nogo/sanity/sanity.go create mode 100644 tools/nogo/sanity/sanity.yaml create mode 100755 tools/nogo/sanity/sanity_test.sh create mode 100644 tools/rules_go_sdk.patch delete mode 100644 tools/worker/BUILD delete mode 100644 tools/worker/worker.go diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index b623eaf8e..c3e91ec2e 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -41,6 +41,9 @@ steps: - <<: *common label: ":fire: Smoke tests" command: make smoke-tests + - <<: *common + label: ":fire: Smoke race tests" + command: make smoke-race-tests - wait # Check that the Go branch builds. diff --git a/Makefile b/Makefile index 55b0bfe9c..f45345471 100644 --- a/Makefile +++ b/Makefile @@ -185,11 +185,14 @@ debian: ## Builds the debian packages. @$(call build,-c opt //debian:debian) .PHONY: debian -smoke-tests: ## Runs a simple smoke test after build runsc. +smoke-tests: ## Runs a simple smoke test after building runsc. @$(call run,//runsc,--alsologtostderr --network none --debug --TESTONLY-unsafe-nonroot=true --rootless do true) - @$(call run,$(RACE_FLAGS) //runsc:runsc-race,--alsologtostderr --network none --debug --TESTONLY-unsafe-nonroot=true --rootless do true) .PHONY: smoke-tests +smoke-race-tests: ## Runs a smoke test after build building runsc in race configuration. + @$(call run,$(RACE_FLAGS) //runsc:runsc-race,--alsologtostderr --network none --debug --TESTONLY-unsafe-nonroot=true --rootless do true) +.PHONY: smoke-race-tests + nogo-tests: @$(call test,--build_tag_filters=nogo --test_tag_filters=nogo //:all pkg/... tools/...) .PHONY: nogo-tests diff --git a/WORKSPACE b/WORKSPACE index 16d19c79a..4f99f856f 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -39,36 +39,52 @@ http_archive( # Newer versions of the rules_go rules will automatically strip test # binaries of symbols, which we don't want. "//tools:rules_go_symbols.patch", + # Allow for patching of the go_sdk. + "//tools:rules_go_sdk.patch", ], - sha256 = "8e968b5fcea1d2d64071872b12737bbb5514524ee5f0a4f54f5920266c261acb", + sha256 = "d6b2513456fe2229811da7eb67a444be7785f5323c6708b38d851d2b51e54d83", urls = [ - "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.28.0/rules_go-v0.28.0.zip", - "https://github.com/bazelbuild/rules_go/releases/download/v0.28.0/rules_go-v0.28.0.zip", + "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.30.0/rules_go-v0.30.0.zip", + "https://github.com/bazelbuild/rules_go/releases/download/v0.30.0/rules_go-v0.30.0.zip", ], ) http_archive( name = "bazel_gazelle", - sha256 = "62ca106be173579c0a167deb23358fdfe71ffa1e4cfdddf5582af26520f1c66f", + sha256 = "de69a09dc70417580aabf20a28619bb3ef60d038470c7cf8442fafcf627c21cb", urls = [ - "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.23.0/bazel-gazelle-v0.23.0.tar.gz", - "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.23.0/bazel-gazelle-v0.23.0.tar.gz", + "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.24.0/bazel-gazelle-v0.24.0.tar.gz", + "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.24.0/bazel-gazelle-v0.24.0.tar.gz", ], ) -load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") +load("@io_bazel_rules_go//go:deps.bzl", "go_download_sdk", "go_rules_dependencies") +load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository") go_rules_dependencies() -go_register_toolchains(go_version = "1.16.8") - -load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository") +go_download_sdk( + name = "go_sdk", + # This implements a fix in the types package which dramatically speeds up + # analysis. Without this fix, the nogo rules will often fail to run in + # time on our continuous integration. + patch = "//tools:go_types_memoize.patch", + patch_strip = 2, + version = "1.17.6", +) gazelle_dependencies() # Some repository below has a transitive dependency on these repositories. # These declarations must precede any later declarations that transitively -# depend on older versions, since only the first declaration is considered. +# depend on older versions, since only the first declaration is considered. go_repository( +go_repository( + name = "org_golang_x_tools", + importpath = "golang.org/x/tools", + sum = "h1:j9KsMiaP1c3B0OTQGth0/k+miLGTgLsAFUCrF2vLcF8=", + version = "v0.1.9", +) + go_repository( name = "org_golang_x_sys", importpath = "golang.org/x/sys", @@ -83,6 +99,20 @@ go_repository( version = "v0.0.0-20210503060351-7fd8e65b6420", ) +go_repository( + name = "co_honnef_go_tools", + importpath = "honnef.co/go/tools", + sum = "h1:MNh1AVMyVX23VUHE2O27jm6lNj3vjO5DexS4A1xvnzk=", + version = "v0.2.2", +) + +go_repository( + name = "org_golang_x_oauth2", + importpath = "golang.org/x/oauth2", + sum = "h1:RerP+noqYHUQ8CMRcPlC2nvTa4dcBIjegkuWdcUDuqg=", + version = "v0.0.0-20211104180415-d3ed0bb246c8", +) + # Load C++ rules. http_archive( name = "rules_cc", @@ -585,11 +615,11 @@ rbe_autoconfig(name = "rbe_default") http_archive( name = "rules_pkg", + sha256 = "62eeb544ff1ef41d786e329e1536c1d541bb9bcad27ae984d57f18f314018e66", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/rules_pkg/releases/download/0.6.0/rules_pkg-0.6.0.tar.gz", "https://github.com/bazelbuild/rules_pkg/releases/download/0.6.0/rules_pkg-0.6.0.tar.gz", ], - sha256 = "62eeb544ff1ef41d786e329e1536c1d541bb9bcad27ae984d57f18f314018e66", ) load("@rules_pkg//:deps.bzl", "rules_pkg_dependencies") @@ -820,13 +850,6 @@ go_repository( version = "v0.0.0-20191024005414-555d28b269f0", ) -go_repository( - name = "org_golang_x_tools", - importpath = "golang.org/x/tools", - sum = "h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA=", - version = "v0.1.5", -) - go_repository( name = "org_golang_x_xerrors", importpath = "golang.org/x/xerrors", @@ -848,13 +871,6 @@ go_repository( version = "v1.5.2", ) -go_repository( - name = "org_golang_x_oauth2", - importpath = "golang.org/x/oauth2", - sum = "h1:B333XXssMuKQeBwiNODx4TupZy7bf4sxFZnN2ZOcvUE=", - version = "v0.0.0-20211005180243-6b3c2da341f1", -) - go_repository( name = "com_github_docker_docker", importpath = "github.com/docker/docker", @@ -974,13 +990,6 @@ go_repository( version = "v0.23.0", ) -go_repository( - name = "co_honnef_go_tools", - importpath = "honnef.co/go/tools", - sum = "h1:/EPr//+UMMXwMTkXvCCoaJDq8cpjMO80Ou+L4PDo2mY=", - version = "v0.2.1", -) - go_repository( name = "com_github_burntsushi_toml", importpath = "github.com/BurntSushi/toml", diff --git a/go.mod b/go.mod index d94a1d08f..bc0f6eab6 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.17 require ( github.com/BurntSushi/toml v0.3.1 - github.com/bazelbuild/rules_go v0.27.0 + github.com/bazelbuild/rules_go v0.30.0 github.com/cenkalti/backoff v1.1.1-0.20190506075156-2146c9339422 github.com/containerd/cgroups v1.0.1 github.com/containerd/console v1.0.1 diff --git a/go.sum b/go.sum index 22d22c41b..85cf2c214 100644 --- a/go.sum +++ b/go.sum @@ -57,6 +57,8 @@ github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kd github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/bazelbuild/rules_go v0.27.0 h1:KViqR7qKXwz+LrNdIauCDU21kneCk+4DnYjpvlJwH50= github.com/bazelbuild/rules_go v0.27.0/go.mod h1:MC23Dc/wkXEyk3Wpq6lCqz0ZAYOZDw2DR5y3N1q2i7M= +github.com/bazelbuild/rules_go v0.30.0 h1:kX4jVcstqrsRqKPJSn2mq2o+TI21edRzEJSrEOMQtr0= +github.com/bazelbuild/rules_go v0.30.0/go.mod h1:MC23Dc/wkXEyk3Wpq6lCqz0ZAYOZDw2DR5y3N1q2i7M= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= github.com/cenkalti/backoff v1.1.1-0.20190506075156-2146c9339422 h1:8eZxmY1yvxGHzdzTEhI09npjMVGzNAdrqzruTX6jcK4= diff --git a/images/default/Dockerfile b/images/default/Dockerfile index a9f54498e..83a4b0a86 100644 --- a/images/default/Dockerfile +++ b/images/default/Dockerfile @@ -24,6 +24,6 @@ RUN curl https://dl.google.com/dl/cloudsdk/channels/rapid/downloads/google-cloud ln -s /google-cloud-sdk/bin/gcloud /usr/bin/gcloud # Download the official bazel binary. The APT repository isn't used because there is not packages for arm64. -RUN sh -c 'curl -o /usr/local/bin/bazel https://releases.bazel.build/4.0.0/release/bazel-4.0.0-linux-$(uname -m | sed s/aarch64/arm64/) && chmod ugo+x /usr/local/bin/bazel' +RUN sh -c 'curl -o /usr/local/bin/bazel https://releases.bazel.build/4.2.1/release/bazel-4.2.1-linux-$(uname -m | sed s/aarch64/arm64/) && chmod ugo+x /usr/local/bin/bazel' WORKDIR /workspace ENTRYPOINT ["/usr/local/bin/bazel"] diff --git a/nogo.yaml b/nogo.yaml index 8d16e9a2b..280e46e4e 100644 --- a/nogo.yaml +++ b/nogo.yaml @@ -51,6 +51,20 @@ global: - "may require checklocks annotation for" # Generated proto code creates declarations like 'var start int = iNdEx' - "should omit type .* from declaration; it will be inferred from the right-hand side" + external: + suppress: + # buildssa can't handle certain packages (cmd/...). + - "panic recovered: interface conversion: types.Type is nil" + - "panic recovered: runtime error: invalid memory address or nil" + - "panic recovered: no type for \\*ast.CallExpr" + - "panic recovered: interface conversion: types.Type is \\*types.Basic" + - "panic recovered: no type for \\*ast.BinaryExpr" + - "panic recovered: no type for \\*ast.SelectorExpr" + - "panic recovered: no types.Object for ast.Ident SetTypeErrors" + - "panic recovered: unexpected CompositeLit type: invalid type" + exclude: + - ".*/vet/testdata/.*" + - ".*/runtime/testdata/.*" internal: suppress: # We use ALL_CAPS for system definitions, @@ -100,6 +114,9 @@ analyzers: exclude: [".*"] errorsas: external: # Enabled. + exclude: + # Specific broken case. + - ".*/cmd/go/internal/modload/list.go" httpresponse: external: # Enabled. loopclosure: @@ -111,6 +128,7 @@ analyzers: exclude: - pkg/sentry/platform/kvm/kvm_test.go # Intentional. - tools/bigquery/bigquery.go # False positive. + - "-" # No filename. printf: external: # Enabled. suppress: @@ -157,6 +175,10 @@ analyzers: external: # Enabled. checkescape: external: # Enabled. + suppress: + # External libraries may not have binaries (e.g. stdlib testdata, etc.), + # so these cases can be safely ignored. + - "no such file or directory" checklinkname: external: # Enabled. suppress: @@ -168,6 +190,8 @@ analyzers: # targets in the standard library, so we still need to run # checklinkname on stdlib generally. - "linkname to unknown symbol" + exclude: + - ".*/containerd/sys/subprocess_unsafe_linux.go" SA1019: # Use of deprecated identifier. # disable for now due to misattribution from golang.org/issue/44195. generated: diff --git a/pkg/coverage/coverage.go b/pkg/coverage/coverage.go index 0fabee92b..8b7e567ad 100644 --- a/pkg/coverage/coverage.go +++ b/pkg/coverage/coverage.go @@ -39,7 +39,7 @@ import ( ) var ( - // coverageMu must be held while accessing coverdata.Cover. This prevents + // coverageMu must be held while accessing coverdata.*. This prevents // concurrent reads/writes from multiple threads collecting coverage data. coverageMu sync.RWMutex @@ -61,7 +61,7 @@ const blockBitLength = 16 // Available returns whether any coverage data is available. func Available() bool { - return len(coverdata.Cover.Blocks) > 0 + return len(coverdata.Blocks) > 0 } // EnableReport sets up coverage reporting. @@ -102,7 +102,7 @@ func ClearCoverageData() { // We do not use atomic operations while reading/writing to the counters, // which would drastically degrade performance. Slight discrepancies due to // racing is okay for the purposes of kcov. - for _, counters := range coverdata.Cover.Counters { + for _, counters := range coverdata.Counters { for index := 0; index < len(counters); index++ { counters[index] = 0 } @@ -155,7 +155,7 @@ func ConsumeCoverageData(w io.Writer) int { total := 0 var pcBuffer [8]byte for fileNum, file := range globalData.files { - counters := coverdata.Cover.Counters[file] + counters := coverdata.Counters[file] for index := 0; index < len(counters); index++ { // We do not use atomic operations while reading/writing to the counters, // which would drastically degrade performance. Slight discrepancies due to @@ -194,13 +194,13 @@ func InitCoverageData() { globalData.once.Do(func() { // First, order all files. Then calculate synthetic PCs for every block // (using the well-defined ordering for files as well). - for file := range coverdata.Cover.Blocks { + for file := range coverdata.Blocks { globalData.files = append(globalData.files, file) } sort.Strings(globalData.files) for fileNum, file := range globalData.files { - blocks := coverdata.Cover.Blocks[file] + blocks := coverdata.Blocks[file] pcs := make([]uint64, 0, len(blocks)) for blockNum := range blocks { pcs = append(pcs, calculateSyntheticPC(fileNum, blockNum)) @@ -226,8 +226,8 @@ func Report() error { var err error reportOnce.Do(func() { - for file, counters := range coverdata.Cover.Counters { - blocks := coverdata.Cover.Blocks[file] + for file, counters := range coverdata.Counters { + blocks := coverdata.Blocks[file] for i := 0; i < len(counters); i++ { if atomic.LoadUint32(&counters[i]) > 0 { err = writeBlock(reportOutput, file, blocks[i]) @@ -260,7 +260,7 @@ func Symbolize(out io.Writer, pc uint64) error { // corresponding synthetic PCs. func WriteAllBlocks(out io.Writer) error { for fileNum, file := range globalData.files { - for blockNum, block := range coverdata.Cover.Blocks[file] { + for blockNum, block := range coverdata.Blocks[file] { if err := writeBlockWithPC(out, calculateSyntheticPC(fileNum, blockNum), file, block); err != nil { return err } @@ -300,7 +300,7 @@ func fileFromIndex(i int) (string, error) { // blockFromIndex returns the i-th block in the given file. func blockFromIndex(file string, i int) (testing.CoverBlock, error) { - blocks, ok := coverdata.Cover.Blocks[file] + blocks, ok := coverdata.Blocks[file] if !ok { return testing.CoverBlock{}, fmt.Errorf("instrumented file %s does not exist", file) } diff --git a/pkg/sentry/fsimpl/fuse/connection_test.go b/pkg/sentry/fsimpl/fuse/connection_test.go index d98d2832b..11eff255b 100644 --- a/pkg/sentry/fsimpl/fuse/connection_test.go +++ b/pkg/sentry/fsimpl/fuse/connection_test.go @@ -74,7 +74,9 @@ func TestConnectionAbort(t *testing.T) { testObj := primitive.Uint32(rand.Uint32()) for i := 0; i < int(numRequests); i++ { req := conn.NewRequest(creds, uint32(i), uint64(i), 0, &testObj) + conn.fd.mu.Lock() fut, err := conn.callFutureLocked(task, req) + conn.fd.mu.Unlock() if err != nil { t.Fatalf("callFutureLocked failed: %v", err) } diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 268d01ce7..e72f9a480 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -303,12 +303,12 @@ func (fs *filesystem) MountOptions() string { return fs.opts.mopts } -// Fh data returned by newEntry +// NewFhData is returned by newEntry. type NewFhData struct { - // file handler + // fh is the file handler. fh uint64 - // Flags of the file. + // flags is the flags of the file. flags uint32 } diff --git a/tools/bazeldefs/BUILD b/tools/bazeldefs/BUILD index 5295f4a85..528ea4dd3 100644 --- a/tools/bazeldefs/BUILD +++ b/tools/bazeldefs/BUILD @@ -1,4 +1,4 @@ -load("//tools:defs.bzl", "bzl_library", "go_proto_library") +load("//tools:defs.bzl", "bzl_library") package( default_visibility = ["//:sandbox"], @@ -53,9 +53,3 @@ genrule( ], visibility = ["//:sandbox"], ) - -go_proto_library( - name = "worker_protocol_go_proto", - importpath = "gvisor.dev/bazel/worker_protocol_go_proto", - proto = "@bazel_tools//src/main/protobuf:worker_protocol_proto", -) diff --git a/tools/bazeldefs/go.bzl b/tools/bazeldefs/go.bzl index 0c360df1f..9874b690d 100644 --- a/tools/bazeldefs/go.bzl +++ b/tools/bazeldefs/go.bzl @@ -11,8 +11,6 @@ go_embed_data = _go_embed_data go_path = _go_path -bazel_worker_proto = "//tools/bazeldefs:worker_protocol_go_proto" - def _go_proto_or_grpc_library(go_library_func, name, **kwargs): if "importpath" in kwargs: # If importpath is explicit, pass straight through. diff --git a/tools/checkescape/BUILD b/tools/checkescape/BUILD index 833aa1269..97909e78b 100644 --- a/tools/checkescape/BUILD +++ b/tools/checkescape/BUILD @@ -8,6 +8,7 @@ go_library( nogo = False, visibility = ["//tools/nogo:__subpackages__"], deps = [ + "//pkg/log", "//tools/nogo/flags", "@org_golang_x_tools//go/analysis:go_default_library", "@org_golang_x_tools//go/analysis/passes/buildssa:go_default_library", diff --git a/tools/checkescape/checkescape.go b/tools/checkescape/checkescape.go index 0ddfae17a..579d59611 100644 --- a/tools/checkescape/checkescape.go +++ b/tools/checkescape/checkescape.go @@ -67,15 +67,16 @@ import ( "go/types" "io" "io/ioutil" - "log" "os" "os/exec" "path/filepath" "strings" + "sync" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/buildssa" "golang.org/x/tools/go/ssa" + "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/tools/nogo/flags" ) @@ -165,21 +166,6 @@ var escapingBuiltins = []string{ "mallocgc", } -// packageEscapeFacts is the set of all functions in a package, and whether or -// not they recursively pass escape analysis. -// -// All the type names for receivers are encoded in the full key. The key -// represents the fully qualified package and type name used at link time. -// -// Note that each Escapes object is a summary. Local findings may be reported -// using more detailed information. -type packageEscapeFacts struct { - Funcs map[string]Escapes -} - -// AFact implements analysis.Fact.AFact. -func (*packageEscapeFacts) AFact() {} - // objdumpAnalyzer accepts the objdump parameter. type objdumpAnalyzer struct { analysis.Analyzer @@ -202,7 +188,7 @@ var Analyzer = &objdumpAnalyzer{ Doc: "escape analysis checks based on +checkescape annotations", Run: nil, // Must be invoked via Run above. Requires: []*analysis.Analyzer{buildssa.Analyzer}, - FactTypes: []analysis.Fact{(*packageEscapeFacts)(nil)}, + FactTypes: []analysis.Fact{(*Escapes)(nil)}, }, } @@ -252,6 +238,9 @@ type Escapes struct { Omitted [reasonCount]int } +// AFact implements analysis.Fact.AFact. +func (*Escapes) AFact() {} + // add is called by Add and Merge. func (es *Escapes) add(r EscapeReason, detail string, omitted int, callSites ...CallSite) { if es.CallSites[r] != nil { @@ -407,11 +396,16 @@ func loadObjdump(binary io.Reader) (finalResults map[string][]string, finalErr e // Execute go tool objdump ggiven the input. cmd := exec.Command(flags.Go, "tool", "objdump", input.Name()) - cmd.Stderr = os.Stderr - pipeOut, pipeErr := cmd.StdoutPipe() - if pipeErr != nil { - return nil, fmt.Errorf("unable to load objdump: %w", pipeErr) + pipeOut, err := cmd.StdoutPipe() + if err != nil { + return nil, fmt.Errorf("unable to load objdump: %w", err) } + defer pipeOut.Close() + pipeErr, err := cmd.StderrPipe() + if err != nil { + return nil, fmt.Errorf("unable to load objdump: %w", err) + } + defer pipeErr.Close() if startErr := cmd.Start(); startErr != nil { return nil, fmt.Errorf("unable to start objdump: %w", startErr) } @@ -421,9 +415,20 @@ func loadObjdump(binary io.Reader) (finalResults map[string][]string, finalErr e // indicate that the dump was incomplete and we could be missed some // escapes that would have appeared. We need to force failure. defer func() { - if waitErr := cmd.Wait(); finalErr == nil && waitErr != nil { + var ( + wg sync.WaitGroup + buf bytes.Buffer + ) + wg.Add(1) + go func() { + defer wg.Done() + io.Copy(&buf, pipeErr) + }() + waitErr := cmd.Wait() + wg.Wait() + if finalErr == nil && waitErr != nil { // Override the function's return value in this case. - finalErr = fmt.Errorf("error running objdump: %v", waitErr) + finalErr = fmt.Errorf("error running objdump %s: %v (%s)", input.Name(), waitErr, buf.Bytes()) } }() @@ -636,7 +641,7 @@ func run(pass *analysis.Pass, binary io.Reader) (interface{}, error) { // Note that if this analysis fails, then we don't actually // fail the analyzer itself. We simply report every possible // escape. In most cases this will work just fine. - log.Printf("WARNING: unable to load objdump: %v", callsErr) + log.Warningf("unable to load objdump: %v", callsErr) } allEscapes := make(map[string][]Escapes) mergedEscapes := make(map[string]Escapes) @@ -707,40 +712,42 @@ func run(pass *analysis.Pass, binary io.Reader) (interface{}, error) { } switch x := x.Call.Value.(type) { case *ssa.Function: - if x.Pkg == nil { - // Can't resolve the package. - es.Add(unknownPackage, "no package", cs) - return - } - // Is this a local function? If yes, call the // function to load the local function. The // local escapes are the escapes found in the // local function. - if x.Pkg.Pkg == pass.Pkg { + if x.Pkg != nil && x.Pkg.Pkg == pass.Pkg { es.MergeWithCall(loadFunc(x), cs) return } // If this package is the atomic package, the implementation // may be replaced by instrinsics that don't have analysis. - if x.Pkg.Pkg.Path() == "sync/atomic" { + if x.Pkg != nil && x.Pkg.Pkg.Path() == "sync/atomic" { return } // Recursively collect information. - var imp packageEscapeFacts - if !pass.ImportPackageFact(x.Pkg.Pkg, &imp) { + var funcEscapes Escapes + if !pass.ImportObjectFact(x.Object(), &funcEscapes) { + // If this is the unix package, and the + // function is RawSyscall, we can also + // ignore this case. + if x.Pkg != nil && x.Pkg.Pkg.Name() == "unix" && (x.Name() == "RawSyscall" || x.Name() == "RawSyscall6") { + return + } + // Unable to import the dependency; we must // declare these as escaping. - es.Add(unknownPackage, "no analysis", cs) + message := fmt.Sprintf("no analysis for %q", x.Object().String()) + es.Add(unknownPackage, message, cs) return } // The escapes of this instruction are the // escapes of the called function directly. // Note that this may record many escapes. - es.MergeWithCall(imp.Funcs[x.RelString(x.Pkg.Pkg)], cs) + es.MergeWithCall(funcEscapes, cs) return case *ssa.Builtin: // Ignore elided escapes. @@ -846,7 +853,10 @@ func run(pass *analysis.Pass, binary io.Reader) (interface{}, error) { // Complete all local functions. for _, fn := range state.SrcFuncs { - loadFunc(fn) + funcEscapes := loadFunc(fn) + if obj := fn.Object(); obj != nil { + pass.ExportObjectFact(obj, &funcEscapes) + } } // Scan all functions for violations. diff --git a/tools/checklocks/facts.go b/tools/checklocks/facts.go index f6dfeaec9..0915e2243 100644 --- a/tools/checklocks/facts.go +++ b/tools/checklocks/facts.go @@ -164,8 +164,8 @@ func (*lockGuardFacts) AFact() {} // globalGuard is a global value. type globalGuard struct { - // Object indicates the object from which resolution should occur. - Object types.Object + // ObjectName indicates the object from which resolution should occur. + ObjectName string // FieldList is the traversal path from object. FieldList fieldList @@ -179,7 +179,7 @@ type ssaPackager interface { // resolveCommon implements resolution for all cases. func (g *globalGuard) resolveCommon(pc *passContext, ls *lockState) resolvedValue { state := pc.pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA) - v := state.Pkg.Members[g.Object.Name()].(ssa.Value) + v := state.Pkg.Members[g.ObjectName].(ssa.Value) return makeResolvedValue(v, g.FieldList) } @@ -627,8 +627,8 @@ func (pc *passContext) findGlobalGuard(pos token.Pos, guardName string) (*global return nil, false } return &globalGuard{ - Object: globalObj, - FieldList: fl, + ObjectName: parts[0], + FieldList: fl, }, true } diff --git a/tools/defs.bzl b/tools/defs.bzl index d588241b6..1840c7a37 100644 --- a/tools/defs.bzl +++ b/tools/defs.bzl @@ -10,7 +10,7 @@ load("//tools/go_marshal:defs.bzl", "go_marshal", "marshal_deps", "marshal_test_ load("//tools/nogo:defs.bzl", "nogo_test") load("//tools/bazeldefs:defs.bzl", _amd64_config = "amd64_config", _arch_config = "arch_config", _arm64_config = "arm64_config", _build_test = "build_test", _bzl_library = "bzl_library", _coreutil = "coreutil", _default_net_util = "default_net_util", _more_shards = "more_shards", _most_shards = "most_shards", _proto_library = "proto_library", _select_arch = "select_arch", _select_system = "select_system", _short_path = "short_path", _transition_allowlist = "transition_allowlist", _version = "version") load("//tools/bazeldefs:cc.bzl", _cc_binary = "cc_binary", _cc_flags_supplier = "cc_flags_supplier", _cc_grpc_library = "cc_grpc_library", _cc_library = "cc_library", _cc_proto_library = "cc_proto_library", _cc_test = "cc_test", _cc_toolchain = "cc_toolchain", _gbenchmark = "gbenchmark", _gbenchmark_internal = "gbenchmark_internal", _grpcpp = "grpcpp", _gtest = "gtest", _vdso_linker_option = "vdso_linker_option") -load("//tools/bazeldefs:go.bzl", _bazel_worker_proto = "bazel_worker_proto", _gazelle = "gazelle", _go_binary = "go_binary", _go_embed_data = "go_embed_data", _go_grpc_and_proto_libraries = "go_grpc_and_proto_libraries", _go_library = "go_library", _go_path = "go_path", _go_proto_library = "go_proto_library", _go_rule = "go_rule", _go_test = "go_test", _select_goarch = "select_goarch", _select_goos = "select_goos") +load("//tools/bazeldefs:go.bzl", _gazelle = "gazelle", _go_binary = "go_binary", _go_embed_data = "go_embed_data", _go_grpc_and_proto_libraries = "go_grpc_and_proto_libraries", _go_library = "go_library", _go_path = "go_path", _go_proto_library = "go_proto_library", _go_rule = "go_rule", _go_test = "go_test", _select_goarch = "select_goarch", _select_goos = "select_goos") load("//tools/bazeldefs:pkg.bzl", _pkg_deb = "pkg_deb", _pkg_tar = "pkg_tar") load("//tools/bazeldefs:platforms.bzl", _default_platform = "default_platform", _platforms = "platforms") load("//tools/bazeldefs:tags.bzl", "go_suffixes") @@ -47,7 +47,6 @@ select_goos = _select_goos select_goarch = _select_goarch go_embed_data = _go_embed_data go_proto_library = _go_proto_library -bazel_worker_proto = _bazel_worker_proto # Packaging rules. pkg_deb = _pkg_deb diff --git a/tools/go_types_memoize.patch b/tools/go_types_memoize.patch new file mode 100644 index 000000000..215206eba --- /dev/null +++ b/tools/go_types_memoize.patch @@ -0,0 +1,39 @@ +diff '--color=auto' -r -u2 a/go/src/go/types/scope.go b/go/src/go/types/scope.go +--- a/go/src/go/types/scope.go 2022-01-06 19:14:25.000000000 +0000 ++++ b/go/src/go/types/scope.go 2022-02-03 18:50:35.526563967 +0000 +@@ -24,4 +24,5 @@ + children []*Scope + elems map[string]Object // lazily allocated ++ sorted []string // lazily allocated + pos, end token.Pos // scope extent; may be invalid + comment string // for debugging only +@@ -32,5 +33,5 @@ + // scope, if any. The comment is for debugging only. + func NewScope(parent *Scope, pos, end token.Pos, comment string) *Scope { +- s := &Scope{parent, nil, nil, pos, end, comment, false} ++ s := &Scope{parent, nil, nil, nil, pos, end, comment, false} + // don't add children to Universe scope! + if parent != nil && parent != Universe { +@@ -48,12 +49,14 @@ + // Names returns the scope's element names in sorted order. + func (s *Scope) Names() []string { +- names := make([]string, len(s.elems)) +- i := 0 +- for name := range s.elems { +- names[i] = name +- i++ ++ if len(s.sorted) != len(s.elems) { ++ s.sorted = make([]string, len(s.elems)) ++ i := 0 ++ for name := range s.elems { ++ s.sorted[i] = name ++ i++ ++ } ++ sort.Strings(s.sorted) + } +- sort.Strings(names) +- return names ++ return s.sorted + } + +Only in b/go/src/go/types: scope.go.orig diff --git a/tools/nogo/check/BUILD b/tools/nogo/check/BUILD index 514442531..1f602ac56 100644 --- a/tools/nogo/check/BUILD +++ b/tools/nogo/check/BUILD @@ -20,7 +20,6 @@ go_library( "//tools/checkunsafe", "//tools/nogo/facts", "//tools/nogo/flags", - "//tools/worker", "@co_honnef_go_tools//staticcheck:go_default_library", "@co_honnef_go_tools//stylecheck:go_default_library", "@org_golang_x_tools//go/analysis:go_default_library", diff --git a/tools/nogo/check/build.go b/tools/nogo/check/build.go index 3b225996e..9f74a4bbb 100644 --- a/tools/nogo/check/build.go +++ b/tools/nogo/check/build.go @@ -15,6 +15,7 @@ package check import ( + "errors" "fmt" "go/build" "io" @@ -37,7 +38,11 @@ var findStdPkg = func(path string) (io.ReadCloser, error) { } // Attempt to resolve the library, and propagate this error. - return os.Open(fmt.Sprintf("%s/pkg/%s_%s/%s.a", root, flags.GOOS, flags.GOARCH, path)) + f, err := os.Open(fmt.Sprintf("%s/pkg/%s_%s/%s.a", root, flags.GOOS, flags.GOARCH, path)) + if err != nil && errors.Is(err, os.ErrNotExist) { + return nil, ErrSkip + } + return f, err } // releaseTags returns the default release tags. diff --git a/tools/nogo/check/check.go b/tools/nogo/check/check.go index c8b14b6bd..8a9f55500 100644 --- a/tools/nogo/check/check.go +++ b/tools/nogo/check/check.go @@ -33,28 +33,20 @@ import ( "path/filepath" "reflect" "regexp" + "runtime/debug" "strings" "sync" - "sync/atomic" - "time" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/gcexportdata" "gvisor.dev/gvisor/runsc/flag" "gvisor.dev/gvisor/tools/nogo/facts" "gvisor.dev/gvisor/tools/nogo/flags" - "gvisor.dev/gvisor/tools/worker" ) var ( // ErrSkip indicates the package should be skipped. - ErrSkip = errors.New("skipped2") - - // cachedFacts caches by file (just byte data). - cachedFacts = worker.NewCache("facts") - - // bundleCachedFacts caches the standard library (bundleFacts). - bundleCachedFacts = worker.NewCache("stdlib") + ErrSkip = errors.New("skipped") // showTimes indicates we should show analyzer times. showTimes = flag.Bool("show_times", false, "show all analyzer times") @@ -103,8 +95,9 @@ type importerEntry struct { ready sync.WaitGroup pkg *types.Package findings FindingSet - facts *facts.Package err error + factsMu sync.Mutex + facts *facts.Package } // importer is an almost-implementation of go/types.Importer. @@ -116,85 +109,126 @@ type importer struct { fset *token.FileSet sources map[string][]string - // mu protects cache. + // mu protects cache & bundles (see below). mu sync.Mutex cache map[string]*importerEntry + // bundles is protected by mu, but once set is immutable. + bundles []*facts.Bundle + // importsMu protects imports. importsMu sync.Mutex imports map[string]*types.Package } -// allFacts returns all package facts for the given name. +// loadBundles loads all bundle files. // -// This attempts to load via the FactMap (global flags) or the Bundles (global -// flags), but falls back to attempting a direct import. -func (i *importer) allFacts(pkg *types.Package) (*facts.Package, error) { +// This should only be called from loadFacts, below. After calling this +// function, i.bundles may be read freely without holding a lock. +func (i *importer) loadBundles() error { + i.mu.Lock() + defer i.mu.Unlock() + + // Are bundles already available? + if i.bundles != nil { + return nil + } + + // Scan all bundle files. + for _, filename := range flags.Bundles { + // Open the given filename as a bundle. + loadedFacts, err := facts.BundleFrom(filename) + if err != nil { + return fmt.Errorf("error loading bundled facts: %w", err) + } + + // Add to the set of available bundles. + i.bundles = append(i.bundles, loadedFacts) + } + + return nil +} + +// loadFacts returns all package facts for the given name. +// +// This should be called only from importPackage, as this may deserialize a +// facts file (which is an expensive operation). Callers should generally rely +// on fastFacts to access facts for packages that have already been imported. +func (i *importer) loadFacts(pkg *types.Package) (*facts.Package, error) { // Attempt to load from the fact map. filename, ok := flags.FactMap[pkg.Path()] if ok { - cb, err := cachedFacts.Lookup([]string{filename}, func() (worker.Sizer, error) { - r, openErr := os.Open(filename) - if openErr != nil { - return nil, fmt.Errorf("error loading facts from %q: %w", filename, openErr) - } - defer r.Close() - loadedFacts := facts.NewPackage(pkg) - if _, readErr := loadedFacts.ReadFrom(r); readErr != nil { - return nil, fmt.Errorf("error loading facts: %w", readErr) - } - return loadedFacts, nil - }) - if err != nil { - return nil, err + r, openErr := os.Open(filename) + if openErr != nil { + return nil, fmt.Errorf("error loading facts from %q: %w", filename, openErr) } - return cb.(*facts.Package), nil + defer r.Close() + loadedFacts := facts.NewPackage() + if readErr := loadedFacts.ReadFrom(pkg, r); readErr != nil { + return nil, fmt.Errorf("error loading facts: %w", readErr) + } + return loadedFacts, nil } // Attempt to load any bundles. - for _, filename := range flags.Bundles { - cb, err := bundleCachedFacts.Lookup([]string{filename}, func() (worker.Sizer, error) { - r, openErr := os.Open(filename) - if openErr != nil { - return nil, fmt.Errorf("error loading bundled facts from %q: %w", filename, openErr) - } - defer r.Close() - loadedFacts := facts.NewBundle(i) - if _, readErr := loadedFacts.ReadFrom(r); readErr != nil { - // If the file is length zero, we skip it. This - // is because stray fact files may been left - // behind that are attempting to recreate now. - fi, err := r.Stat() - if err == nil && fi.Size() == 0 { - return nil, ErrSkip - } - return nil, fmt.Errorf("error loading bundled facts: %w", readErr) - } - return loadedFacts, nil - }) - if err == ErrSkip { - continue // See above. - } + if err := i.loadBundles(); err != nil { + return nil, fmt.Errorf("error loading bundles: %w", err) + } + + // Try to import from the bundle. + for _, bundleFacts := range i.bundles { + localFacts, err := bundleFacts.Package(pkg) if err != nil { - return nil, err + return nil, fmt.Errorf("error loading from a bundle: %w", err) } - if loadedFacts, ok := cb.(*facts.Bundle).Packages[pkg.Path()]; ok { - return loadedFacts, nil + if localFacts != nil { + return localFacts, nil } } - // Attempt to resolve the package via import. - _, parsedFacts, err := i.importPackage(pkg.Path()) - return parsedFacts, err + // Nothing available for this package? + return nil, nil } -// fastFact returns facts for the given package. -func (i *importer) fastFact(pkg *types.Package, obj types.Object, ptr analysis.Fact) bool { - foundFacts, err := i.allFacts(pkg) - if err != nil || foundFacts == nil { - return false +// fastFacts returns facts for the given package. +// +// This relies exclusively on loaded packages, as the parameter is +// *types.Package and therefore the package data must already be available. +func (i *importer) fastFacts(pkg *types.Package) *facts.Package { + i.mu.Lock() + e, ok := i.cache[pkg.Path()] + i.mu.Unlock() + if !ok { + return nil } - return foundFacts.ImportFact(obj, ptr) + + e.factsMu.Lock() + defer e.factsMu.Unlock() + + // Do we have them already? + if e.facts != nil { + return e.facts + } + + // Load the facts. + facts, err := i.loadFacts(pkg) + if err != nil { + // We have no available to propagate an error when attempting + // to import a fact, so we must simply issue a warning. + log.Printf("WARNING: error loading facts for %s: %v", pkg.Path(), err) + return nil + } + e.facts = facts // Cache the result. + return facts +} + +// findArchive finds the archive for the given package. +func (i *importer) findArchive(path string) (rc io.ReadCloser, err error) { + realPath, ok := flags.ArchiveMap[path] + if !ok { + return i.findBinary(path) + } + return os.Open(realPath) } // findBinary finds the binary for the given package. @@ -214,21 +248,21 @@ func (i *importer) findBinary(path string) (rc io.ReadCloser, err error) { // importPackage almost-implements types.Importer.Import. // // This must be called by other methods directly. -func (i *importer) importPackage(path string) (*types.Package, *facts.Package, error) { +func (i *importer) importPackage(path string) (*types.Package, error) { if path == "unsafe" { // Special case: go/types has pre-defined type information for // unsafe. We ensure that this package is correct, in case any // analyzers are specifically looking for this. - return types.Unsafe, nil, nil + return types.Unsafe, nil } // Pull the internal entry. i.mu.Lock() entry, ok := i.cache[path] - if ok { + if ok && entry.pkg != nil { i.mu.Unlock() entry.ready.Wait() - return entry.pkg, entry.facts, entry.err + return entry.pkg, entry.err } // Start preparing this entry. @@ -242,55 +276,49 @@ func (i *importer) importPackage(path string) (*types.Package, *facts.Package, e // analysis from first principles to validate the package and derive // the types. We strictly prefer this to the gcexportdata. if srcs, ok := i.sources[path]; ok && len(srcs) > 0 { - start := time.Now() entry.pkg, entry.findings, entry.facts, entry.err = i.checkPackage(path, srcs) if entry.err != nil { - return nil, nil, entry.err + return nil, entry.err } - // Why does the news already need to be bad? Note that this is - // printed here because this will only happen when multiple - // packages are being analyzed. - log.Printf("SUCCESS: all analyzers successfully completed %q (%v).", path, time.Since(start)) i.importsMu.Lock() defer i.importsMu.Unlock() i.imports[path] = entry.pkg - return entry.pkg, entry.facts, entry.err + return entry.pkg, entry.err } // Load all exported data. Unfortunately, we will have to hold the lock // during this time. The imported may access imports directly. rc, err := i.findBinary(path) if err != nil { - return nil, nil, err + return nil, err } defer rc.Close() r, err := gcexportdata.NewReader(rc) if err != nil { - return nil, nil, err + return nil, err } i.importsMu.Lock() defer i.importsMu.Unlock() entry.pkg, entry.err = gcexportdata.Read(r, i.fset, i.imports, path) - return entry.pkg, entry.facts, entry.err + return entry.pkg, entry.err } // Import implements types.Importer.Import. func (i *importer) Import(path string) (*types.Package, error) { - pkg, _, err := i.importPackage(path) - return pkg, err + return i.importPackage(path) } // errorImporter tracks the last error. type errorImporter struct { *importer - lastErr atomic.Value + lastErr error } // Import implements types.Importer.Import. func (i *errorImporter) Import(path string) (*types.Package, error) { - pkg, _, err := i.importer.importPackage(path) + pkg, err := i.importer.importPackage(path) if err != nil { - i.lastErr.Store(err) + i.lastErr = err } return pkg, err } @@ -333,6 +361,7 @@ func (i *importer) checkPackage(path string, srcs []string) (*types.Package, Fin typesSizes := types.SizesFor("gc", flags.GOARCH) typeConfig := types.Config{ Importer: ei, + Error: func(error) {}, } typesInfo := &types.Info{ Types: make(map[ast.Expr]types.TypeAndValue), @@ -343,19 +372,16 @@ func (i *importer) checkPackage(path string, srcs []string) (*types.Package, Fin Selections: make(map[*ast.SelectorExpr]*types.Selection), } astPackage, err := typeConfig.Check(path, i.fset, syntax, typesInfo) - if err != nil && ei.lastErr.Load() != ErrSkip { + if err != nil && ei.lastErr != ErrSkip { return nil, nil, nil, fmt.Errorf("error checking types: %w", err) } - // We start with completely empty facts. All of our facts are sourced - // via the fastFact function that hits the local caches. - // // Note that facts should be reconcilable between types as of go/tools // commit ee04797aa0b6be5ce3d5f7ac0f91e34716b3acdf. We previously used // to do a sanity check to ensure that binary import data was // compatible with ast-derived data, but this is no longer necessary. // If packages are available locally, we can refer to those directly. - astFacts := facts.NewPackage(astPackage) + astFacts := facts.NewPackage() // Recursively visit all analyzers. var ( @@ -419,7 +445,10 @@ func (i *importer) checkPackage(path string, srcs []string) (*types.Package, Fin }, ImportPackageFact: func(pkg *types.Package, ptr analysis.Fact) bool { if pkg != astPackage { - return i.fastFact(pkg, nil, ptr) + if f := i.fastFacts(pkg); f != nil { + return f.ImportFact(nil, ptr) + } + return false } factsMu.RLock() defer factsMu.RUnlock() @@ -432,7 +461,10 @@ func (i *importer) checkPackage(path string, srcs []string) (*types.Package, Fin }, ImportObjectFact: func(obj types.Object, ptr analysis.Fact) bool { if pkg := obj.Pkg(); pkg != nil && pkg != astPackage { - return i.fastFact(pkg, obj, ptr) + if f := i.fastFacts(pkg); f != nil { + return f.ImportFact(obj, ptr) + } + return false } factsMu.RLock() defer factsMu.RUnlock() @@ -441,14 +473,12 @@ func (i *importer) checkPackage(path string, srcs []string) (*types.Package, Fin ExportObjectFact: func(obj types.Object, fact analysis.Fact) { if obj == nil { // Tried to export nil object? - log.Printf("WARNING: attempted to export fact for nil object") return } if obj.Pkg() != astPackage { // This is not allowed: the // built-in facts library will // also panic in this case. - log.Printf("WARNING: attempted to export fact for package %s", obj.Pkg().Name()) return } factsMu.Lock() @@ -460,8 +490,8 @@ func (i *importer) checkPackage(path string, srcs []string) (*types.Package, Fin defer factsMu.RUnlock() // Pull all dependencies. for _, importedPkg := range astPackage.Imports() { - otherFacts, err := i.allFacts(importedPkg) - if err != nil || otherFacts == nil { + otherFacts := i.fastFacts(importedPkg) + if otherFacts == nil { continue } for typ := range localFactTypes { @@ -520,20 +550,26 @@ func (i *importer) checkPackage(path string, srcs []string) (*types.Package, Fin // analyzers running concurrently // debuggable, capture panic exceptions // and propagate as an analyzer error. - err = fmt.Errorf("panic recovered: %s", r) + err = fmt.Errorf("panic recovered: %s (%s)", r, debug.Stack()) resultsMu.RUnlock() // +checklocksignore - resultsMu.Lock() - errs[a] = err - resultsMu.Unlock() } + resultsMu.Lock() + findings = append(findings, localFindings...) + results[a] = result + errs[a] = err + resultsMu.Unlock() }() found := findAnalyzer(a) resultsMu.RLock() if ba, ok := found.(binaryAnalyzer); ok { // Load the binary and analyze. - rc, loadErr := i.findBinary(path) + rc, loadErr := i.findArchive(path) if loadErr != nil { - err = loadErr + if loadErr != ErrSkip { + err = loadErr + } else { + err = nil // Ignore. + } } else { result, err = ba.Run(p, rc) rc.Close() @@ -542,11 +578,6 @@ func (i *importer) checkPackage(path string, srcs []string) (*types.Package, Fin result, err = a.Run(p) } resultsMu.RUnlock() - resultsMu.Lock() - findings = append(findings, localFindings...) - results[a] = result - errs[a] = err - resultsMu.Unlock() }(a, wg) } for _, wg := range ready { @@ -558,9 +589,13 @@ func (i *importer) checkPackage(path string, srcs []string) (*types.Package, Fin // this as a finding that can be suppressed. Some analyzers // will fail on some packages. if errs[a] != nil { + filename := "" + if len(srcs) > 0 { + filename = srcs[0] + } findings = append(findings, Finding{ Category: a.Name, - Position: token.Position{Filename: path}, + Position: token.Position{Filename: filename}, Message: errs[a].Error(), }) continue @@ -577,32 +612,8 @@ func (i *importer) checkPackage(path string, srcs []string) (*types.Package, Fin return astPackage, findings, astFacts, nil } -// allFindingsAndFacts returns the complete set. -func (i *importer) allFindingsAndFacts() (FindingSet, *facts.Bundle, error) { - var ( - findings = make(FindingSet, 0) - allFacts = facts.NewBundle(i) - ) - for path, entry := range i.cache { - findings = append(findings, entry.findings...) - if entry.facts != nil { - allFacts.Packages[path] = entry.facts - } else if entry.pkg != nil { - pkgFacts, err := i.allFacts(entry.pkg) - if err != nil { - // This should not happen, we should load facts for all packages. - return nil, nil, fmt.Errorf("no facts available for %s: %v", entry.pkg.Path(), err) - } - allFacts.Packages[path] = pkgFacts - } - } - - // Return the results. - return findings, allFacts, nil -} - // Package runs all analyzer on a single package. -func Package(path string, srcs []string) (FindingSet, facts.Writer, error) { +func Package(path string, srcs []string) (FindingSet, facts.Serializer, error) { i := &importer{ fset: token.NewFileSet(), cache: make(map[string]*importerEntry), @@ -615,6 +626,19 @@ func Package(path string, srcs []string) (FindingSet, facts.Writer, error) { return findings, facts, nil } +// allFactsAndFindings returns all factsAndFindings from an importer. +func (i *importer) allFactsAndFindings() (FindingSet, *facts.Bundle) { + var ( + findings = make(FindingSet, 0) + allFacts = facts.NewBundle() + ) + for path, entry := range i.cache { + findings = append(findings, entry.findings...) + allFacts.Add(path, entry.facts) + } + return findings, allFacts +} + // Facts runs all analyzers, and returns human-readable facts. // // These facts are essentially a dictionary tree (split across all '.' @@ -632,11 +656,8 @@ func Facts(path string, srcs []string) (facts.Resolved, error) { // analyzers for these packages. return nil, err } - _, allFacts, err := i.allFindingsAndFacts() - if err != nil { - return nil, err - } - return facts.Resolve(pkg, localFacts, allFacts, allFactNames), nil + _, allFacts := i.allFactsAndFindings() + return facts.Resolve(pkg, localFacts, allFacts, allFactNames) } // FindRoot finds a package root. @@ -688,19 +709,23 @@ func SplitPackages(srcs []string, srcRootPrefix string) map[string][]string { continue // Also not a file. } - // Skip commands where possible. These also have package names - // that do not match the tree structure and will never be - // dependencies. - if strings.HasPrefix(filename, "cmd/") { + // Ignore any files with /testdata/ in the path. + if strings.Contains(filename, "/testdata/") { continue } - // Skip obvious test files; they have bizarre package semantics - // and are never direct dependencies of anything else. + // Ignore all test files since they *may* be in a different + // package than the rest of the sources. if strings.HasSuffix(filename, "_test.go") { continue } + // Skip the "builtin" package, which is only for docs and not a + // real package. Attempting type checking goes crazy. + if pkg == "builtin" { + continue + } + // Skip unsupported packages explicitly. if _, ok := usesTypeParams[pkg]; ok { log.Printf("WARNING: Skipping package %q: type param analysis not yet supported.", pkg) @@ -727,7 +752,7 @@ var usesTypeParams = map[string]struct{}{ } // Bundle checks a bundle of files (typically the standard library). -func Bundle(sources map[string][]string) (FindingSet, facts.Writer, error) { +func Bundle(sources map[string][]string) (FindingSet, facts.Serializer, error) { // Process all packages. i := &importer{ fset: token.NewFileSet(), @@ -736,12 +761,12 @@ func Bundle(sources map[string][]string) (FindingSet, facts.Writer, error) { imports: make(map[string]*types.Package), } for pkg, _ := range sources { - // Was there an error processing this package? Just print a warning. - if _, _, err := i.importPackage(pkg); err != nil && err != ErrSkip { - log.Printf("WARNING: %v.", err) + // Was there an error processing this package? + if _, err := i.importPackage(pkg); err != nil && err != ErrSkip { + return nil, nil, err } } - // Build our findings and facts. - return i.allFindingsAndFacts() + findings, facts := i.allFactsAndFindings() + return findings, facts, nil } diff --git a/tools/nogo/check/findings.go b/tools/nogo/check/findings.go index 3845990ca..a944f4e2b 100644 --- a/tools/nogo/check/findings.go +++ b/tools/nogo/check/findings.go @@ -21,7 +21,6 @@ import ( "go/token" "io" "os" - "reflect" "sort" ) @@ -32,14 +31,6 @@ type Finding struct { Message string } -// findingSize is the size of the finding struct itself. -var findingSize = int64(reflect.TypeOf(Finding{}).Size()) - -// Size implements worker.Sizer.Size. -func (f *Finding) Size() int64 { - return int64(len(f.Category)) + int64(len(f.Message)) + findingSize -} - // String implements fmt.Stringer.String. func (f *Finding) String() string { return fmt.Sprintf("%s: %s: %s", f.Category, f.Position.String(), f.Message) @@ -48,15 +39,6 @@ func (f *Finding) String() string { // FindingSet is a collection of findings. type FindingSet []Finding -// Size implmements worker.Sizer.Size. -func (fs FindingSet) Size() int64 { - size := int64(0) - for _, finding := range fs { - size += finding.Size() - } - return size -} - // Sort sorts all findings. func (fs FindingSet) Sort() { sort.Slice(fs, func(i, j int) bool { diff --git a/tools/nogo/cli/BUILD b/tools/nogo/cli/BUILD index 902eddd86..dcc8c4e18 100644 --- a/tools/nogo/cli/BUILD +++ b/tools/nogo/cli/BUILD @@ -12,7 +12,6 @@ go_library( "//tools/nogo/config", "//tools/nogo/facts", "//tools/nogo/flags", - "//tools/worker", "@com_github_google_subcommands//:go_default_library", "@in_gopkg_yaml_v2//:go_default_library", "@org_golang_x_sys//unix:go_default_library", diff --git a/tools/nogo/cli/cli.go b/tools/nogo/cli/cli.go index 48a57ee31..04969e526 100644 --- a/tools/nogo/cli/cli.go +++ b/tools/nogo/cli/cli.go @@ -32,13 +32,15 @@ import ( "gvisor.dev/gvisor/tools/nogo/config" "gvisor.dev/gvisor/tools/nogo/facts" "gvisor.dev/gvisor/tools/nogo/flags" - "gvisor.dev/gvisor/tools/worker" ) // openOutput opens an output file. -func openOutput(filename string, def io.Writer) (io.Writer, error) { +func openOutput(filename string, def *os.File) (*os.File, error) { if filename == "" { - return def, nil + if def != nil { + return def, nil + } + filename = "/dev/null" // Sink. } f, err := os.OpenFile(filename, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0644) if err != nil { @@ -104,9 +106,9 @@ func (c *checkCommon) setFlags(fs *flag.FlagSet, commandType string) { } // execute runs the common bits for a check command. -func (c *checkCommon) execute(fn func() (check.FindingSet, facts.Writer, error)) error { +func (c *checkCommon) execute(fn func() (check.FindingSet, facts.Serializer, error)) error { // Open outputs. - factsOutput, err := openOutput(c.Facts, io.Discard) + factsOutput, err := openOutput(c.Facts, nil) if err != nil { return fmt.Errorf("opening facts: %w", err) } @@ -124,7 +126,7 @@ func (c *checkCommon) execute(fn func() (check.FindingSet, facts.Writer, error)) } // Save the data. - if _, err := factData.WriteTo(factsOutput); err != nil { + if err := factData.Serialize(factsOutput); err != nil { return fmt.Errorf("writing facts: %w", err) } if !c.Text && !isTerminal(findingsOutput) { @@ -174,7 +176,6 @@ func (*Check) Usage() string { func (c *Check) SetFlags(fs *flag.FlagSet) { c.setFlags(fs, "check") fs.StringVar(&c.Package, "package", "", "package for analysis (required)") - fs.StringVar(&c.Binary, "binary", "", "binary for analysis (optional, omitting may cause some analyzers to fail)") } // Execute implements subcommands.Command.Execute. @@ -183,15 +184,8 @@ func (c *Check) Execute(ctx context.Context, fs *flag.FlagSet, args ...interface c.Package = "main" // Default, no imports. } - // Add the binary to the import map. Note that it may already be - // provided in the import map via the global command line flags, but - // this is able to override that path. - if c.Binary != "" { - flags.ImportMap[c.Package] = c.Binary - } - // Perform the analysis. - if err := c.execute(func() (check.FindingSet, facts.Writer, error) { + if err := c.execute(func() (check.FindingSet, facts.Serializer, error) { return check.Package(c.Package /* path */, fs.Args() /* srcs */) }); err != nil { return failure("%v", err) @@ -238,7 +232,7 @@ func (b *Bundle) SetFlags(fs *flag.FlagSet) { // Execute implements subcommands.Command.Execute. func (b *Bundle) Execute(ctx context.Context, fs *flag.FlagSet, args ...interface{}) subcommands.ExitStatus { // Perform the analysis. - if err := b.execute(func() (check.FindingSet, facts.Writer, error) { + if err := b.execute(func() (check.FindingSet, facts.Serializer, error) { // Discover the correct common root. srcRootPrefix, err := check.FindRoot(fs.Args(), b.Root) if err != nil { @@ -297,7 +291,7 @@ func (s *Stdlib) Execute(ctx context.Context, fs *flag.FlagSet, args ...interfac return subcommands.ExitUsageError // Need no arguments. } - if err := s.execute(func() (check.FindingSet, facts.Writer, error) { + if err := s.execute(func() (check.FindingSet, facts.Serializer, error) { root, err := flags.Env("GOROOT") if err != nil { return nil, nil, err @@ -320,6 +314,7 @@ type Filter struct { Configs flags.StringList Output string Text bool + Test bool } // Name implements subcommands.Command.Name. @@ -347,83 +342,59 @@ func (f *Filter) SetFlags(fs *flag.FlagSet) { fs.Var(&f.Configs, "config", "filter configuration files (in JSON format)") fs.StringVar(&f.Output, "output", "", "findings output (in JSON format by default, unless attached to a terminal)") fs.BoolVar(&f.Text, "text", false, "force text format in all cases (even not attached to a terminal)") + fs.BoolVar(&f.Test, "test", false, "exit with non-zero status if findings are not empty") } -var ( - cachedFindings = worker.NewCache("findings") // With check.FindingSet. - cachedFiltered = worker.NewCache("filtered") // With check.FindingSet. - cachedConfigs = worker.NewCache("configs") // With config.Config. - cachedFullConfigs = worker.NewCache("compiled") // With config.Config. -) - func loadFindings(filename string) (check.FindingSet, error) { - v, err := cachedFindings.Lookup([]string{filename}, func() (worker.Sizer, error) { - r, err := os.Open(filename) - if err != nil { - return nil, fmt.Errorf("unable to open input: %w", err) - } - inputFindings, err := check.ExtractFindingsFrom(r, false /* json */) - if err != nil { - // Seek to reread the file. - if _, err := r.Seek(0, os.SEEK_SET); err != nil { - return nil, fmt.Errorf("unable to reseek in findings %q: %w", filename, err) - } - // Attempt to interpret as a json input. - inputFindings, err = check.ExtractFindingsFrom(r, true /* json */) - if err != nil { - return nil, fmt.Errorf("unable to extract findings from %q: %w", filename, err) - } - } - return inputFindings, nil - }) + r, err := os.Open(filename) if err != nil { - return nil, err + return nil, fmt.Errorf("unable to open input: %w", err) } - return v.(check.FindingSet), nil + inputFindings, err := check.ExtractFindingsFrom(r, false /* json */) + if err != nil { + // Seek to reread the file. + if _, err := r.Seek(0, os.SEEK_SET); err != nil { + return nil, fmt.Errorf("unable to reseek in findings %q: %w", filename, err) + } + // Attempt to interpret as a json input. + inputFindings, err = check.ExtractFindingsFrom(r, true /* json */) + if err != nil { + return nil, fmt.Errorf("unable to extract findings from %q: %w", filename, err) + } + } + return inputFindings, nil } func loadConfig(filename string) (*config.Config, error) { - v, err := cachedConfigs.Lookup([]string{filename}, func() (worker.Sizer, error) { - f, err := os.Open(filename) - if err != nil { - return nil, fmt.Errorf("unable to open config: %w", err) - } - var newConfig config.Config // For current file. - dec := yaml.NewDecoder(f) - dec.SetStrict(true) - if err := dec.Decode(&newConfig); err != nil { - return nil, fmt.Errorf("unable to decode %q: %w", filename, err) - } - return &newConfig, nil - }) + f, err := os.Open(filename) if err != nil { - return nil, err + return nil, fmt.Errorf("unable to open config: %w", err) } - return v.(*config.Config), nil + var newConfig config.Config // For current file. + dec := yaml.NewDecoder(f) + dec.SetStrict(true) + if err := dec.Decode(&newConfig); err != nil { + return nil, fmt.Errorf("unable to decode %q: %w", filename, err) + } + return &newConfig, nil } func loadConfigs(filenames []string) (*config.Config, error) { - v, err := cachedFullConfigs.Lookup(filenames, func() (worker.Sizer, error) { - config := &config.Config{ - Global: make(config.AnalyzerConfig), - Analyzers: make(map[string]config.AnalyzerConfig), - } - for _, filename := range filenames { - next, err := loadConfig(filename) - if err != nil { - return nil, err - } - config.Merge(next) - } - if err := config.Compile(); err != nil { - return nil, fmt.Errorf("error compiling config: %w", err) - } - return config, nil - }) - if err != nil { - return nil, err + config := &config.Config{ + Global: make(config.AnalyzerConfig), + Analyzers: make(map[string]config.AnalyzerConfig), } - return v.(*config.Config), nil + for _, filename := range filenames { + next, err := loadConfig(filename) + if err != nil { + return nil, err + } + config.Merge(next) + } + if err := config.Compile(); err != nil { + return nil, fmt.Errorf("error compiling config: %w", err) + } + return config, nil } // Execute implements subcommands.Command.Execute. @@ -454,23 +425,15 @@ func (f *Filter) Execute(ctx context.Context, fs *flag.FlagSet, args ...interfac // is guaranteed to be safe. This allows us to reuse the same // filter result many times over, because e.g. all standard // library findings will be available to all packages. - v, err := cachedFiltered.Lookup(append(f.Configs, filename), func() (worker.Sizer, error) { - inputFindings, err := loadFindings(filename) - if err != nil { - return nil, err - } - filteredFindings := make(check.FindingSet, 0, len(inputFindings)) - for _, finding := range inputFindings { - if ok := config.ShouldReport(finding); ok { - filteredFindings = append(filteredFindings, finding) - } - } - return filteredFindings, nil - }) + inputFindings, err := loadFindings(filename) if err != nil { - return failure("unable to load filtered findings from %q: %v", filename, err) + return failure("unable to load findings from %q: %v", filename, err) + } + for _, finding := range inputFindings { + if ok := config.ShouldReport(finding); ok { + filteredFindings = append(filteredFindings, finding) + } } - filteredFindings = append(filteredFindings, v.(check.FindingSet)...) } // Write the output. @@ -484,10 +447,12 @@ func (f *Filter) Execute(ctx context.Context, fs *flag.FlagSet, args ...interfac } } - // Treat the run as a test. - if (f.Text || isTerminal(output)) && len(filteredFindings) == 0 { + // Treat the run as a test? + if (f.Text || isTerminal(output)) && f.Test && len(filteredFindings) == 0 { fmt.Fprintf(output, "PASS\n") - return subcommands.ExitSuccess + } + if f.Test && len(filteredFindings) > 0 { + return subcommands.ExitFailure } return subcommands.ExitSuccess @@ -564,7 +529,6 @@ func Main() { subcommands.Register(&Render{}, "") subcommands.Register(subcommands.HelpCommand(), "") subcommands.Register(subcommands.FlagsCommand(), "") - worker.Work(func(args []string) int { - return int(subcommands.Execute(context.Background())) - }) + flag.CommandLine.Parse(os.Args[1:]) + os.Exit(int(subcommands.Execute(context.Background()))) } diff --git a/tools/nogo/config/config.go b/tools/nogo/config/config.go index 427cfd8e3..aab702741 100644 --- a/tools/nogo/config/config.go +++ b/tools/nogo/config/config.go @@ -218,17 +218,6 @@ type Config struct { Analyzers map[string]AnalyzerConfig `yaml:"analyzers"` } -// Size implements worker.Sizer.Size. -func (c *Config) Size() int64 { - count := c.Global.RegexpCount() - for _, config := range c.Analyzers { - count += config.RegexpCount() - } - // The size is measured as the number of regexps that are compiled - // here. We multiply by 1k to produce an estimate. - return 1024 * count -} - // Merge merges two configurations. func (c *Config) Merge(other *Config) { // Merge all groups. diff --git a/tools/nogo/defs.bzl b/tools/nogo/defs.bzl index b1100fa50..77e228b9e 100644 --- a/tools/nogo/defs.bzl +++ b/tools/nogo/defs.bzl @@ -69,30 +69,27 @@ def _nogo_stdlib_impl(ctx): # Build the analyzer command. facts_file = ctx.actions.declare_file(ctx.label.name + ".facts") findings_file = ctx.actions.declare_file(ctx.label.name + ".raw_findings") - args_file = ctx.actions.declare_file(ctx.label.name + "_args_file") - ctx.actions.write( - output = args_file, - content = "\n".join(args + [ - "bundle", - "-findings=%s" % findings_file.path, - "-facts=%s" % facts_file.path, - "-root=.*?/src/", - ] + [f.path for f in go_ctx.stdlib_srcs]), - ) ctx.actions.run( # For the standard library, we need to include the full set of Go # sources in the inputs. - inputs = inputs + go_ctx.stdlib_srcs + [args_file], + inputs = inputs + go_ctx.stdlib_srcs, outputs = [facts_file, findings_file], tools = depset(go_ctx.runfiles.to_list() + ctx.files._nogo), executable = ctx.files._nogo[0], env = go_ctx.env, mnemonic = "GoStandardLibraryAnalysis", - # Note that this does not support work execution currently. There is an - # issue with stdout pollution that is not yet resolved, so this is kept - # as a separate menomic. progress_message = "Analyzing Go Standard Library", - arguments = ["@%s" % args_file.path], + # Since these actions are generally I/O bound, reading source files, + # facts, binaries and serializing results, disable sandboxing. This can + # be enabled without any issues for correctness, but we want to avoid + # paying the FUSE penalty. + execution_requirements = {"no-sandbox": "1"}, + arguments = args + [ + "bundle", + "-findings=%s" % findings_file.path, + "-facts=%s" % facts_file.path, + "-root=.*?/src/", + ] + [f.path for f in go_ctx.stdlib_srcs], ) # Return the stdlib facts as output. @@ -183,7 +180,8 @@ def _nogo_config(ctx, deps): # Configure where to find the binary & fact files. Note that this will # use .x and .a regardless of whether this is a go_binary rule, since # these dependencies must be go_library rules. - _, x_file = _select_objfile(info.binaries) + a_file, x_file = _select_objfile(info.binaries) + args.append("-archive=%s=%s" % (info.importpath, a_file.path)) args.append("-import=%s=%s" % (info.importpath, x_file.path)) args.append("-facts=%s=%s" % (info.importpath, info.facts.path)) @@ -191,6 +189,7 @@ def _nogo_config(ctx, deps): raw_findings.extend(info.raw_findings) # Ensure the above are available as inputs. + inputs.append(a_file) inputs.append(x_file) inputs.append(info.facts) @@ -207,12 +206,12 @@ def _nogo_package_config(ctx, deps, importpath = None, target = None): binaries = [] if target != None: binaries.extend(target.files.to_list()) - target_objfile, target_xfile = _select_objfile(binaries) - if target_objfile != None: - inputs.append(target_objfile) + target_afile, target_xfile = _select_objfile(binaries) if target_xfile != None: - inputs.append(target_xfile) + args.append("-archive=%s=%s" % (importpath, target_afile.path)) args.append("-import=%s=%s" % (importpath, target_xfile.path)) + inputs.append(target_afile) + inputs.append(target_xfile) # Add the standard library facts. stdlib_info = ctx.attr._nogo_stdlib[NogoStdlibInfo] @@ -272,28 +271,24 @@ def _nogo_aspect_impl(target, ctx): go_ctx, args, inputs, raw_findings = _nogo_package_config(ctx, deps, importpath = importpath, target = target) # Build the argument file, and the runner. - args_file = ctx.actions.declare_file(ctx.label.name + "_args_file") facts_file = ctx.actions.declare_file(ctx.label.name + ".facts") findings_file = ctx.actions.declare_file(ctx.label.name + ".findings") - ctx.actions.write( - output = args_file, - content = "\n".join(args + [ - "check", - "-findings=%s" % findings_file.path, - "-facts=%s" % facts_file.path, - "-package=%s" % importpath, - ] + [src.path for src in srcs]), - ) ctx.actions.run( - inputs = inputs + srcs + [args_file], + inputs = inputs + srcs, outputs = [findings_file, facts_file], tools = depset(go_ctx.runfiles.to_list() + ctx.files._nogo), executable = ctx.files._nogo[0], env = go_ctx.env, mnemonic = "GoStaticAnalysis", progress_message = "Analyzing %s" % target.label, - execution_requirements = {"supports-workers": "1"}, - arguments = ["@%s" % args_file.path], + # See above. + execution_requirements = {"no-sandbox": "1"}, + arguments = args + [ + "check", + "-findings=%s" % findings_file.path, + "-facts=%s" % facts_file.path, + "-package=%s" % importpath, + ] + [src.path for src in srcs], ) # Return the package facts as output. @@ -346,25 +341,19 @@ def _nogo_test_impl(ctx): # Build a step that applies the configuration. config_srcs = ctx.attr.config[NogoConfigInfo].srcs findings = ctx.actions.declare_file(ctx.label.name + ".findings") - args_file = ctx.actions.declare_file(ctx.label.name + "_args_file") - ctx.actions.write( - output = args_file, - content = "\n".join( - ["filter"] + - ["-config=%s" % f.path for f in config_srcs] + - ["-output=%s" % findings.path] + - [f.path for f in raw_findings], - ), - ) ctx.actions.run( - inputs = raw_findings + ctx.files.srcs + config_srcs + [args_file], + inputs = raw_findings + ctx.files.srcs + config_srcs, outputs = [findings], tools = depset(ctx.files._nogo), executable = ctx.files._nogo[0], mnemonic = "GoStaticAnalysis", progress_message = "Generating %s" % ctx.label, - execution_requirements = {"supports-workers": "1"}, - arguments = ["@%s" % args_file.path], + # See above. + execution_requirements = {"no-sandbox": "1"}, + arguments = ["filter"] + + ["-config=%s" % f.path for f in config_srcs] + + ["-output=%s" % findings.path] + + [f.path for f in raw_findings], ) # Build a runner that checks the filtered facts. @@ -375,7 +364,7 @@ def _nogo_test_impl(ctx): runner = ctx.actions.declare_file(ctx.label.name) runner_content = [ "#!/bin/bash", - "exec %s filter -text %s" % (ctx.files._nogo[0].short_path, findings.short_path), + "exec %s filter -test -text %s" % (ctx.files._nogo[0].short_path, findings.short_path), "", ] ctx.actions.write(runner, "\n".join(runner_content), is_executable = True) @@ -444,26 +433,22 @@ def _nogo_facts_impl(ctx): # since this will refer to ctx.files (which contains no binaries). go_ctx, args, inputs, _ = _nogo_package_config(ctx, ctx.attr.deps) - # Build the argument file, and the runner. - args_file = ctx.actions.declare_file(ctx.label.name + "_args_file") - ctx.actions.write( - output = args_file, - content = "\n".join(args + [ - "render", - "-template=%s" % ctx.files.template[0].path, - "-output=%s" % ctx.outputs.output.path, - ] + [src.path for src in ctx.files.srcs]), - ) - inputs += ctx.files.template + # Build the runner. ctx.actions.run( - inputs = inputs + ctx.files.srcs + ctx.files.template + [args_file], + inputs = inputs + ctx.files.srcs + ctx.files.template, outputs = [ctx.outputs.output], tools = depset(go_ctx.runfiles.to_list() + ctx.files._nogo), executable = ctx.files._nogo[0], env = go_ctx.env, mnemonic = "GoStaticAnalysis", progress_message = "Generating %s" % ctx.label, - arguments = ["@%s" % args_file.path], + # See above. + execution_requirements = {"no-sandbox": "1"}, + arguments = args + [ + "render", + "-template=%s" % ctx.files.template[0].path, + "-output=%s" % ctx.outputs.output.path, + ] + [src.path for src in ctx.files.srcs], ) # Return the output. diff --git a/tools/nogo/facts/facts.go b/tools/nogo/facts/facts.go index 782058b31..1a76afbed 100644 --- a/tools/nogo/facts/facts.go +++ b/tools/nogo/facts/facts.go @@ -16,22 +16,23 @@ package facts import ( - "bytes" "encoding/gob" + "fmt" "go/types" "io" - "log" "reflect" "sort" + "archive/zip" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/types/objectpath" ) -// Writer is used for fact serialization. -type Writer interface { - io.ReaderFrom - io.WriterTo +// Serializer is used for fact serialization. +// +// It generalizes over the Package and Bundle types. +type Serializer interface { + Serialize(w io.Writer) error } // item is used for serialiation. @@ -40,7 +41,7 @@ type item struct { Value interface{} } -// writeItems is an implementation of io.WriterTo.WriteTo. +// writeItems is an implementation of Serialize. // // This will sort the list as a side effect. func writeItems(w io.Writer, is []item) error { @@ -65,50 +66,53 @@ func readItems(r io.Reader) (is []item, err error) { // because all imports are shared across all packages, there is a single // canonical types.Object shared among all packages being analyzed. type Package struct { - pkg *types.Package Objects map[types.Object][]analysis.Fact } // NewPackage returns a new set of Package facts. -func NewPackage(pkg *types.Package) *Package { +func NewPackage() *Package { return &Package{ - pkg: pkg, Objects: make(map[types.Object][]analysis.Fact), } } -// WriteTo implements io.WriterTo.WriteTo. -func (p *Package) WriteTo(w io.Writer) (int64, error) { - is := make([]item, 0, len(p.Objects)) - for obj, facts := range p.Objects { - var ( - name objectpath.Path - err error - ) - if obj != nil { - name, err = objectpath.For(obj) - } - if err != nil { - continue // Not exported, expected. - } - for _, fact := range facts { - is = append(is, item{ - Key: string(name), - Value: fact, - }) +func extractObjectpath(obj types.Object) (name objectpath.Path, err error) { + defer func() { + // Unfortunately, objectpath.For will occasionally panic for + // certain objects. This happens with basic analysis packages + // (buildssa), and therefore cannot be avoided. + if r := recover(); r != nil { + err = fmt.Errorf("panic: %v", r) } + }() + // Allow empty name for no object. + if obj != nil { + name, err = objectpath.For(obj) } - if err := writeItems(w, is); err != nil { - return 0, err - } - return 1, nil + return } -// ReadFrom implements io.ReaderFrom.ReadFrom. -func (p *Package) ReadFrom(r io.Reader) (int64, error) { +// Serialize implements Serializer.Serialize. +func (p *Package) Serialize(w io.Writer) error { + is := make([]item, 0, len(p.Objects)) + for obj, facts := range p.Objects { + name, err := extractObjectpath(obj) + if err != nil { + continue // Not exported; expected. + } + is = append(is, item{ + Key: string(name), + Value: facts, + }) + } + return writeItems(w, is) +} + +// ReadFrom deserializes a package. +func (p *Package) ReadFrom(pkg *types.Package, r io.Reader) error { is, err := readItems(r) if err != nil { - return 0, err + return err } for _, fi := range is { var ( @@ -116,35 +120,24 @@ func (p *Package) ReadFrom(r io.Reader) (int64, error) { err error ) if fi.Key != "" { - obj, err = objectpath.Object(p.pkg, objectpath.Path(fi.Key)) + obj, err = objectpath.Object(pkg, objectpath.Path(fi.Key)) } if err != nil { // This could simply be a fact saved on an unexported // object. We just suppress this error and ignore it. continue } - p.Objects[obj] = append(p.Objects[obj], fi.Value.(analysis.Fact)) + p.Objects[obj] = fi.Value.([]analysis.Fact) } - return 1, nil -} - -// Size implements worker.Sizer.Size. -func (p *Package) Size() int64 { - total := int64(0) - for _, val := range p.Objects { - total += int64(8) // 8-byte pointer. - total += int64(len(val)) * 16 // 16-bytes per object. - } - return total + return nil } // ExportFact exports an object fact. -func (p Package) ExportFact(obj types.Object, ptr analysis.Fact) { +func (p *Package) ExportFact(obj types.Object, ptr analysis.Fact) { for i, v := range p.Objects[obj] { if reflect.TypeOf(v) == reflect.TypeOf(ptr) { - // Drop this item from the list. - p.Objects[obj] = append(p.Objects[obj][:i], p.Objects[obj][i+1:]...) - break + p.Objects[obj][i] = ptr // Replace. + return } } // Append this new fact. @@ -153,6 +146,9 @@ func (p Package) ExportFact(obj types.Object, ptr analysis.Fact) { // ImportFact imports an object fact. func (p *Package) ImportFact(obj types.Object, ptr analysis.Fact) bool { + if p == nil { + return false // No facts. + } for _, v := range p.Objects[obj] { if reflect.TypeOf(v) == reflect.TypeOf(ptr) { // Set the value to the element saved in our facts. @@ -163,78 +159,101 @@ func (p *Package) ImportFact(obj types.Object, ptr analysis.Fact) bool { return false } -// Bundle is a set of facts about different packages. This is typically -// used for the standard library, but may be used for e.g. module dependencies. +// Bundle is a set of facts about different packages. +// +// This is used to serialize a collection of facts about different packages, +// which will be loaded and evaluated lazily. type Bundle struct { - importer types.Importer - Packages map[string]*Package + reader *zip.ReadCloser + decoded map[string]*Package } -// NewBundle returns a new bundle. -func NewBundle(importer types.Importer) *Bundle { +// NewBundle makes a new package bundle. +func NewBundle() *Bundle { return &Bundle{ - importer: importer, - Packages: make(map[string]*Package), + decoded: make(map[string]*Package), } } -// Size implements worker.Sizer.Size. -func (b *Bundle) Size() int64 { - size := int64(0) - for filename, p := range b.Packages { - size += int64(len(filename)) - size += p.Size() - } - return size -} - -// WriteTo implements io.WriterTo.WriteTo. -func (b *Bundle) WriteTo(w io.Writer) (int64, error) { - is := make([]item, 0, len(b.Packages)) - for pkg, facts := range b.Packages { +// Serialize implements Serializer.Serialize. +func (b *Bundle) Serialize(w io.Writer) error { + zw := zip.NewWriter(w) + for pkg, facts := range b.decoded { if facts == nil { // Some facts may be omitted for bundles, if there is // only type information but no source information. We // omit these completely from the serialized bundle. continue } - var buf bytes.Buffer - if _, err := facts.WriteTo(&buf); err != nil { - return 0, err - } - is = append(is, item{ - Key: pkg, - Value: buf.Bytes(), - }) - } - if err := writeItems(w, is); err != nil { - return 0, err - } - return 1, nil -} - -// ReadFrom implements io.ReaderFrom.ReadFrom. -func (b *Bundle) ReadFrom(r io.Reader) (int64, error) { - is, err := readItems(r) - if err != nil { - return 0, err - } - for _, fi := range is { - pkg, err := b.importer.Import(fi.Key) - if err != nil { - // There's nothing that can be done here, but we can - // report the warning at least. This is not expected. - log.Printf("WARNING: lost facts from %q: %v", fi.Key, err) + if len(facts.Objects) == 0 { + // Similarly prevent serializing any Packages that have + // no facts associated with them. This will speed up + // deserialization since the Package can handle nil. continue } - buf := bytes.NewBuffer(fi.Value.([]byte)) - facts := NewPackage(pkg) - if _, err := facts.ReadFrom(buf); err != nil { - return 0, err + wc, err := zw.Create(pkg) + if err != nil { + return err + } + if err := facts.Serialize(wc); err != nil { + return err } - b.Packages[fi.Key] = facts } - return 1, nil + return zw.Close() +} + +// BundleFrom may be used to create a new bundle that deserializes the contents +// of the given file. +// +// Note that there is no explicit close mechanism, and the underlying file will +// be closed only when the object is finalized. +func BundleFrom(filename string) (*Bundle, error) { + r, err := zip.OpenReader(filename) + if err != nil { + return nil, err + } + return &Bundle{ + reader: r, + decoded: make(map[string]*Package), + }, nil +} + +// Add adds the package to the Bundle. +func (b *Bundle) Add(path string, facts *Package) { + b.decoded[path] = facts +} + +// Package looks up the given package in the bundle. +func (b *Bundle) Package(pkg *types.Package) (*Package, error) { + // Already decoded? + if facts, ok := b.decoded[pkg.Path()]; ok { + return facts, nil + } + + // Find based on the reader. + for _, f := range b.reader.File { + if f.Name != pkg.Path() { + continue + } + + // Extract from the archive. + facts := NewPackage() + rc, err := f.Open() + if err != nil { + return nil, err + } + defer rc.Close() + if err := facts.ReadFrom(pkg, rc); err != nil { + return nil, err + } + + // Memoize the result. + b.Add(pkg.Path(), facts) + return facts, nil + } + + // Nothing available. + return nil, nil } // Resolved is a human-readable fact format. @@ -337,7 +356,7 @@ func (r Resolved) walkScope(parents []string, scope *types.Scope, facts *Package } // Resolve resolves all object facts. -func Resolve(pkg *types.Package, localFacts *Package, allFacts *Bundle, allFactNames map[reflect.Type]string) Resolved { +func Resolve(pkg *types.Package, localFacts *Package, allFacts *Bundle, allFactNames map[reflect.Type]string) (Resolved, error) { // Populate the tree. Allocating this slice up front prevents // allocation during name resolution. We allow for up to 64 names // without allocating a new backing array. @@ -345,12 +364,16 @@ func Resolve(pkg *types.Package, localFacts *Package, allFacts *Bundle, allFactN names := make([]string, 0, 64) r.walkScope(names, pkg.Scope(), localFacts, allFactNames) for _, importPkg := range pkg.Imports() { - importFacts := allFacts.Packages[importPkg.Path()] + importFacts, err := allFacts.Package(importPkg) + if err != nil { + return nil, err + } r.walkScope(append(names, "import", importPkg.Name()), importPkg.Scope(), importFacts, allFactNames) } - return r + return r, nil } func init() { gob.Register((*item)(nil)) + gob.Register(([]analysis.Fact)(nil)) } diff --git a/tools/nogo/flags/flags.go b/tools/nogo/flags/flags.go index 0b85b2ec9..44f9dae9f 100644 --- a/tools/nogo/flags/flags.go +++ b/tools/nogo/flags/flags.go @@ -46,6 +46,9 @@ var ( // ImportMap defines all binary input files. ImportMap = StringMap{} + // ArchiveMap defines all binary archive files. + ArchiveMap = StringMap{} + // FactMap defines all fact input files. FactMap = StringMap{} @@ -60,6 +63,7 @@ func init() { flag.StringVar(&GOARCH, "GOARCH", runtime.GOARCH, "GOARCH for analysis") flag.StringVar(&BuildTags, "tags", "", "comma-separated build tags") flag.Var(&ImportMap, "import", "map of import paths to binaries") + flag.Var(&ArchiveMap, "archive", "map of import paths to archives") flag.Var(&FactMap, "facts", "map of import paths to facts") flag.Var(&Bundles, "bundle", "list of fact bundles") } diff --git a/tools/nogo/sanity/BUILD b/tools/nogo/sanity/BUILD new file mode 100644 index 000000000..057333208 --- /dev/null +++ b/tools/nogo/sanity/BUILD @@ -0,0 +1,36 @@ +load("//tools:defs.bzl", "go_library") +load("//tools/nogo:defs.bzl", "nogo_config", "nogo_test") + +package(licenses = ["notice"]) + +go_library( + name = "sanity", + srcs = ["sanity.go"], + # This would fail, so we need to disable here. Instead, we manually + # construct the nogo test below, and ensure that it is failing. + nogo = False, +) + +nogo_config( + name = "sanity_config", + srcs = ["sanity.yaml"], +) + +nogo_test( + name = "sanity_nogo", + srcs = ["sanity.go"], + config = ":sanity_config", + tags = [ + "manual", + "noguitar", + "notap", + ], + deps = [":sanity"], +) + +sh_test( + name = "sanity_test", + srcs = ["sanity_test.sh"], + args = ["$(location :sanity_nogo)"], + data = [":sanity_nogo"], +) diff --git a/tools/nogo/sanity/sanity.go b/tools/nogo/sanity/sanity.go new file mode 100644 index 000000000..a664e8b15 --- /dev/null +++ b/tools/nogo/sanity/sanity.go @@ -0,0 +1,26 @@ +// Copyright 2022 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package sanity provides a basic sanity test. +package sanity + +import ( + "fmt" + "os" +) + +func improperFormat() { + // This should always be caught by sanitizers. + fmt.Fprintf(os.Stderr, "%d\n", "hello world!") +} diff --git a/tools/nogo/sanity/sanity.yaml b/tools/nogo/sanity/sanity.yaml new file mode 100644 index 000000000..63737605c --- /dev/null +++ b/tools/nogo/sanity/sanity.yaml @@ -0,0 +1,7 @@ +groups: + - name: all + regex: ".*" + default: false +analyzers: + printf: + all: # Enabled. diff --git a/tools/nogo/sanity/sanity_test.sh b/tools/nogo/sanity/sanity_test.sh new file mode 100755 index 000000000..871246c7f --- /dev/null +++ b/tools/nogo/sanity/sanity_test.sh @@ -0,0 +1,37 @@ +#!/bin/bash + +# Copyright 2022 The gVisor Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -xuo pipefail # Allow failure; we check below. + +declare output +declare rc +output="$("${1}")" # Execute the provided parameter. +rc=$? # Record the result. +readonly output +readonly rc + +# Sanity check. +if [[ "${rc}" -eq "0" ]]; then + echo "Expected failure; got success." + exit 1 +fi +if [[ "${output}" =~ ^sanity.go:[0-9]+:[0-9]+:.*%d.*string.*$ ]]; then + echo "Expected format error; not found." + exit 1 +fi + +# All done. +echo "PASS" diff --git a/tools/rules_go_sdk.patch b/tools/rules_go_sdk.patch new file mode 100644 index 000000000..f022e5ff9 --- /dev/null +++ b/tools/rules_go_sdk.patch @@ -0,0 +1,31 @@ +diff --git a/go/private/sdk.bzl b/go/private/sdk.bzl +index cf6f73e6..26c9720c 100644 +--- a/go/private/sdk.bzl ++++ b/go/private/sdk.bzl +@@ -109,6 +109,8 @@ def _go_download_sdk_impl(ctx): + "sdks": ctx.attr.sdks, + "urls": ctx.attr.urls, + "version": version, ++ "patch": ctx.attr.patch, ++ "patch_strip": ctx.attr.patch_strip, + "strip_prefix": ctx.attr.strip_prefix, + } + return None +@@ -121,6 +123,8 @@ _go_download_sdk = repository_rule( + "sdks": attr.string_list_dict(), + "urls": attr.string_list(default = ["https://dl.google.com/go/{}"]), + "version": attr.string(), ++ "patch": attr.label(default = None), ++ "patch_strip": attr.int(default = 0), + "strip_prefix": attr.string(default = "go"), + }, + ) +@@ -210,6 +214,8 @@ def _remote_sdk(ctx, urls, strip_prefix, sha256): + res = ctx.execute(["tar", "-xf", "go_sdk.tar.gz", "--strip-components=1"]) + if res.return_code: + fail("error extracting Go SDK:\n" + res.stdout + res.stderr) ++ if ctx.attr.patch: ++ ctx.patch(ctx.attr.patch, strip=ctx.attr.patch_strip) + ctx.delete("go_sdk.tar.gz") + else: + ctx.download_and_extract( diff --git a/tools/worker/BUILD b/tools/worker/BUILD deleted file mode 100644 index 2c12fed2f..000000000 --- a/tools/worker/BUILD +++ /dev/null @@ -1,51 +0,0 @@ -load("//tools:defs.bzl", "bazel_worker_proto", "go_library") -load("//tools/go_generics:defs.bzl", "go_template_instance") - -package(licenses = ["notice"]) - -# For Google-tooling. -# @unused -glaze_ignore = [ - "worker.go", -] - -go_template_instance( - name = "lru_list", - out = "lru_list.go", - package = "worker", - prefix = "lru", - template = "//pkg/ilist:generic_list", - types = { - "Element": "*cacheEntry", - "Linker": "*cacheEntry", - }, -) - -go_template_instance( - name = "cache_list", - out = "cache_list.go", - package = "worker", - prefix = "caches", - template = "//pkg/ilist:generic_list", - types = { - "Element": "*Cache", - "Linker": "*Cache", - }, -) - -go_library( - name = "worker", - srcs = [ - "worker.go", - ":cache_list", - ":lru_list", - ], - visibility = ["//tools:__subpackages__"], - deps = [ - bazel_worker_proto, - "//runsc/flag", - "@org_golang_google_protobuf//encoding/protowire:go_default_library", - "@org_golang_google_protobuf//proto:go_default_library", - "@org_golang_x_sys//unix:go_default_library", - ], -) diff --git a/tools/worker/worker.go b/tools/worker/worker.go deleted file mode 100644 index 3230bd9c8..000000000 --- a/tools/worker/worker.go +++ /dev/null @@ -1,389 +0,0 @@ -// Copyright 2021 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package worker provides an implementation of the bazel worker protocol. -// -// Tools may be written as a normal command line utility, except the passed -// run function may be invoked multiple times. -package worker - -import ( - "bufio" - "bytes" - "fmt" - "io" - "io/ioutil" - "log" - "net" - "net/http" - "os" - "path/filepath" - "sort" - "strings" - "sync" - - _ "net/http/pprof" // For profiling. - - "golang.org/x/sys/unix" - "google.golang.org/protobuf/encoding/protowire" - "google.golang.org/protobuf/proto" - wpb "gvisor.dev/bazel/worker_protocol_go_proto" - "gvisor.dev/gvisor/runsc/flag" -) - -var ( - persistentWorker = flag.Bool("persistent_worker", false, "enable persistent worker.") - workerDebug = flag.Bool("worker_debug", false, "debug persistent workers.") - maximumCacheUsage = flag.Int64("maximum_cache_usage", 1024*1024*1024, "maximum cache size.") -) - -var ( - // inputFiles is the last set of input files. - // - // This is used for cache invalidation. The key is the *absolute* path - // name, and the value is the digest in the current run. - inputFiles = make(map[string]string) -) - -// LookupDigest returns a digest for the given file. -func LookupDigest(filename string) (string, bool) { - digest, ok := inputFiles[filename] - return digest, ok -} - -var ( - // allCaches is a global list of caches. - allCaches cachesList - - // globalMu is a globalMutex for globalLRU. - // - // Note that this has a strict lock ordering requirement. No cache locks - // may be held when acquiring this lock. - globalMu sync.Mutex - - // globalLRU is a globalLRU for all entries. - // - // Protected by globalMu. - globalLRU lruList - - // totalCacheUsage is the total usage of all caches. - // - // Protected by globalMu. - totalCacheUsage int64 -) - -// Sizer returns a size. -type Sizer interface { - Size() int64 -} - -// cacheEntry is a cache entry. -// -// The cacheEntry object is immutable, with the exception of the ready -// WaitGroup, which may be signalled. -type cacheEntry struct { - cache *Cache - key string - sizer Sizer - err error - ready sync.WaitGroup - lruEntry // in globalLRU. -} - -// Cache is a worker cache. -// -// They can be created via NewCache. -type Cache struct { - name string - mu sync.Mutex - entries map[string]*cacheEntry - size int64 - hits int64 - misses int64 - cachesEntry // in allCaches. -} - -// NewCache returns a new cache. -// -// Precondition: this must be called at init. -func NewCache(name string) *Cache { - c := &Cache{ - name: name, - entries: make(map[string]*cacheEntry), - } - allCaches.PushBack(c) - return c -} - -// mustAbs returns the absolute path of a filename or dies. -func mustAbs(filename string) string { - abs, err := filepath.Abs(filename) - if err != nil { - log.Fatalf("error getting absolute path: %v", err) - } - return abs -} - -// Lookup looks up an entry in the cache. -func (c *Cache) Lookup(filenames []string, generate func() (Sizer, error)) (Sizer, error) { - digests := make([]string, 0, len(filenames)) - for _, filename := range filenames { - digest, ok := inputFiles[mustAbs(filename)] - if !ok { - // This is not a valid input. We may not be running as - // persistent worker in this cache. If that's the case, - // then the file's contents will not change across the - // run, and we just use the filename itself. - digest = filename - } - digests = append(digests, digest) - } - - // Attempt the lookup. - sort.Slice(digests, func(i, j int) bool { - return digests[i] < digests[j] - }) - cacheKey := strings.Join(digests, "+") - - c.mu.Lock() - entry, ok := c.entries[cacheKey] - if ok { - c.hits++ - c.mu.Unlock() // See ordering requirement. - if entry.sizer != nil { - globalMu.Lock() - globalLRU.Remove(entry) - globalLRU.PushBack(entry) - globalMu.Unlock() - } - entry.ready.Wait() - return entry.sizer, entry.err - } - - // Generate a new entry. - c.misses++ - entry = &cacheEntry{ - cache: c, - key: cacheKey, - } - entry.ready.Add(1) - c.entries[cacheKey] = entry - c.mu.Unlock() // Unlock for generate. - entry.sizer, entry.err = generate() - entry.ready.Done() - - // Does this need to be accounted? We consider negative cache entries - // to be free, in order to avoid extra work. - if entry.sizer == nil { - return entry.sizer, entry.err - } - - // Account for the size of this item. This is complex, but we may clear - // out other caches based on the globalLRU. Only items with non-zero - // size are added here. This routine is the reason for the locking - // order requirement on globalMu and must be respected. - globalMu.Lock() - globalLRU.PushBack(entry) - totalCacheUsage += entry.sizer.Size() - if totalCacheUsage > *maximumCacheUsage { - for entry := globalLRU.Front(); entry != nil && totalCacheUsage > *maximumCacheUsage; entry = globalLRU.Front() { - sz := entry.sizer.Size() - - // Remove from its cache. - entry.cache.mu.Lock() - delete(entry.cache.entries, entry.key) - entry.cache.size -= sz - entry.cache.mu.Unlock() - - // Remove from the global list. - globalLRU.Remove(entry) - totalCacheUsage -= sz - } - } - globalMu.Unlock() - - // Return the value. - return entry.sizer, entry.err -} - -// allCacheStats returns stats for all caches. -func allCacheStats() string { - var ( - sb strings.Builder - count int - ) - for c := allCaches.Front(); c != nil; c = c.Next() { - c.mu.Lock() - if len(c.entries) == 0 { - c.mu.Unlock() - continue // Not active. - } - count++ // At least one active cache. - ratio := float64(c.hits) / float64(c.hits+c.misses) - fmt.Fprintf(&sb, - "% 10s: count: % 5d size: % 10d hits: % 7d misses: % 7d ratio: %2.2f\n", - c.name, len(c.entries), c.size, c.hits, c.misses, ratio) - c.mu.Unlock() - } - if count > 0 { - fmt.Fprintf(&sb, "total: % 10d\n", totalCacheUsage) - } - return sb.String() -} - -// safeBuffer is a trivial wrapper around bytes.Buffer. -type safeBuffer struct { - mu sync.Mutex - bytes.Buffer -} - -// Write implements io.Writer.Write. -func (s *safeBuffer) Write(p []byte) (int, error) { - s.mu.Lock() - defer s.mu.Unlock() - return s.Buffer.Write(p) -} - -// Work invokes the main function. -func Work(run func([]string) int) { - flag.CommandLine.Parse(os.Args[1:]) - if !*persistentWorker { - // Handle the argument file. - args := flag.CommandLine.Args() - if len(args) == 1 && len(args[0]) > 1 && args[0][0] == '@' { - content, err := ioutil.ReadFile(args[0][1:]) - if err != nil { - log.Fatalf("unable to parse args file: %v", err) - } - // Pull arguments from the file. - args = strings.Split(string(content), "\n") - for i := 0; i < len(args); { - if args[i] == "" { - // Remove empty arguments. - copy(args[i:], args[i+1:]) - args = args[:len(args)-1] - continue - } - i++ // Visit next. - } - flag.CommandLine.Parse(args) - args = flag.CommandLine.Args() - } - os.Exit(run(args)) - } - - var listenHeader string // Emitted always. - if *workerDebug { - // Bind a server for profiling. - listener, err := net.Listen("tcp", "localhost:0") - if err != nil { - log.Fatalf("unable to bind a server: %v", err) - } - // Construct the header for stats output, below. - listenHeader = fmt.Sprintf("Listening @ http://localhost:%d\n", listener.Addr().(*net.TCPAddr).Port) - go http.Serve(listener, nil) - } - - // Move stdout. This is done to prevent anything else from accidentally - // printing to stdout, which must contain only the valid WorkerResponse - // serialized protos. - newOutput, err := unix.Dup(1) - if err != nil { - log.Fatalf("unable to move stdout: %v", err) - } - // Stderr may be closed or may be a copy of stdout. We make sure that - // we have an output that is in a completely separate range. - for newOutput <= 2 { - newOutput, err = unix.Dup(newOutput) - if err != nil { - log.Fatalf("unable to move stdout: %v", err) - } - } - rPipe, wPipe, err := os.Pipe() - if err != nil { - log.Fatalf("unable to create pipe: %v", err) - } - if err := unix.Dup2(int(wPipe.Fd()), 1); err != nil { - log.Fatalf("error duping over stdout: %v", err) - } - if err := unix.Dup2(int(wPipe.Fd()), 2); err != nil { - log.Fatalf("error duping over stderr: %v", err) - } - wPipe.Close() // Still open at stdout, stderr. - rPipe.Close() // Read end of pipe is now closed. - - // Read requests from stdin. - input := bufio.NewReader(os.NewFile(0, "input")) - output := bufio.NewWriter(os.NewFile(uintptr(newOutput), "output")) - for { - szBuf, err := input.Peek(4) - if err != nil { - log.Fatalf("unabel to read header: %v", err) - } - - // Parse the size, and discard bits. - sz, szBytes := protowire.ConsumeVarint(szBuf) - if szBytes < 0 { - szBytes = 0 - } - if _, err := input.Discard(szBytes); err != nil { - log.Fatalf("error discarding size: %v", err) - } - - // Read a full message. - msg := make([]byte, int(sz)) - if _, err := io.ReadFull(input, msg); err != nil { - log.Fatalf("error reading worker request: %v", err) - } - var wreq wpb.WorkRequest - if err := proto.Unmarshal(msg, &wreq); err != nil { - log.Fatalf("error unmarshaling worker request: %v", err) - } - - // Flush relevant caches. - inputFiles = make(map[string]string) - for _, input := range wreq.GetInputs() { - inputFiles[mustAbs(input.GetPath())] = string(input.GetDigest()) - } - - // Prepare logging. - var outputBuffer safeBuffer - outputBuffer.WriteString(listenHeader) - log.SetOutput(&outputBuffer) - - // Parse all arguments. - flag.CommandLine.Parse(wreq.GetArguments()) - exitCode := run(flag.CommandLine.Args()) - - // Attach all cache stats. - if *workerDebug { - outputBuffer.WriteString(allCacheStats()) - } - - // Send the response. - var wresp wpb.WorkResponse - wresp.ExitCode = int32(exitCode) - wresp.Output = string(outputBuffer.Bytes()) - rmsg, err := proto.Marshal(&wresp) - if err != nil { - log.Fatalf("error marshaling response: %v", err) - } - if _, err := output.Write(append(protowire.AppendVarint(nil, uint64(len(rmsg))), rmsg...)); err != nil { - log.Fatalf("error sending worker response: %v", err) - } - if err := output.Flush(); err != nil { - log.Fatalf("error flushing output: %v", err) - } - } -}