diff options
Diffstat (limited to 'tools')
39 files changed, 1494 insertions, 1553 deletions
diff --git a/tools/bazel.mk b/tools/bazel.mk index 25575c02c..88431ce66 100644 --- a/tools/bazel.mk +++ b/tools/bazel.mk @@ -14,12 +14,16 @@ # See the License for the specific language governing permissions and # limitations under the License. +# Make hacks. +EMPTY := +SPACE := $(EMPTY) $(EMPTY) + # See base Makefile. SHELL=/bin/bash -o pipefail BRANCH_NAME := $(shell (git branch --show-current 2>/dev/null || \ git rev-parse --abbrev-ref HEAD 2>/dev/null) | \ xargs -n 1 basename 2>/dev/null) -BUILD_ROOT := $(CURDIR)/bazel-bin/ +BUILD_ROOTS := bazel-bin/ bazel-out/ # Bazel container configuration (see below). USER ?= gvisor @@ -152,10 +156,12 @@ build_cmd = docker exec $(FULL_DOCKER_EXEC_OPTIONS) $(DOCKER_NAME) sh -o pipefai build_paths = $(build_cmd) 2>&1 \ | tee /proc/self/fd/2 \ - | grep " bazel-bin/" \ + | grep -A1 -E '^Target' \ + | grep -E '^ ($(subst $(SPACE),|,$(BUILD_ROOTS)))' \ | sed "s/ /\n/g" \ | strings -n 10 \ | awk '{$$1=$$1};1' \ + | xargs -n 1 -I {} readlink -f "{}" \ | xargs -n 1 -I {} sh -c "$(1)" build: bazel-server diff --git a/tools/bazeldefs/BUILD b/tools/bazeldefs/BUILD index 8d4356119..d043caf06 100644 --- a/tools/bazeldefs/BUILD +++ b/tools/bazeldefs/BUILD @@ -26,43 +26,6 @@ rbe_platform( remote_execution_properties = """ properties: { name: "container-image" - value:"docker://gcr.io/cloud-marketplace/google/rbe-ubuntu16-04@sha256:93f7e127196b9b653d39830c50f8b05d49ef6fd8739a9b5b8ab16e1df5399e50" - } - properties: { - name: "dockerAddCapabilities" - value: "SYS_ADMIN" - } - properties: { - name: "dockerPrivileged" - value: "true" - } - """, -) - -rbe_toolchain( - name = "cc-toolchain-clang-x86_64-default", - exec_compatible_with = [], - tags = [ - "manual", - ], - target_compatible_with = [], - toolchain = "@bazel_toolchains//configs/ubuntu16_04_clang/10.0.0/bazel_2.0.0/cc:cc-compiler-k8", - toolchain_type = "@bazel_tools//tools/cpp:toolchain_type", -) - -# Updated versions of the above, compatible with bazel3. -rbe_platform( - name = "rbe_ubuntu1604_bazel3", - constraint_values = [ - "@bazel_tools//platforms:x86_64", - "@bazel_tools//platforms:linux", - "@bazel_tools//tools/cpp:clang", - "@bazel_toolchains_bazel3//constraints:xenial", - "@bazel_toolchains_bazel3//constraints/sanitizers:support_msan", - ], - remote_execution_properties = """ - properties: { - name: "container-image" value:"docker://gcr.io/cloud-marketplace/google/rbe-ubuntu16-04@sha256:b516a2d69537cb40a7c6a7d92d0008abb29fba8725243772bdaf2c83f1be2272" } properties: { @@ -77,13 +40,13 @@ rbe_platform( ) rbe_toolchain( - name = "cc-toolchain-clang-x86_64-default_bazel3", + name = "cc-toolchain-clang-x86_64-default", exec_compatible_with = [], tags = [ "manual", ], target_compatible_with = [], - toolchain = "@bazel_toolchains_bazel3//configs/ubuntu16_04_clang/11.0.0/bazel_3.1.0/cc:cc-compiler-k8", + toolchain = "@bazel_toolchains//configs/ubuntu16_04_clang/11.0.0/bazel_3.1.0/cc:cc-compiler-k8", toolchain_type = "@bazel_tools//tools/cpp:toolchain_type", ) diff --git a/tools/bazeldefs/cc.bzl b/tools/bazeldefs/cc.bzl new file mode 100644 index 000000000..7f41a0142 --- /dev/null +++ b/tools/bazeldefs/cc.bzl @@ -0,0 +1,43 @@ +"""C++ rules.""" + +load("@bazel_tools//tools/cpp:cc_flags_supplier.bzl", _cc_flags_supplier = "cc_flags_supplier") +load("@rules_cc//cc:defs.bzl", _cc_binary = "cc_binary", _cc_library = "cc_library", _cc_proto_library = "cc_proto_library", _cc_test = "cc_test") +load("@com_github_grpc_grpc//bazel:cc_grpc_library.bzl", _cc_grpc_library = "cc_grpc_library") + +cc_library = _cc_library +cc_flags_supplier = _cc_flags_supplier +cc_proto_library = _cc_proto_library +cc_test = _cc_test +cc_toolchain = "@bazel_tools//tools/cpp:current_cc_toolchain" +gtest = "@com_google_googletest//:gtest" +gbenchmark = "@com_google_benchmark//:benchmark" +grpcpp = "@com_github_grpc_grpc//:grpc++" +vdso_linker_option = "-fuse-ld=gold " + +def cc_grpc_library(name, **kwargs): + _cc_grpc_library(name = name, grpc_only = True, **kwargs) + +def cc_binary(name, static = False, **kwargs): + """Run cc_binary. + + Args: + name: name of the target. + static: make a static binary if True + **kwargs: the rest of the args. + """ + if static: + # How to statically link a c++ program that uses threads, like for gRPC: + # https://gcc.gnu.org/legacy-ml/gcc-help/2010-05/msg00029.html + if "linkopts" not in kwargs: + kwargs["linkopts"] = [] + kwargs["linkopts"] += [ + "-static", + "-lstdc++", + "-Wl,--whole-archive", + "-lpthread", + "-Wl,--no-whole-archive", + ] + _cc_binary( + name = name, + **kwargs + ) diff --git a/tools/bazeldefs/defs.bzl b/tools/bazeldefs/defs.bzl index cf5b1dc0d..ba186aace 100644 --- a/tools/bazeldefs/defs.bzl +++ b/tools/bazeldefs/defs.bzl @@ -1,35 +1,13 @@ -"""Bazel implementations of standard rules.""" +"""Meta and miscellaneous rules.""" -load("@bazel_gazelle//:def.bzl", _gazelle = "gazelle") load("@bazel_skylib//rules:build_test.bzl", _build_test = "build_test") load("@bazel_skylib//:bzl_library.bzl", _bzl_library = "bzl_library") -load("@bazel_tools//tools/cpp:cc_flags_supplier.bzl", _cc_flags_supplier = "cc_flags_supplier") -load("@io_bazel_rules_go//go:def.bzl", "GoLibrary", _go_binary = "go_binary", _go_context = "go_context", _go_embed_data = "go_embed_data", _go_library = "go_library", _go_path = "go_path", _go_test = "go_test") -load("@io_bazel_rules_go//proto:def.bzl", _go_grpc_library = "go_grpc_library", _go_proto_library = "go_proto_library") -load("@rules_cc//cc:defs.bzl", _cc_binary = "cc_binary", _cc_library = "cc_library", _cc_proto_library = "cc_proto_library", _cc_test = "cc_test") -load("@rules_pkg//:pkg.bzl", _pkg_deb = "pkg_deb", _pkg_tar = "pkg_tar") -load("@com_github_grpc_grpc//bazel:cc_grpc_library.bzl", _cc_grpc_library = "cc_grpc_library") build_test = _build_test bzl_library = _bzl_library -cc_library = _cc_library -cc_flags_supplier = _cc_flags_supplier -cc_proto_library = _cc_proto_library -cc_test = _cc_test -cc_toolchain = "@bazel_tools//tools/cpp:current_cc_toolchain" -gazelle = _gazelle -go_embed_data = _go_embed_data -go_path = _go_path -gtest = "@com_google_googletest//:gtest" -grpcpp = "@com_github_grpc_grpc//:grpc++" -gbenchmark = "@com_google_benchmark//:benchmark" loopback = "//tools/bazeldefs:loopback" -pkg_deb = _pkg_deb -pkg_tar = _pkg_tar -py_binary = native.py_binary rbe_platform = native.platform rbe_toolchain = native.toolchain -vdso_linker_option = "-fuse-ld=gold " def short_path(path): return path @@ -40,140 +18,6 @@ def proto_library(name, has_services = None, **kwargs): **kwargs ) -def cc_grpc_library(name, **kwargs): - _cc_grpc_library(name = name, grpc_only = True, **kwargs) - -def _go_proto_or_grpc_library(go_library_func, name, **kwargs): - deps = [ - dep.replace("_proto", "_go_proto") - for dep in (kwargs.pop("deps", []) or []) - ] - go_library_func( - name = name + "_go_proto", - importpath = "gvisor.dev/gvisor/" + native.package_name() + "/" + name + "_go_proto", - proto = ":" + name + "_proto", - deps = deps, - **kwargs - ) - -def go_proto_library(name, **kwargs): - _go_proto_or_grpc_library(_go_proto_library, name, **kwargs) - -def go_grpc_and_proto_libraries(name, **kwargs): - _go_proto_or_grpc_library(_go_grpc_library, name, **kwargs) - -def cc_binary(name, static = False, **kwargs): - """Run cc_binary. - - Args: - name: name of the target. - static: make a static binary if True - **kwargs: the rest of the args. - """ - if static: - # How to statically link a c++ program that uses threads, like for gRPC: - # https://gcc.gnu.org/legacy-ml/gcc-help/2010-05/msg00029.html - if "linkopts" not in kwargs: - kwargs["linkopts"] = [] - kwargs["linkopts"] += [ - "-static", - "-lstdc++", - "-Wl,--whole-archive", - "-lpthread", - "-Wl,--no-whole-archive", - ] - _cc_binary( - name = name, - **kwargs - ) - -def go_binary(name, static = False, pure = False, x_defs = None, **kwargs): - """Build a go binary. - - Args: - name: name of the target. - static: build a static binary. - pure: build without cgo. - x_defs: additional definitions. - **kwargs: rest of the arguments are passed to _go_binary. - """ - if static: - kwargs["static"] = "on" - if pure: - kwargs["pure"] = "on" - _go_binary( - name = name, - x_defs = x_defs, - **kwargs - ) - -def go_importpath(target): - """Returns the importpath for the target.""" - return target[GoLibrary].importpath - -def go_library(name, **kwargs): - _go_library( - name = name, - importpath = "gvisor.dev/gvisor/" + native.package_name(), - **kwargs - ) - -def go_test(name, pure = False, library = None, **kwargs): - """Build a go test. - - Args: - name: name of the output binary. - pure: should it be built without cgo. - library: the library to embed. - **kwargs: rest of the arguments to pass to _go_test. - """ - if pure: - kwargs["pure"] = "on" - if library: - kwargs["embed"] = [library] - _go_test( - name = name, - **kwargs - ) - -def go_rule(rule, implementation, **kwargs): - """Wraps a rule definition with Go attributes. - - Args: - rule: rule function (typically rule or aspect). - implementation: implementation function. - **kwargs: other arguments to pass to rule. - - Returns: - The result of invoking the rule. - """ - attrs = kwargs.pop("attrs", dict()) - attrs["_go_context_data"] = attr.label(default = "@io_bazel_rules_go//:go_context_data") - attrs["_stdlib"] = attr.label(default = "@io_bazel_rules_go//:stdlib") - toolchains = kwargs.get("toolchains", []) + ["@io_bazel_rules_go//go:toolchain"] - return rule(implementation, attrs = attrs, toolchains = toolchains, **kwargs) - -def go_test_library(target): - if hasattr(target.attr, "embed") and len(target.attr.embed) > 0: - return target.attr.embed[0] - return None - -def go_context(ctx, std = False): - # We don't change anything for the standard library analysis. All Go files - # are available in all instances. Note that this includes the standard - # library sources, which are analyzed by nogo. - go_ctx = _go_context(ctx) - return struct( - go = go_ctx.go, - env = go_ctx.env, - nogo_args = [], - stdlib_srcs = go_ctx.sdk.srcs, - runfiles = depset([go_ctx.go] + go_ctx.sdk.srcs + go_ctx.sdk.tools + go_ctx.stdlib.libs), - goos = go_ctx.sdk.goos, - goarch = go_ctx.sdk.goarch, - tags = go_ctx.tags, - ) - def select_arch(amd64 = "amd64", arm64 = "arm64", default = None, **kwargs): values = { "@bazel_tools//src/conditions:linux_x86_64": amd64, diff --git a/tools/bazeldefs/go.bzl b/tools/bazeldefs/go.bzl new file mode 100644 index 000000000..d388346a5 --- /dev/null +++ b/tools/bazeldefs/go.bzl @@ -0,0 +1,142 @@ +"""Go rules.""" + +load("@bazel_gazelle//:def.bzl", _gazelle = "gazelle") +load("@io_bazel_rules_go//go:def.bzl", "GoLibrary", _go_binary = "go_binary", _go_context = "go_context", _go_embed_data = "go_embed_data", _go_library = "go_library", _go_path = "go_path", _go_test = "go_test") +load("@io_bazel_rules_go//proto:def.bzl", _go_grpc_library = "go_grpc_library", _go_proto_library = "go_proto_library") +load("//tools/bazeldefs:defs.bzl", "select_arch", "select_system") + +gazelle = _gazelle +go_embed_data = _go_embed_data +go_path = _go_path + +def _go_proto_or_grpc_library(go_library_func, name, **kwargs): + deps = [ + dep.replace("_proto", "_go_proto") + for dep in (kwargs.pop("deps", []) or []) + ] + go_library_func( + name = name + "_go_proto", + importpath = "gvisor.dev/gvisor/" + native.package_name() + "/" + name + "_go_proto", + proto = ":" + name + "_proto", + deps = deps, + **kwargs + ) + +def go_proto_library(name, **kwargs): + _go_proto_or_grpc_library(_go_proto_library, name, **kwargs) + +def go_grpc_and_proto_libraries(name, **kwargs): + _go_proto_or_grpc_library(_go_grpc_library, name, **kwargs) + +def go_binary(name, static = False, pure = False, x_defs = None, **kwargs): + """Build a go binary. + + Args: + name: name of the target. + static: build a static binary. + pure: build without cgo. + x_defs: additional definitions. + **kwargs: rest of the arguments are passed to _go_binary. + """ + if static: + kwargs["static"] = "on" + if pure: + kwargs["pure"] = "on" + _go_binary( + name = name, + x_defs = x_defs, + **kwargs + ) + +def go_importpath(target): + """Returns the importpath for the target.""" + return target[GoLibrary].importpath + +def go_library(name, **kwargs): + _go_library( + name = name, + importpath = "gvisor.dev/gvisor/" + native.package_name(), + **kwargs + ) + +def go_test(name, pure = False, library = None, **kwargs): + """Build a go test. + + Args: + name: name of the output binary. + pure: should it be built without cgo. + library: the library to embed. + **kwargs: rest of the arguments to pass to _go_test. + """ + if pure: + kwargs["pure"] = "on" + if library: + kwargs["embed"] = [library] + _go_test( + name = name, + **kwargs + ) + +def go_rule(rule, implementation, **kwargs): + """Wraps a rule definition with Go attributes. + + Args: + rule: rule function (typically rule or aspect). + implementation: implementation function. + **kwargs: other arguments to pass to rule. + + Returns: + The result of invoking the rule. + """ + attrs = kwargs.pop("attrs", dict()) + attrs["_go_context_data"] = attr.label(default = "@io_bazel_rules_go//:go_context_data") + attrs["_stdlib"] = attr.label(default = "@io_bazel_rules_go//:stdlib") + toolchains = kwargs.get("toolchains", []) + ["@io_bazel_rules_go//go:toolchain"] + return rule(implementation, attrs = attrs, toolchains = toolchains, **kwargs) + +def go_test_library(target): + if hasattr(target.attr, "embed") and len(target.attr.embed) > 0: + return target.attr.embed[0] + return None + +def go_context(ctx, goos = None, goarch = None, std = False): + """Extracts a standard Go context struct. + + Args: + ctx: the starlark context (required). + goos: the GOOS value. + goarch: the GOARCH value. + std: ignored. + + Returns: + A context Go struct with pointers to Go toolchain components. + """ + + # We don't change anything for the standard library analysis. All Go files + # are available in all instances. Note that this includes the standard + # library sources, which are analyzed by nogo. + go_ctx = _go_context(ctx) + if goos == None: + goos = go_ctx.sdk.goos + elif goos != go_ctx.sdk.goos: + fail("Internal GOOS (%s) doesn't match GoSdk GOOS (%s)." % (goos, go_ctx.sdk.goos)) + if goarch == None: + goarch = go_ctx.sdk.goarch + elif goarch != go_ctx.sdk.goarch: + fail("Internal GOARCH (%s) doesn't match GoSdk GOARCH (%s)." % (goarch, go_ctx.sdk.goarch)) + return struct( + go = go_ctx.go, + env = go_ctx.env, + nogo_args = [], + stdlib_srcs = go_ctx.sdk.srcs, + runfiles = depset([go_ctx.go] + go_ctx.sdk.srcs + go_ctx.sdk.tools + go_ctx.stdlib.libs), + goos = go_ctx.sdk.goos, + goarch = go_ctx.sdk.goarch, + tags = go_ctx.tags, + ) + +def select_goarch(): + return select_arch(arm64 = "arm64", amd64 = "amd64") + +def select_goos(): + return select_system(linux = "linux") diff --git a/tools/bazeldefs/pkg.bzl b/tools/bazeldefs/pkg.bzl new file mode 100644 index 000000000..56317d93f --- /dev/null +++ b/tools/bazeldefs/pkg.bzl @@ -0,0 +1,6 @@ +"""Packaging rules.""" + +load("@rules_pkg//:pkg.bzl", _pkg_deb = "pkg_deb", _pkg_tar = "pkg_tar") + +pkg_deb = _pkg_deb +pkg_tar = _pkg_tar diff --git a/tools/bigquery/BUILD b/tools/bigquery/BUILD index 2b0062a63..1cea9e1c9 100644 --- a/tools/bigquery/BUILD +++ b/tools/bigquery/BUILD @@ -9,5 +9,8 @@ go_library( visibility = [ "//:sandbox", ], - deps = ["@com_google_cloud_go_bigquery//:go_default_library"], + deps = [ + "@com_google_cloud_go_bigquery//:go_default_library", + "@org_golang_google_api//option:go_default_library", + ], ) diff --git a/tools/bigquery/bigquery.go b/tools/bigquery/bigquery.go index 5f1a882de..34b270cc0 100644 --- a/tools/bigquery/bigquery.go +++ b/tools/bigquery/bigquery.go @@ -25,22 +25,30 @@ import ( "time" bq "cloud.google.com/go/bigquery" + "google.golang.org/api/option" ) -// Benchmark is the top level structure of recorded benchmark data. BigQuery +// Suite is the top level structure for a benchmark run. BigQuery // will infer the schema from this. +type Suite struct { + Name string `bq:"name"` + Conditions []*Condition `bq:"conditions"` + Benchmarks []*Benchmark `bq:"benchmarks"` + Official bool `bq:"official"` + Timestamp time.Time `bq:"timestamp"` +} + +// Benchmark represents an individual benchmark in a suite. type Benchmark struct { Name string `bq:"name"` Condition []*Condition `bq:"condition"` - Timestamp time.Time `bq:"timestamp"` - Official bool `bq:"official"` Metric []*Metric `bq:"metric"` - Metadata *Metadata `bq:"metadata"` } -// Condition represents qualifiers for the benchmark. For example: +// Condition represents qualifiers for the benchmark or suite. For example: // Get_Pid/1/real_time would have Benchmark Name "Get_Pid" with "1" -// and "real_time" parameters as conditions. +// and "real_time" parameters as conditions. Suite conditions include +// information such as the CL number and platform name. type Condition struct { Name string `bq:"name"` Value string `bq:"value"` @@ -53,19 +61,9 @@ type Metric struct { Sample float64 `bq:"sample"` } -// Metadata about this benchmark. -type Metadata struct { - CL string `bq:"changelist"` - IterationID string `bq:"iteration_id"` - PendingCL string `bq:"pending_cl"` - Workflow string `bq:"workflow"` - Platform string `bq:"platform"` - Gofer string `bq:"gofer"` -} - // InitBigQuery initializes a BigQuery dataset/table in the project. If the dataset/table already exists, it is not duplicated. -func InitBigQuery(ctx context.Context, projectID, datasetID, tableID string) error { - client, err := bq.NewClient(ctx, projectID) +func InitBigQuery(ctx context.Context, projectID, datasetID, tableID string, opts []option.ClientOption) error { + client, err := bq.NewClient(ctx, projectID, opts...) if err != nil { return fmt.Errorf("failed to initialize client on project %s: %v", projectID, err) } @@ -77,7 +75,7 @@ func InitBigQuery(ctx context.Context, projectID, datasetID, tableID string) err } table := dataset.Table(tableID) - schema, err := bq.InferSchema(Benchmark{}) + schema, err := bq.InferSchema(Suite{}) if err != nil { return fmt.Errorf("failed to infer schema: %v", err) } @@ -109,24 +107,32 @@ func (bm *Benchmark) AddMetric(metricName, unit string, sample float64) { // NewBenchmark initializes a new benchmark. func NewBenchmark(name string, iters int, official bool) *Benchmark { return &Benchmark{ - Name: name, - Timestamp: time.Now().UTC(), - Official: official, - Metric: make([]*Metric, 0), + Name: name, + Metric: make([]*Metric, 0), + } +} + +// NewSuite initializes a new Suite. +func NewSuite(name string) *Suite { + return &Suite{ + Name: name, + Timestamp: time.Now().UTC(), + Benchmarks: make([]*Benchmark, 0), + Conditions: make([]*Condition, 0), } } // SendBenchmarks sends the slice of benchmarks to the BigQuery dataset/table. -func SendBenchmarks(ctx context.Context, benchmarks []*Benchmark, projectID, datasetID, tableID string) error { - client, err := bq.NewClient(ctx, projectID) +func SendBenchmarks(ctx context.Context, suite *Suite, projectID, datasetID, tableID string, opts []option.ClientOption) error { + client, err := bq.NewClient(ctx, projectID, opts...) if err != nil { return fmt.Errorf("failed to initialize client on project: %s: %v", projectID, err) } defer client.Close() uploader := client.Dataset(datasetID).Table(tableID).Uploader() - if err = uploader.Put(ctx, benchmarks); err != nil { - return fmt.Errorf("failed to upload benchmarks to proejct %s, table %s.%s: %v", projectID, datasetID, tableID, err) + if err = uploader.Put(ctx, suite); err != nil { + return fmt.Errorf("failed to upload benchmarks %s to project %s, table %s.%s: %v", suite.Name, projectID, datasetID, tableID, err) } return nil diff --git a/tools/checkescape/checkescape.go b/tools/checkescape/checkescape.go index 523a42692..e5a7e23c7 100644 --- a/tools/checkescape/checkescape.go +++ b/tools/checkescape/checkescape.go @@ -67,6 +67,7 @@ import ( "go/token" "go/types" "io" + "log" "os" "os/exec" "path/filepath" @@ -619,7 +620,10 @@ func findReasons(pass *analysis.Pass, fdecl *ast.FuncDecl) ([]EscapeReason, bool func run(pass *analysis.Pass, localEscapes bool) (interface{}, error) { calls, err := loadObjdump() if err != nil { - return nil, err + // 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", err) } allEscapes := make(map[string][]Escapes) mergedEscapes := make(map[string]Escapes) @@ -641,6 +645,11 @@ func run(pass *analysis.Pass, localEscapes bool) (interface{}, error) { } hasCall := func(inst poser) (string, bool) { p := linePosition(inst, nil) + if calls == nil { + // See above: we don't have access to the binary + // itself, so need to include every possible call. + return "(possible)", true + } s, ok := calls[p.Simplified()] if !ok { return "", false diff --git a/tools/defs.bzl b/tools/defs.bzl index e2c72a1f6..d75e40863 100644 --- a/tools/defs.bzl +++ b/tools/defs.bzl @@ -7,39 +7,49 @@ change for Google-internal and bazel-compatible rules. load("//tools/go_stateify:defs.bzl", "go_stateify") load("//tools/go_marshal:defs.bzl", "go_marshal", "marshal_deps", "marshal_test_deps") -load("//tools/bazeldefs:defs.bzl", _build_test = "build_test", _bzl_library = "bzl_library", _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", _coreutil = "coreutil", _default_installer = "default_installer", _default_net_util = "default_net_util", _gazelle = "gazelle", _gbenchmark = "gbenchmark", _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_test = "go_test", _grpcpp = "grpcpp", _gtest = "gtest", _loopback = "loopback", _pkg_deb = "pkg_deb", _pkg_tar = "pkg_tar", _proto_library = "proto_library", _py_binary = "py_binary", _rbe_platform = "rbe_platform", _rbe_toolchain = "rbe_toolchain", _select_arch = "select_arch", _select_system = "select_system", _short_path = "short_path", _vdso_linker_option = "vdso_linker_option") +load("//tools/nogo:defs.bzl", "nogo_test") +load("//tools/bazeldefs:defs.bzl", _build_test = "build_test", _bzl_library = "bzl_library", _coreutil = "coreutil", _default_installer = "default_installer", _default_net_util = "default_net_util", _loopback = "loopback", _proto_library = "proto_library", _rbe_platform = "rbe_platform", _rbe_toolchain = "rbe_toolchain", _select_arch = "select_arch", _select_system = "select_system", _short_path = "short_path") +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", _grpcpp = "grpcpp", _gtest = "gtest", _vdso_linker_option = "vdso_linker_option") +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_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") -load("//tools/nogo:defs.bzl", "nogo_test") -# Delegate directly. +# Core rules. build_test = _build_test bzl_library = _bzl_library +default_installer = _default_installer +default_net_util = _default_net_util +loopback = _loopback +select_arch = _select_arch +select_system = _select_system +short_path = _short_path +rbe_platform = _rbe_platform +rbe_toolchain = _rbe_toolchain +coreutil = _coreutil + +# C++ rules. cc_binary = _cc_binary cc_flags_supplier = _cc_flags_supplier cc_grpc_library = _cc_grpc_library cc_library = _cc_library cc_test = _cc_test cc_toolchain = _cc_toolchain -default_installer = _default_installer -default_net_util = _default_net_util gbenchmark = _gbenchmark +gtest = _gtest +grpcpp = _grpcpp +vdso_linker_option = _vdso_linker_option + +# Go rules. gazelle = _gazelle go_embed_data = _go_embed_data go_path = _go_path -gtest = _gtest -grpcpp = _grpcpp -loopback = _loopback +select_goos = _select_goos +select_goarch = _select_goarch + +# Packaging rules. pkg_deb = _pkg_deb pkg_tar = _pkg_tar -py_binary = _py_binary -select_arch = _select_arch -select_system = _select_system -short_path = _short_path -rbe_platform = _rbe_platform -rbe_toolchain = _rbe_toolchain -vdso_linker_option = _vdso_linker_option -coreutil = _coreutil # Platform options. default_platform = _default_platform @@ -66,14 +76,20 @@ def go_binary(name, nogo = True, pure = False, static = False, x_defs = None, ** if nogo: # Note that the nogo rule applies only for go_library and go_test # targets, therefore we construct a library from the binary sources. + # This is done because the binary may not be in a form that objdump + # supports (i.e. a pure Go binary). _go_library( name = name + "_nogo_library", - **kwargs + srcs = kwargs.get("srcs", []), + deps = kwargs.get("deps", []), + testonly = 1, ) nogo_test( name = name + "_nogo", + config = "//:nogo_config", srcs = kwargs.get("srcs", []), - library = ":" + name + "_nogo_library", + deps = [":" + name + "_nogo_library"], + tags = ["nogo"], ) def calculate_sets(srcs): @@ -204,8 +220,10 @@ def go_library(name, srcs, deps = [], imports = [], stateify = True, marshal = F if nogo: nogo_test( name = name + "_nogo", + config = "//:nogo_config", srcs = all_srcs, - library = ":" + name, + deps = [":" + name], + tags = ["nogo"], ) if marshal: @@ -241,8 +259,10 @@ def go_test(name, nogo = True, **kwargs): if nogo: nogo_test( name = name + "_nogo", + config = "//:nogo_config", srcs = kwargs.get("srcs", []), - library = ":" + name, + deps = [":" + name], + tags = ["nogo"], ) def proto_library(name, srcs, deps = None, has_services = 0, **kwargs): diff --git a/tools/github/main.go b/tools/github/main.go index 7a74dc033..681003eef 100644 --- a/tools/github/main.go +++ b/tools/github/main.go @@ -20,6 +20,7 @@ import ( "flag" "fmt" "io/ioutil" + "log" "os" "os/exec" "strings" @@ -34,21 +35,43 @@ var ( owner string repo string tokenFile string - path string + paths stringList commit string dryRun bool ) +type stringList []string + +func (s *stringList) String() string { + return strings.Join(*s, ",") +} + +func (s *stringList) Set(value string) error { + *s = append(*s, value) + return nil +} + // Keep the options simple for now. Supports only a single path and repo. func init() { flag.StringVar(&owner, "owner", "", "GitHub project org/owner (required, except nogo dry-run)") flag.StringVar(&repo, "repo", "", "GitHub repo (required, except nogo dry-run)") flag.StringVar(&tokenFile, "oauth-token-file", "", "file containing the GitHub token (or GITHUB_TOKEN is set)") - flag.StringVar(&path, "path", ".", "path to scan (required for revive and nogo)") + flag.Var(&paths, "path", "path(s) to scan (required for revive and nogo)") flag.StringVar(&commit, "commit", "", "commit to associated (required for nogo, except dry-run)") flag.BoolVar(&dryRun, "dry-run", false, "just print changes to be made") } +func filterPaths(paths []string) (existing []string) { + for _, path := range paths { + if _, err := os.Stat(path); err != nil { + log.Printf("WARNING: skipping %v: %v", path, err) + continue + } + existing = append(existing, path) + } + return +} + func main() { // Set defaults from the environment. repository := os.Getenv("GITHUB_REPOSITORY") @@ -83,8 +106,9 @@ func main() { flag.Usage() os.Exit(1) } - if len(path) == 0 { - fmt.Fprintln(flag.CommandLine.Output(), "missing --path option.") + filteredPaths := filterPaths(paths) + if len(filteredPaths) == 0 { + fmt.Fprintln(flag.CommandLine.Output(), "no valid --path options provided.") flag.Usage() os.Exit(1) } @@ -123,7 +147,7 @@ func main() { os.Exit(1) } // Scan the provided path. - rev := reviver.New([]string{path}, []reviver.Bugger{bugger}) + rev := reviver.New(filteredPaths, []reviver.Bugger{bugger}) if errs := rev.Run(); len(errs) > 0 { fmt.Fprintf(os.Stderr, "Encountered %d errors:\n", len(errs)) for _, err := range errs { @@ -145,7 +169,7 @@ func main() { } // Scan all findings. poster := nogo.NewFindingsPoster(client, owner, repo, commit, dryRun) - if err := poster.Walk(path); err != nil { + if err := poster.Walk(filteredPaths); err != nil { fmt.Fprintln(os.Stderr, "Error finding nogo findings:", err) os.Exit(1) } diff --git a/tools/github/nogo/BUILD b/tools/github/nogo/BUILD index 0633eaf19..19b7eec4d 100644 --- a/tools/github/nogo/BUILD +++ b/tools/github/nogo/BUILD @@ -10,7 +10,7 @@ go_library( "//tools/github:__subpackages__", ], deps = [ - "//tools/nogo/util", + "//tools/nogo", "@com_github_google_go_github_v28//github:go_default_library", ], ) diff --git a/tools/github/nogo/nogo.go b/tools/github/nogo/nogo.go index b70dfe63b..27ab1b8eb 100644 --- a/tools/github/nogo/nogo.go +++ b/tools/github/nogo/nogo.go @@ -24,7 +24,7 @@ import ( "time" "github.com/google/go-github/github" - "gvisor.dev/gvisor/tools/nogo/util" + "gvisor.dev/gvisor/tools/nogo" ) // FindingsPoster is a simple wrapper around the GitHub api. @@ -35,7 +35,7 @@ type FindingsPoster struct { dryRun bool startTime time.Time - findings map[util.Finding]struct{} + findings map[nogo.Finding]struct{} client *github.Client } @@ -47,32 +47,37 @@ func NewFindingsPoster(client *github.Client, owner, repo, commit string, dryRun commit: commit, dryRun: dryRun, startTime: time.Now(), - findings: make(map[util.Finding]struct{}), + findings: make(map[nogo.Finding]struct{}), client: client, } } // Walk walks the given path tree for findings files. -func (p *FindingsPoster) Walk(path string) error { - return filepath.Walk(path, func(filename string, info os.FileInfo, err error) error { - if err != nil { - return err - } - // Skip any directories or files not ending in .findings. - if !strings.HasSuffix(filename, ".findings") || info.IsDir() { +func (p *FindingsPoster) Walk(paths []string) error { + for _, path := range paths { + if err := filepath.Walk(path, func(filename string, info os.FileInfo, err error) error { + if err != nil { + return err + } + // Skip any directories or files not ending in .findings. + if !strings.HasSuffix(filename, ".findings") || info.IsDir() { + return nil + } + findings, err := nogo.ExtractFindingsFromFile(filename) + if err != nil { + return err + } + // Add all findings to the list. We use a map to ensure + // that each finding is unique. + for _, finding := range findings { + p.findings[finding] = struct{}{} + } return nil - } - findings, err := util.ExtractFindingsFromFile(filename) - if err != nil { + }); err != nil { return err } - // Add all findings to the list. We use a map to ensure - // that each finding is unique. - for _, finding := range findings { - p.findings[finding] = struct{}{} - } - return nil - }) + } + return nil } // Post posts all results to the GitHub API as a check run. @@ -81,7 +86,7 @@ func (p *FindingsPoster) Post() error { if p.dryRun { for finding, _ := range p.findings { // Pretty print, so that this is useful for debugging. - fmt.Printf("%s: (%s+%d) %s\n", finding.Category, finding.Path, finding.Line, finding.Message) + fmt.Printf("%s: (%s+%d) %s\n", finding.Category, finding.Position.Filename, finding.Position.Line, finding.Message) } return nil } @@ -110,12 +115,13 @@ func (p *FindingsPoster) Post() error { } annotationLevel := "failure" // Always. for finding, _ := range p.findings { + title := string(finding.Category) opts.Output.Annotations = append(opts.Output.Annotations, &github.CheckRunAnnotation{ - Path: &finding.Path, - StartLine: &finding.Line, - EndLine: &finding.Line, + Path: &finding.Position.Filename, + StartLine: &finding.Position.Line, + EndLine: &finding.Position.Line, Message: &finding.Message, - Title: &finding.Category, + Title: &title, AnnotationLevel: &annotationLevel, }) } diff --git a/tools/github/reviver/github.go b/tools/github/reviver/github.go index a95df0fb6..c4b624f2a 100644 --- a/tools/github/reviver/github.go +++ b/tools/github/reviver/github.go @@ -121,13 +121,24 @@ func (b *GitHubBugger) Activate(todo *Todo) (bool, error) { return true, nil } +var issuePrefixes = []string{ + "gvisor.dev/issue/", + "gvisor.dev/issues/", +} + // parseIssueNo parses the issue number out of the issue url. +// +// 0 is returned if url does not correspond to an issue. func parseIssueNo(url string) (int, error) { - const prefix = "gvisor.dev/issue/" - // First check if I can handle the TODO. - idStr := strings.TrimPrefix(url, prefix) - if len(url) == len(idStr) { + var idStr string + for _, p := range issuePrefixes { + if str := strings.TrimPrefix(url, p); len(str) < len(url) { + idStr = str + break + } + } + if len(idStr) == 0 { return 0, nil } diff --git a/tools/github/reviver/reviver_test.go b/tools/github/reviver/reviver_test.go index a9fb1f9f1..851306c9d 100644 --- a/tools/github/reviver/reviver_test.go +++ b/tools/github/reviver/reviver_test.go @@ -33,6 +33,15 @@ func TestProcessLine(t *testing.T) { }, }, { + line: "// TODO(foobar.com/issues/123): comment, bla. blabla.", + want: &Todo{ + Issue: "foobar.com/issues/123", + Locations: []Location{ + {Comment: "comment, bla. blabla."}, + }, + }, + }, + { line: "// FIXME(b/123): internal bug", want: &Todo{ Issue: "b/123", diff --git a/tools/go_generics/defs.bzl b/tools/go_generics/defs.bzl index 33329cf28..ad97208a8 100644 --- a/tools/go_generics/defs.bzl +++ b/tools/go_generics/defs.bzl @@ -1,25 +1,32 @@ -"""Generics support via go_generics.""" +"""Generics support via go_generics. + +A Go template is similar to a go library, except that it has certain types that +can be replaced before usage. For example, one could define a templatized List +struct, whose elements are of type T, then instantiate that template for +T=segment, where "segment" is the concrete type. +""" TemplateInfo = provider( + "Information about a go_generics template.", fields = { + "unsafe": "whether the template requires unsafe code", "types": "required types", "opt_types": "optional types", "consts": "required consts", "opt_consts": "optional consts", "deps": "package dependencies", - "file": "merged template", + "template": "merged template source file", }, ) def _go_template_impl(ctx): srcs = ctx.files.srcs - output = ctx.outputs.out - - args = ["-o=%s" % output.path] + [f.path for f in srcs] + template = ctx.actions.declare_file(ctx.label.name + "_template.go") + args = ["-o=%s" % template.path] + [f.path for f in srcs] ctx.actions.run( inputs = srcs, - outputs = [output], + outputs = [template], mnemonic = "GoGenericsTemplate", progress_message = "Building Go template %s" % ctx.label, arguments = args, @@ -32,74 +39,48 @@ def _go_template_impl(ctx): consts = ctx.attr.consts, opt_consts = ctx.attr.opt_consts, deps = ctx.attr.deps, - file = output, + template = template, )] -""" -Generates a Go template from a set of Go files. - -A Go template is similar to a go library, except that it has certain types that -can be replaced before usage. For example, one could define a templatized List -struct, whose elements are of type T, then instantiate that template for -T=segment, where "segment" is the concrete type. - -Args: - name: the name of the template. - srcs: the list of source files that comprise the template. - types: the list of generic types in the template that are required to be specified. - opt_types: the list of generic types in the template that can but aren't required to be specified. - consts: the list of constants in the template that are required to be specified. - opt_consts: the list of constants in the template that can but aren't required to be specified. - deps: the list of dependencies. -""" go_template = rule( implementation = _go_template_impl, attrs = { - "srcs": attr.label_list(mandatory = True, allow_files = True), - "deps": attr.label_list(allow_files = True, cfg = "target"), - "types": attr.string_list(), - "opt_types": attr.string_list(), - "consts": attr.string_list(), - "opt_consts": attr.string_list(), + "srcs": attr.label_list(doc = "the list of source files that comprise the template", mandatory = True, allow_files = True), + "deps": attr.label_list(doc = "the standard dependency list", allow_files = True, cfg = "target"), + "types": attr.string_list(doc = "the list of generic types in the template that are required to be specified"), + "opt_types": attr.string_list(doc = "the list of generic types in the template that can but aren't required to be specified"), + "consts": attr.string_list(doc = "the list of constants in the template that are required to be specified"), + "opt_consts": attr.string_list(doc = "the list of constants in the template that can but aren't required to be specified"), "_tool": attr.label(executable = True, cfg = "host", default = Label("//tools/go_generics/go_merge")), }, - outputs = { - "out": "%{name}_template.go", - }, -) - -TemplateInstanceInfo = provider( - fields = { - "srcs": "source files", - }, ) def _go_template_instance_impl(ctx): - template = ctx.attr.template[TemplateInfo] + info = ctx.attr.template[TemplateInfo] output = ctx.outputs.out # Check that all required types are defined. - for t in template.types: + for t in info.types: if t not in ctx.attr.types: fail("Missing value for type %s in %s" % (t, ctx.attr.template.label)) # Check that all defined types are expected by the template. for t in ctx.attr.types: - if (t not in template.types) and (t not in template.opt_types): + if (t not in info.types) and (t not in info.opt_types): fail("Type %s it not a parameter to %s" % (t, ctx.attr.template.label)) # Check that all required consts are defined. - for t in template.consts: + for t in info.consts: if t not in ctx.attr.consts: fail("Missing value for constant %s in %s" % (t, ctx.attr.template.label)) # Check that all defined consts are expected by the template. for t in ctx.attr.consts: - if (t not in template.consts) and (t not in template.opt_consts): + if (t not in info.consts) and (t not in info.opt_consts): fail("Const %s it not a parameter to %s" % (t, ctx.attr.template.label)) # Build the argument list. - args = ["-i=%s" % template.file.path, "-o=%s" % output.path] + args = ["-i=%s" % info.template.path, "-o=%s" % output.path] if ctx.attr.package: args.append("-p=%s" % ctx.attr.package) @@ -117,7 +98,7 @@ def _go_template_instance_impl(ctx): args.append("-anon") ctx.actions.run( - inputs = [template.file], + inputs = [info.template], outputs = [output], mnemonic = "GoGenericsInstance", progress_message = "Building Go template instance %s" % ctx.label, @@ -125,35 +106,22 @@ def _go_template_instance_impl(ctx): executable = ctx.executable._tool, ) - return [TemplateInstanceInfo( - srcs = [output], + return [DefaultInfo( + files = depset([output]), )] -""" -Instantiates a Go template by replacing all generic types with concrete ones. - -Args: - name: the name of the template instance. - template: the label of the template to be instatiated. - prefix: a prefix to be added to globals in the template. - suffix: a suffix to be added to global in the template. - types: the map from generic type names to concrete ones. - consts: the map from constant names to their values. - imports: the map from imports used in types/consts to their import paths. - package: the name of the package the instantiated template will be compiled into. -""" go_template_instance = rule( implementation = _go_template_instance_impl, attrs = { - "template": attr.label(mandatory = True), - "prefix": attr.string(), - "suffix": attr.string(), - "types": attr.string_dict(), - "consts": attr.string_dict(), - "imports": attr.string_dict(), - "anon": attr.bool(mandatory = False, default = False), - "package": attr.string(mandatory = False), - "out": attr.output(mandatory = True), + "template": attr.label(doc = "the label of the template to be instantiated", mandatory = True), + "prefix": attr.string(doc = "a prefix to be added to globals in the template"), + "suffix": attr.string(doc = "a suffix to be added to globals in the template"), + "types": attr.string_dict(doc = "the map from generic type names to concrete ones"), + "consts": attr.string_dict(doc = "the map from constant names to their values"), + "imports": attr.string_dict(doc = "the map from imports used in types/consts to their import paths"), + "anon": attr.bool(doc = "whether anoymous fields should be processed", mandatory = False, default = False), + "package": attr.string(doc = "the package for the generated source file", mandatory = False), + "out": attr.output(doc = "output file", mandatory = True), "_tool": attr.label(executable = True, cfg = "host", default = Label("//tools/go_generics")), }, ) diff --git a/tools/nogo/BUILD b/tools/nogo/BUILD index dd4b46f58..12b8b597c 100644 --- a/tools/nogo/BUILD +++ b/tools/nogo/BUILD @@ -1,8 +1,15 @@ -load("//tools:defs.bzl", "bzl_library", "go_library") -load("//tools/nogo:defs.bzl", "nogo_objdump_tool", "nogo_stdlib") +load("//tools:defs.bzl", "bzl_library", "go_library", "select_goarch", "select_goos") +load("//tools/nogo:defs.bzl", "nogo_objdump_tool", "nogo_stdlib", "nogo_target") package(licenses = ["notice"]) +nogo_target( + name = "target", + goarch = select_goarch(), + goos = select_goos(), + visibility = ["//visibility:public"], +) + nogo_objdump_tool( name = "objdump_tool", visibility = ["//visibility:public"], @@ -13,20 +20,14 @@ nogo_stdlib( visibility = ["//visibility:public"], ) -sh_binary( - name = "gentest", - srcs = ["gentest.sh"], - visibility = ["//visibility:public"], -) - go_library( name = "nogo", srcs = [ + "analyzers.go", "build.go", "config.go", - "matchers.go", + "findings.go", "nogo.go", - "register.go", ], nogo = False, visibility = ["//:sandbox"], diff --git a/tools/nogo/analyzers.go b/tools/nogo/analyzers.go new file mode 100644 index 000000000..b919bc2f8 --- /dev/null +++ b/tools/nogo/analyzers.go @@ -0,0 +1,131 @@ +// Copyright 2019 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 nogo + +import ( + "encoding/gob" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/asmdecl" + "golang.org/x/tools/go/analysis/passes/assign" + "golang.org/x/tools/go/analysis/passes/atomic" + "golang.org/x/tools/go/analysis/passes/bools" + "golang.org/x/tools/go/analysis/passes/buildtag" + "golang.org/x/tools/go/analysis/passes/cgocall" + "golang.org/x/tools/go/analysis/passes/composite" + "golang.org/x/tools/go/analysis/passes/copylock" + "golang.org/x/tools/go/analysis/passes/errorsas" + "golang.org/x/tools/go/analysis/passes/httpresponse" + "golang.org/x/tools/go/analysis/passes/loopclosure" + "golang.org/x/tools/go/analysis/passes/lostcancel" + "golang.org/x/tools/go/analysis/passes/nilfunc" + "golang.org/x/tools/go/analysis/passes/nilness" + "golang.org/x/tools/go/analysis/passes/printf" + "golang.org/x/tools/go/analysis/passes/shadow" + "golang.org/x/tools/go/analysis/passes/shift" + "golang.org/x/tools/go/analysis/passes/stdmethods" + "golang.org/x/tools/go/analysis/passes/stringintconv" + "golang.org/x/tools/go/analysis/passes/structtag" + "golang.org/x/tools/go/analysis/passes/tests" + "golang.org/x/tools/go/analysis/passes/unmarshal" + "golang.org/x/tools/go/analysis/passes/unreachable" + "golang.org/x/tools/go/analysis/passes/unsafeptr" + "golang.org/x/tools/go/analysis/passes/unusedresult" + "honnef.co/go/tools/staticcheck" + "honnef.co/go/tools/stylecheck" + + "gvisor.dev/gvisor/tools/checkescape" + "gvisor.dev/gvisor/tools/checkunsafe" +) + +// AllAnalyzers is a list of all available analyzers. +var AllAnalyzers = []*analysis.Analyzer{ + asmdecl.Analyzer, + assign.Analyzer, + atomic.Analyzer, + bools.Analyzer, + buildtag.Analyzer, + cgocall.Analyzer, + composite.Analyzer, + copylock.Analyzer, + errorsas.Analyzer, + httpresponse.Analyzer, + loopclosure.Analyzer, + lostcancel.Analyzer, + nilfunc.Analyzer, + nilness.Analyzer, + printf.Analyzer, + shift.Analyzer, + stdmethods.Analyzer, + stringintconv.Analyzer, + shadow.Analyzer, + structtag.Analyzer, + tests.Analyzer, + unmarshal.Analyzer, + unreachable.Analyzer, + unsafeptr.Analyzer, + unusedresult.Analyzer, + checkescape.Analyzer, + checkunsafe.Analyzer, +} + +// EscapeAnalyzers is a list of escape-related analyzers. +var EscapeAnalyzers = []*analysis.Analyzer{ + checkescape.EscapeAnalyzer, +} + +func register(all []*analysis.Analyzer) { + // Register all fact types. + // + // N.B. This needs to be done recursively, because there may be + // analyzers in the Requires list that do not appear explicitly above. + registered := make(map[*analysis.Analyzer]struct{}) + var registerOne func(*analysis.Analyzer) + registerOne = func(a *analysis.Analyzer) { + if _, ok := registered[a]; ok { + return + } + + // Register dependencies. + for _, da := range a.Requires { + registerOne(da) + } + + // Register local facts. + for _, f := range a.FactTypes { + gob.Register(f) + } + + registered[a] = struct{}{} // Done. + } + for _, a := range all { + registerOne(a) + } +} + +func init() { + // Add all staticcheck analyzers. + for _, a := range staticcheck.Analyzers { + AllAnalyzers = append(AllAnalyzers, a) + } + // Add all stylecheck analyzers. + for _, a := range stylecheck.Analyzers { + AllAnalyzers = append(AllAnalyzers, a) + } + + // Register lists. + register(AllAnalyzers) + register(EscapeAnalyzers) +} diff --git a/tools/nogo/build.go b/tools/nogo/build.go index 55d34760f..d173cff1f 100644 --- a/tools/nogo/build.go +++ b/tools/nogo/build.go @@ -20,22 +20,6 @@ import ( "os" ) -var ( - // internalPrefix is the internal path prefix. Note that this is not - // special, as paths should be passed relative to the repository root - // and should not have any special prefix applied. - internalPrefix = fmt.Sprintf("^") - - // internalDefault is applied when no paths are provided. - internalDefault = fmt.Sprintf("%s/.*", notPath("external")) - - // generatedPrefix is a regex for generated files. - generatedPrefix = "^(.*/)?(bazel-genfiles|bazel-out|bazel-bin)/" - - // externalPrefix is external workspace packages. - externalPrefix = "^external/" -) - // findStdPkg needs to find the bundled standard library packages. func findStdPkg(GOOS, GOARCH, path string) (io.ReadCloser, error) { if path == "C" { diff --git a/tools/nogo/check/BUILD b/tools/nogo/check/BUILD index 21ba2c306..e18483a18 100644 --- a/tools/nogo/check/BUILD +++ b/tools/nogo/check/BUILD @@ -2,8 +2,6 @@ load("//tools:defs.bzl", "go_binary") package(licenses = ["notice"]) -# Note that the check binary must be public, since an aspect may be applied -# across lots of different rules in different repositories. go_binary( name = "check", srcs = ["main.go"], diff --git a/tools/nogo/check/main.go b/tools/nogo/check/main.go index 3828edf3a..69bdfe502 100644 --- a/tools/nogo/check/main.go +++ b/tools/nogo/check/main.go @@ -16,9 +16,99 @@ package main import ( + "encoding/json" + "flag" + "fmt" + "io/ioutil" + "log" + "os" + "gvisor.dev/gvisor/tools/nogo" ) +var ( + packageFile = flag.String("package", "", "package configuration file (in JSON format)") + stdlibFile = flag.String("stdlib", "", "stdlib configuration file (in JSON format)") + findingsOutput = flag.String("findings", "", "output file (or stdout, if not specified)") + factsOutput = flag.String("facts", "", "output file for facts (optional)") + escapesOutput = flag.String("escapes", "", "output file for escapes (optional)") +) + +func loadConfig(file string, config interface{}) interface{} { + // Load the configuration. + f, err := os.Open(file) + if err != nil { + log.Fatalf("unable to open configuration %q: %v", file, err) + } + defer f.Close() + dec := json.NewDecoder(f) + dec.DisallowUnknownFields() + if err := dec.Decode(config); err != nil { + log.Fatalf("unable to decode configuration: %v", err) + } + return config +} + func main() { - nogo.Main() + // Parse all flags. + flag.Parse() + + var ( + findings []nogo.Finding + factData []byte + err error + ) + + // Check & load the configuration. + if *packageFile != "" && *stdlibFile != "" { + log.Fatalf("unable to perform stdlib and package analysis; provide only one!") + } + + // Run the configuration. + if *stdlibFile != "" { + // Perform basic analysis. + c := loadConfig(*stdlibFile, new(nogo.StdlibConfig)).(*nogo.StdlibConfig) + findings, factData, err = nogo.CheckStdlib(c, nogo.AllAnalyzers) + + } else if *packageFile != "" { + // Perform basic analysis. + c := loadConfig(*packageFile, new(nogo.PackageConfig)).(*nogo.PackageConfig) + findings, factData, err = nogo.CheckPackage(c, nogo.AllAnalyzers, nil) + + // Do we need to do escape analysis? + if *escapesOutput != "" { + escapes, _, err := nogo.CheckPackage(c, nogo.EscapeAnalyzers, nil) + if err != nil { + log.Fatalf("error performing escape analysis: %v", err) + } + if err := nogo.WriteFindingsToFile(escapes, *escapesOutput); err != nil { + log.Fatalf("error writing escapes to %q: %v", *escapesOutput, err) + } + } + } else { + log.Fatalf("please provide at least one of package or stdlib!") + } + + // Check that analysis was successful. + if err != nil { + log.Fatalf("error performing analysis: %v", err) + } + + // Save facts. + if *factsOutput != "" { + if err := ioutil.WriteFile(*factsOutput, factData, 0644); err != nil { + log.Fatalf("error saving findings to %q: %v", *factsOutput, err) + } + } + + // Write all findings. + if *findingsOutput != "" { + if err := nogo.WriteFindingsToFile(findings, *findingsOutput); err != nil { + log.Fatalf("error writing findings to %q: %v", *findingsOutput, err) + } + } else { + for _, finding := range findings { + fmt.Fprintf(os.Stdout, "%s\n", finding.String()) + } + } } diff --git a/tools/nogo/config.go b/tools/nogo/config.go index 8079618ab..2fea5b3e1 100644 --- a/tools/nogo/config.go +++ b/tools/nogo/config.go @@ -15,543 +15,247 @@ package nogo import ( - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/asmdecl" - "golang.org/x/tools/go/analysis/passes/assign" - "golang.org/x/tools/go/analysis/passes/atomic" - "golang.org/x/tools/go/analysis/passes/bools" - "golang.org/x/tools/go/analysis/passes/buildtag" - "golang.org/x/tools/go/analysis/passes/cgocall" - "golang.org/x/tools/go/analysis/passes/composite" - "golang.org/x/tools/go/analysis/passes/copylock" - "golang.org/x/tools/go/analysis/passes/errorsas" - "golang.org/x/tools/go/analysis/passes/httpresponse" - "golang.org/x/tools/go/analysis/passes/loopclosure" - "golang.org/x/tools/go/analysis/passes/lostcancel" - "golang.org/x/tools/go/analysis/passes/nilfunc" - "golang.org/x/tools/go/analysis/passes/nilness" - "golang.org/x/tools/go/analysis/passes/printf" - "golang.org/x/tools/go/analysis/passes/shadow" - "golang.org/x/tools/go/analysis/passes/shift" - "golang.org/x/tools/go/analysis/passes/stdmethods" - "golang.org/x/tools/go/analysis/passes/stringintconv" - "golang.org/x/tools/go/analysis/passes/structtag" - "golang.org/x/tools/go/analysis/passes/tests" - "golang.org/x/tools/go/analysis/passes/unmarshal" - "golang.org/x/tools/go/analysis/passes/unreachable" - "golang.org/x/tools/go/analysis/passes/unsafeptr" - "golang.org/x/tools/go/analysis/passes/unusedresult" - "honnef.co/go/tools/staticcheck" - "honnef.co/go/tools/stylecheck" - - "gvisor.dev/gvisor/tools/checkescape" - "gvisor.dev/gvisor/tools/checkunsafe" + "fmt" + "regexp" ) -var analyzerConfig = map[*analysis.Analyzer]matcher{ - // Standard analyzers. - asmdecl.Analyzer: alwaysMatches(), - assign.Analyzer: externalExcluded( - ".*gazelle/walk/walk.go", // False positive. - ), - atomic.Analyzer: alwaysMatches(), - bools.Analyzer: alwaysMatches(), - buildtag.Analyzer: alwaysMatches(), - cgocall.Analyzer: alwaysMatches(), - composite.Analyzer: and( - disableMatches(), // Disabled for now. - resultExcluded{ - "Object_", - "Range{", - }, - ), - copylock.Analyzer: internalMatches(), // Common external issues (e.g. protos). - errorsas.Analyzer: alwaysMatches(), - httpresponse.Analyzer: alwaysMatches(), - loopclosure.Analyzer: alwaysMatches(), - lostcancel.Analyzer: internalMatches(), // Common external issues. - nilfunc.Analyzer: alwaysMatches(), - nilness.Analyzer: and( - internalMatches(), // Common "tautological checks". - internalExcluded( - "pkg/sentry/platform/kvm/kvm_test.go", // Intentional. - "tools/bigquery/bigquery.go", // False positive. - ), - ), - printf.Analyzer: alwaysMatches(), - shift.Analyzer: alwaysMatches(), - stdmethods.Analyzer: internalMatches(), // Common external issues (e.g. methods named "Write"). - stringintconv.Analyzer: and( - internalExcluded(), - externalExcluded( - ".*protobuf/.*.go", // Bad conversions. - ".*flate/huffman_bit_writer.go", // Bad conversion. +// GroupName is a named group. +type GroupName string + +// AnalyzerName is a named analyzer. +type AnalyzerName string + +// Group represents a named collection of files. +type Group struct { + // Name is the short name for the group. + Name GroupName `yaml:"name"` + + // Regex matches all full paths in the group. + Regex string `yaml:"regex"` + regex *regexp.Regexp `yaml:"-"` + + // Default determines the default group behavior. + // + // If Default is true, all Analyzers are enabled for this + // group. Otherwise, Analyzers must be individually enabled + // by specifying a (possible empty) ItemConfig for the group + // in the AnalyzerConfig. + Default bool `yaml:"default"` +} + +func (g *Group) compile() error { + r, err := regexp.Compile(g.Regex) + if err != nil { + return err + } + g.regex = r + return nil +} + +// ItemConfig is an (Analyzer,Group) configuration. +type ItemConfig struct { + // Exclude are analyzer exclusions. + // + // Exclude is a list of regular expressions. If the corresponding + // Analyzer emits a Finding for which Finding.Position.String() + // matches a regular expression in Exclude, the finding will not + // be reported. + Exclude []string `yaml:"exclude,omitempty"` + exclude []*regexp.Regexp `yaml:"-"` + + // Suppress are analyzer suppressions. + // + // Suppress is a list of regular expressions. If the corresponding + // Analyzer emits a Finding for which Finding.Message matches a regular + // expression in Suppress, the finding will not be reported. + Suppress []string `yaml:"suppress,omitempty"` + suppress []*regexp.Regexp `yaml:"-"` +} + +func compileRegexps(ss []string, rs *[]*regexp.Regexp) error { + *rs = make([]*regexp.Regexp, 0, len(ss)) + for _, s := range ss { + r, err := regexp.Compile(s) + if err != nil { + return err + } + *rs = append(*rs, r) + } + return nil +} + +func (i *ItemConfig) compile() error { + if i == nil { + // This may be nil if nothing is included in the + // item configuration. That's fine, there's nothing + // to compile and nothing to exclude & suppress. + return nil + } + if err := compileRegexps(i.Exclude, &i.exclude); err != nil { + return fmt.Errorf("in exclude: %w", err) + } + if err := compileRegexps(i.Suppress, &i.suppress); err != nil { + return fmt.Errorf("in suppress: %w", err) + } + return nil +} + +func (i *ItemConfig) merge(other *ItemConfig) { + i.Exclude = append(i.Exclude, other.Exclude...) + i.Suppress = append(i.Suppress, other.Suppress...) +} + +func (i *ItemConfig) shouldReport(fullPos, msg string) bool { + if i == nil { + // See above. + return true + } + for _, r := range i.exclude { + if r.MatchString(fullPos) { + return false + } + } + for _, r := range i.suppress { + if r.MatchString(msg) { + return false + } + } + return true +} + +// AnalyzerConfig is the configuration for a single analyzers. +// +// This map is keyed by individual Group names, to allow for different +// configurations depending on what Group the file belongs to. +type AnalyzerConfig map[GroupName]*ItemConfig + +func (a AnalyzerConfig) compile() error { + for name, gc := range a { + if err := gc.compile(); err != nil { + return fmt.Errorf("invalid group %q: %v", name, err) + } + } + return nil +} + +func (a AnalyzerConfig) merge(other AnalyzerConfig) { + // Merge all the groups. + for name, gc := range other { + old, ok := a[name] + if !ok || old == nil { + a[name] = gc // Not configured in a. + continue + } + old.merge(gc) + } +} + +func (a AnalyzerConfig) shouldReport(groupConfig *Group, fullPos, msg string) bool { + gc, ok := a[groupConfig.Name] + if !ok { + return groupConfig.Default + } + + // Note that if a section appears for a particular group + // for a particular analyzer, then it will now be enabled, + // and the group default no longer applies. + return gc.shouldReport(fullPos, msg) +} + +// Config is a nogo configuration. +type Config struct { + // Prefixes defines a set of regular expressions that + // are standard "prefixes", so that files can be grouped + // and specific rules applied to individual groups. + Groups []Group `yaml:"groups"` - // Runtime internal violations. - ".*reflect/value.go", - ".*encoding/xml/xml.go", - ".*runtime/pprof/internal/profile/proto.go", - ".*fmt/scan.go", - ".*go/types/conversions.go", - ".*golang.org/x/net/dns/dnsmessage/message.go", - ), - ), - shadow.Analyzer: disableMatches(), // Disabled for now. - structtag.Analyzer: internalMatches(), // External not subject to rules. - tests.Analyzer: alwaysMatches(), - unmarshal.Analyzer: alwaysMatches(), - unreachable.Analyzer: internalMatches(), - unsafeptr.Analyzer: and( - internalMatches(), - internalExcluded( - ".*_test.go", // Exclude tests. - "pkg/flipcall/.*_unsafe.go", // Special case. - "pkg/gohacks/gohacks_unsafe.go", // Special case. - "pkg/sentry/fs/fsutil/host_file_mapper_unsafe.go", // Special case. - "pkg/sentry/platform/kvm/bluepill_unsafe.go", // Special case. - "pkg/sentry/platform/kvm/machine_unsafe.go", // Special case. - "pkg/sentry/platform/ring0/pagetables/allocator_unsafe.go", // Special case. - "pkg/sentry/platform/safecopy/safecopy_unsafe.go", // Special case. - "pkg/sentry/vfs/mount_unsafe.go", // Special case. - "pkg/sentry/platform/systrap/stub_unsafe.go", // Special case. - "pkg/sentry/platform/systrap/switchto_google_unsafe.go", // Special case. - "pkg/sentry/platform/systrap/sysmsg_thread_unsafe.go", // Special case. - ), - ), - unusedresult.Analyzer: alwaysMatches(), + // Global is the global analyzer config. + Global AnalyzerConfig `yaml:"global"` - // Internal analyzers: external packages not subject. - checkescape.Analyzer: internalMatches(), - checkunsafe.Analyzer: internalMatches(), + // Analyzers are individual analyzer configurations. The + // key for each analyzer is the name of the analyzer. The + // value is either a boolean (enable/disable), or a map to + // the groups above. + Analyzers map[AnalyzerName]AnalyzerConfig `yaml:"analyzers"` } -func init() { - staticMatcher := and( - // Only match internal, non-generated files. - internalMatches(), - generatedExcluded(), +// Merge merges two configurations. +func (c *Config) Merge(other *Config) { + // Merge all groups. + for _, g := range other.Groups { + // Is there a matching group? If yes, we just delete + // it. This will preserve the order provided in the + // overriding file, even if it differs. + for i := 0; i < len(c.Groups); i++ { + if g.Name == c.Groups[i].Name { + copy(c.Groups[i:], c.Groups[i+1:]) + c.Groups = c.Groups[:len(c.Groups)-1] + break + } + } + c.Groups = append(c.Groups, g) + } - // We use ALL_CAPS for system definitions, - // which are common enough in the code base - // that we shouldn't annotate exceptions. - // - // Same story for underscores. - resultExcluded([]string{ - "should not use ALL_CAPS in Go names", - "should not use underscores in Go names", - }), + // Merge global configurations. + c.Global.merge(other.Global) - // Exclude existing matches. - internalExcluded( - "pkg/abi/linux/fuse.go:22", - "pkg/abi/linux/fuse.go:25", - "pkg/abi/linux/socket.go:113", - "pkg/abi/linux/tty.go:73", - "pkg/bpf/decoder.go:112", - "pkg/cpuid/cpuid_x86.go:675", - "pkg/eventchannel/event.go:193", - "pkg/eventchannel/event.go:27", - "pkg/eventchannel/event_test.go:22", - "pkg/eventchannel/rate.go:19", - "pkg/gohacks/gohacks_unsafe.go:33", - "pkg/log/json.go:30", - "pkg/log/log.go:359", - "pkg/merkletree/merkletree.go:230", - "pkg/merkletree/merkletree.go:243", - "pkg/merkletree/merkletree.go:249", - "pkg/merkletree/merkletree.go:266", - "pkg/merkletree/merkletree.go:355", - "pkg/merkletree/merkletree.go:369", - "pkg/metric/metric_test.go:20", - "pkg/p9/p9test/client_test.go:687", - "pkg/p9/transport_test.go:196", - "pkg/pool/pool.go:15", - "pkg/refs/refcounter.go:510", - "pkg/refs/refcounter_test.go:169", - "pkg/refs_vfs2/refs.go:16", - "pkg/safemem/block_unsafe.go:89", - "pkg/seccomp/seccomp.go:82", - "pkg/segment/test/set_functions.go:15", - "pkg/sentry/arch/signal.go:166", - "pkg/sentry/arch/signal.go:171", - "pkg/sentry/control/pprof.go:196", - "pkg/sentry/devices/memdev/full.go:58", - "pkg/sentry/devices/memdev/null.go:59", - "pkg/sentry/devices/memdev/random.go:68", - "pkg/sentry/devices/memdev/zero.go:86", - "pkg/sentry/fdimport/fdimport.go:15", - "pkg/sentry/fs/attr.go:257", - "pkg/sentry/fsbridge/fs.go:116", - "pkg/sentry/fsbridge/vfs.go:124", - "pkg/sentry/fsbridge/vfs.go:70", - "pkg/sentry/fs/copy_up.go:365", - "pkg/sentry/fs/copy_up_test.go:65", - "pkg/sentry/fs/dev/net_tun.go:161", - "pkg/sentry/fs/dev/net_tun.go:63", - "pkg/sentry/fs/dev/null.go:97", - "pkg/sentry/fs/dirent_cache.go:64", - "pkg/sentry/fs/file_overlay.go:327", - "pkg/sentry/fs/file_overlay.go:524", - "pkg/sentry/fs/filetest/filetest.go:55", - "pkg/sentry/fs/filetest/filetest.go:60", - "pkg/sentry/fs/fs.go:77", - "pkg/sentry/fs/fsutil/file.go:290", - "pkg/sentry/fs/fsutil/file.go:346", - "pkg/sentry/fs/fsutil/host_file_mapper.go:105", - "pkg/sentry/fs/fsutil/inode_cached.go:676", - "pkg/sentry/fs/fsutil/inode_cached.go:772", - "pkg/sentry/fs/gofer/attr.go:120", - "pkg/sentry/fs/gofer/fifo.go:33", - "pkg/sentry/fs/gofer/inode.go:410", - "pkg/sentry/fsimpl/devpts/devpts.go:110", - "pkg/sentry/fsimpl/devpts/devpts.go:246", - "pkg/sentry/fsimpl/devpts/devpts.go:50", - "pkg/sentry/fsimpl/devpts/master.go:110", - "pkg/sentry/fsimpl/devpts/master.go:55", - "pkg/sentry/fsimpl/devpts/replica.go:113", - "pkg/sentry/fsimpl/devpts/replica.go:57", - "pkg/sentry/fsimpl/devtmpfs/devtmpfs.go:54", - "pkg/sentry/fsimpl/ext/disklayout/superblock_64.go:97", - "pkg/sentry/fsimpl/ext/disklayout/superblock_old.go:92", - "pkg/sentry/fsimpl/ext/disklayout/block_group_32.go:44", - "pkg/sentry/fsimpl/ext/disklayout/inode_new.go:91", - "pkg/sentry/fsimpl/ext/disklayout/inode_old.go:93", - "pkg/sentry/fsimpl/ext/disklayout/superblock_32.go:66", - "pkg/sentry/fsimpl/ext/disklayout/block_group_64.go:53", - "pkg/sentry/fsimpl/eventfd/eventfd.go:268", - "pkg/sentry/fsimpl/ext/directory.go:163", - "pkg/sentry/fsimpl/ext/directory.go:164", - "pkg/sentry/fsimpl/ext/extent_file.go:142", - "pkg/sentry/fsimpl/ext/extent_file.go:143", - "pkg/sentry/fsimpl/ext/ext.go:105", - "pkg/sentry/fsimpl/ext/filesystem.go:287", - "pkg/sentry/fsimpl/ext/regular_file.go:153", - "pkg/sentry/fsimpl/ext/symlink.go:113", - "pkg/sentry/fsimpl/fuse/connection_control.go:194", - "pkg/sentry/fsimpl/fuse/dev.go:387", - "pkg/sentry/fsimpl/fuse/dev_test.go:318", - "pkg/sentry/fsimpl/fuse/fusefs.go:102", - "pkg/sentry/fsimpl/fuse/read_write.go:129", - "pkg/sentry/fsimpl/fuse/request_response.go:71", - "pkg/sentry/fsimpl/gofer/directory.go:135", - "pkg/sentry/fsimpl/gofer/filesystem.go:679", - "pkg/sentry/fsimpl/gofer/gofer.go:1694", - "pkg/sentry/fsimpl/gofer/gofer.go:276", - "pkg/sentry/fsimpl/gofer/regular_file.go:81", - "pkg/sentry/fsimpl/gofer/special_file.go:141", - "pkg/sentry/fsimpl/host/host.go:184", - "pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go:50", - "pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go:90", - "pkg/sentry/fsimpl/kernfs/fd_impl_util.go:273", - "pkg/sentry/fsimpl/kernfs/filesystem.go:247", - "pkg/sentry/fsimpl/kernfs/inode_impl_util.go:320", - "pkg/sentry/fsimpl/kernfs/inode_impl_util.go:497", - "pkg/sentry/fsimpl/kernfs/synthetic_directory.go:52", - "pkg/sentry/fsimpl/overlay/directory.go:119", - "pkg/sentry/fsimpl/overlay/filesystem.go:527", - "pkg/sentry/fsimpl/overlay/non_directory.go:152", - "pkg/sentry/fsimpl/overlay/overlay.go:115", - "pkg/sentry/fsimpl/overlay/overlay.go:719", - "pkg/sentry/fsimpl/pipefs/pipefs.go:74", - "pkg/sentry/fsimpl/proc/filesystem.go:52", - "pkg/sentry/fsimpl/proc/filesystem.go:81", - "pkg/sentry/fsimpl/proc/subtasks.go:126", - "pkg/sentry/fsimpl/proc/subtasks.go:189", - "pkg/sentry/fsimpl/proc/task_fds.go:168", - "pkg/sentry/fsimpl/proc/task_fds.go:228", - "pkg/sentry/fsimpl/proc/task_fds.go:301", - "pkg/sentry/fsimpl/proc/task_fds.go:318", - "pkg/sentry/fsimpl/proc/task_fds.go:67", - "pkg/sentry/fsimpl/proc/task_files.go:112", - "pkg/sentry/fsimpl/proc/task_files.go:158", - "pkg/sentry/fsimpl/proc/task_files.go:259", - "pkg/sentry/fsimpl/proc/task_files.go:285", - "pkg/sentry/fsimpl/proc/task_files.go:305", - "pkg/sentry/fsimpl/proc/task_files.go:384", - "pkg/sentry/fsimpl/proc/task_files.go:403", - "pkg/sentry/fsimpl/proc/task_files.go:428", - "pkg/sentry/fsimpl/proc/task_files.go:691", - "pkg/sentry/fsimpl/proc/task_files.go:770", - "pkg/sentry/fsimpl/proc/task_files.go:797", - "pkg/sentry/fsimpl/proc/task_files.go:828", - "pkg/sentry/fsimpl/proc/task_files.go:879", - "pkg/sentry/fsimpl/proc/task_files.go:910", - "pkg/sentry/fsimpl/proc/task_files.go:961", - "pkg/sentry/fsimpl/proc/task.go:127", - "pkg/sentry/fsimpl/proc/task.go:193", - "pkg/sentry/fsimpl/proc/task_net.go:134", - "pkg/sentry/fsimpl/proc/task_net.go:475", - "pkg/sentry/fsimpl/proc/task_net.go:491", - "pkg/sentry/fsimpl/proc/task_net.go:508", - "pkg/sentry/fsimpl/proc/task_net.go:665", - "pkg/sentry/fsimpl/proc/task_net.go:715", - "pkg/sentry/fsimpl/proc/task_net.go:779", - "pkg/sentry/fsimpl/proc/tasks_files.go:113", - "pkg/sentry/fsimpl/proc/tasks_files.go:388", - "pkg/sentry/fsimpl/proc/tasks.go:232", - "pkg/sentry/fsimpl/proc/tasks_sys.go:145", - "pkg/sentry/fsimpl/proc/tasks_sys.go:181", - "pkg/sentry/fsimpl/proc/tasks_sys.go:239", - "pkg/sentry/fsimpl/proc/tasks_sys.go:291", - "pkg/sentry/fsimpl/proc/tasks_sys.go:375", - "pkg/sentry/fsimpl/signalfd/signalfd.go:124", - "pkg/sentry/fsimpl/signalfd/signalfd.go:15", - "pkg/sentry/fsimpl/signalfd/signalfd.go:126", - "pkg/sentry/fsimpl/sockfs/sockfs.go:36", - "pkg/sentry/fsimpl/sockfs/sockfs.go:79", - "pkg/sentry/fsimpl/sys/kcov.go:49", - "pkg/sentry/fsimpl/sys/kcov.go:99", - "pkg/sentry/fsimpl/sys/sys.go:118", - "pkg/sentry/fsimpl/sys/sys.go:56", - "pkg/sentry/fsimpl/testutil/testutil.go:257", - "pkg/sentry/fsimpl/testutil/testutil.go:260", - "pkg/sentry/fsimpl/timerfd/timerfd.go:87", - "pkg/sentry/fsimpl/tmpfs/directory.go:112", - "pkg/sentry/fsimpl/tmpfs/filesystem.go:195", - "pkg/sentry/fsimpl/tmpfs/regular_file.go:226", - "pkg/sentry/fsimpl/tmpfs/regular_file.go:346", - "pkg/sentry/fsimpl/tmpfs/tmpfs.go:103", - "pkg/sentry/fsimpl/tmpfs/tmpfs.go:733", - "pkg/sentry/fsimpl/verity/filesystem.go:490", - "pkg/sentry/fsimpl/verity/verity.go:156", - "pkg/sentry/fsimpl/verity/verity.go:629", - "pkg/sentry/fsimpl/verity/verity.go:672", - "pkg/sentry/fs/mount.go:162", - "pkg/sentry/fs/mount.go:256", - "pkg/sentry/fs/mount_overlay.go:144", - "pkg/sentry/fs/mounts.go:432", - "pkg/sentry/fs/proc/exec_args.go:104", - "pkg/sentry/fs/proc/exec_args.go:73", - "pkg/sentry/fs/proc/fds.go:269", - "pkg/sentry/fs/proc/loadavg.go:33", - "pkg/sentry/fs/proc/meminfo.go:39", - "pkg/sentry/fs/proc/mounts.go:193", - "pkg/sentry/fs/proc/mounts.go:84", - "pkg/sentry/fs/proc/net.go:125", - "pkg/sentry/fs/proc/proc.go:146", - "pkg/sentry/fs/proc/proc.go:204", - "pkg/sentry/fs/proc/seqfile/seqfile.go:210", - "pkg/sentry/fs/proc/sys.go:146", - "pkg/sentry/fs/proc/sys.go:43", - "pkg/sentry/fs/proc/sys_net.go:113", - "pkg/sentry/fs/proc/sys_net.go:205", - "pkg/sentry/fs/proc/sys_net.go:233", - "pkg/sentry/fs/proc/sys_net.go:307", - "pkg/sentry/fs/proc/sys_net.go:335", - "pkg/sentry/fs/proc/sys_net.go:446", - "pkg/sentry/fs/proc/sys_net.go:456", - "pkg/sentry/fs/proc/sys_net.go:89", - "pkg/sentry/fs/proc/task.go:170", - "pkg/sentry/fs/proc/task.go:322", - "pkg/sentry/fs/proc/task.go:427", - "pkg/sentry/fs/proc/task.go:467", - "pkg/sentry/fs/proc/task.go:500", - "pkg/sentry/fs/proc/task.go:784", - "pkg/sentry/fs/proc/task.go:839", - "pkg/sentry/fs/proc/task.go:920", - "pkg/sentry/fs/proc/uid_gid_map.go:108", - "pkg/sentry/fs/proc/uid_gid_map.go:79", - "pkg/sentry/fs/proc/uptime.go:75", - "pkg/sentry/fs/ramfs/dir.go:447", - "pkg/sentry/fs/tmpfs/inode_file.go:436", - "pkg/sentry/fs/tmpfs/inode_file.go:537", - "pkg/sentry/fs/tty/dir.go:313", - "pkg/sentry/fs/tty/master.go:131", - "pkg/sentry/fs/tty/master.go:91", - "pkg/sentry/fs/tty/replica.go:116", - "pkg/sentry/fs/tty/replica.go:88", - "pkg/sentry/kernel/auth/id_map.go:269", - "pkg/sentry/kernel/fasync/fasync.go:67", - "pkg/sentry/kernel/kcov.go:209", - "pkg/sentry/kernel/kcov.go:223", - "pkg/sentry/kernel/kernel.go:343", - "pkg/sentry/kernel/kernel.go:368", - "pkg/sentry/kernel/pipe/node_test.go:112", - "pkg/sentry/kernel/pipe/node_test.go:119", - "pkg/sentry/kernel/pipe/node_test.go:130", - "pkg/sentry/kernel/pipe/node_test.go:137", - "pkg/sentry/kernel/pipe/node_test.go:149", - "pkg/sentry/kernel/pipe/node_test.go:150", - "pkg/sentry/kernel/pipe/node_test.go:158", - "pkg/sentry/kernel/pipe/node_test.go:174", - "pkg/sentry/kernel/pipe/node_test.go:180", - "pkg/sentry/kernel/pipe/node_test.go:193", - "pkg/sentry/kernel/pipe/node_test.go:202", - "pkg/sentry/kernel/pipe/node_test.go:205", - "pkg/sentry/kernel/pipe/node_test.go:216", - "pkg/sentry/kernel/pipe/node_test.go:219", - "pkg/sentry/kernel/pipe/node_test.go:271", - "pkg/sentry/kernel/pipe/node_test.go:290", - "pkg/sentry/kernel/pipe/pipe_test.go:93", - "pkg/sentry/kernel/pipe/reader_writer.go:65", - "pkg/sentry/kernel/posixtimer.go:157", - "pkg/sentry/kernel/ptrace.go:218", - "pkg/sentry/kernel/semaphore/semaphore.go:323", - "pkg/sentry/kernel/sessions.go:123", - "pkg/sentry/kernel/sessions.go:508", - "pkg/sentry/kernel/signal_handlers.go:57", - "pkg/sentry/kernel/task_context.go:72", - "pkg/sentry/kernel/task_exit.go:67", - "pkg/sentry/kernel/task_sched.go:255", - "pkg/sentry/kernel/task_sched.go:280", - "pkg/sentry/kernel/task_sched.go:323", - "pkg/sentry/kernel/task_stop.go:192", - "pkg/sentry/kernel/thread_group.go:530", - "pkg/sentry/kernel/timekeeper.go:316", - "pkg/sentry/kernel/vdso.go:106", - "pkg/sentry/kernel/vdso.go:118", - "pkg/sentry/memmap/memmap.go:103", - "pkg/sentry/memmap/memmap.go:163", - "pkg/sentry/mm/address_space.go:42", - "pkg/sentry/mm/address_space.go:42", - "pkg/sentry/mm/aio_context.go:208", - "pkg/sentry/mm/aio_context.go:288", - "pkg/sentry/mm/pma.go:683", - "pkg/sentry/mm/special_mappable.go:80", - "pkg/sentry/platform/systrap/subprocess.go:370", - "pkg/sentry/platform/systrap/usertrap/usertrap_amd64.go:124", - "pkg/sentry/socket/control/control.go:260", - "pkg/sentry/socket/control/control.go:94", - "pkg/sentry/socket/control/control_vfs2.go:37", - "pkg/sentry/socket/hostinet/stack.go:433", - "pkg/sentry/socket/hostinet/stack.go:438", - "pkg/sentry/socket/hostinet/stack.go:444", - "pkg/sentry/socket/hostinet/stack.go:460", - "pkg/sentry/socket/netfilter/tcp_matcher.go:74", - "pkg/sentry/socket/netfilter/udp_matcher.go:71", - "pkg/sentry/socket/netlink/route/protocol.go:38", - "pkg/sentry/socket/socket.go:332", - "pkg/sentry/socket/unix/transport/connectioned.go:394", - "pkg/sentry/socket/unix/transport/connectionless.go:152", - "pkg/sentry/socket/unix/transport/unix.go:436", - "pkg/sentry/socket/unix/transport/unix.go:490", - "pkg/sentry/socket/unix/transport/unix.go:685", - "pkg/sentry/socket/unix/transport/unix.go:795", - "pkg/sentry/syscalls/linux/sys_sem.go:62", - "pkg/sentry/syscalls/linux/sys_time.go:189", - "pkg/sentry/usage/cpu.go:42", - "pkg/sentry/vfs/anonfs.go:302", - "pkg/sentry/vfs/anonfs.go:99", - "pkg/sentry/vfs/dentry.go:214", - "pkg/sentry/vfs/epoll.go:168", - "pkg/sentry/vfs/epoll.go:314", - "pkg/sentry/vfs/file_description.go:549", - "pkg/sentry/vfs/file_description_impl_util.go:304", - "pkg/sentry/vfs/file_description_impl_util.go:412", - "pkg/sentry/vfs/filesystem.go:76", - "pkg/sentry/vfs/lock.go:15", - "pkg/sentry/vfs/lock.go:47", - "pkg/sentry/vfs/memxattr/xattr.go:37", - "pkg/sentry/vfs/mount.go:510", - "pkg/sentry/vfs/mount.go:667", - "pkg/sentry/vfs/mount_test.go:106", - "pkg/sentry/vfs/mount_test.go:160", - "pkg/sentry/vfs/mount_test.go:215", - "pkg/sentry/vfs/mount_unsafe.go:153", - "pkg/sentry/vfs/resolving_path.go:228", - "pkg/sentry/vfs/vfs.go:897", - "pkg/shim/runsc/runsc.go:16", - "pkg/shim/runsc/utils.go:16", - "pkg/shim/v1/proc/deleted_state.go:16", - "pkg/shim/v1/proc/exec.go:16", - "pkg/shim/v1/proc/exec_state.go:16", - "pkg/shim/v1/proc/init.go:16", - "pkg/shim/v1/proc/init_state.go:16", - "pkg/shim/v1/proc/io.go:16", - "pkg/shim/v1/proc/process.go:16", - "pkg/shim/v1/proc/types.go:16", - "pkg/shim/v1/proc/utils.go:16", - "pkg/shim/v1/shim/api.go:16", - "pkg/shim/v1/shim/platform.go:16", - "pkg/shim/v1/shim/service.go:16", - "pkg/shim/v1/utils/annotations.go:15", - "pkg/shim/v1/utils/utils.go:15", - "pkg/shim/v1/utils/volumes.go:15", - "pkg/shim/v2/api.go:16", - "pkg/shim/v2/epoll.go:18", - "pkg/shim/v2/options/options.go:15", - "pkg/shim/v2/options/options.go:24", - "pkg/shim/v2/options/options.go:26", - "pkg/shim/v2/runtimeoptions/runtimeoptions.go:16", - "pkg/shim/v2/runtimeoptions/runtimeoptions_test.go:22", - "pkg/shim/v2/service.go:15", - "pkg/shim/v2/service_linux.go:18", - "pkg/state/tests/integer_test.go:23", - "pkg/state/tests/integer_test.go:28", - "pkg/sync/rwmutex_test.go:105", - "pkg/syserr/host_linux.go:35", - "pkg/tcpip/adapters/gonet/gonet_test.go:144", - "pkg/tcpip/adapters/gonet/gonet_test.go:415", - "pkg/tcpip/adapters/gonet/gonet_test.go:99", - "pkg/tcpip/buffer/view.go:238", - "pkg/tcpip/buffer/view.go:238", - "pkg/tcpip/buffer/view.go:246", - "pkg/tcpip/header/tcp.go:151", - "pkg/tcpip/link/sharedmem/pipe/pipe_test.go:493", - "pkg/tcpip/stack/iptables.go:293", - "pkg/tcpip/stack/iptables_types.go:277", - "pkg/tcpip/stack/stack.go:553", - "pkg/tcpip/stack/transport_test.go:30", - "pkg/tcpip/transport/packet/endpoint.go:126", - "pkg/tcpip/transport/raw/endpoint.go:145", - "pkg/tcpip/transport/tcp/sack_scoreboard.go:167", - "pkg/unet/unet_test.go:634", - "pkg/unet/unet_test.go:662", - "pkg/unet/unet_test.go:703", - "pkg/unet/unet_test.go:98", - "pkg/usermem/addr.go:34", - "pkg/usermem/usermem.go:171", - "pkg/usermem/usermem.go:170", - "runsc/boot/compat.go:22", - "runsc/boot/compat.go:56", - "runsc/boot/loader.go:1115", - "runsc/boot/loader.go:1120", - "runsc/cmd/checkpoint.go:151", - "runsc/config/flags.go:32", - "runsc/container/container.go:641", - "runsc/container/container.go:988", - "runsc/specutils/specutils.go:172", - "runsc/specutils/specutils.go:428", - "runsc/specutils/specutils.go:436", - "runsc/specutils/specutils.go:442", - "runsc/specutils/specutils.go:447", - "runsc/specutils/specutils.go:454", - "test/cmd/test_app/fds.go:171", - "test/iptables/filter_output.go:251", - "test/packetimpact/testbench/connections.go:77", - "tools/bigquery/bigquery.go:106", - "tools/checkescape/test1/test1.go:108", - "tools/checkescape/test1/test1.go:122", - "tools/checkescape/test1/test1.go:137", - "tools/checkescape/test1/test1.go:151", - "tools/checkescape/test1/test1.go:170", - "tools/checkescape/test1/test1.go:39", - "tools/checkescape/test1/test1.go:45", - "tools/checkescape/test1/test1.go:50", - "tools/checkescape/test1/test1.go:64", - "tools/checkescape/test1/test1.go:80", - "tools/checkescape/test1/test1.go:94", - "tools/go_generics/imports.go:51", - "tools/go_generics/imports.go:75", - "tools/go_marshal/gomarshal/generator.go:177", - "tools/go_marshal/gomarshal/generator.go:81", - "tools/go_marshal/gomarshal/generator.go:85", - "tools/go_marshal/test/escape/escape.go:15", - "tools/go_marshal/test/test.go:164", - ), - ) + // Merge all analyzer configurations. + for name, ac := range other.Analyzers { + old, ok := c.Analyzers[name] + if !ok { + c.Analyzers[name] = ac // No analyzer in original config. + continue + } + old.merge(ac) + } +} - // Add all staticcheck analyzers; internal only. - for _, a := range staticcheck.Analyzers { - analyzerConfig[a] = staticMatcher +// Compile compiles a configuration to make it useable. +func (c *Config) Compile() error { + for i := 0; i < len(c.Groups); i++ { + if err := c.Groups[i].compile(); err != nil { + return fmt.Errorf("invalid group %q: %w", c.Groups[i].Name, err) + } } - // Add all stylecheck analyzers; internal only. - for _, a := range stylecheck.Analyzers { - analyzerConfig[a] = staticMatcher + if err := c.Global.compile(); err != nil { + return fmt.Errorf("invalid global: %w", err) } + for name, ac := range c.Analyzers { + if err := ac.compile(); err != nil { + return fmt.Errorf("invalid analyzer %q: %w", name, err) + } + } + return nil } -var escapesConfig = map[*analysis.Analyzer]matcher{ - // Informational only: include all packages. - checkescape.EscapeAnalyzer: alwaysMatches(), +// ShouldReport returns true iff the finding should match the Config. +func (c *Config) ShouldReport(finding Finding) bool { + fullPos := finding.Position.String() + + // Find the matching group. + var groupConfig *Group + for i := 0; i < len(c.Groups); i++ { + if c.Groups[i].regex.MatchString(fullPos) { + groupConfig = &c.Groups[i] + break + } + } + + // If there is no group matching this path, then + // we default to accept the finding. + if groupConfig == nil { + return true + } + + // Suppress via global rule? + if !c.Global.shouldReport(groupConfig, fullPos, finding.Message) { + return false + } + + // Try the analyzer config. + ac, ok := c.Analyzers[finding.Category] + if !ok { + return groupConfig.Default + } + return ac.shouldReport(groupConfig, fullPos, finding.Message) } diff --git a/tools/nogo/defs.bzl b/tools/nogo/defs.bzl index c6fcfd402..b3d297308 100644 --- a/tools/nogo/defs.bzl +++ b/tools/nogo/defs.bzl @@ -1,10 +1,59 @@ """Nogo rules.""" -load("//tools/bazeldefs:defs.bzl", "go_context", "go_importpath", "go_rule", "go_test_library") +load("//tools/bazeldefs:go.bzl", "go_context", "go_importpath", "go_rule", "go_test_library") -def _nogo_objdump_tool_impl(ctx): - go_ctx = go_context(ctx) +NogoConfigInfo = provider( + "information about a nogo configuration", + fields = { + "srcs": "the collection of configuration files", + }, +) + +def _nogo_config_impl(ctx): + return [NogoConfigInfo( + srcs = ctx.files.srcs, + )] + +nogo_config = rule( + implementation = _nogo_config_impl, + attrs = { + "srcs": attr.label_list( + doc = "a list of yaml files (schema defined by tool/nogo/config.go).", + allow_files = True, + ), + }, +) + +NogoTargetInfo = provider( + "information about the Go target", + fields = { + "goarch": "the build architecture (GOARCH)", + "goos": "the build OS target (GOOS)", + }, +) + +def _nogo_target_impl(ctx): + return [NogoTargetInfo( + goarch = ctx.attr.goarch, + goos = ctx.attr.goos, + )] +nogo_target = go_rule( + rule, + implementation = _nogo_target_impl, + attrs = { + "goarch": attr.string( + doc = "the Go build architecture (propagated to other rules).", + mandatory = True, + ), + "goos": attr.string( + doc = "the Go OS target (propagated to other rules).", + mandatory = True, + ), + }, +) + +def _nogo_objdump_tool_impl(ctx): # Construct the magic dump command. # # Note that in some cases, the input is being fed into the tool via stdin. @@ -12,6 +61,8 @@ def _nogo_objdump_tool_impl(ctx): # we need the tool to handle this case by creating a temporary file. # # [1] https://github.com/golang/go/issues/41051 + nogo_target_info = ctx.attr._nogo_target[NogoTargetInfo] + go_ctx = go_context(ctx, goos = nogo_target_info.goos, goarch = nogo_target_info.goarch) env_prefix = " ".join(["%s=%s" % (key, value) for (key, value) in go_ctx.env.items()]) dumper = ctx.actions.declare_file(ctx.label.name) ctx.actions.write(dumper, "\n".join([ @@ -42,6 +93,12 @@ def _nogo_objdump_tool_impl(ctx): nogo_objdump_tool = go_rule( rule, implementation = _nogo_objdump_tool_impl, + attrs = { + "_nogo_target": attr.label( + default = "//tools/nogo:target", + cfg = "target", + ), + }, ) # NogoStdlibInfo is the set of standard library facts. @@ -49,16 +106,16 @@ NogoStdlibInfo = provider( "information for nogo analysis (standard library facts)", fields = { "facts": "serialized standard library facts", - "findings": "package findings (if relevant)", + "raw_findings": "raw package findings (if relevant)", }, ) def _nogo_stdlib_impl(ctx): - go_ctx = go_context(ctx) - # Build the standard library facts. + nogo_target_info = ctx.attr._nogo_target[NogoTargetInfo] + go_ctx = go_context(ctx, goos = nogo_target_info.goos, goarch = nogo_target_info.goarch) facts = ctx.actions.declare_file(ctx.label.name + ".facts") - findings = ctx.actions.declare_file(ctx.label.name + ".findings") + raw_findings = ctx.actions.declare_file(ctx.label.name + ".raw_findings") config = struct( Srcs = [f.path for f in go_ctx.stdlib_srcs], GOOS = go_ctx.goos, @@ -69,15 +126,15 @@ def _nogo_stdlib_impl(ctx): ctx.actions.write(config_file, config.to_json()) ctx.actions.run( inputs = [config_file] + go_ctx.stdlib_srcs, - outputs = [facts, findings], - tools = depset(go_ctx.runfiles.to_list() + ctx.files._objdump_tool), - executable = ctx.files._nogo[0], - mnemonic = "GoStandardLibraryAnalysis", + outputs = [facts, raw_findings], + tools = depset(go_ctx.runfiles.to_list() + ctx.files._nogo_objdump_tool), + executable = ctx.files._nogo_check[0], + mnemonic = "NogoStandardLibraryAnalysis", progress_message = "Analyzing Go Standard Library", arguments = go_ctx.nogo_args + [ - "-objdump_tool=%s" % ctx.files._objdump_tool[0].path, + "-objdump_tool=%s" % ctx.files._nogo_objdump_tool[0].path, "-stdlib=%s" % config_file.path, - "-findings=%s" % findings.path, + "-findings=%s" % raw_findings.path, "-facts=%s" % facts.path, ], ) @@ -85,18 +142,24 @@ def _nogo_stdlib_impl(ctx): # Return the stdlib facts as output. return [NogoStdlibInfo( facts = facts, - findings = findings, + raw_findings = raw_findings, )] nogo_stdlib = go_rule( rule, implementation = _nogo_stdlib_impl, attrs = { - "_nogo": attr.label( + "_nogo_check": attr.label( default = "//tools/nogo/check:check", + cfg = "host", ), - "_objdump_tool": attr.label( + "_nogo_objdump_tool": attr.label( default = "//tools/nogo:objdump_tool", + cfg = "host", + ), + "_nogo_target": attr.label( + default = "//tools/nogo:target", + cfg = "target", ), }, ) @@ -110,23 +173,22 @@ NogoInfo = provider( "information for nogo analysis", fields = { "facts": "serialized package facts", - "findings": "package findings (if relevant)", + "raw_findings": "raw package findings (if relevant)", + "escapes": "escape-only findings (if relevant)", "importpath": "package import path", "binaries": "package binary files", - "srcs": "original source files (for go_test support)", - "deps": "original deps (for go_test support)", + "srcs": "srcs (for go_test support)", + "deps": "deps (for go_test support)", }, ) def _nogo_aspect_impl(target, ctx): - go_ctx = go_context(ctx) - # If this is a nogo rule itself (and not the shadow of a go_library or # go_binary rule created by such a rule), then we simply return nothing. # All work is done in the shadow properties for go rules. For a proto # library, we simply skip the analysis portion but still need to return a # valid NogoInfo to reference the generated binary. - if ctx.rule.kind in ("go_library", "go_binary", "go_test", "go_tool_library"): + if ctx.rule.kind in ("go_library", "go_tool_library", "go_binary", "go_test"): srcs = ctx.rule.files.srcs deps = ctx.rule.attr.deps elif ctx.rule.kind in ("go_proto_library", "go_wrap_cc"): @@ -178,6 +240,7 @@ def _nogo_aspect_impl(target, ctx): # Collect all info from shadow dependencies. fact_map = dict() import_map = dict() + all_raw_findings = [] for dep in deps: # There will be no file attribute set for all transitive dependencies # that are not go_library or go_binary rules, such as a proto rules. @@ -195,17 +258,23 @@ def _nogo_aspect_impl(target, ctx): import_map[info.importpath] = x_files[0] fact_map[info.importpath] = info.facts.path + # Collect all findings; duplicates are resolved at the end. + all_raw_findings.extend(info.raw_findings) + # Ensure the above are available as inputs. inputs.append(info.facts) inputs += info.binaries # Add the standard library facts. - stdlib_facts = ctx.attr._nogo_stdlib[NogoStdlibInfo].facts + stdlib_info = ctx.attr._nogo_stdlib[NogoStdlibInfo] + stdlib_facts = stdlib_info.facts inputs.append(stdlib_facts) # The nogo tool operates on a configuration serialized in JSON format. + nogo_target_info = ctx.attr._nogo_target[NogoTargetInfo] + go_ctx = go_context(ctx, goos = nogo_target_info.goos, goarch = nogo_target_info.goarch) facts = ctx.actions.declare_file(target.label.name + ".facts") - findings = ctx.actions.declare_file(target.label.name + ".findings") + raw_findings = ctx.actions.declare_file(target.label.name + ".raw_findings") escapes = ctx.actions.declare_file(target.label.name + ".escapes") config = struct( ImportPath = importpath, @@ -223,39 +292,39 @@ def _nogo_aspect_impl(target, ctx): inputs.append(config_file) ctx.actions.run( inputs = inputs, - outputs = [facts, findings, escapes], - tools = depset(go_ctx.runfiles.to_list() + ctx.files._objdump_tool), - executable = ctx.files._nogo[0], - mnemonic = "GoStaticAnalysis", + outputs = [facts, raw_findings, escapes], + tools = depset(go_ctx.runfiles.to_list() + ctx.files._nogo_objdump_tool), + executable = ctx.files._nogo_check[0], + mnemonic = "NogoAnalysis", progress_message = "Analyzing %s" % target.label, arguments = go_ctx.nogo_args + [ "-binary=%s" % target_objfile.path, - "-objdump_tool=%s" % ctx.files._objdump_tool[0].path, + "-objdump_tool=%s" % ctx.files._nogo_objdump_tool[0].path, "-package=%s" % config_file.path, - "-findings=%s" % findings.path, + "-findings=%s" % raw_findings.path, "-facts=%s" % facts.path, "-escapes=%s" % escapes.path, ], ) + # Flatten all findings from all dependencies. + # + # This is done because all the filtering must be done at the + # top-level nogo_test to dynamically apply a configuration. + # This does not actually add any additional work here, but + # will simply propagate the full list of files. + all_raw_findings = [stdlib_info.raw_findings] + depset(all_raw_findings).to_list() + [raw_findings] + # Return the package facts as output. - return [ - NogoInfo( - facts = facts, - findings = findings, - importpath = importpath, - binaries = binaries, - srcs = srcs, - deps = deps, - ), - OutputGroupInfo( - # Expose all findings (should just be a single file). This can be - # used for build analysis of the nogo findings. - nogo_findings = depset([findings]), - # Expose all escape analysis findings (see above). - nogo_escapes = depset([escapes]), - ), - ] + return [NogoInfo( + facts = facts, + raw_findings = all_raw_findings, + escapes = escapes, + importpath = importpath, + binaries = binaries, + srcs = srcs, + deps = deps, + )] nogo_aspect = go_rule( aspect, @@ -266,50 +335,94 @@ nogo_aspect = go_rule( "embed", ], attrs = { - "_nogo": attr.label(default = "//tools/nogo/check:check"), - "_nogo_stdlib": attr.label(default = "//tools/nogo:stdlib"), - "_objdump_tool": attr.label(default = "//tools/nogo:objdump_tool"), + "_nogo_check": attr.label( + default = "//tools/nogo/check:check", + cfg = "host", + ), + "_nogo_stdlib": attr.label( + default = "//tools/nogo:stdlib", + cfg = "host", + ), + "_nogo_objdump_tool": attr.label( + default = "//tools/nogo:objdump_tool", + cfg = "host", + ), + "_nogo_target": attr.label( + default = "//tools/nogo:target", + cfg = "target", + ), }, ) def _nogo_test_impl(ctx): """Check nogo findings.""" - # Build a runner that checks the facts files. - findings = [dep[NogoInfo].findings for dep in ctx.attr.deps] - runner = ctx.actions.declare_file(ctx.label.name) + # Ensure there's a single dependency. + if len(ctx.attr.deps) != 1: + fail("nogo_test requires exactly one dep.") + raw_findings = ctx.attr.deps[0][NogoInfo].raw_findings + escapes = ctx.attr.deps[0][NogoInfo].escapes + + # Build a step that applies the configuration. + config_srcs = ctx.attr.config[NogoConfigInfo].srcs + findings = ctx.actions.declare_file(ctx.label.name + ".findings") ctx.actions.run( - inputs = findings + ctx.files.srcs, - outputs = [runner], - tools = depset(ctx.files._gentest), - executable = ctx.files._gentest[0], - mnemonic = "Gentest", + inputs = raw_findings + ctx.files.srcs + config_srcs, + outputs = [findings], + tools = depset(ctx.files._filter), + executable = ctx.files._filter[0], + mnemonic = "GoStaticAnalysis", progress_message = "Generating %s" % ctx.label, - arguments = [runner.path] + [f.path for f in findings], + arguments = ["-input=%s" % f.path for f in raw_findings] + + ["-config=%s" % f.path for f in config_srcs] + + ["-output=%s" % findings.path], ) + + # Build a runner that checks the filtered facts. + # + # Note that this calls the filter binary without any configuration, so all + # findings will be included. But this is expected, since we've already + # filtered out everything that should not be included. + runner = ctx.actions.declare_file(ctx.label.name) + runner_content = [ + "#!/bin/bash", + "exec %s -input=%s" % (ctx.files._filter[0].short_path, findings.short_path), + "", + ] + ctx.actions.write(runner, "\n".join(runner_content), is_executable = True) + return [DefaultInfo( + # The runner just executes the filter again, on the + # newly generated filtered findings. We still need + # the filter tool as part of our runfiles, however. + runfiles = ctx.runfiles(files = ctx.files._filter + [findings]), executable = runner, + ), OutputGroupInfo( + # Propagate the filtered filters, for consumption by + # build tooling. Note that the build tooling typically + # pays attention to the mnemoic above, so this must be + # what is expected by the tooling. + nogo_findings = depset([findings]), + # Expose all escape analysis findings (see above). + nogo_escapes = depset([escapes]), )] -_nogo_test = rule( +nogo_test = rule( implementation = _nogo_test_impl, attrs = { - # deps should have only a single element. - "deps": attr.label_list(aspects = [nogo_aspect]), - # srcs exist here only to ensure that this target is - # directly affected by changes to the source files. - "srcs": attr.label_list(allow_files = True), - "_gentest": attr.label(default = "//tools/nogo:gentest"), + "config": attr.label( + mandatory = True, + doc = "A rule of kind nogo_config.", + ), + "deps": attr.label_list( + aspects = [nogo_aspect], + doc = "Exactly one Go dependency to be analyzed.", + ), + "srcs": attr.label_list( + allow_files = True, + doc = "Relevant src files. This is ignored except to make the nogo_test directly affected by the files.", + ), + "_filter": attr.label(default = "//tools/nogo/filter:filter"), }, test = True, ) - -def nogo_test(name, srcs, library, **kwargs): - tags = kwargs.pop("tags", []) + ["nogo"] - _nogo_test( - name = name, - srcs = srcs, - deps = [library], - tags = tags, - **kwargs - ) diff --git a/tools/nogo/filter/BUILD b/tools/nogo/filter/BUILD new file mode 100644 index 000000000..e56a783e2 --- /dev/null +++ b/tools/nogo/filter/BUILD @@ -0,0 +1,14 @@ +load("//tools:defs.bzl", "go_binary") + +package(licenses = ["notice"]) + +go_binary( + name = "filter", + srcs = ["main.go"], + nogo = False, + visibility = ["//visibility:public"], + deps = [ + "//tools/nogo", + "@in_gopkg_yaml_v2//:go_default_library", + ], +) diff --git a/tools/nogo/filter/main.go b/tools/nogo/filter/main.go new file mode 100644 index 000000000..9cf41b3b0 --- /dev/null +++ b/tools/nogo/filter/main.go @@ -0,0 +1,131 @@ +// Copyright 2019 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. + +// Binary check is the nogo entrypoint. +package main + +import ( + "flag" + "fmt" + "io/ioutil" + "log" + "os" + "strings" + + yaml "gopkg.in/yaml.v2" + "gvisor.dev/gvisor/tools/nogo" +) + +type stringList []string + +func (s *stringList) String() string { + return strings.Join(*s, ",") +} + +func (s *stringList) Set(value string) error { + *s = append(*s, value) + return nil +} + +var ( + inputFiles stringList + configFiles stringList + outputFile string + showConfig bool +) + +func init() { + flag.Var(&inputFiles, "input", "findings input files") + flag.StringVar(&outputFile, "output", "", "findings output file") + flag.Var(&configFiles, "config", "findings configuration files") + flag.BoolVar(&showConfig, "show-config", false, "dump configuration only") +} + +func main() { + flag.Parse() + + // Load all available findings. + var findings []nogo.Finding + for _, filename := range inputFiles { + inputFindings, err := nogo.ExtractFindingsFromFile(filename) + if err != nil { + log.Fatalf("unable to extract findings from %s: %v", filename, err) + } + findings = append(findings, inputFindings...) + } + + // Open and merge all configuations. + config := &nogo.Config{ + Global: make(nogo.AnalyzerConfig), + Analyzers: make(map[nogo.AnalyzerName]nogo.AnalyzerConfig), + } + for _, filename := range configFiles { + content, err := ioutil.ReadFile(filename) + if err != nil { + log.Fatalf("unable to read %s: %v", filename, err) + } + var newConfig nogo.Config // For current file. + if err := yaml.Unmarshal(content, &newConfig); err != nil { + log.Fatalf("unable to decode %s: %v", filename, err) + } + config.Merge(&newConfig) + if showConfig { + bytes, err := yaml.Marshal(&newConfig) + if err != nil { + log.Fatalf("error marshalling config: %v", err) + } + mergedBytes, err := yaml.Marshal(config) + if err != nil { + log.Fatalf("error marshalling config: %v", err) + } + fmt.Fprintf(os.Stdout, "Loaded configuration from %s:\n%s\n", filename, string(bytes)) + fmt.Fprintf(os.Stdout, "Merged configuration:\n%s\n", string(mergedBytes)) + } + } + if err := config.Compile(); err != nil { + log.Fatalf("error compiling config: %v", err) + } + if showConfig { + os.Exit(0) + } + + // Filter the findings (and aggregate by group). + filteredFindings := make([]nogo.Finding, 0, len(findings)) + for _, finding := range findings { + if ok := config.ShouldReport(finding); ok { + filteredFindings = append(filteredFindings, finding) + } + } + + // Write the output (if required). + // + // If the outputFile is specified, then we exit here. Otherwise, + // we continue to write to stdout and treat like a test. + if outputFile != "" { + if err := nogo.WriteFindingsToFile(filteredFindings, outputFile); err != nil { + log.Fatalf("unable to write findings: %v", err) + } + return + } + + // Treat the run as a test. + if len(filteredFindings) == 0 { + fmt.Fprintf(os.Stdout, "PASS\n") + os.Exit(0) + } + for _, finding := range filteredFindings { + fmt.Fprintf(os.Stdout, "%s\n", finding.String()) + } + os.Exit(1) +} diff --git a/tools/nogo/findings.go b/tools/nogo/findings.go new file mode 100644 index 000000000..5bd850269 --- /dev/null +++ b/tools/nogo/findings.go @@ -0,0 +1,63 @@ +// Copyright 2019 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 nogo + +import ( + "encoding/json" + "fmt" + "go/token" + "io/ioutil" +) + +// Finding is a single finding. +type Finding struct { + Category AnalyzerName + Position token.Position + Message string +} + +// String implements fmt.Stringer.String. +func (f *Finding) String() string { + return fmt.Sprintf("%s: %s: %s", f.Category, f.Position.String(), f.Message) +} + +// WriteFindingsToFile writes findings to a file. +func WriteFindingsToFile(findings []Finding, filename string) error { + content, err := WriteFindingsToBytes(findings) + if err != nil { + return err + } + return ioutil.WriteFile(filename, content, 0644) +} + +// WriteFindingsToBytes serializes findings as bytes. +func WriteFindingsToBytes(findings []Finding) ([]byte, error) { + return json.Marshal(findings) +} + +// ExtractFindingsFromFile loads findings from a file. +func ExtractFindingsFromFile(filename string) ([]Finding, error) { + content, err := ioutil.ReadFile(filename) + if err != nil { + return nil, err + } + return ExtractFindingsFromBytes(content) +} + +// ExtractFindingsFromBytes loads findings from bytes. +func ExtractFindingsFromBytes(content []byte) (findings []Finding, err error) { + err = json.Unmarshal(content, &findings) + return findings, err +} diff --git a/tools/nogo/gentest.sh b/tools/nogo/gentest.sh deleted file mode 100755 index 033da11ad..000000000 --- a/tools/nogo/gentest.sh +++ /dev/null @@ -1,47 +0,0 @@ -#!/bin/bash -# Copyright 2019 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 -euo pipefail - -if [[ "$#" -lt 2 ]]; then - echo "usage: $0 <output> <findings...>" - exit 2 -fi -declare violations=0 -declare output=$1 -shift - -# Start the script. -echo "#!/bin/sh" > "${output}" - -# Read a list of findings files. -declare filename -declare line -for filename in "$@"; do - if [[ -z "${filename}" ]]; then - continue - fi - while read -r line; do - violations=$((${violations}+1)); - echo "echo -e '\\033[0;31m${line}\\033[0;31m\\033[0m'" >> "${output}" - done < "${filename}" -done - -# Show violations. -if [[ "${violations}" -eq 0 ]]; then - echo "echo -e '\\033[0;32mPASS\\033[0;31m\\033[0m'" >> "${output}" -else - echo "exit 1" >> "${output}" -fi diff --git a/tools/nogo/io_bazel_rules_go-visibility.patch b/tools/nogo/io_bazel_rules_go-visibility.patch deleted file mode 100644 index 6b64b2e85..000000000 --- a/tools/nogo/io_bazel_rules_go-visibility.patch +++ /dev/null @@ -1,25 +0,0 @@ -diff --git a/third_party/org_golang_x_tools-extras.patch b/third_party/org_golang_x_tools-extras.patch -index 133fbccc..5f0d9a47 100644 ---- a/third_party/org_golang_x_tools-extras.patch -+++ b/third_party/org_golang_x_tools-extras.patch -@@ -32,7 +32,7 @@ diff -urN c/go/analysis/internal/facts/BUILD.bazel d/go/analysis/internal/facts/ - - go_library( - name = "go_default_library", --@@ -14,6 +14,23 @@ -+@@ -14,6 +14,20 @@ - ], - ) - -@@ -43,10 +43,7 @@ diff -urN c/go/analysis/internal/facts/BUILD.bazel d/go/analysis/internal/facts/ - + "imports.go", - + ], - + importpath = "golang.org/x/tools/go/analysis/internal/facts", --+ visibility = [ --+ "//go/analysis:__subpackages__", --+ "@io_bazel_rules_go//go/tools/builders:__pkg__", --+ ], -++ visibility = ["//visibility:public"], - + deps = [ - + "//go/analysis:go_tool_library", - + "//go/types/objectpath:go_tool_library", diff --git a/tools/nogo/matchers.go b/tools/nogo/matchers.go deleted file mode 100644 index b7b73fa27..000000000 --- a/tools/nogo/matchers.go +++ /dev/null @@ -1,172 +0,0 @@ -// Copyright 2019 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 nogo - -import ( - "go/token" - "regexp" - "strings" - - "golang.org/x/tools/go/analysis" -) - -type matcher interface { - ShouldReport(d analysis.Diagnostic, fs *token.FileSet) bool -} - -// pathRegexps filters explicit paths. -type pathRegexps struct { - expr []*regexp.Regexp - - // include, if true, indicates that paths matching any regexp in expr - // match. - // - // If false, paths matching no regexps in expr match. - include bool -} - -// buildRegexps builds a list of regular expressions. -// -// This will panic on error. -func buildRegexps(prefix string, args ...string) []*regexp.Regexp { - result := make([]*regexp.Regexp, 0, len(args)) - for _, arg := range args { - result = append(result, regexp.MustCompile(prefix+arg)) - } - return result -} - -// notPath works around the lack of backtracking. -// -// It is used to construct a regular expression for non-matching components. -func notPath(name string) string { - sb := strings.Builder{} - sb.WriteString("(") - for i := range name { - if i > 0 { - sb.WriteString("|") - } - sb.WriteString(name[:i]) - sb.WriteString("[^") - sb.WriteByte(name[i]) - sb.WriteString("/][^/]*") - } - sb.WriteString(")") - return sb.String() -} - -// ShouldReport implements matcher.ShouldReport. -func (p *pathRegexps) ShouldReport(d analysis.Diagnostic, fs *token.FileSet) bool { - fullPos := fs.Position(d.Pos).String() - for _, path := range p.expr { - if path.MatchString(fullPos) { - return p.include - } - } - return !p.include -} - -// internalExcluded excludes specific internal paths. -func internalExcluded(paths ...string) *pathRegexps { - return &pathRegexps{ - expr: buildRegexps(internalPrefix, paths...), - include: false, - } -} - -// excludedExcluded excludes specific external paths. -func externalExcluded(paths ...string) *pathRegexps { - return &pathRegexps{ - expr: buildRegexps(externalPrefix, paths...), - include: false, - } -} - -// internalMatches returns a path matcher for internal packages. -func internalMatches() *pathRegexps { - return &pathRegexps{ - expr: buildRegexps(internalPrefix, internalDefault), - include: true, - } -} - -// generatedExcluded excludes all generated code. -func generatedExcluded() *pathRegexps { - return &pathRegexps{ - expr: buildRegexps(generatedPrefix, ".*"), - include: false, - } -} - -// resultExcluded excludes explicit message contents. -type resultExcluded []string - -// ShouldReport implements matcher.ShouldReport. -func (r resultExcluded) ShouldReport(d analysis.Diagnostic, _ *token.FileSet) bool { - for _, str := range r { - if strings.Contains(d.Message, str) { - return false - } - } - return true // Not excluded. -} - -// andMatcher is a composite matcher. -type andMatcher struct { - all []matcher -} - -// ShouldReport implements matcher.ShouldReport. -func (a *andMatcher) ShouldReport(d analysis.Diagnostic, fs *token.FileSet) bool { - for _, m := range a.all { - if !m.ShouldReport(d, fs) { - return false - } - } - return true -} - -// and is a syntactic convension for andMatcher. -func and(ms ...matcher) *andMatcher { - return &andMatcher{ - all: ms, - } -} - -// anyMatcher matches everything. -type anyMatcher struct{} - -// ShouldReport implements matcher.ShouldReport. -func (anyMatcher) ShouldReport(analysis.Diagnostic, *token.FileSet) bool { - return true -} - -// alwaysMatches returns an anyMatcher instance. -func alwaysMatches() anyMatcher { - return anyMatcher{} -} - -// neverMatcher will never match. -type neverMatcher struct{} - -// ShouldReport implements matcher.ShouldReport. -func (neverMatcher) ShouldReport(analysis.Diagnostic, *token.FileSet) bool { - return false -} - -// disableMatches returns a neverMatcher instance. -func disableMatches() neverMatcher { - return neverMatcher{} -} diff --git a/tools/nogo/nogo.go b/tools/nogo/nogo.go index 120fdcff5..779d4d6d8 100644 --- a/tools/nogo/nogo.go +++ b/tools/nogo/nogo.go @@ -21,7 +21,6 @@ package nogo import ( "encoding/json" "errors" - "flag" "fmt" "go/ast" "go/build" @@ -45,20 +44,20 @@ import ( "gvisor.dev/gvisor/tools/checkescape" ) -// stdlibConfig is serialized as the configuration. +// StdlibConfig is serialized as the configuration. // // This contains everything required for stdlib analysis. -type stdlibConfig struct { +type StdlibConfig struct { Srcs []string GOOS string GOARCH string Tags []string } -// packageConfig is serialized as the configuration. +// PackageConfig is serialized as the configuration. // // This contains everything required for single package analysis. -type packageConfig struct { +type PackageConfig struct { ImportPath string GoFiles []string NonGoFiles []string @@ -84,7 +83,7 @@ type saver func([]byte) error // // This is done because all stdlib data is stored together, and we don't want // to load this data many times over. -func (c *packageConfig) factLoader() (loader, error) { +func (c *PackageConfig) factLoader() (loader, error) { allFacts := make(map[string][]byte) if c.StdlibFacts != "" { data, err := ioutil.ReadFile(c.StdlibFacts) @@ -114,7 +113,7 @@ func (c *packageConfig) factLoader() (loader, error) { // shouldInclude indicates whether the file should be included. // // NOTE: This does only basic parsing of tags. -func (c *packageConfig) shouldInclude(path string) (bool, error) { +func (c *PackageConfig) shouldInclude(path string) (bool, error) { ctx := build.Default ctx.GOOS = c.GOOS ctx.GOARCH = c.GOARCH @@ -128,7 +127,7 @@ func (c *packageConfig) shouldInclude(path string) (bool, error) { // files, and the facts. Note that this importer implementation will always // pass when a given package is not available. type importer struct { - *packageConfig + *PackageConfig fset *token.FileSet cache map[string]*types.Package lastErr error @@ -185,14 +184,14 @@ func (i *importer) Import(path string) (*types.Package, error) { // ErrSkip indicates the package should be skipped. var ErrSkip = errors.New("skipped") -// checkStdlib checks the standard library. +// CheckStdlib checks the standard library. // // This constructs a synthetic package configuration for each library in the -// standard library sources, and call checkPackage repeatedly. +// standard library sources, and call CheckPackage repeatedly. // // Note that not all parts of the source are expected to build. We skip obvious // test files, and cmd files, which should not be dependencies. -func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]string, []byte, error) { +func CheckStdlib(config *StdlibConfig, analyzers []*analysis.Analyzer) (allFindings []Finding, facts []byte, err error) { if len(config.Srcs) == 0 { return nil, nil, nil } @@ -225,7 +224,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str } // Aggregate all files by directory. - packages := make(map[string]*packageConfig) + packages := make(map[string]*PackageConfig) for _, file := range config.Srcs { if !strings.HasPrefix(file, rootSrcPrefix) { // Superflouous file. @@ -243,7 +242,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str } c, ok := packages[pkg] if !ok { - c = &packageConfig{ + c = &PackageConfig{ ImportPath: pkg, GOOS: config.GOOS, GOARCH: config.GOARCH, @@ -262,14 +261,18 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str } // Closure to check a single package. - allFindings := make([]string, 0) stdlibFacts := make(map[string][]byte) + stdlibErrs := make(map[string]error) var checkOne func(pkg string) error // Recursive. checkOne = func(pkg string) error { // Is this already done? if _, ok := stdlibFacts[pkg]; ok { return nil } + // Did this fail previously? + if _, ok := stdlibErrs[pkg]; ok { + return nil + } // Lookup the configuration. config, ok := packages[pkg] @@ -283,6 +286,7 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str // If there's no binary for this package, it is likely // not built with the distribution. That's fine, we can // just skip analysis. + stdlibErrs[pkg] = err return nil } @@ -295,10 +299,11 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str }() // Run the analysis. - findings, factData, err := checkPackage(config, ac, checkOne) + findings, factData, err := CheckPackage(config, analyzers, checkOne) if err != nil { // If we can't analyze a package from the standard library, // then we skip it. It will simply not have any findings. + stdlibErrs[pkg] = err return nil } stdlibFacts[pkg] = factData @@ -312,7 +317,9 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str // to evaluate in the order provided here. We do ensure however, that // all packages are evaluated. for pkg := range packages { - checkOne(pkg) + if err := checkOne(pkg); err != nil { + return nil, nil, err + } } // Sanity check. @@ -326,11 +333,16 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str return nil, nil, fmt.Errorf("error saving stdlib facts: %w", err) } + // Write out all errors. + for pkg, err := range stdlibErrs { + log.Printf("WARNING: error while processing %v: %v", pkg, err) + } + // Return all findings. return allFindings, factData, nil } -// checkPackage runs all analyzers. +// CheckPackage runs all given analyzers. // // The implementation was adapted from [1], which was in turn adpated from [2]. // This returns a list of matching analysis issues, or an error if the analysis @@ -338,9 +350,9 @@ func checkStdlib(config *stdlibConfig, ac map[*analysis.Analyzer]matcher) ([]str // // [1] bazelbuid/rules_go/tools/builders/nogo_main.go // [2] golang.org/x/tools/go/checker/internal/checker -func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, importCallback func(string) error) ([]string, []byte, error) { +func CheckPackage(config *PackageConfig, analyzers []*analysis.Analyzer, importCallback func(string) error) (findings []Finding, factData []byte, err error) { imp := &importer{ - packageConfig: config, + PackageConfig: config, fset: token.NewFileSet(), cache: make(map[string]*types.Package), callback: importCallback, @@ -392,7 +404,6 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo // Register fact types and establish dependencies between analyzers. // The visit closure will execute recursively, and populate results // will all required analysis results. - diagnostics := make(map[*analysis.Analyzer][]analysis.Diagnostic) results := make(map[*analysis.Analyzer]interface{}) var visit func(*analysis.Analyzer) error // For recursion. visit = func(a *analysis.Analyzer) error { @@ -407,27 +418,25 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo } } - // Prepare the matcher. - m := ac[a] - report := func(d analysis.Diagnostic) { - if m.ShouldReport(d, imp.fset) { - diagnostics[a] = append(diagnostics[a], d) - } - } - // Run the analysis. factFilter := make(map[reflect.Type]bool) for _, f := range a.FactTypes { factFilter[reflect.TypeOf(f)] = true } p := &analysis.Pass{ - Analyzer: a, - Fset: imp.fset, - Files: syntax, - Pkg: types, - TypesInfo: typesInfo, - ResultOf: results, // All results. - Report: report, + Analyzer: a, + Fset: imp.fset, + Files: syntax, + Pkg: types, + TypesInfo: typesInfo, + ResultOf: results, // All results. + Report: func(d analysis.Diagnostic) { + findings = append(findings, Finding{ + Category: AnalyzerName(a.Name), + Position: imp.fset.Position(d.Pos), + Message: d.Message, + }) + }, ImportPackageFact: facts.ImportPackageFact, ExportPackageFact: facts.ExportPackageFact, ImportObjectFact: facts.ImportObjectFact, @@ -450,7 +459,7 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo } // Visit all analyzers recursively. - for a, _ := range ac { + for _, a := range analyzers { if imp.lastErr == ErrSkip { continue // No local analysis. } @@ -459,114 +468,6 @@ func checkPackage(config *packageConfig, ac map[*analysis.Analyzer]matcher, impo } } - // Convert all diagnostics to strings. - findings := make([]string, 0, len(diagnostics)) - for a, ds := range diagnostics { - for _, d := range ds { - // Include the anlyzer name for debugability and configuration. - findings = append(findings, fmt.Sprintf("%s: %s: %s", a.Name, imp.fset.Position(d.Pos), d.Message)) - } - } - // Return all findings. - factData := facts.Encode() - return findings, factData, nil -} - -var ( - packageFile = flag.String("package", "", "package configuration file (in JSON format)") - stdlibFile = flag.String("stdlib", "", "stdlib configuration file (in JSON format)") - findingsOutput = flag.String("findings", "", "output file (or stdout, if not specified)") - factsOutput = flag.String("facts", "", "output file for facts (optional)") - escapesOutput = flag.String("escapes", "", "output file for escapes (optional)") -) - -func loadConfig(file string, config interface{}) interface{} { - // Load the configuration. - f, err := os.Open(file) - if err != nil { - log.Fatalf("unable to open configuration %q: %v", file, err) - } - defer f.Close() - dec := json.NewDecoder(f) - dec.DisallowUnknownFields() - if err := dec.Decode(config); err != nil { - log.Fatalf("unable to decode configuration: %v", err) - } - return config -} - -// Main is the entrypoint; it should be called directly from main. -// -// N.B. This package registers it's own flags. -func Main() { - // Parse all flags. - flag.Parse() - - var ( - findings []string - factData []byte - err error - ) - - // Check the configuration. - if *packageFile != "" && *stdlibFile != "" { - log.Fatalf("unable to perform stdlib and package analysis; provide only one!") - } else if *stdlibFile != "" { - // Perform basic analysis. - c := loadConfig(*stdlibFile, new(stdlibConfig)).(*stdlibConfig) - findings, factData, err = checkStdlib(c, analyzerConfig) - } else if *packageFile != "" { - // Perform basic analysis. - c := loadConfig(*packageFile, new(packageConfig)).(*packageConfig) - findings, factData, err = checkPackage(c, analyzerConfig, nil) - // Do we need to do escape analysis? - if *escapesOutput != "" { - escapes, _, err := checkPackage(c, escapesConfig, nil) - if err != nil { - log.Fatalf("error performing escape analysis: %v", err) - } - f, err := os.OpenFile(*escapesOutput, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) - if err != nil { - log.Fatalf("unable to open output %q: %v", *escapesOutput, err) - } - defer f.Close() - for _, escape := range escapes { - fmt.Fprintf(f, "%s\n", escape) - } - } - } else { - log.Fatalf("please provide at least one of package or stdlib!") - } - - // Save facts. - if *factsOutput != "" { - if err := ioutil.WriteFile(*factsOutput, factData, 0644); err != nil { - log.Fatalf("error saving findings to %q: %v", *factsOutput, err) - } - } - - // Open the output file. - var w io.Writer = os.Stdout - if *findingsOutput != "" { - f, err := os.OpenFile(*findingsOutput, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) - if err != nil { - log.Fatalf("unable to open output %q: %v", *findingsOutput, err) - } - defer f.Close() - w = f - } - - // Handle findings & errors. - if err != nil { - log.Fatalf("error checking package: %v", err) - } - if len(findings) == 0 { - return - } - - // Print findings. - for _, finding := range findings { - fmt.Fprintf(w, "%s\n", finding) - } + return findings, facts.Encode(), nil } diff --git a/tools/nogo/register.go b/tools/nogo/register.go deleted file mode 100644 index 34b173937..000000000 --- a/tools/nogo/register.go +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2019 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 nogo - -import ( - "encoding/gob" - "log" - - "golang.org/x/tools/go/analysis" -) - -// analyzers returns all configured analyzers. -func analyzers() (all []*analysis.Analyzer) { - for a, _ := range analyzerConfig { - all = append(all, a) - } - for a, _ := range escapesConfig { - all = append(all, a) - } - return all -} - -func init() { - // Validate basic configuration. - if err := analysis.Validate(analyzers()); err != nil { - log.Fatalf("unable to validate analyzer: %v", err) - } - - // Register all fact types. - // - // N.B. This needs to be done recursively, because there may be - // analyzers in the Requires list that do not appear explicitly above. - registered := make(map[*analysis.Analyzer]struct{}) - var register func(*analysis.Analyzer) - register = func(a *analysis.Analyzer) { - if _, ok := registered[a]; ok { - return - } - - // Regsiter dependencies. - for _, da := range a.Requires { - register(da) - } - - // Register local facts. - for _, f := range a.FactTypes { - gob.Register(f) - } - - registered[a] = struct{}{} // Done. - } - for _, a := range analyzers() { - register(a) - } -} diff --git a/tools/nogo/util/BUILD b/tools/nogo/util/BUILD deleted file mode 100644 index 7ab340b51..000000000 --- a/tools/nogo/util/BUILD +++ /dev/null @@ -1,9 +0,0 @@ -load("//tools:defs.bzl", "go_library") - -package(licenses = ["notice"]) - -go_library( - name = "util", - srcs = ["util.go"], - visibility = ["//visibility:public"], -) diff --git a/tools/nogo/util/util.go b/tools/nogo/util/util.go deleted file mode 100644 index 919fec799..000000000 --- a/tools/nogo/util/util.go +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright 2019 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 util contains nogo-related utilities. -package util - -import ( - "fmt" - "io/ioutil" - "regexp" - "strconv" - "strings" -) - -// findingRegexp is used to parse findings. -var findingRegexp = regexp.MustCompile(`([a-zA-Z0-9_\/\.-]+): (-|([a-zA-Z0-9_\/\.-]+):([0-9]+)(:([0-9]+))?): (.*)`) - -const ( - categoryIndex = 1 - fullPathAndLineIndex = 2 - fullPathIndex = 3 - lineIndex = 4 - messageIndex = 7 -) - -// Finding is a single finding. -type Finding struct { - Category string - Path string - Line int - Message string -} - -// ExtractFindingsFromFile loads findings from a file. -func ExtractFindingsFromFile(filename string) ([]Finding, error) { - content, err := ioutil.ReadFile(filename) - if err != nil { - return nil, err - } - return ExtractFindingsFromBytes(content) -} - -// ExtractFindingsFromBytes loads findings from bytes. -func ExtractFindingsFromBytes(content []byte) (findings []Finding, err error) { - lines := strings.Split(string(content), "\n") - for _, singleLine := range lines { - // Skip blank lines. - singleLine = strings.TrimSpace(singleLine) - if singleLine == "" { - continue - } - m := findingRegexp.FindStringSubmatch(singleLine) - if m == nil { - // We shouldn't see findings like this. - return findings, fmt.Errorf("poorly formated line: %v", singleLine) - } - if m[fullPathAndLineIndex] == "-" { - continue // No source file available. - } - // Cleanup the message. - message := m[messageIndex] - message = strings.Replace(message, " → ", "\n → ", -1) - message = strings.Replace(message, " or ", "\n or ", -1) - // Construct a new annotation. - lineNumber, _ := strconv.ParseUint(m[lineIndex], 10, 32) - findings = append(findings, Finding{ - Category: m[categoryIndex], - Path: m[fullPathIndex], - Line: int(lineNumber), - Message: message, - }) - } - return findings, nil -} diff --git a/tools/parsers/BUILD b/tools/parsers/BUILD index 7d9c9a3fb..dab954e25 100644 --- a/tools/parsers/BUILD +++ b/tools/parsers/BUILD @@ -1,4 +1,4 @@ -load("//tools:defs.bzl", "go_library", "go_test") +load("//tools:defs.bzl", "go_binary", "go_library", "go_test") package(licenses = ["notice"]) @@ -7,6 +7,7 @@ go_test( size = "small", srcs = ["go_parser_test.go"], library = ":parsers", + nogo = False, deps = [ "//tools/bigquery", "@com_github_google_go_cmp//cmp:go_default_library", @@ -19,9 +20,22 @@ go_library( srcs = [ "go_parser.go", ], + nogo = False, visibility = ["//:sandbox"], deps = [ "//test/benchmarks/tools", "//tools/bigquery", ], ) + +go_binary( + name = "parser", + testonly = 1, + srcs = ["parser_main.go"], + nogo = False, + deps = [ + ":parsers", + "//runsc/flag", + "//tools/bigquery", + ], +) diff --git a/tools/parsers/go_parser.go b/tools/parsers/go_parser.go index 2cf74c883..df4875e6a 100644 --- a/tools/parsers/go_parser.go +++ b/tools/parsers/go_parser.go @@ -27,20 +27,21 @@ import ( "gvisor.dev/gvisor/tools/bigquery" ) -// parseOutput expects golang benchmark output returns a Benchmark struct formatted for BigQuery. -func parseOutput(output string, metadata *bigquery.Metadata, official bool) ([]*bigquery.Benchmark, error) { - var benchmarks []*bigquery.Benchmark +// ParseOutput expects golang benchmark output and returns a struct formatted +// for BigQuery. +func ParseOutput(output string, name string, official bool) (*bigquery.Suite, error) { + suite := bigquery.NewSuite(name) lines := strings.Split(output, "\n") for _, line := range lines { - bm, err := parseLine(line, metadata, official) + bm, err := parseLine(line, official) if err != nil { return nil, fmt.Errorf("failed to parse line '%s': %v", line, err) } if bm != nil { - benchmarks = append(benchmarks, bm) + suite.Benchmarks = append(suite.Benchmarks, bm) } } - return benchmarks, nil + return suite, nil } // parseLine handles parsing a benchmark line into a bigquery.Benchmark. @@ -58,9 +59,8 @@ func parseOutput(output string, metadata *bigquery.Metadata, official bool) ([]* // {Name: ns/op, Unit: ns/op, Sample: 1397875880} // {Name: requests_per_second, Unit: QPS, Sample: 140 } // } -// Metadata: metadata //} -func parseLine(line string, metadata *bigquery.Metadata, official bool) (*bigquery.Benchmark, error) { +func parseLine(line string, official bool) (*bigquery.Benchmark, error) { fields := strings.Fields(line) // Check if this line is a Benchmark line. Otherwise ignore the line. @@ -79,7 +79,6 @@ func parseLine(line string, metadata *bigquery.Metadata, official bool) (*bigque } bm := bigquery.NewBenchmark(name, iters, official) - bm.Metadata = metadata for _, p := range params { bm.AddCondition(p.Name, p.Value) } diff --git a/tools/parsers/go_parser_test.go b/tools/parsers/go_parser_test.go index 36996b7c8..0aa1152a2 100644 --- a/tools/parsers/go_parser_test.go +++ b/tools/parsers/go_parser_test.go @@ -94,13 +94,11 @@ func TestParseLine(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - got, err := parseLine(tc.data, nil, false) + got, err := parseLine(tc.data, false) if err != nil { t.Fatalf("parseLine failed with: %v", err) } - tc.want.Timestamp = got.Timestamp - if !cmp.Equal(tc.want, got, nil) { for _, c := range got.Condition { t.Logf("Cond: %+v", c) @@ -150,14 +148,14 @@ BenchmarkRuby/server_threads.5-6 1 1416003331 ns/op 0.00950 average_latency.s 46 for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - bms, err := parseOutput(tc.data, nil, false) + suite, err := ParseOutput(tc.data, "", false) if err != nil { t.Fatalf("parseOutput failed: %v", err) - } else if len(bms) != tc.numBenchmarks { - t.Fatalf("NumBenchmarks failed want: %d got: %d %+v", tc.numBenchmarks, len(bms), bms) + } else if len(suite.Benchmarks) != tc.numBenchmarks { + t.Fatalf("NumBenchmarks failed want: %d got: %d %+v", tc.numBenchmarks, len(suite.Benchmarks), suite.Benchmarks) } - for _, bm := range bms { + for _, bm := range suite.Benchmarks { if len(bm.Metric) != tc.numMetrics { t.Fatalf("NumMetrics failed want: %d got: %d %+v", tc.numMetrics, len(bm.Metric), bm.Metric) } diff --git a/tools/parsers/parser_main.go b/tools/parsers/parser_main.go new file mode 100644 index 000000000..6c6182464 --- /dev/null +++ b/tools/parsers/parser_main.go @@ -0,0 +1,129 @@ +// Copyright 2020 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. + +// Binary parser parses Benchmark data from golang benchmarks, +// puts it into a Schema for BigQuery, and sends it to BigQuery. +// parser will also initialize a table with the Benchmarks BigQuery schema. +package main + +import ( + "context" + "fmt" + "io/ioutil" + "os" + + "gvisor.dev/gvisor/runsc/flag" + bq "gvisor.dev/gvisor/tools/bigquery" + "gvisor.dev/gvisor/tools/parsers" +) + +const ( + initString = "init" + initDescription = "initializes a new table with benchmarks schema" + parseString = "parse" + parseDescription = "parses given benchmarks file and sends it to BigQuery table." +) + +var ( + // The init command will create a new dataset/table in the given project and initialize + // the table with the schema in //tools/bigquery/bigquery.go. If the table/dataset exists + // or has been initialized, init has no effect and successfully returns. + initCmd = flag.NewFlagSet(initString, flag.ContinueOnError) + initProject = initCmd.String("project", "", "GCP project to send benchmarks.") + initDataset = initCmd.String("dataset", "", "dataset to send benchmarks data.") + initTable = initCmd.String("table", "", "table to send benchmarks data.") + + // The parse command parses benchmark data in `file` and sends it to the + // requested table. + parseCmd = flag.NewFlagSet(parseString, flag.ContinueOnError) + file = parseCmd.String("file", "", "file to parse for benchmarks") + name = parseCmd.String("suite_name", "", "name of the benchmark suite") + clNumber = parseCmd.String("cl", "", "changelist number of this run") + gitCommit = parseCmd.String("git_commit", "", "git commit sha for this run") + parseProject = parseCmd.String("project", "", "GCP project to send benchmarks.") + parseDataset = parseCmd.String("dataset", "", "dataset to send benchmarks data.") + parseTable = parseCmd.String("table", "", "table to send benchmarks data.") + official = parseCmd.Bool("official", false, "mark input data as official.") +) + +// initBenchmarks initializes a dataset/table in a BigQuery project. +func initBenchmarks(ctx context.Context) error { + return bq.InitBigQuery(ctx, *initProject, *initDataset, *initTable, nil) +} + +// parseBenchmarks parses the given file into the BigQuery schema, +// adds some custom data for the commit, and sends the data to BigQuery. +func parseBenchmarks(ctx context.Context) error { + data, err := ioutil.ReadFile(*file) + if err != nil { + return fmt.Errorf("failed to read file: %v", err) + } + suite, err := parsers.ParseOutput(string(data), *name, *official) + if err != nil { + return fmt.Errorf("failed parse data: %v", err) + } + extraConditions := []*bq.Condition{ + { + Name: "change_list", + Value: *clNumber, + }, + { + Name: "commit", + Value: *gitCommit, + }, + } + + suite.Conditions = append(suite.Conditions, extraConditions...) + return bq.SendBenchmarks(ctx, suite, *parseProject, *parseDataset, *parseTable, nil) +} + +func main() { + ctx := context.Background() + switch { + // the "init" command + case len(os.Args) >= 2 && os.Args[1] == initString: + if err := initCmd.Parse(os.Args[2:]); err != nil { + fmt.Fprintf(os.Stderr, "failed parse flags: %v", err) + os.Exit(1) + } + if err := initBenchmarks(ctx); err != nil { + failure := "failed to initialize project: %s dataset: %s table: %s: %v" + fmt.Fprintf(os.Stderr, failure, *parseProject, *parseDataset, *parseTable, err) + os.Exit(1) + } + // the "parse" command. + case len(os.Args) >= 2 && os.Args[1] == parseString: + if err := parseCmd.Parse(os.Args[2:]); err != nil { + fmt.Fprintf(os.Stderr, "failed parse flags: %v", err) + os.Exit(1) + } + if err := parseBenchmarks(ctx); err != nil { + fmt.Fprintf(os.Stderr, "failed parse benchmarks: %v", err) + os.Exit(1) + } + default: + printUsage() + } +} + +// printUsage prints the top level usage string. +func printUsage() { + usage := `Usage: parser <command> <flags> ... + +Available commands: + %s %s + %s %s +` + fmt.Fprintf(os.Stderr, usage, initCmd.Name(), initDescription, parseCmd.Name(), parseDescription) +} diff --git a/tools/rules_go.patch b/tools/rules_go.patch new file mode 100644 index 000000000..5e1e87084 --- /dev/null +++ b/tools/rules_go.patch @@ -0,0 +1,14 @@ +diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl +index 17516ad7..76b6c68c 100644 +--- a/go/private/rules/test.bzl ++++ b/go/private/rules/test.bzl +@@ -121,9 +121,6 @@ def _go_test_impl(ctx): + ) + + test_gc_linkopts = gc_linkopts(ctx) +- if not go.mode.debug: +- # Disable symbol table and DWARF generation for test binaries. +- test_gc_linkopts.extend(["-s", "-w"]) + + # Now compile the test binary itself + test_library = GoLibrary( diff --git a/tools/tag_release.sh b/tools/tag_release.sh index b0bab74b4..50378065e 100755 --- a/tools/tag_release.sh +++ b/tools/tag_release.sh @@ -43,7 +43,7 @@ fi closest_commit() { while read line; do - if [[ "$line" =~ "commit " ]]; then + if [[ "$line" =~ ^"commit " ]]; then current_commit="${line#commit }" continue elif [[ "$line" =~ "PiperOrigin-RevId: " ]]; then @@ -57,7 +57,9 @@ closest_commit() { # Is the passed identifier a sha commit? if ! git show "${target_commit}" &> /dev/null; then # Extract the commit given a piper ID. - declare -r commit="$(git log | closest_commit "${target_commit}")" + commit="$(set +o pipefail; \ + git log --first-parent | closest_commit "${target_commit}")" + declare -r commit else declare -r commit="${target_commit}" fi |