From 6a4908b26239ec7ab027e1c2624ca00d740937fe Mon Sep 17 00:00:00 2001 From: Adin Scannell Date: Tue, 21 Feb 2023 17:02:56 -0800 Subject: [PATCH] Process nogo targets across all architectures. PiperOrigin-RevId: 511343590 --- debian/BUILD | 3 +- pkg/abi/linux/BUILD | 3 +- pkg/cpuid/BUILD | 5 ++ pkg/goid/BUILD | 3 +- pkg/hosttid/BUILD | 3 +- pkg/ring0/BUILD | 3 +- pkg/sentry/loader/vdsodata/BUILD | 3 +- pkg/sentry/platform/kvm/BUILD | 3 +- pkg/sync/BUILD | 3 +- test/syscalls/linux/BUILD | 3 +- test/syscalls/linux/rseq/BUILD | 3 +- tools/BUILD | 8 +++ tools/arch.bzl | 62 +++++++++++++++++++++++ tools/bazeldefs/defs.bzl | 23 ++++++--- tools/defs.bzl | 58 +--------------------- tools/nogo/defs.bzl | 84 ++++++++++++++++++-------------- vdso/BUILD | 3 +- 17 files changed, 163 insertions(+), 110 deletions(-) create mode 100644 tools/arch.bzl diff --git a/debian/BUILD b/debian/BUILD index 7e9a0727d..3d58b5c62 100644 --- a/debian/BUILD +++ b/debian/BUILD @@ -1,4 +1,5 @@ -load("//tools:defs.bzl", "pkg_deb", "pkg_tar", "select_arch", "version") +load("//tools:arch.bzl", "select_arch") +load("//tools:defs.bzl", "pkg_deb", "pkg_tar", "version") package(licenses = ["notice"]) diff --git a/pkg/abi/linux/BUILD b/pkg/abi/linux/BUILD index 2f54d0158..aec99b169 100644 --- a/pkg/abi/linux/BUILD +++ b/pkg/abi/linux/BUILD @@ -1,4 +1,5 @@ -load("//tools:defs.bzl", "arch_genrule", "go_library", "go_test") +load("//tools:arch.bzl", "arch_genrule") +load("//tools:defs.bzl", "go_library", "go_test") load("//tools/nogo:defs.bzl", "nogo_facts") # Package linux contains the constants and types needed to interface with a diff --git a/pkg/cpuid/BUILD b/pkg/cpuid/BUILD index 2b63ec6b3..535dd6df3 100644 --- a/pkg/cpuid/BUILD +++ b/pkg/cpuid/BUILD @@ -29,6 +29,11 @@ go_test( "cpuid_test.go", ], library = ":cpuid", + # NOTE: It seems that bazel code generation does not properly parse tags + # when run via the architecture transition for nogo. This should be fixed + # at some point in the future, but for now we can simply skip nogo analysis + # on the test itself. It still applies to the core library. + nogo = False, ) go_test( diff --git a/pkg/goid/BUILD b/pkg/goid/BUILD index 29cb83578..49e0bdc4c 100644 --- a/pkg/goid/BUILD +++ b/pkg/goid/BUILD @@ -1,4 +1,5 @@ -load("//tools:defs.bzl", "arch_genrule", "go_library", "go_test", "select_arch") +load("//tools:arch.bzl", "arch_genrule", "select_arch") +load("//tools:defs.bzl", "go_library", "go_test") load("//tools/nogo:defs.bzl", "nogo_facts") package(licenses = ["notice"]) diff --git a/pkg/hosttid/BUILD b/pkg/hosttid/BUILD index 97a84d216..7ae6a87c8 100644 --- a/pkg/hosttid/BUILD +++ b/pkg/hosttid/BUILD @@ -1,4 +1,5 @@ -load("//tools:defs.bzl", "arch_genrule", "go_library", "go_test", "select_arch") +load("//tools:arch.bzl", "arch_genrule", "select_arch") +load("//tools:defs.bzl", "go_library", "go_test") load("//tools/nogo:defs.bzl", "nogo_facts") package(licenses = ["notice"]) diff --git a/pkg/ring0/BUILD b/pkg/ring0/BUILD index e04080eda..c0c37a99b 100644 --- a/pkg/ring0/BUILD +++ b/pkg/ring0/BUILD @@ -1,4 +1,5 @@ -load("//tools:defs.bzl", "arch_genrule", "go_library", "select_arch") +load("//tools:arch.bzl", "arch_genrule", "select_arch") +load("//tools:defs.bzl", "go_library") load("//tools/nogo:defs.bzl", "nogo_facts") package(licenses = ["notice"]) diff --git a/pkg/sentry/loader/vdsodata/BUILD b/pkg/sentry/loader/vdsodata/BUILD index 51ffabdb9..809c497cf 100644 --- a/pkg/sentry/loader/vdsodata/BUILD +++ b/pkg/sentry/loader/vdsodata/BUILD @@ -1,4 +1,5 @@ -load("//tools:defs.bzl", "arch_genrule", "go_embed_data", "go_library") +load("//tools:arch.bzl", "arch_genrule") +load("//tools:defs.bzl", "go_embed_data", "go_library") package(licenses = ["notice"]) diff --git a/pkg/sentry/platform/kvm/BUILD b/pkg/sentry/platform/kvm/BUILD index 97140924f..ced668564 100644 --- a/pkg/sentry/platform/kvm/BUILD +++ b/pkg/sentry/platform/kvm/BUILD @@ -1,4 +1,5 @@ -load("//tools:defs.bzl", "arch_genrule", "go_library", "go_test", "select_arch") +load("//tools:arch.bzl", "arch_genrule", "select_arch") +load("//tools:defs.bzl", "go_library", "go_test") load("//tools/go_generics:defs.bzl", "go_template_instance") load("//tools/nogo:defs.bzl", "nogo_facts") diff --git a/pkg/sync/BUILD b/pkg/sync/BUILD index 547f12c9f..47847ac6b 100644 --- a/pkg/sync/BUILD +++ b/pkg/sync/BUILD @@ -1,4 +1,5 @@ -load("//tools:defs.bzl", "arch_genrule", "go_library", "go_test", "select_arch") +load("//tools:arch.bzl", "arch_genrule", "select_arch") +load("//tools:defs.bzl", "go_library", "go_test") load("//tools/nogo:defs.bzl", "nogo_facts") package( diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 84007ee7e..c4dae36ff 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -1,4 +1,5 @@ -load("//tools:defs.bzl", "cc_binary", "cc_library", "gbenchmark", "gtest", "select_arch", "select_system") +load("//tools:arch.bzl", "select_arch") +load("//tools:defs.bzl", "cc_binary", "cc_library", "gbenchmark", "gtest", "select_system") package( default_visibility = ["//:sandbox"], diff --git a/test/syscalls/linux/rseq/BUILD b/test/syscalls/linux/rseq/BUILD index 853258b04..4168b4931 100644 --- a/test/syscalls/linux/rseq/BUILD +++ b/test/syscalls/linux/rseq/BUILD @@ -1,7 +1,8 @@ # This package contains a standalone rseq test binary. This binary must not # depend on libc, which might use rseq itself. -load("//tools:defs.bzl", "cc_flags_supplier", "cc_library", "cc_toolchain", "select_arch") +load("//tools:arch.bzl", "select_arch") +load("//tools:defs.bzl", "cc_flags_supplier", "cc_library", "cc_toolchain") package(licenses = ["notice"]) diff --git a/tools/BUILD b/tools/BUILD index bacb62f9a..4064c80de 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -19,6 +19,14 @@ config_setting( ], ) +bzl_library( + name = "arch_bzl", + srcs = ["arch.bzl"], + visibility = [ + "//:sandbox", + ], +) + bzl_library( name = "defs_bzl", srcs = ["defs.bzl"], diff --git a/tools/arch.bzl b/tools/arch.bzl new file mode 100644 index 000000000..047451a01 --- /dev/null +++ b/tools/arch.bzl @@ -0,0 +1,62 @@ +"""Wrappers for architecture-specific rules.""" + +load("//tools/bazeldefs:defs.bzl", _amd64_config = "amd64_config", _arch_config = "arch_config", _arm64_config = "arm64_config", _select_arch = "select_arch", _transition_allowlist = "transition_allowlist") + +# Export arch rules. +select_arch = _select_arch +transition_allowlist = _transition_allowlist + +def _arch_transition_impl(settings, attr): + return { + "arm64": _arm64_config(settings, attr), + "amd64": _amd64_config(settings, attr), + } + +arch_transition = transition( + implementation = _arch_transition_impl, + inputs = [], + outputs = _arch_config, +) + +def _arch_genrule_impl(ctx): + """Runs a command with inputs from multiple architectures. + + The command will be run multiple times, with the provided + template rendered using the architecture for the output. + """ + outputs = [] + for (arch, src) in ctx.split_attr.src.items(): + # Calculate the template for this output file. + output = ctx.actions.declare_file(ctx.attr.template % arch) + outputs.append(output) + + # Copy the specific generated source. + input_files = src[DefaultInfo].files + ctx.actions.run_shell( + inputs = input_files, + outputs = [output], + command = "cp %s %s" % ( + " ".join([f.path for f in input_files.to_list()]), + output.path, + ), + ) + return [DefaultInfo( + files = depset(outputs), + )] + +arch_genrule = rule( + implementation = _arch_genrule_impl, + attrs = { + "src": attr.label( + doc = "Sources for the genrule.", + cfg = arch_transition, + ), + "template": attr.string( + doc = "Template for the output files.", + mandatory = True, + ), + "_allowlist_function_transition": attr.label( + default = _transition_allowlist, + ), + }, +) diff --git a/tools/bazeldefs/defs.bzl b/tools/bazeldefs/defs.bzl index 02b0edb08..57d8e5477 100644 --- a/tools/bazeldefs/defs.bzl +++ b/tools/bazeldefs/defs.bzl @@ -21,12 +21,23 @@ def proto_library(name, has_services = None, **kwargs): **kwargs ) -def select_arch(amd64 = "amd64", arm64 = "arm64", default = None, **kwargs): - values = { - "//tools/bazeldefs:amd64": amd64, - "//tools/bazeldefs:arm64": arm64, - } - if default: +def select_arch(amd64 = None, arm64 = None, default = None, **kwargs): + """Select an option against standard architectures. + + Args: + amd64: the option if the architecture is amd64. + arm64: the option if the architecture is arm64. + default: the option if no matching architecture is provided. + **kwargs: extra select arguments. + + Returns: + An appropriate select.""" + values = dict() + if amd64 != None: + values["//tools/bazeldefs:amd64"] = amd64 + if arm64 != None: + values["//tools/bazeldefs:arm64"] = arm64 + if default != None: values["//conditions:default"] = default return select(values, **kwargs) diff --git a/tools/defs.bzl b/tools/defs.bzl index 36765c166..cc131a53b 100644 --- a/tools/defs.bzl +++ b/tools/defs.bzl @@ -8,7 +8,7 @@ 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/nogo:defs.bzl", "nogo_test") -load("//tools/bazeldefs:defs.bzl", _BuildSettingInfo = "BuildSettingInfo", _amd64_config = "amd64_config", _arch_config = "arch_config", _arm64_config = "arm64_config", _bool_flag = "bool_flag", _bpf_program = "bpf_program", _build_test = "build_test", _bzl_library = "bzl_library", _coreutil = "coreutil", _default_net_util = "default_net_util", _more_shards = "more_shards", _most_shards = "most_shards", _proto_library = "proto_library", _select_arch = "select_arch", _select_system = "select_system", _short_path = "short_path", _transition_allowlist = "transition_allowlist", _version = "version") +load("//tools/bazeldefs:defs.bzl", _BuildSettingInfo = "BuildSettingInfo", _bool_flag = "bool_flag", _bpf_program = "bpf_program", _build_test = "build_test", _bzl_library = "bzl_library", _coreutil = "coreutil", _default_net_util = "default_net_util", _more_shards = "more_shards", _most_shards = "most_shards", _proto_library = "proto_library", _select_system = "select_system", _short_path = "short_path", _version = "version") load("//tools/bazeldefs:cc.bzl", _cc_binary = "cc_binary", _cc_flags_supplier = "cc_flags_supplier", _cc_grpc_library = "cc_grpc_library", _cc_library = "cc_library", _cc_proto_library = "cc_proto_library", _cc_test = "cc_test", _cc_toolchain = "cc_toolchain", _gbenchmark = "gbenchmark", _gbenchmark_internal = "gbenchmark_internal", _grpcpp = "grpcpp", _gtest = "gtest", _vdso_linker_option = "vdso_linker_option") load("//tools/bazeldefs:go.bzl", _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", _gotsan_flag_values = "gotsan_flag_values", _gotsan_values = "gotsan_values", _select_goarch = "select_goarch", _select_goos = "select_goos") load("//tools/bazeldefs:pkg.bzl", _pkg_deb = "pkg_deb", _pkg_tar = "pkg_tar") @@ -21,7 +21,6 @@ bzl_library = _bzl_library bool_flag = _bool_flag BuildSettingInfo = _BuildSettingInfo default_net_util = _default_net_util -select_arch = _select_arch select_system = _select_system short_path = _short_path coreutil = _coreutil @@ -325,58 +324,3 @@ def proto_library(name, srcs, deps = None, has_services = 0, **kwargs): deps = [":" + name + "_cc_proto"], **kwargs ) - -def _arch_transition_impl(settings, attr): - return { - "arm64": _arm64_config(settings, attr), - "amd64": _amd64_config(settings, attr), - } - -arch_transition = transition( - implementation = _arch_transition_impl, - inputs = [], - outputs = _arch_config, -) - -def _arch_genrule_impl(ctx): - """Runs a command with inputs from multiple architectures. - - The command will be run multiple times, with the provided - template rendered using the architecture for the output. - """ - outputs = [] - for (arch, src) in ctx.split_attr.src.items(): - # Calculate the template for this output file. - output = ctx.actions.declare_file(ctx.attr.template % arch) - outputs.append(output) - - # Copy the specific generated source. - input_files = src[DefaultInfo].files - ctx.actions.run_shell( - inputs = input_files, - outputs = [output], - command = "cp %s %s" % ( - " ".join([f.path for f in input_files.to_list()]), - output.path, - ), - ) - return [DefaultInfo( - files = depset(outputs), - )] - -arch_genrule = rule( - implementation = _arch_genrule_impl, - attrs = { - "src": attr.label( - doc = "Sources for the genrule.", - cfg = arch_transition, - ), - "template": attr.string( - doc = "Template for the output files.", - mandatory = True, - ), - "_allowlist_function_transition": attr.label( - default = _transition_allowlist, - ), - }, -) diff --git a/tools/nogo/defs.bzl b/tools/nogo/defs.bzl index e23e56a06..733d8dc9a 100644 --- a/tools/nogo/defs.bzl +++ b/tools/nogo/defs.bzl @@ -2,6 +2,7 @@ load("//tools/bazeldefs:defs.bzl", "BuildSettingInfo") load("//tools/bazeldefs:go.bzl", "go_context", "go_embed_libraries", "go_importpath", "go_rule") +load("//tools:arch.bzl", "arch_transition", "transition_allowlist") NogoConfigInfo = provider( "information about a nogo configuration", @@ -343,54 +344,61 @@ nogo_aspect = go_rule( def _nogo_test_impl(ctx): """Check nogo findings.""" - # 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 - - # 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 = raw_findings + ctx.files.srcs + config_srcs, - outputs = [findings], - tools = depset(ctx.files._nogo), - executable = ctx.files._nogo[0], - mnemonic = "GoStaticAnalysis", - progress_message = "Generating %s" % ctx.label, - # See above. - execution_requirements = {"no-sandbox": "1"}, - arguments = ["filter"] + - ["-config=%s" % f.path for f in config_srcs] + - ["-output=%s" % findings.path] + - [f.path for f in raw_findings], - ) - # Build a runner that checks the filtered facts. - # - # 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 filter -test -text %s" % (ctx.files._nogo[0].short_path, findings.short_path), - "", - ] - ctx.actions.write(runner, "\n".join(runner_content), is_executable = True) + runner_content = ["#!/bin/bash"] + runner_footer = list() + all_findings = list() + # Collect all architecture-targets. + for (arch, deps) in ctx.split_attr.deps.items(): + # Ensure there's a single dependency. + if len(deps) != 1: + fail("nogo_test requires exactly one dep.") + raw_findings = deps[0][NogoInfo].raw_findings + + # Build a step that applies the configuration. + config_srcs = ctx.attr.config[NogoConfigInfo].srcs + findings = ctx.actions.declare_file(ctx.label.name + "." + arch + ".findings") + ctx.actions.run( + inputs = raw_findings + ctx.files.srcs + config_srcs, + outputs = [findings], + tools = depset(ctx.files._nogo), + executable = ctx.files._nogo[0], + mnemonic = "GoStaticAnalysis", + progress_message = "Generating %s" % ctx.label, + # See above. + execution_requirements = {"no-sandbox": "1"}, + arguments = ["filter"] + + ["-config=%s" % f.path for f in config_srcs] + + ["-output=%s" % findings.path] + + [f.path for f in raw_findings], + ) + + # 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. The + # runner will always run all tests, and then exit if any have failed. + runner_content.append("echo -n %s..." % arch) + runner_content.append("%s filter -test -text %s" % (ctx.files._nogo[0].short_path, findings.short_path)) + runner_content.append("rc_%s=$?" % arch) + runner_footer.append("if [[ $rc_%s -ne 0 ]]; then exit $rc_%s; fi" % (arch, arch)) + all_findings.append(findings) + runner_content.extend(runner_footer) + runner_content.append("") # Ensure empty line. + 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._nogo + [findings]), + runfiles = ctx.runfiles(files = ctx.files._nogo + all_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]), + nogo_findings = depset(all_findings), )] nogo_test = rule( @@ -403,6 +411,7 @@ nogo_test = rule( "deps": attr.label_list( aspects = [nogo_aspect], doc = "Exactly one Go dependency to be analyzed.", + cfg = arch_transition, ), "srcs": attr.label_list( allow_files = True, @@ -414,12 +423,15 @@ nogo_test = rule( ), "_target": attr.label( default = "//tools/nogo:target", - cfg = "target", + cfg = arch_transition, ), "_nogo_full": attr.label( default = "//tools/nogo:full", cfg = "exec", ), + "_allowlist_function_transition": attr.label( + default = transition_allowlist, + ), }, test = True, ) diff --git a/vdso/BUILD b/vdso/BUILD index 1eaa4b506..37fb26a16 100644 --- a/vdso/BUILD +++ b/vdso/BUILD @@ -3,7 +3,8 @@ # normal system VDSO (time, gettimeofday, clock_gettimeofday) but which uses # timekeeping parameters managed by the sandbox kernel. -load("//tools:defs.bzl", "cc_flags_supplier", "cc_toolchain", "select_arch", "vdso_linker_option") +load("//tools:arch.bzl", "select_arch") +load("//tools:defs.bzl", "cc_flags_supplier", "cc_toolchain", "vdso_linker_option") package(licenses = ["notice"])