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