From f7acafe7d0f4b81b2b66e6eee54672923828594c Mon Sep 17 00:00:00 2001 From: Genevieve Mendoza Date: Wed, 12 Aug 2026 00:24:27 -0700 Subject: [PATCH 1/4] feat: add crate_identity attribute and per-link-unit crate-instance validation Prep work to fix the upstream hermeticbuild/rules_rs issue 144. Motivation A crate can be pulled from two different registries or resolved differently across Bzlmod hubs, producing two identical-name rlibs in one binary. For crates with global/static state (e.g., log, tracing), that breaks shared-state assumptions. There should be at most one configured instance per source-qualified identity per link unit. Adds an opt-in crate_identity attribute to library-producing Rust rules and a RustCrateIdentityInfo/RustLinkClosureInfo provider pair so a configured Rust library can carry a source-qualified logical identity. This commit does NOT merge or deduplicate crate instances; it only carries the identity and enforces the per-link-unit invariant (at most one configured instance of each source-qualified identity per native link unit) at analysis time, intrinsically for Rust terminal link units and via an opt-in aspect or the rust_link_checked_* wrappers for native C/C++ links. Deduplication of compatible same-source packages across independent hubs is performed upstream by the consuming crate generator (rules_rs interner). --- rust/BUILD.bazel | 1 + rust/private/common.bzl | 6 +- rust/private/providers.bzl | 38 +++ rust/private/rust.bzl | 40 ++- rust/private/rust_crate_identity.bzl | 73 +++++ rust/private/rustc.bzl | 77 ++++++ rust/rust_common.bzl | 4 + rust/rust_link_validation.bzl | 163 ++++++++++++ test/unit/rust_link_validation/BUILD.bazel | 4 + test/unit/rust_link_validation/lib.rs | 3 + test/unit/rust_link_validation/main.rs | 1 + test/unit/rust_link_validation/proc.rs | 8 + .../rust_link_validation_test.bzl | 251 ++++++++++++++++++ .../rust_link_validation_native/BUILD.bazel | 4 + test/unit/rust_link_validation_native/lib.rs | 3 + test/unit/rust_link_validation_native/main.cc | 3 + .../rust_link_validation_native_test.bzl | 238 +++++++++++++++++ 17 files changed, 910 insertions(+), 7 deletions(-) create mode 100644 rust/private/rust_crate_identity.bzl create mode 100644 rust/rust_link_validation.bzl create mode 100644 test/unit/rust_link_validation/BUILD.bazel create mode 100644 test/unit/rust_link_validation/lib.rs create mode 100644 test/unit/rust_link_validation/main.rs create mode 100644 test/unit/rust_link_validation/proc.rs create mode 100644 test/unit/rust_link_validation/rust_link_validation_test.bzl create mode 100644 test/unit/rust_link_validation_native/BUILD.bazel create mode 100644 test/unit/rust_link_validation_native/lib.rs create mode 100644 test/unit/rust_link_validation_native/main.cc create mode 100644 test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl diff --git a/rust/BUILD.bazel b/rust/BUILD.bazel index 399bdc0201..210e3ae545 100644 --- a/rust/BUILD.bazel +++ b/rust/BUILD.bazel @@ -27,6 +27,7 @@ bzl_library( "//rust/private:bzl_lib", "//rust/settings:bzl_lib", "@bazel_features//:features", + "@rules_cc//cc:defs_bzl", ], ) diff --git a/rust/private/common.bzl b/rust/private/common.bzl index 81b58acc0d..461b5796c4 100644 --- a/rust/private/common.bzl +++ b/rust/private/common.bzl @@ -23,7 +23,7 @@ which exports the `rust_common` struct. In the Bazel lingo, `rust_common` gives the access to the Rust Sandwich API. """ -load(":providers.bzl", "CrateGroupInfo", "CrateInfo", "DepInfo", "DepVariantInfo", "StdLibInfo", "TestCrateInfo") +load(":providers.bzl", "CrateGroupInfo", "CrateInfo", "DepInfo", "DepVariantInfo", "RustCrateIdentityInfo", "RustLinkClosureInfo", "StdLibInfo", "TestCrateInfo") # This constant only represents the default value for attributes and macros # defined in `rules_rust`. Like any attribute public attribute, it can be @@ -67,6 +67,8 @@ def _create_crate_info(**kwargs): kwargs.update({"root_path": ""}) if not "owner" in kwargs: kwargs.update({"owner": None}) + if not "crate_identity" in kwargs: + kwargs.update({"crate_identity": None}) return CrateInfo(**kwargs) rust_common = struct( @@ -74,6 +76,8 @@ rust_common = struct( crate_info = CrateInfo, dep_info = DepInfo, dep_variant_info = DepVariantInfo, + rust_crate_identity_info = RustCrateIdentityInfo, + rust_link_closure_info = RustLinkClosureInfo, stdlib_info = StdLibInfo, test_crate_info = TestCrateInfo, crate_group_info = CrateGroupInfo, diff --git a/rust/private/providers.bzl b/rust/private/providers.bzl index 099c6713ba..4a0621700d 100644 --- a/rust/private/providers.bzl +++ b/rust/private/providers.bzl @@ -22,6 +22,10 @@ CrateInfo = provider( "List[str]: The set of enabled cfgs for this crate. Note that this field is populated only " + "when @rules_rust//rust/settings:collect_cfgs is set." ), + "crate_identity": ( + "RustCrateIdentityInfo, optional: The logical Rust library identity and configured crate " + + "instance, if this target carries one. Absent for ordinary handwritten targets." + ), "compile_data": "depset[File]: Compile data required by this crate.", "compile_data_targets": "depset[Label]: Compile data targets required by this crate.", "data": "depset[File]: Runtime data associated with the target. Not passed to `Rustc` actions, except for `proc-macro` targets where `Rustc` is the runtime.", @@ -71,6 +75,40 @@ DepInfo = provider( }, ) +RustCrateIdentityInfo = provider( + doc = ( + "The logical identity of a Rust library together with the specific configured crate " + + "instance that implements it.\n\n" + + "`logical_id` answers \"does this represent the same logical upstream Rust library?\" " + + "The `crate_instance` artifact answers \"is this actually the same compiled instance?\" " + + "Equality of `crate_instance` means the same configured compilation output; target " + + "configuration, enabled features, cfgs, toolchain, transitions, and recursively selected " + + "dependencies are already reflected in the configured action that owns that artifact. " + + "Different artifacts are conservatively treated as different instances even if their " + + "bytes or compilation flags happen to match.\n\n" + + "`owner` and `display_name` are diagnostic-only fields and do not participate in equality." + ), + fields = { + "logical_id": "str: Logical identity of the upstream Rust library whose runtime/type identity should be unique.", + "crate_instance": "File: The configured crate output artifact (CrateInfo.output).", + "owner": "Label: The label of the target that produced the crate only for diagnostics.", + "display_name": "str: Short human-readable description for diagnostics only.", + }, +) + +RustLinkClosureInfo = provider( + doc = ( + "The Rust library identity closure absorbed into a native-boundary artifact.\n\n" + + "A native consumer may fold a `static` closure into its own link unit. A `dynamic` " + + "artifact is independently linked and its internal Rust crates must not be combined with " + + "a consuming unit's closure." + ), + fields = { + "crates": "depset[RustCrateIdentityInfo]: The Rust library identity closure.", + "linkage": "str: 'static' or 'dynamic'.", + }, +) + CrateGroupInfo = provider( doc = "A provider containing a group of crates.", fields = { diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 0de8fff40f..e5682c2231 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -1091,11 +1091,39 @@ _RUST_TEST_ATTRS = { ), } | _COVERAGE_ATTRS | _EXPERIMENTAL_USE_CC_COMMON_LINK_ATTRS +# `crate_identity` is intentionally restricted to library-producing rules and is +# NOT part of `_COMMON_ATTRS`, so binaries, tests, examples, and build-script +# executables never carry it. A Cargo package can legitimately contain both a +# library and binaries; tagging every Cargo target with only its package ID +# would incorrectly equate distinct Cargo targets. +_CRATE_IDENTITY_ATTRS = { + "crate_identity": attr.string( + doc = dedent("""\ + Optional logical identity of the upstream Rust library this target + represents. + + When set, the configured library carries a `RustCrateIdentityInfo` + and participates in cross-link-unit crate-instance validation: a + native link unit may contain at most one configured crate instance + for each non-empty logical identity. + + The `cargo:` prefix is reserved for crate generators (crate_universe + sets `crate_identity = "cargo:" + fully_qualified_cargo_package_id` + on generated library targets). Handwritten libraries should use a + reverse-domain or repository-qualified identifier, e.g. + `com.example:mylib`. + + Leave empty (the default) for ordinary targets that opt out of this + facility. + """), + ), +} + rust_library = rule( implementation = _rust_library_impl, provides = COMMON_PROVIDERS, cfg = per_crate_flag_trim_transition, - attrs = _COMMON_ATTRS | { + attrs = _COMMON_ATTRS | _CRATE_IDENTITY_ATTRS | { "disable_pipelining": attr.bool( default = False, doc = dedent("""\ @@ -1268,7 +1296,7 @@ _rust_static_library_transition = transition( rust_static_library = rule( implementation = _rust_static_library_impl, - attrs = _COMMON_ATTRS | _PLATFORM_ATTRS, + attrs = _COMMON_ATTRS | _CRATE_IDENTITY_ATTRS | _PLATFORM_ATTRS, fragments = ["cpp"], cfg = _rust_static_library_transition, toolchains = [ @@ -1300,7 +1328,7 @@ def _rust_shared_library_transition_impl(settings, attr): _PER_CRATE_FLAG_SETTING: per_crate_flags, } -_rust_shared_library_transition = transition( +_rust_cdylib_library_transition = transition( implementation = _rust_shared_library_transition_impl, inputs = [ "//command_line_option:platforms", @@ -1330,9 +1358,9 @@ _CC_RUNTIME_LINKAGE_ATTRS = { rust_cdylib_library = rule( implementation = _rust_cdylib_library_impl, - attrs = _COMMON_ATTRS | _PLATFORM_ATTRS | _EXPERIMENTAL_USE_CC_COMMON_LINK_ATTRS | _CC_RUNTIME_LINKAGE_ATTRS, + attrs = _COMMON_ATTRS | _CRATE_IDENTITY_ATTRS | _PLATFORM_ATTRS | _EXPERIMENTAL_USE_CC_COMMON_LINK_ATTRS | _CC_RUNTIME_LINKAGE_ATTRS, fragments = ["cpp"], - cfg = _rust_shared_library_transition, + cfg = _rust_cdylib_library_transition, toolchains = [ str(Label("//rust:toolchain_type")), config_common.toolchain_type("@bazel_tools//tools/cpp:toolchain_type", mandatory = False), @@ -1362,7 +1390,7 @@ rust_proc_macro = rule( "_allowlist_function_transition": attr.label( default = "@bazel_tools//tools/allowlists/function_transition_allowlist", ), - }, + } | _CRATE_IDENTITY_ATTRS, fragments = ["cpp"], toolchains = [ str(Label("//rust:toolchain_type")), diff --git a/rust/private/rust_crate_identity.bzl b/rust/private/rust_crate_identity.bzl new file mode 100644 index 0000000000..ded3b63561 --- /dev/null +++ b/rust/private/rust_crate_identity.bzl @@ -0,0 +1,73 @@ +# Copyright 2021 The Bazel Authors. All rights reserved. +# +# 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. + +"""Shared Rust crate-instance link validation. + +Used by both the intrinsic Rust link rules (`rustc.bzl`) and the native +(C/C++) link-validation aspect so there is a single, consistent definition of +the enforced invariant: + +> Within one configured native link unit, each non-empty logical Rust library +> identity may correspond to at most one configured Rust crate instance. +""" + +def validate_crate_identity_closure(owner, identities): + """Fails when one link unit contains incompatible instances of one identity. + + Args: + owner (Label): The link unit owner, used only in diagnostics. + identities (list[RustCrateIdentityInfo]): The Rust identity records in + the link unit's static closure. + """ + instances_by_id = {} + for identity in identities: + logical_id = identity.logical_id + if not logical_id: + continue + instances = instances_by_id.get(logical_id) + if instances == None: + instances = {} + instances_by_id[logical_id] = instances + instances[str(identity.crate_instance)] = identity + + conflicting_ids = [ + logical_id + for logical_id, instances in instances_by_id.items() + if len(instances) > 1 + ] + if not conflicting_ids: + return + + lines = [ + "{} would link incompatible instances of the same Rust library:".format(owner), + ] + for logical_id in sorted(conflicting_ids): + instances = instances_by_id[logical_id] + lines.extend([ + "", + " logical identity: {}".format(logical_id), + ]) + for identity in sorted(instances.values(), key = lambda i: str(i.owner)): + lines.extend([ + "", + " {}".format(identity.owner), + " crate instance: {}".format(identity.crate_instance), + ]) + + lines.extend([ + "", + "A native link unit may contain at most one configured Rust crate instance", + "for each logical Rust library identity.", + ]) + fail("\n".join(lines)) diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index a7006fb429..b40adf2000 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -43,6 +43,7 @@ load( "UnstableRustFeaturesInfo", _BuildInfo = "BuildInfo", ) +load(":rust_crate_identity.bzl", "validate_crate_identity_closure") load(":rustc_resource_set.bzl", "get_rustc_resource_set", "is_codegen_units_enabled") load(":stamp.bzl", "is_stamping_enabled") load( @@ -180,6 +181,59 @@ def _are_linkstamps_supported(feature_configuration): def _is_proc_macro(crate_info): return "proc-macro" in (crate_info.type, crate_info.wrapped_crate_type) +def _is_terminal_link_unit(crate_info): + """Whether this crate type finalizes a link unit (vs. being linked into one).""" + if crate_info.is_test: + return True + return crate_info.type in ("bin", "cdylib", "dylib", "staticlib", "proc-macro") + +def _linkage_for(crate_info): + """The native linkage reported by a library target's RustLinkClosureInfo. + + A shared (cdylib/dylib) artifact is an independent dynamic link boundary and + must not have its internal Rust closure folded into a consumer's closure. + """ + if crate_info.type in ("cdylib", "dylib"): + return "dynamic" + return "static" + +def _crate_identity_for(ctx, crate_info_dict): + """Builds the RustCrateIdentityInfo for the current crate, or None. + + `crate_instance` is the configured output artifact (CrateInfo.output); + target configuration, features, cfgs, toolchain, transitions, and + recursively selected dependencies are all already reflected in the + configured action that owns that artifact. + """ + logical_id = getattr(ctx.attr, "crate_identity", "") + if not logical_id: + return None + return rust_common.rust_crate_identity_info( + logical_id = logical_id, + crate_instance = crate_info_dict["output"], + owner = ctx.label, + display_name = crate_info_dict["name"], + ) + +def _link_closure_identities(crate_info, dep_info): + """Returns the Rust library identity records in this crate's static closure. + + proc-macro implementation crates are host units: they must not be folded + into a target-runtime closure. `dep_info.transitive_crates` includes them + (they land in `direct_crates`), so filter them out here. + """ + identities = [] + own = crate_info.crate_identity + if own != None: + identities.append(own) + for linked_crate in dep_info.transitive_crates.to_list(): + if _is_proc_macro(linked_crate): + continue + identity = getattr(linked_crate, "crate_identity", None) + if identity != None: + identities.append(identity) + return identities + def collect_deps( deps, proc_macro_deps, @@ -2599,7 +2653,9 @@ def rustc_compile( # ctx.configuration.default_shell_env, which must not leak through # CrateInfo -- it would otherwise clobber cc_toolchain link_env in # downstream rust_test(crate = ...) (see bazelbuild/rules_rust#3989). + crate_identity = _crate_identity_for(ctx, crate_info_dict) crate_info_dict.update({ + "crate_identity": crate_identity, "rustc_env": env_from_args, }) crate_info = rust_common.create_crate_info( @@ -2639,6 +2695,27 @@ def rustc_compile( lto_object = output_o if distributed_thin_lto else None, ) + # The static Rust identity closure absorbed by this crate. Reused for the + # intrinsic link-unit validator below and for the native-boundary provider, + # so there is only one approximation of Rust dependency filtering. + link_identities = _link_closure_identities(crate_info, dep_info) + + # Enforce the per-link-unit invariant during analysis, before any linker + # action is registered for this link unit. + if _is_terminal_link_unit(crate_info): + validate_crate_identity_closure(ctx.label, link_identities) + + # Export the native-boundary closure for Rust targets that expose CcInfo so + # a downstream native (C/C++) linking aspect can fold it. Terminal link + # units (binaries, tests, proc macros) do not need to export it. + if crate_info.type in ("rlib", "lib", "staticlib", "cdylib", "dylib"): + providers.append( + rust_common.rust_link_closure_info( + crates = depset(link_identities), + linkage = _linkage_for(crate_info), + ), + ) + output_group_info = {} if pdb_file: diff --git a/rust/rust_common.bzl b/rust/rust_common.bzl index 8f692f06ad..3d6333b021 100644 --- a/rust/rust_common.bzl +++ b/rust/rust_common.bzl @@ -27,6 +27,8 @@ load( _CrateInfo = "CrateInfo", _DepInfo = "DepInfo", _DepVariantInfo = "DepVariantInfo", + _RustCrateIdentityInfo = "RustCrateIdentityInfo", + _RustLinkClosureInfo = "RustLinkClosureInfo", _TestCrateInfo = "TestCrateInfo", _UnstableRustFeaturesInfo = "UnstableRustFeaturesInfo", ) @@ -38,6 +40,8 @@ CrateGroupInfo = _CrateGroupInfo CrateInfo = _CrateInfo DepInfo = _DepInfo DepVariantInfo = _DepVariantInfo +RustCrateIdentityInfo = _RustCrateIdentityInfo +RustLinkClosureInfo = _RustLinkClosureInfo TestCrateInfo = _TestCrateInfo UnstableRustFeaturesInfo = _UnstableRustFeaturesInfo UnstableSelfProfileInfo = _UnstableSelfProfileInfo diff --git a/rust/rust_link_validation.bzl b/rust/rust_link_validation.bzl new file mode 100644 index 0000000000..31604eb675 --- /dev/null +++ b/rust/rust_link_validation.bzl @@ -0,0 +1,163 @@ +# Copyright 2021 The Bazel Authors. All rights reserved. +# +# 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. + +"""Native (C/C++) link validation for Rust crate instances. + +Rust link rules validate their own target-runtime closure intrinsically (see +`rust/private/rustc.bzl`). A native final link -- e.g. a `cc_binary` linking two +`rust_static_library`s -- is outside that intrinsic check. This module supplies: + +* `rust_link_validation_aspect`: applies the same per-link-unit invariant to + native link units reached transitively. Enable it with + `--aspects=@rules_rust//rust:rust_link_validation.bzl%rust_link_validation_aspect`. +* `rust_link_checked_cc_binary` / `rust_link_checked_cc_test` / + `rust_link_checked_cc_shared_library`: paved-path wrappers that force the + check during analysis without adding linker inputs. +""" + +load("@rules_cc//cc:defs.bzl", _cc_binary = "cc_binary", _cc_shared_library = "cc_shared_library", _cc_test = "cc_test") +load("@rules_cc//cc/common:cc_common.bzl", "cc_common") +load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") +load("//rust/private:providers.bzl", "RustLinkClosureInfo") +load("//rust/private:rust_crate_identity.bzl", "validate_crate_identity_closure") + +# Private aggregation provider returned by the aspect. It is distinct from +# RustLinkClosureInfo because Bazel does not allow an aspect and a target rule +# to return the same provider type, and the aspect needs to carry its result +# along the native shadow graph. +RustLinkAggregationInfo = provider( + doc = "Aggregates static Rust identity closures along native dependency edges.", + fields = { + "crates": "depset[RustCrateIdentityInfo]: Static Rust identity closure reached so far.", + }, +) + +# Native dependency attributes the aspect traverses. Only these have the aspect +# applied via `attr_aspects`, so only these can contribute aggregation results. +_NATIVE_DEP_ATTRS = ["deps", "implementation_deps"] + +# Native link-unit rule kinds where the aggregated closure is finalized and the +# invariant is enforced. +_NATIVE_LINK_UNIT_KINDS = ("cc_binary", "cc_test", "cc_shared_library") + +def _rust_link_validation_aspect_impl(target, ctx): + """Propagates static Rust identity closures across native dep edges. + + A target that exposes RustLinkClosureInfo is a Rust boundary: its closure is + already complete and authoritative, so it is a leaf of this shadow graph. + We never descend into a Rust target's own `deps` -- doing so would pull a + dynamic library's internal crates out of its private link boundary + (observed via cquery traces) and double-count static ones. + """ + if RustLinkClosureInfo in target: + closure = target[RustLinkClosureInfo] + if closure.linkage != "static": + # Dynamic boundary: opaque. Contributes nothing and stops here. + return [] + return [ + RustLinkAggregationInfo(crates = closure.crates), + ] + + crates = [] + for attr_name in _NATIVE_DEP_ATTRS: + if not hasattr(ctx.rule.attr, attr_name): + continue + for dep in getattr(ctx.rule.attr, attr_name): + if RustLinkAggregationInfo in dep: + crates.append(dep[RustLinkAggregationInfo].crates) + + if not crates: + return [] + + aggregation = RustLinkAggregationInfo(crates = depset(transitive = crates)) + + if ctx.rule.kind in _NATIVE_LINK_UNIT_KINDS: + validate_crate_identity_closure(ctx.label, aggregation.crates.to_list()) + + return [aggregation] + +rust_link_validation_aspect = aspect( + implementation = _rust_link_validation_aspect_impl, + attr_aspects = _NATIVE_DEP_ATTRS, + doc = ( + "Traverses native dependency edges, accumulates the static Rust library identity " + + "closures exposed by Rust targets, and enforces that each logical identity appears " + + "with at most one configured crate instance within one native link unit (cc_binary, " + + "cc_test, cc_shared_library)." + ), +) + +def _rust_link_checker_impl(ctx): + """Validates the Rust identity closure of the forwarded native deps.""" + crates = [] + for dep in ctx.attr.deps: + if RustLinkAggregationInfo in dep: + crates.append(dep[RustLinkAggregationInfo].crates) + + identities = [] + for closure in crates: + identities.extend(closure.to_list()) + + validate_crate_identity_closure(ctx.label, identities) + + return [ + DefaultInfo(), + # Empty CcInfo: forces analysis of the checker (and its aspect) when the + # real native target depends on it, without adding any linker inputs. + CcInfo( + linking_context = cc_common.create_linking_context( + linker_inputs = depset([]), + ), + ), + ] + +_rust_link_checker = rule( + implementation = _rust_link_checker_impl, + attrs = { + "deps": attr.label_list( + doc = "Forwarded native deps the aspect should aggregate and validate.", + aspects = [rust_link_validation_aspect], + ), + }, + doc = "Analysis-only Rust crate-instance validator for a native link.", +) + +def rust_link_checked_cc_binary(name, deps = [], **kwargs): + """A `cc_binary` whose Rust identity closure is validated during analysis.""" + _rust_link_checked(_cc_binary, name, deps, **kwargs) + +def rust_link_checked_cc_test(name, deps = [], **kwargs): + """A `cc_test` whose Rust identity closure is validated during analysis.""" + _rust_link_checked(_cc_test, name, deps, **kwargs) + +def rust_link_checked_cc_shared_library(name, deps = [], **kwargs): + """A `cc_shared_library` whose Rust identity closure is validated during analysis.""" + _rust_link_checked(_cc_shared_library, name, deps, **kwargs) + +def _rust_link_checked(cc_rule, name, deps, **kwargs): + checker = "_" + name + "_crate_link_check" + _rust_link_checker( + name = checker, + deps = deps, + tags = ["manual"], + ) + + # Forward the original deps plus the analysis-only checker. The checker + # contributes an empty CcInfo, so it forces the validator to run without + # altering the link inputs. + cc_rule( + name = name, + deps = deps + [":" + checker], + **kwargs + ) diff --git a/test/unit/rust_link_validation/BUILD.bazel b/test/unit/rust_link_validation/BUILD.bazel new file mode 100644 index 0000000000..9ba7cedaa3 --- /dev/null +++ b/test/unit/rust_link_validation/BUILD.bazel @@ -0,0 +1,4 @@ +load(":rust_link_validation_test.bzl", "rust_link_validation_test_suite") + +############################ UNIT TESTS ############################# +rust_link_validation_test_suite(name = "rust_link_validation_test_suite") diff --git a/test/unit/rust_link_validation/lib.rs b/test/unit/rust_link_validation/lib.rs new file mode 100644 index 0000000000..3991d363db --- /dev/null +++ b/test/unit/rust_link_validation/lib.rs @@ -0,0 +1,3 @@ +pub fn ping() -> bool { + true +} diff --git a/test/unit/rust_link_validation/main.rs b/test/unit/rust_link_validation/main.rs new file mode 100644 index 0000000000..f328e4d9d0 --- /dev/null +++ b/test/unit/rust_link_validation/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/test/unit/rust_link_validation/proc.rs b/test/unit/rust_link_validation/proc.rs new file mode 100644 index 0000000000..2a7b987d26 --- /dev/null +++ b/test/unit/rust_link_validation/proc.rs @@ -0,0 +1,8 @@ +extern crate proc_macro; + +use proc_macro::TokenStream; + +#[proc_macro] +pub fn identity(input: TokenStream) -> TokenStream { + input +} diff --git a/test/unit/rust_link_validation/rust_link_validation_test.bzl b/test/unit/rust_link_validation/rust_link_validation_test.bzl new file mode 100644 index 0000000000..70dedd5d6c --- /dev/null +++ b/test/unit/rust_link_validation/rust_link_validation_test.bzl @@ -0,0 +1,251 @@ +"""Analysis tests for the intrinsic Rust crate-instance link validation.""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load("//rust:defs.bzl", "rust_binary", "rust_common", "rust_library", "rust_proc_macro", "rust_shared_library", "rust_static_library", "rust_test") + +_IDENTITY = "cargo:registry+https://index.crates.io/#dup@1.0.0" + +def _conflict_test(): + def _impl(ctx): + env = analysistest.begin(ctx) + asserts.expect_failure( + env, + "would link incompatible instances of the same Rust library", + ) + return analysistest.end(env) + + return analysistest.make(_impl, expect_failure = True) + +link_conflict_test = _conflict_test() + +def _clean_test_impl(ctx): + env = analysistest.begin(ctx) + return analysistest.end(env) + +link_clean_test = analysistest.make(_clean_test_impl) + +def _identity_record_test_impl(ctx): + env = analysistest.begin(ctx) + tut = analysistest.target_under_test(env) + crate_info = tut[rust_common.crate_info] + identity = crate_info.crate_identity + asserts.true(env, identity != None) + asserts.equals(env, _IDENTITY, identity.logical_id) + asserts.equals(env, crate_info.output, identity.crate_instance) + return analysistest.end(env) + +identity_record_test = analysistest.make(_identity_record_test_impl) + +def _no_identity_test_impl(ctx): + env = analysistest.begin(ctx) + tut = analysistest.target_under_test(env) + crate_info = tut[rust_common.crate_info] + asserts.equals(env, None, crate_info.crate_identity) + return analysistest.end(env) + +no_identity_test = analysistest.make(_no_identity_test_impl) + +def rust_link_validation_test_suite(name): + """Creates the intrinsic Rust link-validation analysis test suite. + + Args: + name: Name of the generated test suite. + """ + rust_library( + name = "class_one", + srcs = ["lib.rs"], + crate_identity = _IDENTITY, + crate_name = "dup", + ) + rust_library( + name = "class_two", + srcs = ["lib.rs"], + crate_identity = _IDENTITY, + crate_name = "dup", + ) + rust_library( + name = "plain_lib", + srcs = ["lib.rs"], + crate_name = "plain_lib", + ) + rust_library( + name = "other_identity", + srcs = ["lib.rs"], + crate_identity = "cargo:other@1.0.0", + crate_name = "other_identity", + ) + rust_library( + name = "transitive_class_one", + srcs = ["lib.rs"], + deps = [":class_one"], + crate_name = "transitive_class_one", + ) + + # ---------------- pass: identity sanity ---------------- + identity_record_test( + name = "identity_record_test", + target_under_test = ":class_one", + ) + no_identity_test( + name = "plain_lib_has_no_identity_test", + target_under_test = ":plain_lib", + ) + + # ---------------- pass: single instance ---------------- + rust_binary( + name = "single_identity_bin", + srcs = ["main.rs"], + deps = [":class_one"], + ) + link_clean_test( + name = "single_identity_bin_test", + target_under_test = ":single_identity_bin", + ) + + # Different logical identities in one binary: pass. + rust_binary( + name = "different_identities_bin", + srcs = ["main.rs"], + deps = [":class_one", ":other_identity"], + ) + link_clean_test( + name = "different_identities_bin_test", + target_under_test = ":different_identities_bin", + ) + + # Same instance reached through a parallel transitive path: pass (dedup by artifact). + rust_binary( + name = "shared_path_bin", + srcs = ["main.rs"], + deps = [":class_one", ":transitive_class_one"], + ) + link_clean_test( + name = "shared_path_bin_test", + target_under_test = ":shared_path_bin", + ) + + # Two incompatible instances used by separate binaries: pass. + rust_binary( + name = "separate_bin_one", + srcs = ["main.rs"], + deps = [":class_one"], + ) + rust_binary( + name = "separate_bin_two", + srcs = ["main.rs"], + deps = [":class_two"], + ) + link_clean_test( + name = "separate_bin_one_test", + target_under_test = ":separate_bin_one", + ) + link_clean_test( + name = "separate_bin_two_test", + target_under_test = ":separate_bin_two", + ) + + # Proc-macro host closure is not folded into the target-runtime closure: + # a proc macro with the same logical identity as a runtime lib must pass. + rust_proc_macro( + name = "same_identity_proc_macro", + srcs = ["proc.rs"], + crate_identity = _IDENTITY, + crate_name = "same_identity_proc_macro", + ) + rust_binary( + name = "proc_macro_isolation_bin", + srcs = ["main.rs"], + deps = [":class_one"], + proc_macro_deps = [":same_identity_proc_macro"], + ) + link_clean_test( + name = "proc_macro_isolation_bin_test", + target_under_test = ":proc_macro_isolation_bin", + ) + + # ---------------- fail: two instances in one link unit ---------------- + rust_binary( + name = "direct_conflict_bin", + srcs = ["main.rs"], + deps = [":class_one", ":class_two"], + tags = ["manual"], + ) + link_conflict_test( + name = "direct_conflict_bin_test", + target_under_test = ":direct_conflict_bin", + ) + + rust_binary( + name = "transitive_conflict_bin", + srcs = ["main.rs"], + deps = [":class_two", ":transitive_class_one"], + tags = ["manual"], + ) + link_conflict_test( + name = "transitive_conflict_bin_test", + target_under_test = ":transitive_conflict_bin", + ) + + rust_static_library( + name = "staticlib_conflict", + srcs = ["lib.rs"], + deps = [":class_one", ":class_two"], + tags = ["manual"], + ) + link_conflict_test( + name = "staticlib_conflict_test", + target_under_test = ":staticlib_conflict", + ) + + rust_shared_library( + name = "cdylib_conflict", + srcs = ["lib.rs"], + deps = [":class_one", ":class_two"], + tags = ["manual"], + ) + link_conflict_test( + name = "cdylib_conflict_test", + target_under_test = ":cdylib_conflict", + ) + + rust_proc_macro( + name = "proc_macro_conflict", + srcs = ["proc.rs"], + deps = [":class_one", ":class_two"], + tags = ["manual"], + ) + link_conflict_test( + name = "proc_macro_conflict_test", + target_under_test = ":proc_macro_conflict", + ) + + rust_test( + name = "test_conflict", + srcs = ["lib.rs"], + deps = [":class_one", ":class_two"], + tags = ["manual"], + ) + link_conflict_test( + name = "test_conflict_test", + target_under_test = ":test_conflict", + ) + + native.test_suite( + name = name, + tests = [ + ":cdylib_conflict_test", + ":direct_conflict_bin_test", + ":different_identities_bin_test", + ":identity_record_test", + ":plain_lib_has_no_identity_test", + ":proc_macro_conflict_test", + ":proc_macro_isolation_bin_test", + ":separate_bin_one_test", + ":separate_bin_two_test", + ":shared_path_bin_test", + ":single_identity_bin_test", + ":staticlib_conflict_test", + ":test_conflict_test", + ":transitive_conflict_bin_test", + ], + ) diff --git a/test/unit/rust_link_validation_native/BUILD.bazel b/test/unit/rust_link_validation_native/BUILD.bazel new file mode 100644 index 0000000000..87d98764f2 --- /dev/null +++ b/test/unit/rust_link_validation_native/BUILD.bazel @@ -0,0 +1,4 @@ +load(":rust_link_validation_native_test.bzl", "rust_link_validation_native_test_suite") + +############################ UNIT TESTS ############################# +rust_link_validation_native_test_suite(name = "rust_link_validation_native_test_suite") diff --git a/test/unit/rust_link_validation_native/lib.rs b/test/unit/rust_link_validation_native/lib.rs new file mode 100644 index 0000000000..3991d363db --- /dev/null +++ b/test/unit/rust_link_validation_native/lib.rs @@ -0,0 +1,3 @@ +pub fn ping() -> bool { + true +} diff --git a/test/unit/rust_link_validation_native/main.cc b/test/unit/rust_link_validation_native/main.cc new file mode 100644 index 0000000000..4cce7f667f --- /dev/null +++ b/test/unit/rust_link_validation_native/main.cc @@ -0,0 +1,3 @@ +int main() { + return 0; +} diff --git a/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl b/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl new file mode 100644 index 0000000000..6a79edc1aa --- /dev/null +++ b/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl @@ -0,0 +1,238 @@ +"""Analysis tests for native (C/C++) Rust crate-instance link validation.""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library") +load("//rust:defs.bzl", "rust_common", "rust_library", "rust_shared_library", "rust_static_library") +load("//rust:rust_link_validation.bzl", "rust_link_checked_cc_binary", "rust_link_checked_cc_shared_library") + +_IDENTITY = "cargo:registry+https://index.crates.io/#dup@1.0.0" + +def _closure_provider_test_impl(ctx): + env = analysistest.begin(ctx) + tut = analysistest.target_under_test(env) + closure = tut[rust_common.rust_link_closure_info] + asserts.equals(env, ctx.attr.expect_linkage, closure.linkage) + owners = [] + for identity in closure.crates.to_list(): + owners.append(str(identity.owner)) + expects = ctx.attr.expect_owners + asserts.true(env, len(expects) > 0) + for token in expects: + present = False + for owner in owners: + if token in owner: + present = True + break + asserts.true(env, present, "missing %s among %s" % (token, sorted(owners))) + return analysistest.end(env) + +closure_provider_test = analysistest.make( + _closure_provider_test_impl, + attrs = { + "expect_linkage": attr.string(), + "expect_owners": attr.string_list(), + }, +) + +def _native_conflict_test(): + def _impl(ctx): + env = analysistest.begin(ctx) + asserts.expect_failure( + env, + "would link incompatible instances of the same Rust library", + ) + return analysistest.end(env) + + return analysistest.make(_impl, expect_failure = True) + +native_conflict_test = _native_conflict_test() + +def _native_clean_test_impl(ctx): + env = analysistest.begin(ctx) + return analysistest.end(env) + +native_clean_test = analysistest.make(_native_clean_test_impl) + +def rust_link_validation_native_test_suite(name): + """Creates the native-link validation analysis test suite. + + Args: + name: Name of the generated test suite. + """ + + # Two classes of one logical identity, each wrapped in its own static lib. + rust_library( + name = "class_one", + srcs = ["lib.rs"], + crate_identity = _IDENTITY, + crate_name = "dup", + ) + rust_library( + name = "class_two", + srcs = ["lib.rs"], + crate_identity = _IDENTITY, + crate_name = "dup", + ) + rust_static_library( + name = "platform_static_one", + srcs = ["lib.rs"], + deps = [":class_one"], + crate_name = "platform_static_one", + ) + rust_static_library( + name = "platform_static_two", + srcs = ["lib.rs"], + deps = [":class_two"], + crate_name = "platform_static_two", + ) + + # ---------------- raw negative control ---------------- + # A raw cc_binary without the aspect/wrapper is not checked and must build. + cc_binary( + name = "raw_unchecked", + srcs = ["main.cc"], + deps = [":platform_static_one", ":platform_static_two"], + ) + native_clean_test( + name = "raw_unchecked_test", + target_under_test = ":raw_unchecked", + ) + + # ---------------- checked conflict cases (fail) ---------------- + rust_link_checked_cc_binary( + name = "checked_conflict", + srcs = ["main.cc"], + deps = [":platform_static_one", ":platform_static_two"], + tags = ["manual"], + ) + native_conflict_test( + name = "checked_conflict_test", + target_under_test = ":checked_conflict", + ) + + # Conflict remains when a cc_library sits between the binary and the static libs. + cc_library( + name = "native_mid_one", + deps = [":platform_static_one"], + ) + cc_library( + name = "native_mid_two", + deps = [":platform_static_two"], + ) + rust_link_checked_cc_binary( + name = "checked_transitive_conflict", + srcs = ["main.cc"], + deps = [":native_mid_one", ":native_mid_two"], + tags = ["manual"], + ) + native_conflict_test( + name = "checked_transitive_conflict_test", + target_under_test = ":checked_transitive_conflict", + ) + + # A checked shared library containing conflicting static closures fails. + rust_link_checked_cc_shared_library( + name = "checked_shared_conflict", + deps = [":platform_static_one", ":platform_static_two"], + tags = ["manual"], + ) + native_conflict_test( + name = "checked_shared_conflict_test", + target_under_test = ":checked_shared_conflict", + ) + + # ---------------- checked allowed cases (pass) ---------------- + rust_link_checked_cc_binary( + name = "checked_separate_one", + srcs = ["main.cc"], + deps = [":platform_static_one"], + ) + rust_link_checked_cc_binary( + name = "checked_separate_two", + srcs = ["main.cc"], + deps = [":platform_static_two"], + ) + native_clean_test( + name = "checked_separate_one_test", + target_under_test = ":checked_separate_one", + ) + native_clean_test( + name = "checked_separate_two_test", + target_under_test = ":checked_separate_two", + ) + + # The same instance reached through two native paths: pass (dedup by artifact). + cc_library( + name = "native_wrap", + deps = [":platform_static_one"], + ) + rust_link_checked_cc_binary( + name = "checked_shared_path", + srcs = ["main.cc"], + deps = [":platform_static_one", ":native_wrap"], + ) + native_clean_test( + name = "checked_shared_path_test", + target_under_test = ":checked_shared_path", + ) + + # Two separately valid shared libraries are independent link units: a checked + # binary linking both is allowed even though they share the logical identity. + rust_shared_library( + name = "dyn_lib_one", + srcs = ["lib.rs"], + deps = [":class_one"], + crate_name = "dyn_lib_one", + ) + rust_shared_library( + name = "dyn_lib_two", + srcs = ["lib.rs"], + deps = [":class_two"], + crate_name = "dyn_lib_two", + ) + rust_link_checked_cc_binary( + name = "checked_two_dynamic", + srcs = ["main.cc"], + deps = [":dyn_lib_one", ":dyn_lib_two"], + ) + native_clean_test( + name = "checked_two_dynamic_test", + target_under_test = ":checked_two_dynamic", + ) + + # Provider contract: static vs dynamic linkage + depset content (also + # exercises RustCrateIdentityInfo depset hashability). + rust_static_library( + name = "contract_static", + srcs = ["lib.rs"], + deps = [":class_one"], + crate_name = "contract_static", + ) + closure_provider_test( + name = "static_closure_provider_test", + target_under_test = ":contract_static", + expect_linkage = "static", + expect_owners = ["class_one"], + ) + closure_provider_test( + name = "dynamic_closure_provider_test", + target_under_test = ":dyn_lib_one", + expect_linkage = "dynamic", + expect_owners = ["class_one"], + ) + + native.test_suite( + name = name, + tests = [ + ":checked_conflict_test", + ":checked_separate_one_test", + ":checked_separate_two_test", + ":checked_shared_conflict_test", + ":checked_shared_path_test", + ":checked_transitive_conflict_test", + ":checked_two_dynamic_test", + ":dynamic_closure_provider_test", + ":raw_unchecked_test", + ":static_closure_provider_test", + ], + ) From 474873d8bce4664f1e5d199a00e543dd22a4d128 Mon Sep 17 00:00:00 2001 From: Genevieve Mendoza Date: Wed, 12 Aug 2026 20:12:37 -0700 Subject: [PATCH 2/4] docs: clarify source-qualified crate identity semantics Align crate_identity docs with reality: - The crate generator emits a source-qualified identity (source, name, version) as 'cargo:' + canonical record, not the non-existent fully_qualified_cargo_package_id helper; the generator merges compatible same-source packages, this feature only carries/validates the identity. - State explicitly that cross-registry same-name is a distinct identity accepted by design (private-registry shadowing), and that the enforced invariant is at most one configured instance per source-qualified identity per link unit. - Make the opt-in nature of native link validation explicit. --- rust/private/providers.bzl | 9 ++++++++- rust/private/rust.bzl | 21 ++++++++++++++++----- rust/rust_link_validation.bzl | 15 ++++++++++++--- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/rust/private/providers.bzl b/rust/private/providers.bzl index 4a0621700d..e6857046f6 100644 --- a/rust/private/providers.bzl +++ b/rust/private/providers.bzl @@ -80,6 +80,13 @@ RustCrateIdentityInfo = provider( "The logical identity of a Rust library together with the specific configured crate " + "instance that implements it.\n\n" + "`logical_id` answers \"does this represent the same logical upstream Rust library?\" " + + "For generated Cargo libraries the logical identity is source-qualified: it encodes the " + + "Cargo source/provenance as well as name and version. Two instances with the same name " + + "and version but different sources (e.g. crates.io vs a private registry) therefore have " + + "different logical identities and are treated as distinct libraries; cross-registry " + + "duplication is accepted by design so a private registry may shadow a common name. The " + + "enforced invariant is per identity: within one native link unit there may be at most " + + "one configured crate instance for each source-qualified logical identity.\n\n" + "The `crate_instance` artifact answers \"is this actually the same compiled instance?\" " + "Equality of `crate_instance` means the same configured compilation output; target " + "configuration, enabled features, cfgs, toolchain, transitions, and recursively selected " + @@ -89,7 +96,7 @@ RustCrateIdentityInfo = provider( "`owner` and `display_name` are diagnostic-only fields and do not participate in equality." ), fields = { - "logical_id": "str: Logical identity of the upstream Rust library whose runtime/type identity should be unique.", + "logical_id": "str: Source-qualified logical identity of the upstream Rust library whose runtime/type identity should be unique per link unit.", "crate_instance": "File: The configured crate output artifact (CrateInfo.output).", "owner": "Label: The label of the target that produced the crate only for diagnostics.", "display_name": "str: Short human-readable description for diagnostics only.", diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index e5682c2231..6d009daff1 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -1107,11 +1107,22 @@ _CRATE_IDENTITY_ATTRS = { native link unit may contain at most one configured crate instance for each non-empty logical identity. - The `cargo:` prefix is reserved for crate generators (crate_universe - sets `crate_identity = "cargo:" + fully_qualified_cargo_package_id` - on generated library targets). Handwritten libraries should use a - reverse-domain or repository-qualified identifier, e.g. - `com.example:mylib`. + The `cargo:` prefix is reserved for generated Cargo library targets. + The crate generator emits a source-qualified identity of the form + `cargo:` + a canonical record encoding the Cargo package's source, + name, and version (e.g. + `cargo:["registry","sparse+https://index.crates.io/","log","0.4.22"]`). + The source is deliberately part of the identity: a Cargo package of + the same name and version pulled from two different registries is two + distinct logical libraries (private-registry shadowing of a common + name is intended), so cross-registry duplication is accepted rather + than flagged. The enforced invariant is per identity: within one + native link unit there may be at most one configured instance of each + source-qualified logical identity. + + Handwritten libraries should use a reverse-domain or + repository-qualified identifier, e.g. `com.example:mylib`, and opt + out (or in) consciously. Leave empty (the default) for ordinary targets that opt out of this facility. diff --git a/rust/rust_link_validation.bzl b/rust/rust_link_validation.bzl index 31604eb675..037eed4e70 100644 --- a/rust/rust_link_validation.bzl +++ b/rust/rust_link_validation.bzl @@ -15,8 +15,10 @@ """Native (C/C++) link validation for Rust crate instances. Rust link rules validate their own target-runtime closure intrinsically (see -`rust/private/rustc.bzl`). A native final link -- e.g. a `cc_binary` linking two -`rust_static_library`s -- is outside that intrinsic check. This module supplies: +`rust/private/rustc.bzl`) whenever a target carries `crate_identity`. A native +final link -- e.g. a `cc_binary` linking two `rust_static_library`s -- is +outside that intrinsic check and is **not** validated by default. This module is +the opt-in affordance for native links: * `rust_link_validation_aspect`: applies the same per-link-unit invariant to native link units reached transitively. Enable it with @@ -24,6 +26,11 @@ Rust link rules validate their own target-runtime closure intrinsically (see * `rust_link_checked_cc_binary` / `rust_link_checked_cc_test` / `rust_link_checked_cc_shared_library`: paved-path wrappers that force the check during analysis without adding linker inputs. + +Because plain `cc_binary`/`cc_test`/`cc_shared_library` are never checked unless +one of these is applied, a C++ link that embeds duplicate Rust instances will +still build silently by default. Use the wrappers or the command-line aspect +wherever a native link may absorb Rust static crates. """ load("@rules_cc//cc:defs.bzl", _cc_binary = "cc_binary", _cc_shared_library = "cc_shared_library", _cc_test = "cc_test") @@ -94,7 +101,9 @@ rust_link_validation_aspect = aspect( "Traverses native dependency edges, accumulates the static Rust library identity " + "closures exposed by Rust targets, and enforces that each logical identity appears " + "with at most one configured crate instance within one native link unit (cc_binary, " + - "cc_test, cc_shared_library)." + "cc_test, cc_shared_library). NOTE: this aspect is opt-in -- it is only applied via " + + "the command line or the `rust_link_checked_*` wrappers. Plain native rules are not " + + "checked by default." ), ) From 5b5a81d4e6b033bc516269a584d3071e26843bfb Mon Sep 17 00:00:00 2001 From: Genevieve Mendoza Date: Wed, 12 Aug 2026 20:42:41 -0700 Subject: [PATCH 3/4] implement rust duplicate crate checks for native deps like CcLibrary --- rust/private/rust.bzl | 3 + rust/private/rust_crate_identity.bzl | 2 +- rust/private/rust_link_validation.bzl | 74 +++++++++++++++ rust/private/rustc.bzl | 17 +++- rust/rust_link_validation.bzl | 91 ++++--------------- .../rust_link_validation_test.bzl | 52 +++++++++++ .../rust_link_validation_native_test.bzl | 13 +++ 7 files changed, 174 insertions(+), 78 deletions(-) create mode 100644 rust/private/rust_link_validation.bzl diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 6d009daff1..786e88449e 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -34,6 +34,7 @@ load( ":rust_allocator_libraries.bzl", "RUSTC_ALLOCATOR_LIBRARIES_ATTRS", ) +load(":rust_link_validation.bzl", "native_rust_link_validation_aspect") load( ":rustc.bzl", "UnstableSelfProfileInfo", @@ -815,6 +816,7 @@ _COMMON_ATTRS = { These must be targets that provide `CrateInfo`, such as `rust_library`. """), + aspects = [native_rust_link_validation_aspect], ), "edition": attr.string( doc = "The rust edition to use for this crate. Defaults to the edition specified in the rust_toolchain.", @@ -826,6 +828,7 @@ _COMMON_ATTRS = { These are typically `cc_library` targets. """), providers = [[CcInfo], [rust_common.crate_info]], + aspects = [native_rust_link_validation_aspect], ), "lint_config": attr.label( doc = "Set of lints to apply when building this crate.", diff --git a/rust/private/rust_crate_identity.bzl b/rust/private/rust_crate_identity.bzl index ded3b63561..54811ea8b2 100644 --- a/rust/private/rust_crate_identity.bzl +++ b/rust/private/rust_crate_identity.bzl @@ -39,7 +39,7 @@ def validate_crate_identity_closure(owner, identities): if instances == None: instances = {} instances_by_id[logical_id] = instances - instances[str(identity.crate_instance)] = identity + instances[identity.crate_instance] = identity conflicting_ids = [ logical_id diff --git a/rust/private/rust_link_validation.bzl b/rust/private/rust_link_validation.bzl new file mode 100644 index 0000000000..dff67e1d66 --- /dev/null +++ b/rust/private/rust_link_validation.bzl @@ -0,0 +1,74 @@ +# Copyright 2026 The Bazel Authors. All rights reserved. +# +# 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. + +"""Rust identity-closure aggregation across native dependency edges.""" + +load(":providers.bzl", "BuildInfo", "CrateInfo", "RustLinkClosureInfo", "TestCrateInfo") +load(":rust_crate_identity.bzl", "validate_crate_identity_closure") + +RustLinkAggregationInfo = provider( + doc = "Aggregates static Rust identity closures along native dependency edges.", + fields = { + "crates": "depset[RustCrateIdentityInfo]: Static Rust identity closure reached so far.", + }, +) + +_NATIVE_DEP_ATTRS = ["deps", "implementation_deps"] +_NATIVE_LINK_UNIT_KINDS = ("cc_binary", "cc_test", "cc_shared_library") + +def _rust_link_validation_aspect_impl(target, ctx): + """Propagates static Rust identity closures across native dependency edges.""" + if RustLinkClosureInfo in target: + closure = target[RustLinkClosureInfo] + if closure.linkage != "static": + return [] + return [RustLinkAggregationInfo(crates = closure.crates)] + + # Rust executables, tests, proc macros, and build scripts are independent + # link/host units. + # Library-producing rules are handled above through RustLinkClosureInfo. + # Do not look through a legacy/custom Rust boundary that lacks that provider. + if BuildInfo in target or CrateInfo in target or TestCrateInfo in target: + return [] + + crates = [] + for attr_name in _NATIVE_DEP_ATTRS: + if not hasattr(ctx.rule.attr, attr_name): + continue + for dep in getattr(ctx.rule.attr, attr_name): + if RustLinkAggregationInfo in dep: + crates.append(dep[RustLinkAggregationInfo].crates) + + if not crates: + return [] + + aggregation = RustLinkAggregationInfo(crates = depset(transitive = crates)) + if ctx.rule.kind in _NATIVE_LINK_UNIT_KINDS: + validate_crate_identity_closure(ctx.label, aggregation.crates.to_list()) + if ctx.rule.kind == "cc_shared_library": + # The shared library is independently linked. Its internal static + # Rust crates do not belong to a consuming link unit. + return [] + + return [aggregation] + +native_rust_link_validation_aspect = aspect( + implementation = _rust_link_validation_aspect_impl, + attr_aspects = _NATIVE_DEP_ATTRS, + doc = ( + "Traverses native dependency edges, accumulates static Rust library identity " + + "closures, validates native terminal link units, and treats shared libraries " + + "as opaque dynamic boundaries." + ), +) diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index b40adf2000..f4c7368579 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -44,6 +44,7 @@ load( _BuildInfo = "BuildInfo", ) load(":rust_crate_identity.bzl", "validate_crate_identity_closure") +load(":rust_link_validation.bzl", "RustLinkAggregationInfo") load(":rustc_resource_set.bzl", "get_rustc_resource_set", "is_codegen_units_enabled") load(":stamp.bzl", "is_stamping_enabled") load( @@ -234,6 +235,17 @@ def _link_closure_identities(crate_info, dep_info): identities.append(identity) return identities +def _native_link_closure_identities(ctx): + """Returns Rust identities recovered through target-runtime native edges.""" + identities = [] + for attr_name in ("deps", "link_deps"): + if not hasattr(ctx.attr, attr_name): + continue + for dep in getattr(ctx.attr, attr_name): + if RustLinkAggregationInfo in dep: + identities.extend(dep[RustLinkAggregationInfo].crates.to_list()) + return identities + def collect_deps( deps, proc_macro_deps, @@ -2699,9 +2711,10 @@ def rustc_compile( # intrinsic link-unit validator below and for the native-boundary provider, # so there is only one approximation of Rust dependency filtering. link_identities = _link_closure_identities(crate_info, dep_info) + link_identities.extend(_native_link_closure_identities(ctx)) - # Enforce the per-link-unit invariant during analysis, before any linker - # action is registered for this link unit. + # Enforce the per-link-unit invariant during analysis. Analysis failure + # prevents the invalid configured target's actions from becoming executable. if _is_terminal_link_unit(crate_info): validate_crate_identity_closure(ctx.label, link_identities) diff --git a/rust/rust_link_validation.bzl b/rust/rust_link_validation.bzl index 037eed4e70..3bbf817fd1 100644 --- a/rust/rust_link_validation.bzl +++ b/rust/rust_link_validation.bzl @@ -14,11 +14,12 @@ """Native (C/C++) link validation for Rust crate instances. -Rust link rules validate their own target-runtime closure intrinsically (see -`rust/private/rustc.bzl`) whenever a target carries `crate_identity`. A native -final link -- e.g. a `cc_binary` linking two `rust_static_library`s -- is -outside that intrinsic check and is **not** validated by default. This module is -the opt-in affordance for native links: +Rust link rules validate their target-runtime closure intrinsically (see +`rust/private/rustc.bzl`). An internal aspect preserves identity metadata when +static Rust libraries reach a Rust target through ordinary `cc_library` `deps` +or `implementation_deps`. A native final link -- e.g. a `cc_binary` linking two +`rust_static_library`s -- is outside that intrinsic check and is **not** +validated by default. This module is the opt-in affordance for native links: * `rust_link_validation_aspect`: applies the same per-link-unit invariant to native link units reached transitively. Enable it with @@ -31,88 +32,28 @@ Because plain `cc_binary`/`cc_test`/`cc_shared_library` are never checked unless one of these is applied, a C++ link that embeds duplicate Rust instances will still build silently by default. Use the wrappers or the command-line aspect wherever a native link may absorb Rust static crates. + +The aspect can follow declared `deps` and `implementation_deps`. It cannot +recover identity after a custom rule discards the dependency graph, or from raw +archives supplied through `srcs`, `linkopts`, linker scripts, or other +untraversed attributes. Prebuilt Rust archives also remain invisible unless a +rule attaches identity metadata to them. """ load("@rules_cc//cc:defs.bzl", _cc_binary = "cc_binary", _cc_shared_library = "cc_shared_library", _cc_test = "cc_test") load("@rules_cc//cc/common:cc_common.bzl", "cc_common") load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") -load("//rust/private:providers.bzl", "RustLinkClosureInfo") load("//rust/private:rust_crate_identity.bzl", "validate_crate_identity_closure") +load("//rust/private:rust_link_validation.bzl", _RustLinkAggregationInfo = "RustLinkAggregationInfo", _native_rust_link_validation_aspect = "native_rust_link_validation_aspect") -# Private aggregation provider returned by the aspect. It is distinct from -# RustLinkClosureInfo because Bazel does not allow an aspect and a target rule -# to return the same provider type, and the aspect needs to carry its result -# along the native shadow graph. -RustLinkAggregationInfo = provider( - doc = "Aggregates static Rust identity closures along native dependency edges.", - fields = { - "crates": "depset[RustCrateIdentityInfo]: Static Rust identity closure reached so far.", - }, -) - -# Native dependency attributes the aspect traverses. Only these have the aspect -# applied via `attr_aspects`, so only these can contribute aggregation results. -_NATIVE_DEP_ATTRS = ["deps", "implementation_deps"] - -# Native link-unit rule kinds where the aggregated closure is finalized and the -# invariant is enforced. -_NATIVE_LINK_UNIT_KINDS = ("cc_binary", "cc_test", "cc_shared_library") - -def _rust_link_validation_aspect_impl(target, ctx): - """Propagates static Rust identity closures across native dep edges. - - A target that exposes RustLinkClosureInfo is a Rust boundary: its closure is - already complete and authoritative, so it is a leaf of this shadow graph. - We never descend into a Rust target's own `deps` -- doing so would pull a - dynamic library's internal crates out of its private link boundary - (observed via cquery traces) and double-count static ones. - """ - if RustLinkClosureInfo in target: - closure = target[RustLinkClosureInfo] - if closure.linkage != "static": - # Dynamic boundary: opaque. Contributes nothing and stops here. - return [] - return [ - RustLinkAggregationInfo(crates = closure.crates), - ] - - crates = [] - for attr_name in _NATIVE_DEP_ATTRS: - if not hasattr(ctx.rule.attr, attr_name): - continue - for dep in getattr(ctx.rule.attr, attr_name): - if RustLinkAggregationInfo in dep: - crates.append(dep[RustLinkAggregationInfo].crates) - - if not crates: - return [] - - aggregation = RustLinkAggregationInfo(crates = depset(transitive = crates)) - - if ctx.rule.kind in _NATIVE_LINK_UNIT_KINDS: - validate_crate_identity_closure(ctx.label, aggregation.crates.to_list()) - - return [aggregation] - -rust_link_validation_aspect = aspect( - implementation = _rust_link_validation_aspect_impl, - attr_aspects = _NATIVE_DEP_ATTRS, - doc = ( - "Traverses native dependency edges, accumulates the static Rust library identity " + - "closures exposed by Rust targets, and enforces that each logical identity appears " + - "with at most one configured crate instance within one native link unit (cc_binary, " + - "cc_test, cc_shared_library). NOTE: this aspect is opt-in -- it is only applied via " + - "the command line or the `rust_link_checked_*` wrappers. Plain native rules are not " + - "checked by default." - ), -) +rust_link_validation_aspect = _native_rust_link_validation_aspect def _rust_link_checker_impl(ctx): """Validates the Rust identity closure of the forwarded native deps.""" crates = [] for dep in ctx.attr.deps: - if RustLinkAggregationInfo in dep: - crates.append(dep[RustLinkAggregationInfo].crates) + if _RustLinkAggregationInfo in dep: + crates.append(dep[_RustLinkAggregationInfo].crates) identities = [] for closure in crates: diff --git a/test/unit/rust_link_validation/rust_link_validation_test.bzl b/test/unit/rust_link_validation/rust_link_validation_test.bzl index 70dedd5d6c..fe2da0d945 100644 --- a/test/unit/rust_link_validation/rust_link_validation_test.bzl +++ b/test/unit/rust_link_validation/rust_link_validation_test.bzl @@ -1,6 +1,7 @@ """Analysis tests for the intrinsic Rust crate-instance link validation.""" load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load("@rules_cc//cc:defs.bzl", "cc_library") load("//rust:defs.bzl", "rust_binary", "rust_common", "rust_library", "rust_proc_macro", "rust_shared_library", "rust_static_library", "rust_test") _IDENTITY = "cargo:registry+https://index.crates.io/#dup@1.0.0" @@ -163,6 +164,41 @@ def rust_link_validation_test_suite(name): target_under_test = ":proc_macro_isolation_bin", ) + # Rust identities remain visible when static Rust libraries travel through + # ordinary native dependency edges on their way to a Rust terminal link. + rust_static_library( + name = "hidden_static_one", + srcs = ["lib.rs"], + deps = [":class_one"], + crate_name = "hidden_static_one", + ) + rust_static_library( + name = "hidden_static_two", + srcs = ["lib.rs"], + deps = [":class_two"], + crate_name = "hidden_static_two", + ) + cc_library( + name = "native_hidden_one", + deps = [":hidden_static_one"], + ) + cc_library( + name = "native_hidden_two", + deps = [":hidden_static_two"], + ) + rust_binary( + name = "native_hidden_same_instance_bin", + srcs = ["main.rs"], + link_deps = [ + ":hidden_static_one", + ":native_hidden_one", + ], + ) + link_clean_test( + name = "native_hidden_same_instance_bin_test", + target_under_test = ":native_hidden_same_instance_bin", + ) + # ---------------- fail: two instances in one link unit ---------------- rust_binary( name = "direct_conflict_bin", @@ -186,6 +222,20 @@ def rust_link_validation_test_suite(name): target_under_test = ":transitive_conflict_bin", ) + rust_binary( + name = "native_hidden_conflict_bin", + srcs = ["main.rs"], + link_deps = [ + ":native_hidden_one", + ":native_hidden_two", + ], + tags = ["manual"], + ) + link_conflict_test( + name = "native_hidden_conflict_bin_test", + target_under_test = ":native_hidden_conflict_bin", + ) + rust_static_library( name = "staticlib_conflict", srcs = ["lib.rs"], @@ -237,6 +287,8 @@ def rust_link_validation_test_suite(name): ":direct_conflict_bin_test", ":different_identities_bin_test", ":identity_record_test", + ":native_hidden_conflict_bin_test", + ":native_hidden_same_instance_bin_test", ":plain_lib_has_no_identity_test", ":proc_macro_conflict_test", ":proc_macro_isolation_bin_test", diff --git a/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl b/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl index 6a79edc1aa..a94e03713f 100644 --- a/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl +++ b/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl @@ -214,6 +214,18 @@ def rust_link_validation_native_test_suite(name): expect_linkage = "static", expect_owners = ["class_one"], ) + rust_library( + name = "native_bridge", + srcs = ["lib.rs"], + link_deps = [":native_mid_one"], + crate_name = "native_bridge", + ) + closure_provider_test( + name = "native_bridge_closure_provider_test", + target_under_test = ":native_bridge", + expect_linkage = "static", + expect_owners = ["class_one"], + ) closure_provider_test( name = "dynamic_closure_provider_test", target_under_test = ":dyn_lib_one", @@ -232,6 +244,7 @@ def rust_link_validation_native_test_suite(name): ":checked_transitive_conflict_test", ":checked_two_dynamic_test", ":dynamic_closure_provider_test", + ":native_bridge_closure_provider_test", ":raw_unchecked_test", ":static_closure_provider_test", ], From 0adc30e4e5c154cf8238a6a319f1118bdec92c7e Mon Sep 17 00:00:00 2001 From: Genevieve Mendoza Date: Wed, 12 Aug 2026 20:55:37 -0700 Subject: [PATCH 4/4] fix: rust link checker did not propagate testonly --- rust/rust_link_validation.bzl | 18 +++++++--- .../rust_link_validation_native_test.bzl | 35 ++++++++++++++++++- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/rust/rust_link_validation.bzl b/rust/rust_link_validation.bzl index 3bbf817fd1..38221b736b 100644 --- a/rust/rust_link_validation.bzl +++ b/rust/rust_link_validation.bzl @@ -85,22 +85,31 @@ _rust_link_checker = rule( def rust_link_checked_cc_binary(name, deps = [], **kwargs): """A `cc_binary` whose Rust identity closure is validated during analysis.""" - _rust_link_checked(_cc_binary, name, deps, **kwargs) + _rust_link_checked(_cc_binary, name, deps, _default_testonly = False, **kwargs) def rust_link_checked_cc_test(name, deps = [], **kwargs): """A `cc_test` whose Rust identity closure is validated during analysis.""" - _rust_link_checked(_cc_test, name, deps, **kwargs) + _rust_link_checked(_cc_test, name, deps, _default_testonly = True, **kwargs) def rust_link_checked_cc_shared_library(name, deps = [], **kwargs): """A `cc_shared_library` whose Rust identity closure is validated during analysis.""" - _rust_link_checked(_cc_shared_library, name, deps, **kwargs) + _rust_link_checked(_cc_shared_library, name, deps, _default_testonly = False, **kwargs) -def _rust_link_checked(cc_rule, name, deps, **kwargs): +def _rust_link_checked(cc_rule, name, deps, _default_testonly, **kwargs): checker = "_" + name + "_crate_link_check" + + # Mirror the wrapped target's testonly on the hidden checker. A checked + # *_test is implicitly a test target (and may wrap test-only Rust deps), so + # the checker must be testonly too, or the non-testonly checker tripping + # testonly validation would break an otherwise valid test. An explicit + # `testonly` passed to the wrapped target overrides the per-wrapper default. + testonly = kwargs.pop("testonly", _default_testonly) + _rust_link_checker( name = checker, deps = deps, tags = ["manual"], + testonly = testonly, ) # Forward the original deps plus the analysis-only checker. The checker @@ -109,5 +118,6 @@ def _rust_link_checked(cc_rule, name, deps, **kwargs): cc_rule( name = name, deps = deps + [":" + checker], + testonly = testonly, **kwargs ) diff --git a/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl b/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl index a94e03713f..331afdab6c 100644 --- a/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl +++ b/test/unit/rust_link_validation_native/rust_link_validation_native_test.bzl @@ -3,7 +3,7 @@ load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library") load("//rust:defs.bzl", "rust_common", "rust_library", "rust_shared_library", "rust_static_library") -load("//rust:rust_link_validation.bzl", "rust_link_checked_cc_binary", "rust_link_checked_cc_shared_library") +load("//rust:rust_link_validation.bzl", "rust_link_checked_cc_binary", "rust_link_checked_cc_shared_library", "rust_link_checked_cc_test") _IDENTITY = "cargo:registry+https://index.crates.io/#dup@1.0.0" @@ -98,6 +98,15 @@ def rust_link_validation_native_test_suite(name): target_under_test = ":raw_unchecked", ) + # A test-only Rust static library for the testonly-propagation checks. + rust_static_library( + name = "testonly_static", + srcs = ["lib.rs"], + deps = [":class_one"], + crate_name = "testonly_static", + testonly = True, + ) + # ---------------- checked conflict cases (fail) ---------------- rust_link_checked_cc_binary( name = "checked_conflict", @@ -233,6 +242,28 @@ def rust_link_validation_native_test_suite(name): expect_owners = ["class_one"], ) + rust_link_checked_cc_test( + name = "checked_testonly_test", + srcs = ["main.cc"], + deps = [":testonly_static"], + ) + native_clean_test( + name = "checked_testonly_test_ok", + target_under_test = ":checked_testonly_test", + ) + + # An explicitly test-only checked binary with a test-only Rust dep too. + rust_link_checked_cc_binary( + name = "checked_testonly_bin", + srcs = ["main.cc"], + deps = [":testonly_static"], + testonly = True, + ) + native_clean_test( + name = "checked_testonly_bin_ok", + target_under_test = ":checked_testonly_bin", + ) + native.test_suite( name = name, tests = [ @@ -241,6 +272,8 @@ def rust_link_validation_native_test_suite(name): ":checked_separate_two_test", ":checked_shared_conflict_test", ":checked_shared_path_test", + ":checked_testonly_bin_ok", + ":checked_testonly_test_ok", ":checked_transitive_conflict_test", ":checked_two_dynamic_test", ":dynamic_closure_provider_test",