diff options
author | Adin Scannell <ascannell@google.com> | 2020-10-19 16:26:42 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-10-19 16:28:40 -0700 |
commit | 54e989ec3a2d9c6928047ea09a3d6053cbf2441f (patch) | |
tree | d64ef5014191725e66de7c1f81795927be9d96d3 /tools | |
parent | 4b4d12d5bb9c4902380fa999b5f49d3ed7029938 (diff) |
Remove legacy bazel configurations.
Using the newer bazel rules necessitates a transition from proto1 to
proto2. In order to resolve the incompatibility between proto2 and
gogoproto, the cri runtimeoptions proto must be vendored.
Further, some of the semantics of bazel caching changed during the
transition. It is now necessary to:
- Ensure that :gopath depends only on pure library targets, as the
propagation of go_binary build attributes (pure, static) will
affected the generated files (though content remains the same,
there are conflicts with respect to the gopath).
- Update bazel.mk to include the possibility of binaries in the
bazel-out directory, as it will now put runsc and others there.
This required some refinements to the mechanism of extracting
paths, since some the existing regex resulted in false positives.
- Change nogo rules to prevent escape generation on binary targets.
For some reason, the newer version of bazel attempted to run the
nogo analysis on the binary targets, which fails due to the fact
that objdump does not work on the final binary. This must be due
to a change in the semantics of aspects in bazel3.
PiperOrigin-RevId: 337958324
Diffstat (limited to 'tools')
-rw-r--r-- | tools/bazel.mk | 10 | ||||
-rw-r--r-- | tools/bazeldefs/BUILD | 41 | ||||
-rw-r--r-- | tools/checkescape/checkescape.go | 11 | ||||
-rw-r--r-- | tools/defs.bzl | 6 | ||||
-rw-r--r-- | tools/github/main.go | 36 | ||||
-rw-r--r-- | tools/github/nogo/nogo.go | 39 | ||||
-rw-r--r-- | tools/go_generics/defs.bzl | 108 | ||||
-rw-r--r-- | tools/nogo/config.go | 1 | ||||
-rw-r--r-- | tools/nogo/defs.bzl | 6 | ||||
-rwxr-xr-x | tools/nogo/gentest.sh | 1 | ||||
-rw-r--r-- | tools/nogo/io_bazel_rules_go-visibility.patch | 25 | ||||
-rw-r--r-- | tools/nogo/nogo.go | 8 | ||||
-rw-r--r-- | tools/rules_go.patch | 14 |
13 files changed, 138 insertions, 168 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/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 019506d3e..bb291c512 100644 --- a/tools/defs.bzl +++ b/tools/defs.bzl @@ -76,9 +76,13 @@ 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", 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/nogo.go b/tools/github/nogo/nogo.go index b70dfe63b..b2bc63459 100644 --- a/tools/github/nogo/nogo.go +++ b/tools/github/nogo/nogo.go @@ -53,26 +53,31 @@ func NewFindingsPoster(client *github.Client, owner, repo, commit string, dryRun } // 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 := util.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. 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/config.go b/tools/nogo/config.go index 8079618ab..0853f03cf 100644 --- a/tools/nogo/config.go +++ b/tools/nogo/config.go @@ -473,6 +473,7 @@ func init() { "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_cri.go", // Generated: exempt all. "pkg/shim/v2/runtimeoptions/runtimeoptions_test.go:22", "pkg/shim/v2/service.go:15", "pkg/shim/v2/service_linux.go:18", diff --git a/tools/nogo/defs.bzl b/tools/nogo/defs.bzl index 29898cfda..543598b52 100644 --- a/tools/nogo/defs.bzl +++ b/tools/nogo/defs.bzl @@ -151,8 +151,8 @@ NogoInfo = provider( "findings": "package 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)", }, ) @@ -162,7 +162,7 @@ def _nogo_aspect_impl(target, ctx): # 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"): diff --git a/tools/nogo/gentest.sh b/tools/nogo/gentest.sh index 033da11ad..0a762f9f6 100755 --- a/tools/nogo/gentest.sh +++ b/tools/nogo/gentest.sh @@ -34,6 +34,7 @@ for filename in "$@"; do continue fi while read -r line; do + line="${line@Q}" violations=$((${violations}+1)); echo "echo -e '\\033[0;31m${line}\\033[0;31m\\033[0m'" >> "${output}" done < "${filename}" 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/nogo.go b/tools/nogo/nogo.go index b178f63ab..e19e3c237 100644 --- a/tools/nogo/nogo.go +++ b/tools/nogo/nogo.go @@ -536,15 +536,15 @@ func Main() { 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() + escapes, _, err := checkPackage(c, escapesConfig, nil) + if err != nil { + log.Fatalf("error performing escape analysis: %v", err) + } for _, escape := range escapes { fmt.Fprintf(f, "%s\n", escape) } 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( |