From cdc82e252c8c18e038c88847f35883a8d095267f Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Thu, 26 Oct 2023 15:07:09 +0000 Subject: [PATCH 01/11] Patch github.com/bazelbuild/rules_rust/pull/1899 --- .bazelci/presubmit.yml | 81 ++++++++++++ BUILD.bazel | 12 ++ rust/private/rust.bzl | 12 +- rust/private/rustc.bzl | 35 ++++- rust/private/rustdoc_test.bzl | 6 +- rust/private/utils.bzl | 9 ++ rust/toolchain.bzl | 123 +++++++++++++----- test/panic_style/BUILD.bazel | 53 ++++++++ test/panic_style/defs.bzl | 29 +++++ test/panic_style/src/try_catch_panic.rs | 11 ++ test/panic_style/src/verify_abort.rs | 37 ++++++ test/panic_style/src/verify_unwind.rs | 30 +++++ test/panic_style/tests/catch_panic.rs | 6 + test/unit/stdlib/stdlib.bzl | 7 +- .../toolchain_make_variables_test.bzl | 18 +-- 15 files changed, 415 insertions(+), 54 deletions(-) create mode 100644 test/panic_style/BUILD.bazel create mode 100644 test/panic_style/defs.bzl create mode 100644 test/panic_style/src/try_catch_panic.rs create mode 100644 test/panic_style/src/verify_abort.rs create mode 100644 test/panic_style/src/verify_unwind.rs create mode 100644 test/panic_style/tests/catch_panic.rs diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index e95a173c5b..ed6fa57d14 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -570,6 +570,87 @@ tasks: test_flags: - "--@rules_rust//rust/toolchain/channel=nightly" - "--@rules_rust//:no_std=alloc" + cc_common_link_panic_abort_ubuntu2004: + name: Build via cc_common.link and -Cpanic=abort + platform: ubuntu2004 + working_directory: test/cc_common_link + build_targets: + - "--" + - "//..." + # The with_global_alloc directory is a repository on its own tested in the 'Build via cc_common.link using a global allocator' task. + - "-//with_global_alloc/..." + test_targets: + - "--" + - "//..." + # The with_global_alloc directory is a repository on its own tested in the 'Build via cc_common.link using a global allocator' task. + - "-//with_global_alloc/..." + build_flags: + - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" + - "--@rules_rust//:panic_style=abort" + test_flags: + - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" + - "--@rules_rust//:panic_style=abort" + cc_common_link_with_global_alloc_panic_abort_ubuntu2004: + name: Build via cc_common.link using a global allocator and -Cpanic=abort + platform: ubuntu2004 + working_directory: test/cc_common_link/with_global_alloc + build_targets: + - "//..." + test_targets: + - "//..." + build_flags: + - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" + - "--@rules_rust//rust/settings:experimental_use_global_allocator=True" + - "--@rules_rust//:panic_style=abort" + test_flags: + - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" + - "--@rules_rust//rust/settings:experimental_use_global_allocator=True" + - "--@rules_rust//:panic_style=abort" + cc_common_link_no_std_panic_abort_ubuntu2004: + name: Build with no_std + alloc using cc_common.link infrastructure for linking and -Cpanic=abort + platform: ubuntu2004 + working_directory: test/no_std + build_targets: + - "//..." + test_targets: + - "//..." + build_flags: + - "--@rules_rust//rust/toolchain/channel=nightly" + - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" + - "--@rules_rust//rust/settings:experimental_use_global_allocator=True" + - "--@rules_rust//:no_std=alloc" + - "--@rules_rust//:panic_style=abort" + test_flags: + - "--@rules_rust//rust/toolchain/channel=nightly" + - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" + - "--@rules_rust//rust/settings:experimental_use_global_allocator=True" + - "--@rules_rust//:no_std=alloc" + - "--@rules_rust//:panic_style=abort" + no_std_panic_abort_ubuntu2004: + name: Build with no_std + alloc + -Cpanic=abort + platform: ubuntu2004 + working_directory: test/no_std + build_targets: + - "//..." + test_targets: + - "//..." + build_flags: + - "--@rules_rust//rust/toolchain/channel=nightly" + - "--@rules_rust//:no_std=alloc" + - "--@rules_rust//:panic_style=abort" + test_flags: + - "--@rules_rust//rust/toolchain/channel=nightly" + - "--@rules_rust//:no_std=alloc" + - "--@rules_rust//:panic_style=abort" + panic_abort_ubuntu2004: + name: Build and test with -Cpanic=abort + platform: ubuntu2004 + build_targets: *default_linux_targets + test_targets: *default_linux_targets + build_flags: + - "--@rules_rust//:panic_style=abort" + test_flags: + - "--@rules_rust//:panic_style=abort" android_examples_ubuntu2004: name: Android Examples platform: ubuntu2004 diff --git a/BUILD.bazel b/BUILD.bazel index 2eb9e03a25..ceb522c349 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -32,6 +32,18 @@ error_format( visibility = ["//visibility:public"], ) +# This setting may be changed from the command line to panic immediately upon abort. +string_flag( + name = "panic_style", + build_setting_default = "", + values = [ + "abort", + "unwind", + "", + ], + visibility = ["//visibility:public"], +) + # This setting may be used to pass extra options to clippy from the command line. # It applies across all targets. clippy_flags( diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 6f375defb7..ba44160f94 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -28,6 +28,7 @@ load( "determine_output_hash", "expand_dict_value_locations", "find_toolchain", + "force_panic_unwind_transition", "get_edition", "get_import_macro_deps", "transform_deps", @@ -631,6 +632,9 @@ _common_attrs = { "_is_proc_macro_dep_enabled": attr.label( default = Label("//:is_proc_macro_dep_enabled"), ), + "_panic_style": attr.label( + default = Label("//:panic_style"), + ), "_per_crate_rustc_flag": attr.label( default = Label("//:experimental_per_crate_rustc_flag"), ), @@ -890,6 +894,7 @@ _proc_macro_dep_transition = transition( rust_proc_macro = rule( implementation = _rust_proc_macro_impl, provides = _common_providers, + cfg = force_panic_unwind_transition, # Start by copying the common attributes, then override the `deps` attribute # to apply `_proc_macro_dep_transition`. To add this transition we additionally # need to declare `_allowlist_function_transition`, see @@ -1106,7 +1111,11 @@ rust_test = rule( implementation = _rust_test_impl, provides = _common_providers, attrs = dict(_common_attrs.items() + - _rust_test_attrs.items()), + _rust_test_attrs.items() + { + "_allowlist_function_transition": attr.label( + default = "@bazel_tools//tools/allowlists/function_transition_allowlist", + ), + }.items()), executable = True, fragments = ["cpp"], host_fragments = ["cpp"], @@ -1115,6 +1124,7 @@ rust_test = rule( str(Label("//rust:toolchain_type")), "@bazel_tools//tools/cpp:toolchain_type", ], + cfg = force_panic_unwind_transition, incompatible_use_toolchain_transition = True, doc = dedent("""\ Builds a Rust test crate. diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 9ba3115e76..07f553cfad 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -657,7 +657,7 @@ def collect_inputs( force_depend_on_objects (bool, optional): Forces dependencies of this rule to be objects rather than metadata, even for libraries. This is used in rustdoc tests. experimental_use_cc_common_link (bool, optional): Whether rules_rust uses cc_common.link to link - rust binaries. + rust binaries. Returns: tuple: A tuple: A tuple of the following items: @@ -932,6 +932,8 @@ def construct_arguments( rustc_flags.add(error_format, format = "--error-format=%s") + rustc_flags.add("-Cpanic=" + _get_panic_style(ctx, toolchain)) + # Mangle symbols to disambiguate crates with the same name. This could # happen only for non-final artifacts where we compute an output_hash, # e.g., rust_library. @@ -1080,6 +1082,15 @@ def construct_arguments( return args, env +def _get_panic_style(ctx, toolchain): + panic_style = toolchain.default_panic_style + + if hasattr(ctx.attr, "_panic_style"): + flag_value = ctx.attr._panic_style[BuildSettingInfo].value + if flag_value: + panic_style = flag_value + return panic_style + def rustc_compile_action( ctx, attr, @@ -1469,15 +1480,29 @@ def _is_no_std(ctx, toolchain, crate_info): return True def _get_std_and_alloc_info(ctx, toolchain, crate_info): + panic_style = _get_panic_style(ctx, toolchain) + if panic_style != "unwind" and panic_style != "abort": + fail("Unrecognized panic style: " + panic_style) + if is_exec_configuration(ctx): - return toolchain.libstd_and_allocator_ccinfo + if panic_style == "unwind": + return toolchain.libstd_and_allocator_unwind_ccinfo + else: + return toolchain.libstd_and_allocator_abort_ccinfo if toolchain._experimental_use_global_allocator: if _is_no_std(ctx, toolchain, crate_info): - return toolchain.nostd_and_global_allocator_cc_info + if panic_style == "unwind": + return toolchain.nostd_and_global_allocator_unwind_ccinfo + else: + return toolchain.nostd_and_global_allocator_abort_ccinfo + elif panic_style == "unwind": + return toolchain.libstd_and_global_allocator_unwind_ccinfo else: - return toolchain.libstd_and_global_allocator_ccinfo + return toolchain.libstd_and_global_allocator_abort_ccinfo + elif panic_style == "unwind": + return toolchain.libstd_and_allocator_unwind_ccinfo else: - return toolchain.libstd_and_allocator_ccinfo + return toolchain.libstd_and_allocator_abort_ccinfo def _is_dylib(dep): return not bool(dep.static_library or dep.pic_static_library) diff --git a/rust/private/rustdoc_test.bzl b/rust/private/rustdoc_test.bzl index 79b0f4dd84..8a4bdf17e7 100644 --- a/rust/private/rustdoc_test.bzl +++ b/rust/private/rustdoc_test.bzl @@ -17,7 +17,7 @@ load("//rust/private:common.bzl", "rust_common") load("//rust/private:providers.bzl", "CrateInfo") load("//rust/private:rustdoc.bzl", "rustdoc_compile_action") -load("//rust/private:utils.bzl", "dedent", "find_toolchain", "transform_deps") +load("//rust/private:utils.bzl", "dedent", "find_toolchain", "force_panic_unwind_transition", "transform_deps") def _construct_writer_arguments(ctx, test_runner, opt_test_params, action, crate_info): """Construct arguments and environment variables specific to `rustdoc_test_writer`. @@ -207,6 +207,9 @@ rust_doc_test = rule( """), providers = [[CrateInfo], [CcInfo]], ), + "_allowlist_function_transition": attr.label( + default = "@bazel_tools//tools/allowlists/function_transition_allowlist", + ), "_cc_toolchain": attr.label( doc = ( "In order to use find_cc_toolchain, your rule has to depend " + @@ -229,6 +232,7 @@ rust_doc_test = rule( ), }, test = True, + cfg = force_panic_unwind_transition, fragments = ["cpp"], host_fragments = ["cpp"], toolchains = [ diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl index 7b0fe23e9b..b8766acc6b 100644 --- a/rust/private/utils.bzl +++ b/rust/private/utils.bzl @@ -849,3 +849,12 @@ def _symlink_for_non_generated_source(ctx, src_file, package_root): return src_symlink else: return src_file + +def _force_panic_unwind_transition_impl(_settings, _attr): + return {"//:panic_style": "unwind"} + +force_panic_unwind_transition = transition( + implementation = _force_panic_unwind_transition_impl, + inputs = [], + outputs = ["//:panic_style"], +) diff --git a/rust/toolchain.bzl b/rust/toolchain.bzl index 3a59d49d73..b0b4ccb509 100644 --- a/rust/toolchain.bzl +++ b/rust/toolchain.bzl @@ -127,15 +127,15 @@ def _ltl(library, ctx, cc_toolchain, feature_configuration): pic_static_library = library, ) -def _make_libstd_and_allocator_ccinfo(ctx, rust_std, allocator_library, std = "std"): +def _make_libstd_and_allocator_ccinfo(ctx, rust_std, allocator_library, std, panic): """Make the CcInfo (if possible) for libstd and allocator libraries. Args: ctx (ctx): The rule's context object. rust_std: The Rust standard library. allocator_library: The target to use for providing allocator functions. - std: Standard library flavor. Currently only "std" and "no_std_with_alloc" are supported, - accompanied with the default panic behavior. + std: Standard library flavor. Currently only "std" and "no_std_with_alloc" are supported. + panic: Either "unwind" or "abort" to select which panic implementation to include. Returns: @@ -187,39 +187,29 @@ def _make_libstd_and_allocator_ccinfo(ctx, rust_std, allocator_library, std = "s # The libraries panic_abort and panic_unwind are alternatives. # The std by default requires panic_unwind. - # Exclude panic_abort if panic_unwind is present. - # TODO: Provide a setting to choose between panic_abort and panic_unwind. + # Exclude panic_abort if panic_unwind is present, and vice versa. filtered_between_core_and_std_files = rust_stdlib_info.between_core_and_std_files - has_panic_unwind = [ + if panic == "abort": + panic_filter = "panic_unwind" + elif panic == "unwind": + panic_filter = "panic_abort" + else: + fail("Unrecognized panic style: %s" % panic) + filtered_between_core_and_std_files = [ f for f in filtered_between_core_and_std_files - if "panic_unwind" in f.basename + if panic_filter not in f.basename ] - if has_panic_unwind: - filtered_between_core_and_std_files = [ - f - for f in filtered_between_core_and_std_files - if "abort" not in f.basename - ] - core_alloc_and_panic_inputs = depset( - [ - _ltl(f, ctx, cc_toolchain, feature_configuration) - for f in rust_stdlib_info.panic_files - if "unwind" not in f.basename - ], - transitive = [core_inputs], - order = "topological", - ) - else: - core_alloc_and_panic_inputs = depset( - [ - _ltl(f, ctx, cc_toolchain, feature_configuration) - for f in rust_stdlib_info.panic_files - if "unwind" not in f.basename - ], - transitive = [core_inputs], - order = "topological", - ) + core_alloc_and_panic_inputs = depset( + [ + _ltl(f, ctx, cc_toolchain, feature_configuration) + for f in rust_stdlib_info.panic_files + if panic_filter not in f.basename + ], + transitive = [core_inputs], + order = "topological", + ) + memchr_inputs = depset( [ _ltl(f, ctx, cc_toolchain, feature_configuration) @@ -453,6 +443,65 @@ def _generate_sysroot( sysroot_anchor = sysroot_anchor, ) +# Most targets default to unwind, but a few default to abort. Can't find a list in the +# documentation, this list is extracted from rust 1.72.1 via this command: +# for f in $(git grep -l PanicStrategy::Abort compiler/rustc_target/src/spec | fgrep -v mod.rs) +# do grep -E "\<$(basename -s .rs $f)\>" compiler/rustc_target/src/spec/mod.rs +# done +_DEFAULT_ABORT_TRIPLES = [ + "aarch64-nintendo-switch-freestanding", + "aarch64-unknown-none", + "aarch64-unknown-none-softfloat", + "armebv7r-none-eabi", + "armebv7r-none-eabihf", + "armv4t-none-eabi", + "armv7a-none-eabi", + "armv7a-none-eabihf", + "armv7r-none-eabi", + "armv7r-none-eabihf", + "loongarch64-unknown-none", + "loongarch64-unknown-none-softfloat", + "mipsel-sony-psx", + "mipsel-unknown-none", + "msp430-none-elf", + "nvptx64-nvidia-cuda", + "riscv32i-unknown-none-elf", + "riscv32im-unknown-none-elf", + "riscv32imac-esp-espidf", + "riscv32imac-unknown-none-elf", + "riscv32imac-unknown-xous-elf", + "riscv32imc-esp-espidf", + "riscv32imc-unknown-none-elf", + "riscv64gc-unknown-none-elf", + "riscv64imac-unknown-none-elf", + "thumbv4t-none-eabi", + "thumbv7a-pc-windows-msvc", + "thumbv7a-uwp-windows-msvc", + "x86_64-unknown-l4re-uclibc", + "x86_64-unknown-none", + "bpfeb-unknown-none", + "bpfel-unknown-none", + "aarch64-unknown-hermit", + "x86_64-unknown-hermit", + "x86_64-unknown-l4re-uclibc", + "armv5te-none-eabi", + "thumbv4t-none-eabi", + "thumbv5te-none-eabi", + "thumbv6m-none-eabi", + "thumbv7em-none-eabi", + "thumbv7em-none-eabihf", + "thumbv7m-none-eabi", + "thumbv8m.base-none-eabi", + "thumbv8m.main-none-eabi", + "aarch64-unknown-uefi", + "i686-unknown-uefi", + "x86_64-unknown-uefi", + "wasm32-unknown-emscripten", + "wasm32-unknown-unknown", + "wasm32-wasi", + "wasm64-unknown-unknown", +] + def _rust_toolchain_impl(ctx): """The rust_toolchain implementation @@ -609,9 +658,13 @@ def _rust_toolchain_impl(ctx): dylib_ext = ctx.attr.dylib_ext, env = ctx.attr.env, exec_triple = exec_triple, - libstd_and_allocator_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.allocator_library, "std"), - libstd_and_global_allocator_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "std"), - nostd_and_global_allocator_cc_info = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "no_std_with_alloc"), + libstd_and_allocator_unwind_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.allocator_library, "std", "unwind"), + libstd_and_allocator_abort_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.allocator_library, "std", "abort"), + libstd_and_global_allocator_unwind_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "std", "unwind"), + libstd_and_global_allocator_abort_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "std", "abort"), + nostd_and_global_allocator_unwind_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "no_std_with_alloc", "unwind"), + nostd_and_global_allocator_abort_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "no_std_with_alloc", "abort"), + default_panic_style = "abort" if target_triple and target_triple.str in _DEFAULT_ABORT_TRIPLES else "unwind", llvm_cov = ctx.file.llvm_cov, llvm_profdata = ctx.file.llvm_profdata, make_variables = make_variable_info, diff --git a/test/panic_style/BUILD.bazel b/test/panic_style/BUILD.bazel new file mode 100644 index 0000000000..eadad9e52f --- /dev/null +++ b/test/panic_style/BUILD.bazel @@ -0,0 +1,53 @@ +load("//rust:defs.bzl", "rust_binary", "rust_test") +load(":defs.bzl", "with_panic_style_cfg") + +rust_binary( + name = "try_catch_panic", + srcs = ["src/try_catch_panic.rs"], + edition = "2018", +) + +# This should always succeed, independent of the flag, because tests should +# always have -Cpanic=unwind. +rust_test( + name = "test_catch_panic", + srcs = ["tests/catch_panic.rs"], + edition = "2018", +) + +with_panic_style_cfg( + name = "try_catch_panic_with_unwind", + srcs = [":try_catch_panic"], + panic_style = "unwind", +) + +with_panic_style_cfg( + name = "try_catch_panic_with_abort", + srcs = [":try_catch_panic"], + panic_style = "abort", +) + +rust_test( + name = "verify_abort", + srcs = ["src/verify_abort.rs"], + data = [ + ":try_catch_panic_with_abort", + ], + edition = "2018", + deps = [ + "//tools/runfiles", + "@libc", + ], +) + +rust_test( + name = "verify_unwind", + srcs = ["src/verify_unwind.rs"], + data = [ + ":try_catch_panic_with_unwind", + ], + edition = "2018", + deps = [ + "//tools/runfiles", + ], +) diff --git a/test/panic_style/defs.bzl b/test/panic_style/defs.bzl new file mode 100644 index 0000000000..7f6d29fd67 --- /dev/null +++ b/test/panic_style/defs.bzl @@ -0,0 +1,29 @@ +"""Test transitions to test panic_style.""" + +def _panic_style_transition_impl(_settings, attr): + return { + "//:panic_style": attr.panic_style, + } + +_panic_style_transition = transition( + implementation = _panic_style_transition_impl, + inputs = [], + outputs = ["//:panic_style"], +) + +def _with_panic_style_cfg_impl(ctx): + return [DefaultInfo(files = depset(ctx.files.srcs))] + +with_panic_style_cfg = rule( + implementation = _with_panic_style_cfg_impl, + attrs = { + "panic_style": attr.string(mandatory = True), + "srcs": attr.label_list( + allow_files = True, + cfg = _panic_style_transition, + ), + "_allowlist_function_transition": attr.label( + default = Label("//tools/allowlists/function_transition_allowlist"), + ), + }, +) diff --git a/test/panic_style/src/try_catch_panic.rs b/test/panic_style/src/try_catch_panic.rs new file mode 100644 index 0000000000..fc241c2099 --- /dev/null +++ b/test/panic_style/src/try_catch_panic.rs @@ -0,0 +1,11 @@ +//! This binary attempts to catch a panic. Depending on how it is built, this may fail. Exit codes +//! are: +//! 0 if successfully caught +//! 101 if catch_unwind returns Ok somehow +//! Exited with SIGABRT if the panic is not caught + +use std::panic::catch_unwind; + +fn main() { + assert!(catch_unwind(|| panic!("Test panic")).is_err()); +} diff --git a/test/panic_style/src/verify_abort.rs b/test/panic_style/src/verify_abort.rs new file mode 100644 index 0000000000..b4a01cd23d --- /dev/null +++ b/test/panic_style/src/verify_abort.rs @@ -0,0 +1,37 @@ +use std::path::PathBuf; +use std::process::Command; + +use libc::SIGABRT; + +use runfiles::Runfiles; + +#[test] +fn test() { + let r = Runfiles::create().unwrap(); + let try_catch_panic = r.rlocation( + [ + "rules_rust", + "test", + "panic_style", + if cfg!(unix) { + "try_catch_panic" + } else { + "try_catch_panic.exe" + }, + ] + .iter() + .collect::(), + ); + let status = Command::new(try_catch_panic) + .status() + .expect("Failed to spawn test process"); + assert!(!status.success(), "Test process should have failed"); + if cfg!(unix) { + use std::os::unix::process::ExitStatusExt; + if status.signal() != Some(SIGABRT) { + panic!("Test process failed in an unexpected way: {:?}", status); + } + } else { + // The Windows API doesn't let us validate anything else. + } +} diff --git a/test/panic_style/src/verify_unwind.rs b/test/panic_style/src/verify_unwind.rs new file mode 100644 index 0000000000..78e31c5653 --- /dev/null +++ b/test/panic_style/src/verify_unwind.rs @@ -0,0 +1,30 @@ +use std::path::PathBuf; +use std::process::Command; + +use runfiles::Runfiles; + +#[test] +fn test() { + let r = Runfiles::create().unwrap(); + let try_catch_panic = r.rlocation( + [ + "rules_rust", + "test", + "panic_style", + if cfg!(unix) { + "try_catch_panic" + } else { + "try_catch_panic.exe" + }, + ] + .iter() + .collect::(), + ); + let status = Command::new(try_catch_panic) + .status() + .expect("Failed to spawn test process"); + assert!( + status.success(), + "Test process should have exited with success" + ); +} diff --git a/test/panic_style/tests/catch_panic.rs b/test/panic_style/tests/catch_panic.rs new file mode 100644 index 0000000000..75735d99d1 --- /dev/null +++ b/test/panic_style/tests/catch_panic.rs @@ -0,0 +1,6 @@ +use std::panic::catch_unwind; + +#[test] +fn catch_panic() { + assert!(catch_unwind(|| panic!("Test panic")).is_err()); +} diff --git a/test/unit/stdlib/stdlib.bzl b/test/unit/stdlib/stdlib.bzl index 3aa18cc3d1..90d172e9cd 100644 --- a/test/unit/stdlib/stdlib.bzl +++ b/test/unit/stdlib/stdlib.bzl @@ -48,14 +48,13 @@ libstd_ordering_test = analysistest.make(_libstd_ordering_test_impl) def _libstd_panic_test_impl(ctx): # The libraries panic_unwind and panic_abort are alternatives. - # Check that they don't occur together. + # Check that we have one or the other. env = analysistest.begin(ctx) tut = analysistest.target_under_test(env) stdlibs = _stdlibs(tut) has_panic_unwind = [lib for lib in stdlibs if "panic_unwind" in lib.basename] - if has_panic_unwind: - has_panic_abort = [lib for lib in stdlibs if "panic_abort" in lib.basename] - asserts.false(env, has_panic_abort) + has_panic_abort = [lib for lib in stdlibs if "panic_abort" in lib.basename] + asserts.false(env, has_panic_unwind == has_panic_abort) return analysistest.end(env) diff --git a/test/unit/toolchain_make_variables/toolchain_make_variables_test.bzl b/test/unit/toolchain_make_variables/toolchain_make_variables_test.bzl index f6dc8ba163..de6a1e484d 100644 --- a/test/unit/toolchain_make_variables/toolchain_make_variables_test.bzl +++ b/test/unit/toolchain_make_variables/toolchain_make_variables_test.bzl @@ -1,8 +1,8 @@ """Tests for make variables provided by `rust_toolchain`""" -load("@bazel_skylib//lib:unittest.bzl", "analysistest") +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_test") -load("//test/unit:common.bzl", "assert_action_mnemonic", "assert_env_value") +load("//test/unit:common.bzl", "assert_action_mnemonic") _RUST_TOOLCHAIN_ENV = { "ENV_VAR_CARGO": "$(CARGO)", @@ -16,6 +16,13 @@ _RUSTFMT_TOOLCHAIN_ENV = { "ENV_VAR_RUSTFMT": "$(RUSTFMT)", } +def _mangle_path(path): + if not path: + return path + components = path.split("/") + components[1] = "" + return "/".join(components) + def _rust_toolchain_make_variable_expansion_test_common_impl(ctx, mnemonic): env = analysistest.begin(ctx) target = analysistest.target_under_test(env) @@ -45,12 +52,7 @@ def _rust_toolchain_make_variable_expansion_test_common_impl(ctx, mnemonic): } for key in env_vars: - assert_env_value( - env = env, - action = action, - key = key, - value = expected_values[key], - ) + asserts.equals(env, _mangle_path(action.env[key]), _mangle_path(expected_values[key])) return analysistest.end(env) From 9a6959ce106a0bfc03aeb578b6622ed7f8d7dfda Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Thu, 26 Oct 2023 15:10:45 +0000 Subject: [PATCH 02/11] Shorten test names --- test/rust_test_suite/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rust_test_suite/BUILD.bazel b/test/rust_test_suite/BUILD.bazel index df8ed74da5..cd9a7a9644 100644 --- a/test/rust_test_suite/BUILD.bazel +++ b/test/rust_test_suite/BUILD.bazel @@ -7,7 +7,7 @@ rust_library( ) rust_test_suite( - name = "tests_suite", + name = "suite", srcs = glob(["tests/**"]), edition = "2018", deps = [":math_lib"], From 19a727fc2e263ae8293a66e2fb8072ec3c17b6d4 Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Thu, 26 Oct 2023 15:19:25 +0000 Subject: [PATCH 03/11] Uncomment unix os verification --- test/panic_style/src/verify_abort.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/panic_style/src/verify_abort.rs b/test/panic_style/src/verify_abort.rs index b4a01cd23d..407d724df1 100644 --- a/test/panic_style/src/verify_abort.rs +++ b/test/panic_style/src/verify_abort.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use std::process::Command; -use libc::SIGABRT; +// use libc::SIGABRT; use runfiles::Runfiles; @@ -26,12 +26,12 @@ fn test() { .status() .expect("Failed to spawn test process"); assert!(!status.success(), "Test process should have failed"); - if cfg!(unix) { - use std::os::unix::process::ExitStatusExt; - if status.signal() != Some(SIGABRT) { - panic!("Test process failed in an unexpected way: {:?}", status); - } - } else { - // The Windows API doesn't let us validate anything else. - } + // if cfg!(unix) { + // use std::os::unix::process::ExitStatusExt; + // if status.signal() != Some(SIGABRT) { + // panic!("Test process failed in an unexpected way: {:?}", status); + // } + // } else { + // // The Windows API doesn't let us validate anything else. + // } } From 1d399428d4dd064fc9302c97c492e6899284ca6c Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Fri, 27 Oct 2023 13:25:18 +0000 Subject: [PATCH 04/11] Revert "Shorten test names" This reverts commit 9a6959ce106a0bfc03aeb578b6622ed7f8d7dfda. --- test/rust_test_suite/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rust_test_suite/BUILD.bazel b/test/rust_test_suite/BUILD.bazel index cd9a7a9644..df8ed74da5 100644 --- a/test/rust_test_suite/BUILD.bazel +++ b/test/rust_test_suite/BUILD.bazel @@ -7,7 +7,7 @@ rust_library( ) rust_test_suite( - name = "suite", + name = "tests_suite", srcs = glob(["tests/**"]), edition = "2018", deps = [":math_lib"], From b7ee2a4759e1820d373f9a9177bb7ffd444271d5 Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Fri, 27 Oct 2023 13:28:48 +0000 Subject: [PATCH 05/11] Shorten output root for windows build --- .bazelrc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.bazelrc b/.bazelrc index bcfb9f203f..ff551422a0 100644 --- a/.bazelrc +++ b/.bazelrc @@ -12,6 +12,10 @@ common --enable_platform_specific_config startup --windows_enable_symlinks build:windows --enable_runfiles +# Shorten output root to avoid maximum path length limitation +# https://bazel.build/configure/windows#long-path-issues +startup:windows --output_user_root=C:/tmp + # Enable the only currently supported report type # https://bazel.build/reference/command-line-reference#flag--combined_report coverage --combined_report=lcov From 21195f01a433a1d496f4597c6dde3e8b521c6ce1 Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Fri, 27 Oct 2023 13:37:45 +0000 Subject: [PATCH 06/11] Revert "Shorten output root for windows build" This reverts commit b7ee2a4759e1820d373f9a9177bb7ffd444271d5. --- .bazelrc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.bazelrc b/.bazelrc index ff551422a0..bcfb9f203f 100644 --- a/.bazelrc +++ b/.bazelrc @@ -12,10 +12,6 @@ common --enable_platform_specific_config startup --windows_enable_symlinks build:windows --enable_runfiles -# Shorten output root to avoid maximum path length limitation -# https://bazel.build/configure/windows#long-path-issues -startup:windows --output_user_root=C:/tmp - # Enable the only currently supported report type # https://bazel.build/reference/command-line-reference#flag--combined_report coverage --combined_report=lcov From 90d542323409d65a449eaf35638180f0f050c15a Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Fri, 27 Oct 2023 13:37:59 +0000 Subject: [PATCH 07/11] Reapply "Shorten test names" This reverts commit 1d399428d4dd064fc9302c97c492e6899284ca6c. --- test/rust_test_suite/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rust_test_suite/BUILD.bazel b/test/rust_test_suite/BUILD.bazel index df8ed74da5..cd9a7a9644 100644 --- a/test/rust_test_suite/BUILD.bazel +++ b/test/rust_test_suite/BUILD.bazel @@ -7,7 +7,7 @@ rust_library( ) rust_test_suite( - name = "tests_suite", + name = "suite", srcs = glob(["tests/**"]), edition = "2018", deps = [":math_lib"], From 550a115dba28a5f46d1b7e608425b7a98e58988a Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Fri, 27 Oct 2023 15:08:28 +0000 Subject: [PATCH 08/11] Disable proto targets on windows --- .bazelci/presubmit.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index ed6fa57d14..8826fcb942 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -24,8 +24,10 @@ default_macos_targets: &default_macos_targets default_windows_targets: &default_windows_targets - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245 - "//..." - - "-//test/proto/..." - "-//test/unit/pipelined_compilation/..." + # The proto rules do not work on windows + - "-//proto/..." + - "-//test/proto/..." crate_universe_vendor_example_targets: &crate_universe_vendor_example_targets - "//vendor_external:crates_vendor" - "//vendor_local_manifests:crates_vendor" From 7da2ab912fb5eb4ced35ac539300d9cd404fa467 Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Fri, 27 Oct 2023 15:29:51 +0000 Subject: [PATCH 09/11] Revert "Reapply "Shorten test names"" This reverts commit 90d542323409d65a449eaf35638180f0f050c15a. --- test/rust_test_suite/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rust_test_suite/BUILD.bazel b/test/rust_test_suite/BUILD.bazel index cd9a7a9644..df8ed74da5 100644 --- a/test/rust_test_suite/BUILD.bazel +++ b/test/rust_test_suite/BUILD.bazel @@ -7,7 +7,7 @@ rust_library( ) rust_test_suite( - name = "suite", + name = "tests_suite", srcs = glob(["tests/**"]), edition = "2018", deps = [":math_lib"], From c791b63a68b0d82881c8b8133e471455a64fe09c Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Fri, 27 Oct 2023 15:32:49 +0000 Subject: [PATCH 10/11] Reapply "Reapply "Shorten test names"" This reverts commit 7da2ab912fb5eb4ced35ac539300d9cd404fa467. --- test/rust_test_suite/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rust_test_suite/BUILD.bazel b/test/rust_test_suite/BUILD.bazel index df8ed74da5..cd9a7a9644 100644 --- a/test/rust_test_suite/BUILD.bazel +++ b/test/rust_test_suite/BUILD.bazel @@ -7,7 +7,7 @@ rust_library( ) rust_test_suite( - name = "tests_suite", + name = "suite", srcs = glob(["tests/**"]), edition = "2018", deps = [":math_lib"], From 94b8be03d7fcec1e781901a14c17edee742b27b9 Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Fri, 27 Oct 2023 15:34:17 +0000 Subject: [PATCH 11/11] Revert "Patch github.com/bazelbuild/rules_rust/pull/1899" This reverts commit cdc82e252c8c18e038c88847f35883a8d095267f. --- .bazelci/presubmit.yml | 81 ------------ BUILD.bazel | 12 -- rust/private/rust.bzl | 12 +- rust/private/rustc.bzl | 35 +---- rust/private/rustdoc_test.bzl | 6 +- rust/private/utils.bzl | 9 -- rust/toolchain.bzl | 123 +++++------------- test/panic_style/BUILD.bazel | 53 -------- test/panic_style/defs.bzl | 29 ----- test/panic_style/src/try_catch_panic.rs | 11 -- test/panic_style/src/verify_abort.rs | 37 ------ test/panic_style/src/verify_unwind.rs | 30 ----- test/panic_style/tests/catch_panic.rs | 6 - test/unit/stdlib/stdlib.bzl | 7 +- .../toolchain_make_variables_test.bzl | 18 ++- 15 files changed, 54 insertions(+), 415 deletions(-) delete mode 100644 test/panic_style/BUILD.bazel delete mode 100644 test/panic_style/defs.bzl delete mode 100644 test/panic_style/src/try_catch_panic.rs delete mode 100644 test/panic_style/src/verify_abort.rs delete mode 100644 test/panic_style/src/verify_unwind.rs delete mode 100644 test/panic_style/tests/catch_panic.rs diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 8826fcb942..36d603cded 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -572,87 +572,6 @@ tasks: test_flags: - "--@rules_rust//rust/toolchain/channel=nightly" - "--@rules_rust//:no_std=alloc" - cc_common_link_panic_abort_ubuntu2004: - name: Build via cc_common.link and -Cpanic=abort - platform: ubuntu2004 - working_directory: test/cc_common_link - build_targets: - - "--" - - "//..." - # The with_global_alloc directory is a repository on its own tested in the 'Build via cc_common.link using a global allocator' task. - - "-//with_global_alloc/..." - test_targets: - - "--" - - "//..." - # The with_global_alloc directory is a repository on its own tested in the 'Build via cc_common.link using a global allocator' task. - - "-//with_global_alloc/..." - build_flags: - - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" - - "--@rules_rust//:panic_style=abort" - test_flags: - - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" - - "--@rules_rust//:panic_style=abort" - cc_common_link_with_global_alloc_panic_abort_ubuntu2004: - name: Build via cc_common.link using a global allocator and -Cpanic=abort - platform: ubuntu2004 - working_directory: test/cc_common_link/with_global_alloc - build_targets: - - "//..." - test_targets: - - "//..." - build_flags: - - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" - - "--@rules_rust//rust/settings:experimental_use_global_allocator=True" - - "--@rules_rust//:panic_style=abort" - test_flags: - - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" - - "--@rules_rust//rust/settings:experimental_use_global_allocator=True" - - "--@rules_rust//:panic_style=abort" - cc_common_link_no_std_panic_abort_ubuntu2004: - name: Build with no_std + alloc using cc_common.link infrastructure for linking and -Cpanic=abort - platform: ubuntu2004 - working_directory: test/no_std - build_targets: - - "//..." - test_targets: - - "//..." - build_flags: - - "--@rules_rust//rust/toolchain/channel=nightly" - - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" - - "--@rules_rust//rust/settings:experimental_use_global_allocator=True" - - "--@rules_rust//:no_std=alloc" - - "--@rules_rust//:panic_style=abort" - test_flags: - - "--@rules_rust//rust/toolchain/channel=nightly" - - "--@rules_rust//rust/settings:experimental_use_cc_common_link=True" - - "--@rules_rust//rust/settings:experimental_use_global_allocator=True" - - "--@rules_rust//:no_std=alloc" - - "--@rules_rust//:panic_style=abort" - no_std_panic_abort_ubuntu2004: - name: Build with no_std + alloc + -Cpanic=abort - platform: ubuntu2004 - working_directory: test/no_std - build_targets: - - "//..." - test_targets: - - "//..." - build_flags: - - "--@rules_rust//rust/toolchain/channel=nightly" - - "--@rules_rust//:no_std=alloc" - - "--@rules_rust//:panic_style=abort" - test_flags: - - "--@rules_rust//rust/toolchain/channel=nightly" - - "--@rules_rust//:no_std=alloc" - - "--@rules_rust//:panic_style=abort" - panic_abort_ubuntu2004: - name: Build and test with -Cpanic=abort - platform: ubuntu2004 - build_targets: *default_linux_targets - test_targets: *default_linux_targets - build_flags: - - "--@rules_rust//:panic_style=abort" - test_flags: - - "--@rules_rust//:panic_style=abort" android_examples_ubuntu2004: name: Android Examples platform: ubuntu2004 diff --git a/BUILD.bazel b/BUILD.bazel index ceb522c349..2eb9e03a25 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -32,18 +32,6 @@ error_format( visibility = ["//visibility:public"], ) -# This setting may be changed from the command line to panic immediately upon abort. -string_flag( - name = "panic_style", - build_setting_default = "", - values = [ - "abort", - "unwind", - "", - ], - visibility = ["//visibility:public"], -) - # This setting may be used to pass extra options to clippy from the command line. # It applies across all targets. clippy_flags( diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index ba44160f94..6f375defb7 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -28,7 +28,6 @@ load( "determine_output_hash", "expand_dict_value_locations", "find_toolchain", - "force_panic_unwind_transition", "get_edition", "get_import_macro_deps", "transform_deps", @@ -632,9 +631,6 @@ _common_attrs = { "_is_proc_macro_dep_enabled": attr.label( default = Label("//:is_proc_macro_dep_enabled"), ), - "_panic_style": attr.label( - default = Label("//:panic_style"), - ), "_per_crate_rustc_flag": attr.label( default = Label("//:experimental_per_crate_rustc_flag"), ), @@ -894,7 +890,6 @@ _proc_macro_dep_transition = transition( rust_proc_macro = rule( implementation = _rust_proc_macro_impl, provides = _common_providers, - cfg = force_panic_unwind_transition, # Start by copying the common attributes, then override the `deps` attribute # to apply `_proc_macro_dep_transition`. To add this transition we additionally # need to declare `_allowlist_function_transition`, see @@ -1111,11 +1106,7 @@ rust_test = rule( implementation = _rust_test_impl, provides = _common_providers, attrs = dict(_common_attrs.items() + - _rust_test_attrs.items() + { - "_allowlist_function_transition": attr.label( - default = "@bazel_tools//tools/allowlists/function_transition_allowlist", - ), - }.items()), + _rust_test_attrs.items()), executable = True, fragments = ["cpp"], host_fragments = ["cpp"], @@ -1124,7 +1115,6 @@ rust_test = rule( str(Label("//rust:toolchain_type")), "@bazel_tools//tools/cpp:toolchain_type", ], - cfg = force_panic_unwind_transition, incompatible_use_toolchain_transition = True, doc = dedent("""\ Builds a Rust test crate. diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 07f553cfad..9ba3115e76 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -657,7 +657,7 @@ def collect_inputs( force_depend_on_objects (bool, optional): Forces dependencies of this rule to be objects rather than metadata, even for libraries. This is used in rustdoc tests. experimental_use_cc_common_link (bool, optional): Whether rules_rust uses cc_common.link to link - rust binaries. + rust binaries. Returns: tuple: A tuple: A tuple of the following items: @@ -932,8 +932,6 @@ def construct_arguments( rustc_flags.add(error_format, format = "--error-format=%s") - rustc_flags.add("-Cpanic=" + _get_panic_style(ctx, toolchain)) - # Mangle symbols to disambiguate crates with the same name. This could # happen only for non-final artifacts where we compute an output_hash, # e.g., rust_library. @@ -1082,15 +1080,6 @@ def construct_arguments( return args, env -def _get_panic_style(ctx, toolchain): - panic_style = toolchain.default_panic_style - - if hasattr(ctx.attr, "_panic_style"): - flag_value = ctx.attr._panic_style[BuildSettingInfo].value - if flag_value: - panic_style = flag_value - return panic_style - def rustc_compile_action( ctx, attr, @@ -1480,29 +1469,15 @@ def _is_no_std(ctx, toolchain, crate_info): return True def _get_std_and_alloc_info(ctx, toolchain, crate_info): - panic_style = _get_panic_style(ctx, toolchain) - if panic_style != "unwind" and panic_style != "abort": - fail("Unrecognized panic style: " + panic_style) - if is_exec_configuration(ctx): - if panic_style == "unwind": - return toolchain.libstd_and_allocator_unwind_ccinfo - else: - return toolchain.libstd_and_allocator_abort_ccinfo + return toolchain.libstd_and_allocator_ccinfo if toolchain._experimental_use_global_allocator: if _is_no_std(ctx, toolchain, crate_info): - if panic_style == "unwind": - return toolchain.nostd_and_global_allocator_unwind_ccinfo - else: - return toolchain.nostd_and_global_allocator_abort_ccinfo - elif panic_style == "unwind": - return toolchain.libstd_and_global_allocator_unwind_ccinfo + return toolchain.nostd_and_global_allocator_cc_info else: - return toolchain.libstd_and_global_allocator_abort_ccinfo - elif panic_style == "unwind": - return toolchain.libstd_and_allocator_unwind_ccinfo + return toolchain.libstd_and_global_allocator_ccinfo else: - return toolchain.libstd_and_allocator_abort_ccinfo + return toolchain.libstd_and_allocator_ccinfo def _is_dylib(dep): return not bool(dep.static_library or dep.pic_static_library) diff --git a/rust/private/rustdoc_test.bzl b/rust/private/rustdoc_test.bzl index 8a4bdf17e7..79b0f4dd84 100644 --- a/rust/private/rustdoc_test.bzl +++ b/rust/private/rustdoc_test.bzl @@ -17,7 +17,7 @@ load("//rust/private:common.bzl", "rust_common") load("//rust/private:providers.bzl", "CrateInfo") load("//rust/private:rustdoc.bzl", "rustdoc_compile_action") -load("//rust/private:utils.bzl", "dedent", "find_toolchain", "force_panic_unwind_transition", "transform_deps") +load("//rust/private:utils.bzl", "dedent", "find_toolchain", "transform_deps") def _construct_writer_arguments(ctx, test_runner, opt_test_params, action, crate_info): """Construct arguments and environment variables specific to `rustdoc_test_writer`. @@ -207,9 +207,6 @@ rust_doc_test = rule( """), providers = [[CrateInfo], [CcInfo]], ), - "_allowlist_function_transition": attr.label( - default = "@bazel_tools//tools/allowlists/function_transition_allowlist", - ), "_cc_toolchain": attr.label( doc = ( "In order to use find_cc_toolchain, your rule has to depend " + @@ -232,7 +229,6 @@ rust_doc_test = rule( ), }, test = True, - cfg = force_panic_unwind_transition, fragments = ["cpp"], host_fragments = ["cpp"], toolchains = [ diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl index b8766acc6b..7b0fe23e9b 100644 --- a/rust/private/utils.bzl +++ b/rust/private/utils.bzl @@ -849,12 +849,3 @@ def _symlink_for_non_generated_source(ctx, src_file, package_root): return src_symlink else: return src_file - -def _force_panic_unwind_transition_impl(_settings, _attr): - return {"//:panic_style": "unwind"} - -force_panic_unwind_transition = transition( - implementation = _force_panic_unwind_transition_impl, - inputs = [], - outputs = ["//:panic_style"], -) diff --git a/rust/toolchain.bzl b/rust/toolchain.bzl index b0b4ccb509..3a59d49d73 100644 --- a/rust/toolchain.bzl +++ b/rust/toolchain.bzl @@ -127,15 +127,15 @@ def _ltl(library, ctx, cc_toolchain, feature_configuration): pic_static_library = library, ) -def _make_libstd_and_allocator_ccinfo(ctx, rust_std, allocator_library, std, panic): +def _make_libstd_and_allocator_ccinfo(ctx, rust_std, allocator_library, std = "std"): """Make the CcInfo (if possible) for libstd and allocator libraries. Args: ctx (ctx): The rule's context object. rust_std: The Rust standard library. allocator_library: The target to use for providing allocator functions. - std: Standard library flavor. Currently only "std" and "no_std_with_alloc" are supported. - panic: Either "unwind" or "abort" to select which panic implementation to include. + std: Standard library flavor. Currently only "std" and "no_std_with_alloc" are supported, + accompanied with the default panic behavior. Returns: @@ -187,29 +187,39 @@ def _make_libstd_and_allocator_ccinfo(ctx, rust_std, allocator_library, std, pan # The libraries panic_abort and panic_unwind are alternatives. # The std by default requires panic_unwind. - # Exclude panic_abort if panic_unwind is present, and vice versa. + # Exclude panic_abort if panic_unwind is present. + # TODO: Provide a setting to choose between panic_abort and panic_unwind. filtered_between_core_and_std_files = rust_stdlib_info.between_core_and_std_files - if panic == "abort": - panic_filter = "panic_unwind" - elif panic == "unwind": - panic_filter = "panic_abort" - else: - fail("Unrecognized panic style: %s" % panic) - filtered_between_core_and_std_files = [ + has_panic_unwind = [ f for f in filtered_between_core_and_std_files - if panic_filter not in f.basename + if "panic_unwind" in f.basename ] - core_alloc_and_panic_inputs = depset( - [ - _ltl(f, ctx, cc_toolchain, feature_configuration) - for f in rust_stdlib_info.panic_files - if panic_filter not in f.basename - ], - transitive = [core_inputs], - order = "topological", - ) - + if has_panic_unwind: + filtered_between_core_and_std_files = [ + f + for f in filtered_between_core_and_std_files + if "abort" not in f.basename + ] + core_alloc_and_panic_inputs = depset( + [ + _ltl(f, ctx, cc_toolchain, feature_configuration) + for f in rust_stdlib_info.panic_files + if "unwind" not in f.basename + ], + transitive = [core_inputs], + order = "topological", + ) + else: + core_alloc_and_panic_inputs = depset( + [ + _ltl(f, ctx, cc_toolchain, feature_configuration) + for f in rust_stdlib_info.panic_files + if "unwind" not in f.basename + ], + transitive = [core_inputs], + order = "topological", + ) memchr_inputs = depset( [ _ltl(f, ctx, cc_toolchain, feature_configuration) @@ -443,65 +453,6 @@ def _generate_sysroot( sysroot_anchor = sysroot_anchor, ) -# Most targets default to unwind, but a few default to abort. Can't find a list in the -# documentation, this list is extracted from rust 1.72.1 via this command: -# for f in $(git grep -l PanicStrategy::Abort compiler/rustc_target/src/spec | fgrep -v mod.rs) -# do grep -E "\<$(basename -s .rs $f)\>" compiler/rustc_target/src/spec/mod.rs -# done -_DEFAULT_ABORT_TRIPLES = [ - "aarch64-nintendo-switch-freestanding", - "aarch64-unknown-none", - "aarch64-unknown-none-softfloat", - "armebv7r-none-eabi", - "armebv7r-none-eabihf", - "armv4t-none-eabi", - "armv7a-none-eabi", - "armv7a-none-eabihf", - "armv7r-none-eabi", - "armv7r-none-eabihf", - "loongarch64-unknown-none", - "loongarch64-unknown-none-softfloat", - "mipsel-sony-psx", - "mipsel-unknown-none", - "msp430-none-elf", - "nvptx64-nvidia-cuda", - "riscv32i-unknown-none-elf", - "riscv32im-unknown-none-elf", - "riscv32imac-esp-espidf", - "riscv32imac-unknown-none-elf", - "riscv32imac-unknown-xous-elf", - "riscv32imc-esp-espidf", - "riscv32imc-unknown-none-elf", - "riscv64gc-unknown-none-elf", - "riscv64imac-unknown-none-elf", - "thumbv4t-none-eabi", - "thumbv7a-pc-windows-msvc", - "thumbv7a-uwp-windows-msvc", - "x86_64-unknown-l4re-uclibc", - "x86_64-unknown-none", - "bpfeb-unknown-none", - "bpfel-unknown-none", - "aarch64-unknown-hermit", - "x86_64-unknown-hermit", - "x86_64-unknown-l4re-uclibc", - "armv5te-none-eabi", - "thumbv4t-none-eabi", - "thumbv5te-none-eabi", - "thumbv6m-none-eabi", - "thumbv7em-none-eabi", - "thumbv7em-none-eabihf", - "thumbv7m-none-eabi", - "thumbv8m.base-none-eabi", - "thumbv8m.main-none-eabi", - "aarch64-unknown-uefi", - "i686-unknown-uefi", - "x86_64-unknown-uefi", - "wasm32-unknown-emscripten", - "wasm32-unknown-unknown", - "wasm32-wasi", - "wasm64-unknown-unknown", -] - def _rust_toolchain_impl(ctx): """The rust_toolchain implementation @@ -658,13 +609,9 @@ def _rust_toolchain_impl(ctx): dylib_ext = ctx.attr.dylib_ext, env = ctx.attr.env, exec_triple = exec_triple, - libstd_and_allocator_unwind_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.allocator_library, "std", "unwind"), - libstd_and_allocator_abort_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.allocator_library, "std", "abort"), - libstd_and_global_allocator_unwind_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "std", "unwind"), - libstd_and_global_allocator_abort_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "std", "abort"), - nostd_and_global_allocator_unwind_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "no_std_with_alloc", "unwind"), - nostd_and_global_allocator_abort_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "no_std_with_alloc", "abort"), - default_panic_style = "abort" if target_triple and target_triple.str in _DEFAULT_ABORT_TRIPLES else "unwind", + libstd_and_allocator_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.allocator_library, "std"), + libstd_and_global_allocator_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "std"), + nostd_and_global_allocator_cc_info = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.global_allocator_library, "no_std_with_alloc"), llvm_cov = ctx.file.llvm_cov, llvm_profdata = ctx.file.llvm_profdata, make_variables = make_variable_info, diff --git a/test/panic_style/BUILD.bazel b/test/panic_style/BUILD.bazel deleted file mode 100644 index eadad9e52f..0000000000 --- a/test/panic_style/BUILD.bazel +++ /dev/null @@ -1,53 +0,0 @@ -load("//rust:defs.bzl", "rust_binary", "rust_test") -load(":defs.bzl", "with_panic_style_cfg") - -rust_binary( - name = "try_catch_panic", - srcs = ["src/try_catch_panic.rs"], - edition = "2018", -) - -# This should always succeed, independent of the flag, because tests should -# always have -Cpanic=unwind. -rust_test( - name = "test_catch_panic", - srcs = ["tests/catch_panic.rs"], - edition = "2018", -) - -with_panic_style_cfg( - name = "try_catch_panic_with_unwind", - srcs = [":try_catch_panic"], - panic_style = "unwind", -) - -with_panic_style_cfg( - name = "try_catch_panic_with_abort", - srcs = [":try_catch_panic"], - panic_style = "abort", -) - -rust_test( - name = "verify_abort", - srcs = ["src/verify_abort.rs"], - data = [ - ":try_catch_panic_with_abort", - ], - edition = "2018", - deps = [ - "//tools/runfiles", - "@libc", - ], -) - -rust_test( - name = "verify_unwind", - srcs = ["src/verify_unwind.rs"], - data = [ - ":try_catch_panic_with_unwind", - ], - edition = "2018", - deps = [ - "//tools/runfiles", - ], -) diff --git a/test/panic_style/defs.bzl b/test/panic_style/defs.bzl deleted file mode 100644 index 7f6d29fd67..0000000000 --- a/test/panic_style/defs.bzl +++ /dev/null @@ -1,29 +0,0 @@ -"""Test transitions to test panic_style.""" - -def _panic_style_transition_impl(_settings, attr): - return { - "//:panic_style": attr.panic_style, - } - -_panic_style_transition = transition( - implementation = _panic_style_transition_impl, - inputs = [], - outputs = ["//:panic_style"], -) - -def _with_panic_style_cfg_impl(ctx): - return [DefaultInfo(files = depset(ctx.files.srcs))] - -with_panic_style_cfg = rule( - implementation = _with_panic_style_cfg_impl, - attrs = { - "panic_style": attr.string(mandatory = True), - "srcs": attr.label_list( - allow_files = True, - cfg = _panic_style_transition, - ), - "_allowlist_function_transition": attr.label( - default = Label("//tools/allowlists/function_transition_allowlist"), - ), - }, -) diff --git a/test/panic_style/src/try_catch_panic.rs b/test/panic_style/src/try_catch_panic.rs deleted file mode 100644 index fc241c2099..0000000000 --- a/test/panic_style/src/try_catch_panic.rs +++ /dev/null @@ -1,11 +0,0 @@ -//! This binary attempts to catch a panic. Depending on how it is built, this may fail. Exit codes -//! are: -//! 0 if successfully caught -//! 101 if catch_unwind returns Ok somehow -//! Exited with SIGABRT if the panic is not caught - -use std::panic::catch_unwind; - -fn main() { - assert!(catch_unwind(|| panic!("Test panic")).is_err()); -} diff --git a/test/panic_style/src/verify_abort.rs b/test/panic_style/src/verify_abort.rs deleted file mode 100644 index 407d724df1..0000000000 --- a/test/panic_style/src/verify_abort.rs +++ /dev/null @@ -1,37 +0,0 @@ -use std::path::PathBuf; -use std::process::Command; - -// use libc::SIGABRT; - -use runfiles::Runfiles; - -#[test] -fn test() { - let r = Runfiles::create().unwrap(); - let try_catch_panic = r.rlocation( - [ - "rules_rust", - "test", - "panic_style", - if cfg!(unix) { - "try_catch_panic" - } else { - "try_catch_panic.exe" - }, - ] - .iter() - .collect::(), - ); - let status = Command::new(try_catch_panic) - .status() - .expect("Failed to spawn test process"); - assert!(!status.success(), "Test process should have failed"); - // if cfg!(unix) { - // use std::os::unix::process::ExitStatusExt; - // if status.signal() != Some(SIGABRT) { - // panic!("Test process failed in an unexpected way: {:?}", status); - // } - // } else { - // // The Windows API doesn't let us validate anything else. - // } -} diff --git a/test/panic_style/src/verify_unwind.rs b/test/panic_style/src/verify_unwind.rs deleted file mode 100644 index 78e31c5653..0000000000 --- a/test/panic_style/src/verify_unwind.rs +++ /dev/null @@ -1,30 +0,0 @@ -use std::path::PathBuf; -use std::process::Command; - -use runfiles::Runfiles; - -#[test] -fn test() { - let r = Runfiles::create().unwrap(); - let try_catch_panic = r.rlocation( - [ - "rules_rust", - "test", - "panic_style", - if cfg!(unix) { - "try_catch_panic" - } else { - "try_catch_panic.exe" - }, - ] - .iter() - .collect::(), - ); - let status = Command::new(try_catch_panic) - .status() - .expect("Failed to spawn test process"); - assert!( - status.success(), - "Test process should have exited with success" - ); -} diff --git a/test/panic_style/tests/catch_panic.rs b/test/panic_style/tests/catch_panic.rs deleted file mode 100644 index 75735d99d1..0000000000 --- a/test/panic_style/tests/catch_panic.rs +++ /dev/null @@ -1,6 +0,0 @@ -use std::panic::catch_unwind; - -#[test] -fn catch_panic() { - assert!(catch_unwind(|| panic!("Test panic")).is_err()); -} diff --git a/test/unit/stdlib/stdlib.bzl b/test/unit/stdlib/stdlib.bzl index 90d172e9cd..3aa18cc3d1 100644 --- a/test/unit/stdlib/stdlib.bzl +++ b/test/unit/stdlib/stdlib.bzl @@ -48,13 +48,14 @@ libstd_ordering_test = analysistest.make(_libstd_ordering_test_impl) def _libstd_panic_test_impl(ctx): # The libraries panic_unwind and panic_abort are alternatives. - # Check that we have one or the other. + # Check that they don't occur together. env = analysistest.begin(ctx) tut = analysistest.target_under_test(env) stdlibs = _stdlibs(tut) has_panic_unwind = [lib for lib in stdlibs if "panic_unwind" in lib.basename] - has_panic_abort = [lib for lib in stdlibs if "panic_abort" in lib.basename] - asserts.false(env, has_panic_unwind == has_panic_abort) + if has_panic_unwind: + has_panic_abort = [lib for lib in stdlibs if "panic_abort" in lib.basename] + asserts.false(env, has_panic_abort) return analysistest.end(env) diff --git a/test/unit/toolchain_make_variables/toolchain_make_variables_test.bzl b/test/unit/toolchain_make_variables/toolchain_make_variables_test.bzl index de6a1e484d..f6dc8ba163 100644 --- a/test/unit/toolchain_make_variables/toolchain_make_variables_test.bzl +++ b/test/unit/toolchain_make_variables/toolchain_make_variables_test.bzl @@ -1,8 +1,8 @@ """Tests for make variables provided by `rust_toolchain`""" -load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load("@bazel_skylib//lib:unittest.bzl", "analysistest") load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_test") -load("//test/unit:common.bzl", "assert_action_mnemonic") +load("//test/unit:common.bzl", "assert_action_mnemonic", "assert_env_value") _RUST_TOOLCHAIN_ENV = { "ENV_VAR_CARGO": "$(CARGO)", @@ -16,13 +16,6 @@ _RUSTFMT_TOOLCHAIN_ENV = { "ENV_VAR_RUSTFMT": "$(RUSTFMT)", } -def _mangle_path(path): - if not path: - return path - components = path.split("/") - components[1] = "" - return "/".join(components) - def _rust_toolchain_make_variable_expansion_test_common_impl(ctx, mnemonic): env = analysistest.begin(ctx) target = analysistest.target_under_test(env) @@ -52,7 +45,12 @@ def _rust_toolchain_make_variable_expansion_test_common_impl(ctx, mnemonic): } for key in env_vars: - asserts.equals(env, _mangle_path(action.env[key]), _mangle_path(expected_values[key])) + assert_env_value( + env = env, + action = action, + key = key, + value = expected_values[key], + ) return analysistest.end(env)