From f22775b245f39fb0cec9b6279124a95b7fee7d03 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 19 Jun 2024 19:34:28 +0300 Subject: [PATCH 01/19] ignore `llvm::Lld` step if `lld` is not enabled People are having trouble when they don't want to build `lld` for their custom distribution tarballs even with `lld = false` in their config.toml. This is because it is not controlled by `lld_enabled` flag. This change ensures that `llvm:Lld` is controlled by lld configuration. Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/dist.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index 54136d2aebda2..3c555f0a211cf 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -2261,9 +2261,6 @@ impl Step for RustDev { builder.ensure(crate::core::build_steps::llvm::Llvm { target }); - // We want to package `lld` to use it with `download-ci-llvm`. - builder.ensure(crate::core::build_steps::llvm::Lld { target }); - let src_bindir = builder.llvm_out(target).join("bin"); // If updating this, you likely want to change // src/bootstrap/download-ci-llvm-stamp as well, otherwise local users @@ -2280,10 +2277,15 @@ impl Step for RustDev { } } - // We don't build LLD on some platforms, so only add it if it exists - let lld_path = builder.lld_out(target).join("bin").join(exe("lld", target)); - if lld_path.exists() { - tarball.add_file(lld_path, "bin", 0o755); + if builder.config.lld_enabled { + // We want to package `lld` to use it with `download-ci-llvm`. + let lld_out = builder.ensure(crate::core::build_steps::llvm::Lld { target }); + + // We don't build LLD on some platforms, so only add it if it exists + let lld_path = lld_out.join("bin").join(exe("lld", target)); + if lld_path.exists() { + tarball.add_file(lld_path, "bin", 0o755); + } } tarball.add_file(builder.llvm_filecheck(target), "bin", 0o755); From c890a81e2e7e125f9cd96444326a378e4794d9db Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 19 Jun 2024 19:53:28 +0300 Subject: [PATCH 02/19] add `lld = true` to default dist profile Make sure lld is enabled for dist profile unless it is explicitly disabled. Signed-off-by: onur-ozkan --- src/bootstrap/defaults/config.dist.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bootstrap/defaults/config.dist.toml b/src/bootstrap/defaults/config.dist.toml index d06930f2b9d9b..d4feffe02271c 100644 --- a/src/bootstrap/defaults/config.dist.toml +++ b/src/bootstrap/defaults/config.dist.toml @@ -16,6 +16,7 @@ download-ci-llvm = false # Make sure they don't get set when installing from source. channel = "nightly" download-rustc = false +lld = true # Build the llvm-bitcode-linker llvm-bitcode-linker = true From bfca652432100bdce47ab897db38c20a2856c92f Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 19 Jun 2024 22:51:40 +0300 Subject: [PATCH 03/19] disable lld if external llvm is used Signed-off-by: onur-ozkan --- src/ci/run.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ci/run.sh b/src/ci/run.sh index efaf70078c4a9..869f75e923d2c 100755 --- a/src/ci/run.sh +++ b/src/ci/run.sh @@ -85,6 +85,10 @@ fi # space required for CI artifacts. RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --dist-compression-formats=xz" +if [ "$EXTERNAL_LLVM" = "1" ]; then + RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.lld=false" +fi + # Enable the `c` feature for compiler_builtins, but only when the `compiler-rt` source is available # (to avoid spending a lot of time cloning llvm) if [ "$EXTERNAL_LLVM" = "" ]; then From 11acf83159edc94edeeafd75eff458d9d584b822 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Fri, 14 Jun 2024 09:45:02 +0300 Subject: [PATCH 04/19] bootstrap: exclude cargo from package metadata Signed-off-by: onur-ozkan --- src/bootstrap/src/core/metadata.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/bootstrap/src/core/metadata.rs b/src/bootstrap/src/core/metadata.rs index 08a96407a6917..12867cb46b811 100644 --- a/src/bootstrap/src/core/metadata.rs +++ b/src/bootstrap/src/core/metadata.rs @@ -66,9 +66,6 @@ pub fn build(build: &mut Build) { } /// Invokes `cargo metadata` to get package metadata of each workspace member. -/// -/// Note that `src/tools/cargo` is no longer a workspace member but we still -/// treat it as one here, by invoking an additional `cargo metadata` command. fn workspace_members(build: &Build) -> impl Iterator { let collect_metadata = |manifest_path| { let mut cargo = Command::new(&build.initial_cargo); @@ -89,12 +86,8 @@ fn workspace_members(build: &Build) -> impl Iterator { // Collects `metadata.packages` from all workspaces. let packages = collect_metadata("Cargo.toml"); - let cargo_packages = collect_metadata("src/tools/cargo/Cargo.toml"); let ra_packages = collect_metadata("src/tools/rust-analyzer/Cargo.toml"); let bootstrap_packages = collect_metadata("src/bootstrap/Cargo.toml"); - // We only care about the root package from `src/tool/cargo` workspace. - let cargo_package = cargo_packages.into_iter().find(|pkg| pkg.name == "cargo").into_iter(); - - packages.into_iter().chain(cargo_package).chain(ra_packages).chain(bootstrap_packages) + packages.into_iter().chain(ra_packages).chain(bootstrap_packages) } From 457ac5d570f1897ac4c59b3db5f86f26a17d14bb Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Fri, 14 Jun 2024 09:45:59 +0300 Subject: [PATCH 05/19] don't fetch/sync cargo submodule by default Signed-off-by: onur-ozkan --- src/bootstrap/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index dfc30298c28d2..afba907ee92b4 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -469,8 +469,7 @@ impl Build { // Make sure we update these before gathering metadata so we don't get an error about missing // Cargo.toml files. - let rust_submodules = - ["src/tools/cargo", "src/doc/book", "library/backtrace", "library/stdarch"]; + let rust_submodules = ["src/doc/book", "library/backtrace", "library/stdarch"]; for s in rust_submodules { build.update_submodule(Path::new(s)); } From 8c3ebf7a4dced401e58f544f1af187f8c41d3472 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Fri, 14 Jun 2024 09:49:28 +0300 Subject: [PATCH 06/19] refactor `tool_doc` macro in bootstrap Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/doc.rs | 42 ++++++++++------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/doc.rs b/src/bootstrap/src/core/build_steps/doc.rs index 6748625f1323d..87c700dad063d 100644 --- a/src/bootstrap/src/core/build_steps/doc.rs +++ b/src/bootstrap/src/core/build_steps/doc.rs @@ -888,12 +888,11 @@ impl Step for Rustc { macro_rules! tool_doc { ( $tool: ident, - $should_run: literal, $path: literal, $(rustc_tool = $rustc_tool:literal, )? - $(in_tree = $in_tree:literal ,)? $(is_library = $is_library:expr,)? $(crates = $crates:expr)? + $(, submodule $(= $submodule:literal)? )? ) => { #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub struct $tool { @@ -907,7 +906,7 @@ macro_rules! tool_doc { fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { let builder = run.builder; - run.crate_or_deps($should_run).default_condition(builder.config.compiler_docs) + run.path($path).default_condition(builder.config.compiler_docs) } fn make_run(run: RunConfig<'_>) { @@ -921,6 +920,15 @@ macro_rules! tool_doc { /// we do not merge it with the other documentation from std, test and /// proc_macros. This is largely just a wrapper around `cargo doc`. fn run(self, builder: &Builder<'_>) { + let source_type = SourceType::InTree; + $( + let _ = source_type; // silence the "unused variable" warning + let source_type = SourceType::Submodule; + + let path = Path::new(submodule_helper!( $path, submodule $( = $submodule )? )); + builder.update_submodule(&path); + )? + let stage = builder.top_stage; let target = self.target; @@ -941,12 +949,6 @@ macro_rules! tool_doc { builder.ensure(compile::Rustc::new(compiler, target)); } - let source_type = if true $(&& $in_tree)? { - SourceType::InTree - } else { - SourceType::Submodule - }; - // Build cargo command. let mut cargo = prepare_tool_cargo( builder, @@ -1008,21 +1010,14 @@ macro_rules! tool_doc { } } -tool_doc!(Rustdoc, "rustdoc-tool", "src/tools/rustdoc", crates = ["rustdoc", "rustdoc-json-types"]); -tool_doc!( - Rustfmt, - "rustfmt-nightly", - "src/tools/rustfmt", - crates = ["rustfmt-nightly", "rustfmt-config_proc_macro"] -); -tool_doc!(Clippy, "clippy", "src/tools/clippy", crates = ["clippy_config", "clippy_utils"]); -tool_doc!(Miri, "miri", "src/tools/miri", crates = ["miri"]); +tool_doc!(Rustdoc, "src/tools/rustdoc", crates = ["rustdoc", "rustdoc-json-types"]); +tool_doc!(Rustfmt, "src/tools/rustfmt", crates = ["rustfmt-nightly", "rustfmt-config_proc_macro"]); +tool_doc!(Clippy, "src/tools/clippy", crates = ["clippy_config", "clippy_utils"]); +tool_doc!(Miri, "src/tools/miri", crates = ["miri"]); tool_doc!( Cargo, - "cargo", "src/tools/cargo", rustc_tool = false, - in_tree = false, crates = [ "cargo", "cargo-credential", @@ -1034,12 +1029,12 @@ tool_doc!( "crates-io", "mdman", "rustfix", - ] + ], + submodule = "src/tools/cargo" ); -tool_doc!(Tidy, "tidy", "src/tools/tidy", rustc_tool = false, crates = ["tidy"]); +tool_doc!(Tidy, "src/tools/tidy", rustc_tool = false, crates = ["tidy"]); tool_doc!( Bootstrap, - "bootstrap", "src/bootstrap", rustc_tool = false, is_library = true, @@ -1047,7 +1042,6 @@ tool_doc!( ); tool_doc!( RunMakeSupport, - "run_make_support", "src/tools/run-make-support", rustc_tool = false, is_library = true, From 51f6e68559bdef1a1b16b3d026552eab51b5f389 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Fri, 14 Jun 2024 12:32:34 +0300 Subject: [PATCH 07/19] handle cargo submodule in a lazy-load way Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/test.rs | 3 +++ src/bootstrap/src/core/build_steps/tool.rs | 2 ++ src/bootstrap/src/core/build_steps/vendor.rs | 5 ++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index efc09c41bf429..1ef5af7cc2daf 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2983,6 +2983,9 @@ impl Step for Bootstrap { let compiler = builder.compiler(0, host); let _guard = builder.msg(Kind::Test, 0, "bootstrap", host, host); + // Some tests require cargo submodule to be present. + builder.build.update_submodule(Path::new("src/tools/cargo")); + let mut check_bootstrap = Command::new(builder.python()); check_bootstrap .args(["-m", "unittest", "bootstrap_test.py"]) diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 850c8bfe2f8b9..d909a39b60a61 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -654,6 +654,8 @@ impl Step for Cargo { } fn run(self, builder: &Builder<'_>) -> PathBuf { + builder.build.update_submodule(Path::new("src/tools/cargo")); + builder.ensure(ToolBuild { compiler: self.compiler, target: self.target, diff --git a/src/bootstrap/src/core/build_steps/vendor.rs b/src/bootstrap/src/core/build_steps/vendor.rs index 68f1b1bef3f39..e92ab57619b6a 100644 --- a/src/bootstrap/src/core/build_steps/vendor.rs +++ b/src/bootstrap/src/core/build_steps/vendor.rs @@ -1,5 +1,5 @@ use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Command; #[derive(Debug, Clone, Hash, PartialEq, Eq)] @@ -34,6 +34,9 @@ impl Step for Vendor { cmd.arg("--versioned-dirs"); } + // cargo submodule must be present for `x vendor` to work. + builder.build.update_submodule(Path::new("src/tools/cargo")); + // Sync these paths by default. for p in [ "src/tools/cargo/Cargo.toml", From ac7595fdb1ee2aafecdd99cd8a3e56192639ada6 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Tue, 12 Dec 2023 13:32:43 -0800 Subject: [PATCH 08/19] Support for -Z patchable-function-entry `-Z patchable-function-entry` works like `-fpatchable-function-entry` on clang/gcc. The arguments are total nop count and function offset. See MCP rust-lang/compiler-team#704 --- compiler/rustc_codegen_llvm/src/attributes.rs | 26 +++++++++++++++++ compiler/rustc_interface/src/tests.rs | 1 + .../src/middle/codegen_fn_attrs.rs | 23 +++++++++++++++ compiler/rustc_session/src/config.rs | 29 ++++++++++++++++++- compiler/rustc_session/src/options.rs | 29 +++++++++++++++++++ .../patchable-function-entry.md | 24 +++++++++++++++ tests/codegen/patchable-function-entry.rs | 8 +++++ 7 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md create mode 100644 tests/codegen/patchable-function-entry.rs diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs index 48693895da13e..7cf789ab5d72e 100644 --- a/compiler/rustc_codegen_llvm/src/attributes.rs +++ b/compiler/rustc_codegen_llvm/src/attributes.rs @@ -53,6 +53,31 @@ fn inline_attr<'ll>(cx: &CodegenCx<'ll, '_>, inline: InlineAttr) -> Option<&'ll } } +#[inline] +fn patchable_function_entry_attrs<'ll>( + cx: &CodegenCx<'ll, '_>, +) -> SmallVec<[&'ll Attribute; 2]> { + let mut attrs = SmallVec::new(); + let patchable_spec = cx.tcx.sess.opts.unstable_opts.patchable_function_entry; + let entry = patchable_spec.entry(); + let prefix = patchable_spec.prefix(); + if entry > 0 { + attrs.push(llvm::CreateAttrStringValue( + cx.llcx, + "patchable-function-entry", + &format!("{}", entry), + )); + } + if prefix > 0 { + attrs.push(llvm::CreateAttrStringValue( + cx.llcx, + "patchable-function-prefix", + &format!("{}", prefix), + )); + } + attrs +} + /// Get LLVM sanitize attributes. #[inline] pub fn sanitize_attrs<'ll>( @@ -421,6 +446,7 @@ pub fn from_fn_attrs<'ll, 'tcx>( llvm::set_alignment(llfn, align); } to_add.extend(sanitize_attrs(cx, codegen_fn_attrs.no_sanitize)); + to_add.extend(patchable_function_entry_attrs(cx)); // Always annotate functions with the target-cpu they are compiled for. // Without this, ThinLTO won't inline Rust functions into Clang generated diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 6ffc518097ef0..e4b50e7f5ff26 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -813,6 +813,7 @@ fn test_unstable_options_tracking_hash() { tracked!(packed_bundled_libs, true); tracked!(panic_abort_tests, true); tracked!(panic_in_drop, PanicStrategy::Abort); + tracked!(patchable_function_entry, PatchableFunctionEntry::from_nop_count_and_offset(3, 4)); tracked!(plt, Some(true)); tracked!(polonius, Polonius::Legacy); tracked!(precise_enum_drop_elaboration, false); diff --git a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs index 3fa5054baed1e..0fce26dcbbdbb 100644 --- a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs +++ b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs @@ -47,6 +47,29 @@ pub struct CodegenFnAttrs { pub alignment: Option, } +#[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)] +pub struct PatchableFunctionEntry { + /// Nops to prepend to the function + prefix: u8, + /// Nops after entry, but before body + entry: u8, +} + +impl PatchableFunctionEntry { + pub fn from_config(config: rustc_session::config::PatchableFunctionEntry) -> Self { + Self { prefix: config.prefix(), entry: config.entry() } + } + pub fn from_prefix_and_entry(prefix: u8, entry: u8) -> Self { + Self { prefix, entry } + } + pub fn prefix(&self) -> u8 { + self.prefix + } + pub fn entry(&self) -> u8 { + self.entry + } +} + #[derive(Clone, Copy, PartialEq, Eq, TyEncodable, TyDecodable, HashStable)] pub struct CodegenFnAttrFlags(u32); bitflags::bitflags! { diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 839cc51efcea8..2534152267e14 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2963,7 +2963,7 @@ pub(crate) mod dep_tracking { CrateType, DebugInfo, DebugInfoCompression, ErrorOutputType, FunctionReturn, InliningThreshold, InstrumentCoverage, InstrumentXRay, LinkerPluginLto, LocationDetail, LtoCli, NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes, - Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, + PatchableFunctionEntry, Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, }; use crate::lint; @@ -3071,6 +3071,7 @@ pub(crate) mod dep_tracking { OomStrategy, LanguageIdentifier, NextSolverConfig, + PatchableFunctionEntry, Polonius, InliningThreshold, FunctionReturn, @@ -3248,6 +3249,32 @@ impl DumpMonoStatsFormat { } } +/// `-Z patchable-function-entry` representation - how many nops to put before and after function +/// entry. +#[derive(Clone, Copy, PartialEq, Hash, Debug, Default)] +pub struct PatchableFunctionEntry { + /// Nops before the entry + prefix: u8, + /// Nops after the entry + entry: u8, +} + +impl PatchableFunctionEntry { + pub fn from_nop_count_and_offset(nop_count: u8, offset: u8) -> Option { + if nop_count < offset { + None + } else { + Some(Self { prefix: offset, entry: nop_count - offset }) + } + } + pub fn prefix(&self) -> u8 { + self.prefix + } + pub fn entry(&self) -> u8 { + self.entry + } +} + /// `-Zpolonius` values, enabling the borrow checker polonius analysis, and which version: legacy, /// or future prototype. #[derive(Clone, Copy, PartialEq, Hash, Debug, Default)] diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 9a10adeb6d1a1..f4f8a16662d75 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -379,6 +379,8 @@ mod desc { pub const parse_passes: &str = "a space-separated list of passes, or `all`"; pub const parse_panic_strategy: &str = "either `unwind` or `abort`"; pub const parse_on_broken_pipe: &str = "either `kill`, `error`, or `inherit`"; + pub const parse_patchable_function_entry: &str = + "nop_count,entry_offset or nop_count (defaulting entry_offset=0)"; pub const parse_opt_panic_strategy: &str = parse_panic_strategy; pub const parse_oom_strategy: &str = "either `panic` or `abort`"; pub const parse_relro_level: &str = "one of: `full`, `partial`, or `off`"; @@ -723,6 +725,7 @@ mod parse { true } + pub(crate) fn parse_on_broken_pipe(slot: &mut OnBrokenPipe, v: Option<&str>) -> bool { match v { // OnBrokenPipe::Default can't be explicitly specified @@ -734,6 +737,30 @@ mod parse { true } + pub(crate) fn parse_patchable_function_entry( + slot: &mut PatchableFunctionEntry, + v: Option<&str>, + ) -> bool { + let mut nop_count = 0; + let mut offset = 0; + + if !parse_number(&mut nop_count, v) { + let parts = v.and_then(|v| v.split_once(',')).unzip(); + if !parse_number(&mut nop_count, parts.0) { + return false; + } + if !parse_number(&mut offset, parts.1) { + return false; + } + } + + if let Some(pfe) = PatchableFunctionEntry::from_nop_count_and_offset(nop_count, offset) { + *slot = pfe; + return true; + } + false + } + pub(crate) fn parse_oom_strategy(slot: &mut OomStrategy, v: Option<&str>) -> bool { match v { Some("panic") => *slot = OomStrategy::Panic, @@ -1859,6 +1886,8 @@ options! { "panic strategy for panics in drops"), parse_only: bool = (false, parse_bool, [UNTRACKED], "parse only; do not compile, assemble, or link (default: no)"), + patchable_function_entry: PatchableFunctionEntry = (PatchableFunctionEntry::default(), parse_patchable_function_entry, [TRACKED], + "nop padding at function entry"), plt: Option = (None, parse_opt_bool, [TRACKED], "whether to use the PLT when calling into shared libraries; only has effect for PIC code on systems with ELF binaries diff --git a/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md b/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md new file mode 100644 index 0000000000000..a701b9e37719c --- /dev/null +++ b/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md @@ -0,0 +1,24 @@ +# `patchable-function-entry` + +-------------------- + +The `-Z patchable-function-entry=M,N` or `-Z patchable-function-entry=M` +compiler flag enables nop padding of function entries with M nops, with +an offset for the entry of the function at N nops. In the second form, +N defaults to 0. + +As an illustrative example, `-Z patchable-function-entry=3,2` would produce: + +``` +nop +nop +function_label: +nop +//Actual function code begins here +``` + +This flag is used for hotpatching, especially in the Linux kernel. The flag +arguments are modeled after hte `-fpatchable-function-entry` flag as defined +for both [Clang](https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fpatchable-function-entry) +and [gcc](https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fpatchable-function-entry) +and is intended to provide the same effect. diff --git a/tests/codegen/patchable-function-entry.rs b/tests/codegen/patchable-function-entry.rs new file mode 100644 index 0000000000000..f06739303d2e2 --- /dev/null +++ b/tests/codegen/patchable-function-entry.rs @@ -0,0 +1,8 @@ +// compile-flags: -Z patchable-function-entry=15,10 + +#![crate_type = "lib"] + +#[no_mangle] +pub fn foo() {} +// CHECK: @foo() unnamed_addr #0 +// CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="5"{{.*}}"patchable-function-prefix"="10" {{.*}} } From 9b0ae75ecc485d668232c9c85f0090fb85668312 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Tue, 12 Dec 2023 13:37:04 -0800 Subject: [PATCH 09/19] Support `#[patchable_function_entries]` See [RFC](https://github.com/maurer/rust-rfcs/blob/patchable-function-entry/text/0000-patchable-function-entry.md) (yet to be numbered) TODO before submission: * Needs an RFC * Improve error reporting for malformed attributes --- compiler/rustc_codegen_llvm/src/attributes.rs | 9 ++++--- .../rustc_codegen_ssa/src/codegen_attrs.rs | 26 ++++++++++++++++++- compiler/rustc_feature/src/builtin_attrs.rs | 7 +++++ compiler/rustc_feature/src/unstable.rs | 3 +++ .../src/middle/codegen_fn_attrs.rs | 4 +++ compiler/rustc_span/src/symbol.rs | 3 +++ tests/codegen/patchable-function-entry.rs | 20 ++++++++++++++ .../feature-gate-patchable-function-entry.rs | 3 +++ ...ature-gate-patchable-function-entry.stderr | 12 +++++++++ 9 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 tests/ui/feature-gates/feature-gate-patchable-function-entry.rs create mode 100644 tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs index 7cf789ab5d72e..cd82894af18eb 100644 --- a/compiler/rustc_codegen_llvm/src/attributes.rs +++ b/compiler/rustc_codegen_llvm/src/attributes.rs @@ -2,7 +2,7 @@ use rustc_codegen_ssa::traits::*; use rustc_hir::def_id::DefId; -use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; +use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, PatchableFunctionEntry}; use rustc_middle::ty::{self, TyCtxt}; use rustc_session::config::{FunctionReturn, OptLevel}; use rustc_span::symbol::sym; @@ -56,9 +56,12 @@ fn inline_attr<'ll>(cx: &CodegenCx<'ll, '_>, inline: InlineAttr) -> Option<&'ll #[inline] fn patchable_function_entry_attrs<'ll>( cx: &CodegenCx<'ll, '_>, + attr: Option, ) -> SmallVec<[&'ll Attribute; 2]> { let mut attrs = SmallVec::new(); - let patchable_spec = cx.tcx.sess.opts.unstable_opts.patchable_function_entry; + let patchable_spec = attr.unwrap_or_else(|| { + PatchableFunctionEntry::from_config(cx.tcx.sess.opts.unstable_opts.patchable_function_entry) + }); let entry = patchable_spec.entry(); let prefix = patchable_spec.prefix(); if entry > 0 { @@ -446,7 +449,7 @@ pub fn from_fn_attrs<'ll, 'tcx>( llvm::set_alignment(llfn, align); } to_add.extend(sanitize_attrs(cx, codegen_fn_attrs.no_sanitize)); - to_add.extend(patchable_function_entry_attrs(cx)); + to_add.extend(patchable_function_entry_attrs(cx, codegen_fn_attrs.patchable_function_entry)); // Always annotate functions with the target-cpu they are compiled for. // Without this, ThinLTO won't inline Rust functions into Clang generated diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index fb71cdaa8ff2a..8924ddb2aedd3 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -5,7 +5,9 @@ use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE}; use rustc_hir::{lang_items, weak_lang_items::WEAK_LANG_ITEMS, LangItem}; -use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; +use rustc_middle::middle::codegen_fn_attrs::{ + CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry, +}; use rustc_middle::mir::mono::Linkage; use rustc_middle::query::Providers; use rustc_middle::ty::{self as ty, TyCtxt}; @@ -463,6 +465,28 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { None }; } + sym::patchable_function_entry => { + codegen_fn_attrs.patchable_function_entry = attr.meta_item_list().and_then(|l| { + let mut prefix = 0; + let mut entry = 0; + for item in l { + if let Some((sym, lit)) = item.name_value_literal() { + let val = match lit.kind { + // FIXME emit error if too many nops requested + rustc_ast::LitKind::Int(i, _) => i as u8, + _ => continue, + }; + match sym { + sym::prefix => prefix = val, + sym::entry => entry = val, + // FIXME possibly emit error here? + _ => continue, + } + } + } + Some(PatchableFunctionEntry::from_prefix_and_entry(prefix, entry)) + }) + } _ => {} } } diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index c53bf96513951..b5f9f2c715f4f 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -584,6 +584,13 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ pointee, Normal, template!(Word), ErrorFollowing, EncodeCrossCrate::No, derive_smart_pointer, experimental!(pointee) ), + + // FIXME RFC + // `#[patchable_function_entry(prefix(n), entry(n))]` + gated!( + patchable_function_entry, Normal, template!(List: "prefix(n), entry(n)"), ErrorPreceding, + experimental!(patchable_function_entry) + ), // ========================================================================== // Internal attributes: Stability, deprecation, and unsafe: diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs index 2dfaac8f6e731..796475e766f6b 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -565,6 +565,9 @@ declare_features! ( (unstable, offset_of_slice, "CURRENT_RUSTC_VERSION", Some(126151)), /// Allows using `#[optimize(X)]`. (unstable, optimize_attribute, "1.34.0", Some(54882)), + /// Allows specifying nop padding on functions for dynamic patching. + // FIXME this needs an RFC # + (unstable, patchable_function_entry, "CURRENT_RUSTC_VERSION", Some(9999)), /// Allows postfix match `expr.match { ... }` (unstable, postfix_match, "1.79.0", Some(121618)), /// Allows `use<'a, 'b, A, B>` in `impl Trait + use<...>` for precise capture of generic args. diff --git a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs index 0fce26dcbbdbb..23fe72c537a1c 100644 --- a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs +++ b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs @@ -45,6 +45,9 @@ pub struct CodegenFnAttrs { /// The `#[repr(align(...))]` attribute. Indicates the value of which the function should be /// aligned to. pub alignment: Option, + /// The `#[patchable_function_entry(...)]` attribute. Indicates how many nops should be around + /// the function entry. + pub patchable_function_entry: Option, } #[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)] @@ -147,6 +150,7 @@ impl CodegenFnAttrs { no_sanitize: SanitizerSet::empty(), instruction_set: None, alignment: None, + patchable_function_entry: None, } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 6d4a8c29bc902..ea7fe7e76c412 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -768,6 +768,7 @@ symbols! { enable, encode, end, + entry, enumerate_method, env, env_CFG_RELEASE: env!("CFG_RELEASE"), @@ -1383,6 +1384,7 @@ symbols! { passes, pat, pat_param, + patchable_function_entry, path, pattern_complexity, pattern_parentheses, @@ -1421,6 +1423,7 @@ symbols! { prefetch_read_instruction, prefetch_write_data, prefetch_write_instruction, + prefix, preg, prelude, prelude_import, diff --git a/tests/codegen/patchable-function-entry.rs b/tests/codegen/patchable-function-entry.rs index f06739303d2e2..dc20c0a2c6dad 100644 --- a/tests/codegen/patchable-function-entry.rs +++ b/tests/codegen/patchable-function-entry.rs @@ -1,8 +1,28 @@ +#![feature(patchable_function_entry)] // compile-flags: -Z patchable-function-entry=15,10 #![crate_type = "lib"] +// This should have the default, as set by the compile flags #[no_mangle] pub fn foo() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix(1), entry(2))] +pub fn bar() {} + +// If we override an attribute to 0 or unset, the attribute should go away +#[no_mangle] +#[patchable_function_entry(entry(0))] +pub fn baz() {} + // CHECK: @foo() unnamed_addr #0 +// CHECK: @bar() unnamed_addr #1 +// CHECK: @baz() unnamed_addr #2 + // CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="5"{{.*}}"patchable-function-prefix"="10" {{.*}} } +// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} } +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-entry{{.*}} } +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} } +// CHECK: attributes #2 = { {{.*}} } diff --git a/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs b/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs new file mode 100644 index 0000000000000..0e16e873a1eaa --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs @@ -0,0 +1,3 @@ +#[patchable_function_entry(entry(1), prefix(1))] +//~^ ERROR: the `#[patchable_function_entry]` attribute is an experimental feature +fn main() {} diff --git a/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr b/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr new file mode 100644 index 0000000000000..c4d57d774e35d --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr @@ -0,0 +1,12 @@ +error[E0658]: the `#[patchable_function_entry]` attribute is an experimental feature + --> $DIR/feature-gate-patchable-function-entry.rs:1:1 + | +LL | #[patchable_function_entry(entry(1), prefix(1))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #9999 for more information + = help: add `#![feature(patchable_function_entry)]` to the crate attributes to enable + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0658`. From 7c56398e912ea9e81d8e56b2ca7459b4e62b6636 Mon Sep 17 00:00:00 2001 From: Florian Schmiderer Date: Thu, 2 May 2024 23:19:02 +0200 Subject: [PATCH 10/19] Updated code for changes to RFC, added additional error handling, added tests --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 62 ++++++++++++----- compiler/rustc_feature/src/builtin_attrs.rs | 10 +-- compiler/rustc_feature/src/unstable.rs | 3 +- compiler/rustc_interface/src/tests.rs | 9 ++- compiler/rustc_session/src/config.rs | 14 ++-- compiler/rustc_session/src/options.rs | 18 ++--- compiler/rustc_span/src/symbol.rs | 4 +- .../patchable-function-entry.md | 12 ++-- tests/codegen/patchable-function-entry.rs | 28 -------- .../patchable-function-entry-both-flags.rs | 64 ++++++++++++++++++ .../patchable-function-entry-no-flag.rs | 39 +++++++++++ .../patchable-function-entry-one-flag.rs | 66 +++++++++++++++++++ .../feature-gate-patchable-function-entry.rs | 2 +- ...ature-gate-patchable-function-entry.stderr | 7 +- .../patchable-function-entry-attribute.rs | 17 +++++ .../patchable-function-entry-attribute.stderr | 32 +++++++++ .../patchable-function-entry-flags.rs | 2 + .../patchable-function-entry-flags.stderr | 2 + 18 files changed, 312 insertions(+), 79 deletions(-) delete mode 100644 tests/codegen/patchable-function-entry.rs create mode 100644 tests/codegen/patchable-function-entry/patchable-function-entry-both-flags.rs create mode 100644 tests/codegen/patchable-function-entry/patchable-function-entry-no-flag.rs create mode 100644 tests/codegen/patchable-function-entry/patchable-function-entry-one-flag.rs create mode 100644 tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs create mode 100644 tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr create mode 100644 tests/ui/patchable-function-entry/patchable-function-entry-flags.rs create mode 100644 tests/ui/patchable-function-entry/patchable-function-entry-flags.stderr diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 8924ddb2aedd3..84041811bbbf2 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -1,5 +1,6 @@ use rustc_ast::{ast, attr, MetaItemKind, NestedMetaItem}; use rustc_attr::{list_contains_name, InlineAttr, InstructionSetAttr, OptimizeAttr}; +use rustc_data_structures::packed::Pu128; use rustc_errors::{codes::*, struct_span_code_err}; use rustc_hir as hir; use rustc_hir::def::DefKind; @@ -467,24 +468,55 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { } sym::patchable_function_entry => { codegen_fn_attrs.patchable_function_entry = attr.meta_item_list().and_then(|l| { - let mut prefix = 0; - let mut entry = 0; + let mut prefix = None; + let mut entry = None; for item in l { - if let Some((sym, lit)) = item.name_value_literal() { - let val = match lit.kind { - // FIXME emit error if too many nops requested - rustc_ast::LitKind::Int(i, _) => i as u8, - _ => continue, - }; - match sym { - sym::prefix => prefix = val, - sym::entry => entry = val, - // FIXME possibly emit error here? - _ => continue, + let Some(meta_item) = item.meta_item() else { + tcx.dcx().span_err(item.span(), "Expected name value pair."); + continue; + }; + + let Some(name_value_lit) = meta_item.name_value_literal() else { + tcx.dcx().span_err(item.span(), "Expected name value pair."); + continue; + }; + + let attrib_to_write = match meta_item.name_or_empty() { + sym::prefix_nops => &mut prefix, + sym::entry_nops => &mut entry, + _ => { + tcx.dcx().span_err( + item.span(), + format!( + "Unexpected parameter name. Allowed names: {}, {}", + sym::prefix_nops, + sym::entry_nops + ), + ); + continue; } - } + }; + + let rustc_ast::LitKind::Int(Pu128(val @ 0..=255), _) = name_value_lit.kind + else { + tcx.dcx().span_err( + name_value_lit.span, + "Expected integer value between 0 and 255.", + ); + continue; + }; + + *attrib_to_write = Some(val.try_into().unwrap()); } - Some(PatchableFunctionEntry::from_prefix_and_entry(prefix, entry)) + + if let (None, None) = (prefix, entry) { + tcx.dcx().span_err(attr.span, "Must specify at least one parameter."); + } + + Some(PatchableFunctionEntry::from_prefix_and_entry( + prefix.unwrap_or(0), + entry.unwrap_or(0), + )) }) } _ => {} diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index b5f9f2c715f4f..9371a288f09f7 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -584,12 +584,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ pointee, Normal, template!(Word), ErrorFollowing, EncodeCrossCrate::No, derive_smart_pointer, experimental!(pointee) ), - - // FIXME RFC - // `#[patchable_function_entry(prefix(n), entry(n))]` + + // RFC 3543 + // `#[patchable_function_entry(prefix_nops = m, entry_nops = n)]` gated!( - patchable_function_entry, Normal, template!(List: "prefix(n), entry(n)"), ErrorPreceding, - experimental!(patchable_function_entry) + patchable_function_entry, Normal, template!(List: "prefix_nops = m, entry_nops = n"), ErrorPreceding, + EncodeCrossCrate::Yes, experimental!(patchable_function_entry) ), // ========================================================================== diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs index 796475e766f6b..29c28b4efd2f8 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -566,8 +566,7 @@ declare_features! ( /// Allows using `#[optimize(X)]`. (unstable, optimize_attribute, "1.34.0", Some(54882)), /// Allows specifying nop padding on functions for dynamic patching. - // FIXME this needs an RFC # - (unstable, patchable_function_entry, "CURRENT_RUSTC_VERSION", Some(9999)), + (unstable, patchable_function_entry, "CURRENT_RUSTC_VERSION", Some(123115)), /// Allows postfix match `expr.match { ... }` (unstable, postfix_match, "1.79.0", Some(121618)), /// Allows `use<'a, 'b, A, B>` in `impl Trait + use<...>` for precise capture of generic args. diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index e4b50e7f5ff26..e23d556ad4610 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -8,8 +8,8 @@ use rustc_session::config::{ ErrorOutputType, ExternEntry, ExternLocation, Externs, FunctionReturn, InliningThreshold, Input, InstrumentCoverage, InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli, NextSolverConfig, OomStrategy, Options, OutFileName, OutputType, OutputTypes, PAuthKey, - PacRet, Passes, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath, - SymbolManglingVersion, WasiExecModel, + PacRet, Passes, PatchableFunctionEntry, Polonius, ProcMacroExecutionStrategy, Strip, + SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, }; use rustc_session::lint::Level; use rustc_session::search_paths::SearchPath; @@ -813,7 +813,10 @@ fn test_unstable_options_tracking_hash() { tracked!(packed_bundled_libs, true); tracked!(panic_abort_tests, true); tracked!(panic_in_drop, PanicStrategy::Abort); - tracked!(patchable_function_entry, PatchableFunctionEntry::from_nop_count_and_offset(3, 4)); + tracked!( + patchable_function_entry, + PatchableFunctionEntry::from_total_and_prefix_nops(10, 5).expect("total >= prefix") + ); tracked!(plt, Some(true)); tracked!(polonius, Polonius::Legacy); tracked!(precise_enum_drop_elaboration, false); diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 2534152267e14..e716abedf0733 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2963,8 +2963,9 @@ pub(crate) mod dep_tracking { CrateType, DebugInfo, DebugInfoCompression, ErrorOutputType, FunctionReturn, InliningThreshold, InstrumentCoverage, InstrumentXRay, LinkerPluginLto, LocationDetail, LtoCli, NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes, - PatchableFunctionEntry, Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, - SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, + PatchableFunctionEntry, Polonius, RemapPathScopeComponents, ResolveDocLinks, + SourceFileHashAlgorithm, SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, + WasiExecModel, }; use crate::lint; use crate::utils::NativeLib; @@ -3260,11 +3261,14 @@ pub struct PatchableFunctionEntry { } impl PatchableFunctionEntry { - pub fn from_nop_count_and_offset(nop_count: u8, offset: u8) -> Option { - if nop_count < offset { + pub fn from_total_and_prefix_nops( + total_nops: u8, + prefix_nops: u8, + ) -> Option { + if total_nops < prefix_nops { None } else { - Some(Self { prefix: offset, entry: nop_count - offset }) + Some(Self { prefix: prefix_nops, entry: total_nops - prefix_nops }) } } pub fn prefix(&self) -> u8 { diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index f4f8a16662d75..80f7ca544f3c5 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -379,8 +379,7 @@ mod desc { pub const parse_passes: &str = "a space-separated list of passes, or `all`"; pub const parse_panic_strategy: &str = "either `unwind` or `abort`"; pub const parse_on_broken_pipe: &str = "either `kill`, `error`, or `inherit`"; - pub const parse_patchable_function_entry: &str = - "nop_count,entry_offset or nop_count (defaulting entry_offset=0)"; + pub const parse_patchable_function_entry: &str = "either two comma separated integers (total_nops,prefix_nops), with prefix_nops <= total_nops, or one integer (total_nops)"; pub const parse_opt_panic_strategy: &str = parse_panic_strategy; pub const parse_oom_strategy: &str = "either `panic` or `abort`"; pub const parse_relro_level: &str = "one of: `full`, `partial`, or `off`"; @@ -725,7 +724,6 @@ mod parse { true } - pub(crate) fn parse_on_broken_pipe(slot: &mut OnBrokenPipe, v: Option<&str>) -> bool { match v { // OnBrokenPipe::Default can't be explicitly specified @@ -741,20 +739,22 @@ mod parse { slot: &mut PatchableFunctionEntry, v: Option<&str>, ) -> bool { - let mut nop_count = 0; - let mut offset = 0; + let mut total_nops = 0; + let mut prefix_nops = 0; - if !parse_number(&mut nop_count, v) { + if !parse_number(&mut total_nops, v) { let parts = v.and_then(|v| v.split_once(',')).unzip(); - if !parse_number(&mut nop_count, parts.0) { + if !parse_number(&mut total_nops, parts.0) { return false; } - if !parse_number(&mut offset, parts.1) { + if !parse_number(&mut prefix_nops, parts.1) { return false; } } - if let Some(pfe) = PatchableFunctionEntry::from_nop_count_and_offset(nop_count, offset) { + if let Some(pfe) = + PatchableFunctionEntry::from_total_and_prefix_nops(total_nops, prefix_nops) + { *slot = pfe; return true; } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index ea7fe7e76c412..3b6147c4c0f64 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -768,7 +768,7 @@ symbols! { enable, encode, end, - entry, + entry_nops, enumerate_method, env, env_CFG_RELEASE: env!("CFG_RELEASE"), @@ -1423,7 +1423,7 @@ symbols! { prefetch_read_instruction, prefetch_write_data, prefetch_write_instruction, - prefix, + prefix_nops, preg, prelude, prelude_import, diff --git a/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md b/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md index a701b9e37719c..4a9bf47a29011 100644 --- a/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md +++ b/src/doc/unstable-book/src/compiler-flags/patchable-function-entry.md @@ -2,14 +2,14 @@ -------------------- -The `-Z patchable-function-entry=M,N` or `-Z patchable-function-entry=M` -compiler flag enables nop padding of function entries with M nops, with -an offset for the entry of the function at N nops. In the second form, -N defaults to 0. +The `-Z patchable-function-entry=total_nops,prefix_nops` or `-Z patchable-function-entry=total_nops` +compiler flag enables nop padding of function entries with 'total_nops' nops, with +an offset for the entry of the function at 'prefix_nops' nops. In the second form, +'prefix_nops' defaults to 0. As an illustrative example, `-Z patchable-function-entry=3,2` would produce: -``` +```text nop nop function_label: @@ -18,7 +18,7 @@ nop ``` This flag is used for hotpatching, especially in the Linux kernel. The flag -arguments are modeled after hte `-fpatchable-function-entry` flag as defined +arguments are modeled after the `-fpatchable-function-entry` flag as defined for both [Clang](https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fpatchable-function-entry) and [gcc](https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fpatchable-function-entry) and is intended to provide the same effect. diff --git a/tests/codegen/patchable-function-entry.rs b/tests/codegen/patchable-function-entry.rs deleted file mode 100644 index dc20c0a2c6dad..0000000000000 --- a/tests/codegen/patchable-function-entry.rs +++ /dev/null @@ -1,28 +0,0 @@ -#![feature(patchable_function_entry)] -// compile-flags: -Z patchable-function-entry=15,10 - -#![crate_type = "lib"] - -// This should have the default, as set by the compile flags -#[no_mangle] -pub fn foo() {} - -// The attribute should override the compile flags -#[no_mangle] -#[patchable_function_entry(prefix(1), entry(2))] -pub fn bar() {} - -// If we override an attribute to 0 or unset, the attribute should go away -#[no_mangle] -#[patchable_function_entry(entry(0))] -pub fn baz() {} - -// CHECK: @foo() unnamed_addr #0 -// CHECK: @bar() unnamed_addr #1 -// CHECK: @baz() unnamed_addr #2 - -// CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="5"{{.*}}"patchable-function-prefix"="10" {{.*}} } -// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} } -// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-entry{{.*}} } -// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} } -// CHECK: attributes #2 = { {{.*}} } diff --git a/tests/codegen/patchable-function-entry/patchable-function-entry-both-flags.rs b/tests/codegen/patchable-function-entry/patchable-function-entry-both-flags.rs new file mode 100644 index 0000000000000..72204c78a4906 --- /dev/null +++ b/tests/codegen/patchable-function-entry/patchable-function-entry-both-flags.rs @@ -0,0 +1,64 @@ +//@ compile-flags: -Z patchable-function-entry=15,10 + +#![feature(patchable_function_entry)] +#![crate_type = "lib"] + +// This should have the default, as set by the compile flags +#[no_mangle] +pub fn fun0() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 1, entry_nops = 2)] +pub fn fun1() {} + +// If we override an attribute to 0 or unset, the attribute should go away +#[no_mangle] +#[patchable_function_entry(entry_nops = 0)] +pub fn fun2() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 20, entry_nops = 1)] +pub fn fun3() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 2, entry_nops = 19)] +pub fn fun4() {} + +// The attribute should override patchable-function-entry to 3 and +// patchable-function-prefix to the default of 0, clearing it entirely +#[no_mangle] +#[patchable_function_entry(entry_nops = 3)] +pub fn fun5() {} + +// The attribute should override patchable-function-prefix to 4 +// and patchable-function-entry to the default of 0, clearing it entirely +#[no_mangle] +#[patchable_function_entry(prefix_nops = 4)] +pub fn fun6() {} + +// CHECK: @fun0() unnamed_addr #0 +// CHECK: @fun1() unnamed_addr #1 +// CHECK: @fun2() unnamed_addr #2 +// CHECK: @fun3() unnamed_addr #3 +// CHECK: @fun4() unnamed_addr #4 +// CHECK: @fun5() unnamed_addr #5 +// CHECK: @fun6() unnamed_addr #6 + +// CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="5"{{.*}}"patchable-function-prefix"="10" {{.*}} } +// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} } + +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-entry{{.*}} } +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} } +// CHECK: attributes #2 = { {{.*}} } + +// CHECK: attributes #3 = { {{.*}}"patchable-function-entry"="1"{{.*}}"patchable-function-prefix"="20" {{.*}} } +// CHECK: attributes #4 = { {{.*}}"patchable-function-entry"="19"{{.*}}"patchable-function-prefix"="2" {{.*}} } + +// CHECK: attributes #5 = { {{.*}}"patchable-function-entry"="3"{{.*}} } +// CHECK-NOT: attributes #5 = { {{.*}}patchable-function-prefix{{.*}} } + +// CHECK: attributes #6 = { {{.*}}"patchable-function-prefix"="4"{{.*}} } +// CHECK-NOT: attributes #6 = { {{.*}}patchable-function-entry{{.*}} } diff --git a/tests/codegen/patchable-function-entry/patchable-function-entry-no-flag.rs b/tests/codegen/patchable-function-entry/patchable-function-entry-no-flag.rs new file mode 100644 index 0000000000000..3a7078fe55107 --- /dev/null +++ b/tests/codegen/patchable-function-entry/patchable-function-entry-no-flag.rs @@ -0,0 +1,39 @@ +#![feature(patchable_function_entry)] +#![crate_type = "lib"] + +// No patchable function entry should be set +#[no_mangle] +pub fn fun0() {} + +// The attribute should work even without compiler flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 1, entry_nops = 2)] +pub fn fun1() {} + +// The attribute should work even without compiler flags +// and only set patchable-function-entry to 3. +#[no_mangle] +#[patchable_function_entry(entry_nops = 3)] +pub fn fun2() {} + +// The attribute should work even without compiler flags +// and only set patchable-function-prefix to 4. +#[no_mangle] +#[patchable_function_entry(prefix_nops = 4)] +pub fn fun3() {} + +// CHECK: @fun0() unnamed_addr #0 +// CHECK: @fun1() unnamed_addr #1 +// CHECK: @fun2() unnamed_addr #2 +// CHECK: @fun3() unnamed_addr #3 + +// CHECK-NOT: attributes #0 = { {{.*}}patchable-function-entry{{.*}} } +// CHECK-NOT: attributes #0 = { {{.*}}patchable-function-prefix{{.*}} } + +// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} } + +// CHECK: attributes #2 = { {{.*}}"patchable-function-entry"="3"{{.*}} } +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} } + +// CHECK: attributes #3 = { {{.*}}"patchable-function-prefix"="4"{{.*}} } +// CHECK-NOT: attributes #3 = { {{.*}}patchable-function-entry{{.*}} } diff --git a/tests/codegen/patchable-function-entry/patchable-function-entry-one-flag.rs b/tests/codegen/patchable-function-entry/patchable-function-entry-one-flag.rs new file mode 100644 index 0000000000000..8bdd61e461b81 --- /dev/null +++ b/tests/codegen/patchable-function-entry/patchable-function-entry-one-flag.rs @@ -0,0 +1,66 @@ +//@ compile-flags: -Z patchable-function-entry=15 + +#![feature(patchable_function_entry)] +#![crate_type = "lib"] + +// This should have the default, as set by the compile flags +#[no_mangle] +pub fn fun0() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 1, entry_nops = 2)] +pub fn fun1() {} + +// If we override an attribute to 0 or unset, the attribute should go away +#[no_mangle] +#[patchable_function_entry(entry_nops = 0)] +pub fn fun2() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 20, entry_nops = 1)] +pub fn fun3() {} + +// The attribute should override the compile flags +#[no_mangle] +#[patchable_function_entry(prefix_nops = 2, entry_nops = 19)] +pub fn fun4() {} + +// The attribute should override patchable-function-entry to 3 +// and patchable-function-prefix to the default of 0, clearing it entirely +#[no_mangle] +#[patchable_function_entry(entry_nops = 3)] +pub fn fun5() {} + +// The attribute should override patchable-function-prefix to 4 +// and patchable-function-entry to the default of 0, clearing it entirely +#[no_mangle] +#[patchable_function_entry(prefix_nops = 4)] +pub fn fun6() {} + +// CHECK: @fun0() unnamed_addr #0 +// CHECK: @fun1() unnamed_addr #1 +// CHECK: @fun2() unnamed_addr #2 +// CHECK: @fun3() unnamed_addr #3 +// CHECK: @fun4() unnamed_addr #4 +// CHECK: @fun5() unnamed_addr #5 +// CHECK: @fun6() unnamed_addr #6 + +// CHECK: attributes #0 = { {{.*}}"patchable-function-entry"="15" {{.*}} } +// CHECK-NOT: attributes #0 = { {{.*}}patchable-function-prefix{{.*}} } + +// CHECK: attributes #1 = { {{.*}}"patchable-function-entry"="2"{{.*}}"patchable-function-prefix"="1" {{.*}} } + +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-entry{{.*}} } +// CHECK-NOT: attributes #2 = { {{.*}}patchable-function-prefix{{.*}} } +// CHECK: attributes #2 = { {{.*}} } + +// CHECK: attributes #3 = { {{.*}}"patchable-function-entry"="1"{{.*}}"patchable-function-prefix"="20" {{.*}} } +// CHECK: attributes #4 = { {{.*}}"patchable-function-entry"="19"{{.*}}"patchable-function-prefix"="2" {{.*}} } + +// CHECK: attributes #5 = { {{.*}}"patchable-function-entry"="3"{{.*}} } +// CHECK-NOT: attributes #5 = { {{.*}}patchable-function-prefix{{.*}} } + +// CHECK: attributes #6 = { {{.*}}"patchable-function-prefix"="4"{{.*}} } +// CHECK-NOT: attributes #6 = { {{.*}}patchable-function-entry{{.*}} } diff --git a/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs b/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs index 0e16e873a1eaa..b9642c7bfd4a6 100644 --- a/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs +++ b/tests/ui/feature-gates/feature-gate-patchable-function-entry.rs @@ -1,3 +1,3 @@ -#[patchable_function_entry(entry(1), prefix(1))] +#[patchable_function_entry(prefix_nops = 1, entry_nops = 1)] //~^ ERROR: the `#[patchable_function_entry]` attribute is an experimental feature fn main() {} diff --git a/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr b/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr index c4d57d774e35d..55fcdb4f7291e 100644 --- a/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr +++ b/tests/ui/feature-gates/feature-gate-patchable-function-entry.stderr @@ -1,11 +1,12 @@ error[E0658]: the `#[patchable_function_entry]` attribute is an experimental feature --> $DIR/feature-gate-patchable-function-entry.rs:1:1 | -LL | #[patchable_function_entry(entry(1), prefix(1))] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #[patchable_function_entry(prefix_nops = 1, entry_nops = 1)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: see issue #9999 for more information + = note: see issue #123115 for more information = help: add `#![feature(patchable_function_entry)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error: aborting due to 1 previous error diff --git a/tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs new file mode 100644 index 0000000000000..d7231ef416077 --- /dev/null +++ b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.rs @@ -0,0 +1,17 @@ +#![feature(patchable_function_entry)] +fn main() {} + +#[patchable_function_entry(prefix_nops = 256, entry_nops = 0)]//~error: Expected integer value between 0 and 255. +pub fn too_high_pnops() {} + +#[patchable_function_entry(prefix_nops = "stringvalue", entry_nops = 0)]//~error: Expected integer value between 0 and 255. +pub fn non_int_nop() {} + +#[patchable_function_entry]//~error: malformed `patchable_function_entry` attribute input +pub fn malformed_attribute() {} + +#[patchable_function_entry(prefix_nops = 10, something = 0)]//~error: Unexpected parameter name. Allowed names: prefix_nops, entry_nops +pub fn unexpected_parameter_name() {} + +#[patchable_function_entry()]//~error: Must specify at least one parameter. +pub fn no_parameters_given() {} diff --git a/tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr new file mode 100644 index 0000000000000..a270106925f44 --- /dev/null +++ b/tests/ui/patchable-function-entry/patchable-function-entry-attribute.stderr @@ -0,0 +1,32 @@ +error: malformed `patchable_function_entry` attribute input + --> $DIR/patchable-function-entry-attribute.rs:10:1 + | +LL | #[patchable_function_entry] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[patchable_function_entry(prefix_nops = m, entry_nops = n)]` + +error: Expected integer value between 0 and 255. + --> $DIR/patchable-function-entry-attribute.rs:4:42 + | +LL | #[patchable_function_entry(prefix_nops = 256, entry_nops = 0)] + | ^^^ + +error: Expected integer value between 0 and 255. + --> $DIR/patchable-function-entry-attribute.rs:7:42 + | +LL | #[patchable_function_entry(prefix_nops = "stringvalue", entry_nops = 0)] + | ^^^^^^^^^^^^^ + +error: Unexpected parameter name. Allowed names: prefix_nops, entry_nops + --> $DIR/patchable-function-entry-attribute.rs:13:46 + | +LL | #[patchable_function_entry(prefix_nops = 10, something = 0)] + | ^^^^^^^^^^^^^ + +error: Must specify at least one parameter. + --> $DIR/patchable-function-entry-attribute.rs:16:1 + | +LL | #[patchable_function_entry()] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors + diff --git a/tests/ui/patchable-function-entry/patchable-function-entry-flags.rs b/tests/ui/patchable-function-entry/patchable-function-entry-flags.rs new file mode 100644 index 0000000000000..cb5bc62b6b347 --- /dev/null +++ b/tests/ui/patchable-function-entry/patchable-function-entry-flags.rs @@ -0,0 +1,2 @@ +//@ compile-flags: -Z patchable-function-entry=1,2 +fn main() {} diff --git a/tests/ui/patchable-function-entry/patchable-function-entry-flags.stderr b/tests/ui/patchable-function-entry/patchable-function-entry-flags.stderr new file mode 100644 index 0000000000000..b09af94a61541 --- /dev/null +++ b/tests/ui/patchable-function-entry/patchable-function-entry-flags.stderr @@ -0,0 +1,2 @@ +error: incorrect value `1,2` for unstable option `patchable-function-entry` - either two comma separated integers (total_nops,prefix_nops), with prefix_nops <= total_nops, or one integer (total_nops) was expected + From d5ff4f4f657766ca03d7b96553baae6aca053596 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 25 Jun 2024 22:34:40 +0200 Subject: [PATCH 11/19] Simplify `str::clone_into` Removes an `unsafe` in favor of just using `String` methods. --- library/alloc/src/str.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/str.rs b/library/alloc/src/str.rs index 3e23612d0c13c..3bb808a6c73ab 100644 --- a/library/alloc/src/str.rs +++ b/library/alloc/src/str.rs @@ -206,15 +206,16 @@ impl BorrowMut for String { #[stable(feature = "rust1", since = "1.0.0")] impl ToOwned for str { type Owned = String; + #[inline] fn to_owned(&self) -> String { unsafe { String::from_utf8_unchecked(self.as_bytes().to_owned()) } } + #[inline] fn clone_into(&self, target: &mut String) { - let mut b = mem::take(target).into_bytes(); - self.as_bytes().clone_into(&mut b); - *target = unsafe { String::from_utf8_unchecked(b) } + target.clear(); + target.push_str(self); } } From 2c9556d28aef345102a394ea1256d7713748388a Mon Sep 17 00:00:00 2001 From: joboet Date: Tue, 25 Jun 2024 23:43:19 +0200 Subject: [PATCH 12/19] fix UI test, simplify error message --- library/core/src/fmt/mod.rs | 6 ++++++ tests/ui/fmt/send-sync.stderr | 30 ++++++------------------------ 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index c25bc5a1b13c9..3bcd4be183465 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -459,6 +459,12 @@ impl<'a> Arguments<'a> { } } +// Manually implementing these results in better error messages. +#[stable(feature = "rust1", since = "1.0.0")] +impl !Send for Arguments<'_> {} +#[stable(feature = "rust1", since = "1.0.0")] +impl !Sync for Arguments<'_> {} + #[stable(feature = "rust1", since = "1.0.0")] impl Debug for Arguments<'_> { fn fmt(&self, fmt: &mut Formatter<'_>) -> Result { diff --git a/tests/ui/fmt/send-sync.stderr b/tests/ui/fmt/send-sync.stderr index bebf575d9a71c..dff9b23ba677c 100644 --- a/tests/ui/fmt/send-sync.stderr +++ b/tests/ui/fmt/send-sync.stderr @@ -1,45 +1,27 @@ -error[E0277]: `core::fmt::rt::Opaque` cannot be shared between threads safely +error[E0277]: `Arguments<'_>` cannot be sent between threads safely --> $DIR/send-sync.rs:8:10 | LL | send(format_args!("{:?}", c)); - | ---- ^^^^^^^^^^^^^^^^^^^^^^^ `core::fmt::rt::Opaque` cannot be shared between threads safely + | ---- ^^^^^^^^^^^^^^^^^^^^^^^ `Arguments<'_>` cannot be sent between threads safely | | | required by a bound introduced by this call | - = help: within `[core::fmt::rt::Argument<'_>]`, the trait `Sync` is not implemented for `core::fmt::rt::Opaque`, which is required by `Arguments<'_>: Send` - = note: required because it appears within the type `&core::fmt::rt::Opaque` -note: required because it appears within the type `core::fmt::rt::ArgumentType<'_>` - --> $SRC_DIR/core/src/fmt/rt.rs:LL:COL -note: required because it appears within the type `core::fmt::rt::Argument<'_>` - --> $SRC_DIR/core/src/fmt/rt.rs:LL:COL - = note: required because it appears within the type `[core::fmt::rt::Argument<'_>]` - = note: required for `&[core::fmt::rt::Argument<'_>]` to implement `Send` -note: required because it appears within the type `Arguments<'_>` - --> $SRC_DIR/core/src/fmt/mod.rs:LL:COL + = help: the trait `Send` is not implemented for `Arguments<'_>` note: required by a bound in `send` --> $DIR/send-sync.rs:1:12 | LL | fn send(_: T) {} | ^^^^ required by this bound in `send` -error[E0277]: `core::fmt::rt::Opaque` cannot be shared between threads safely +error[E0277]: `Arguments<'_>` cannot be shared between threads safely --> $DIR/send-sync.rs:9:10 | LL | sync(format_args!("{:?}", c)); - | ---- ^^^^^^^^^^^^^^^^^^^^^^^ `core::fmt::rt::Opaque` cannot be shared between threads safely + | ---- ^^^^^^^^^^^^^^^^^^^^^^^ `Arguments<'_>` cannot be shared between threads safely | | | required by a bound introduced by this call | - = help: within `Arguments<'_>`, the trait `Sync` is not implemented for `core::fmt::rt::Opaque`, which is required by `Arguments<'_>: Sync` - = note: required because it appears within the type `&core::fmt::rt::Opaque` -note: required because it appears within the type `core::fmt::rt::ArgumentType<'_>` - --> $SRC_DIR/core/src/fmt/rt.rs:LL:COL -note: required because it appears within the type `core::fmt::rt::Argument<'_>` - --> $SRC_DIR/core/src/fmt/rt.rs:LL:COL - = note: required because it appears within the type `[core::fmt::rt::Argument<'_>]` - = note: required because it appears within the type `&[core::fmt::rt::Argument<'_>]` -note: required because it appears within the type `Arguments<'_>` - --> $SRC_DIR/core/src/fmt/mod.rs:LL:COL + = help: the trait `Sync` is not implemented for `Arguments<'_>` note: required by a bound in `sync` --> $DIR/send-sync.rs:2:12 | From 23d1cc4b84ad6d966f5a686ac6a93daf0239c455 Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 26 Jun 2024 00:06:16 +0200 Subject: [PATCH 13/19] core: avoid `extern` types in formatting infrastructure --- library/core/src/fmt/rt.rs | 46 +++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/library/core/src/fmt/rt.rs b/library/core/src/fmt/rt.rs index 92626feabf3d7..5836ce59ec1dc 100644 --- a/library/core/src/fmt/rt.rs +++ b/library/core/src/fmt/rt.rs @@ -5,6 +5,7 @@ use super::*; use crate::hint::unreachable_unchecked; +use crate::ptr::NonNull; #[lang = "format_placeholder"] #[derive(Copy, Clone)] @@ -66,7 +67,13 @@ pub(super) enum Flag { #[derive(Copy, Clone)] enum ArgumentType<'a> { - Placeholder { value: &'a Opaque, formatter: fn(&Opaque, &mut Formatter<'_>) -> Result }, + Placeholder { + // INVARIANT: if `formatter` had type `fn(&T, _) -> _` then `value` + // was derived from a `&T` with lifetime `'a`. + value: NonNull<()>, + formatter: unsafe fn(NonNull<()>, &mut Formatter<'_>) -> Result, + _lifetime: PhantomData<&'a ()>, + }, Count(usize), } @@ -90,21 +97,15 @@ pub struct Argument<'a> { impl<'a> Argument<'a> { #[inline(always)] fn new<'b, T>(x: &'b T, f: fn(&T, &mut Formatter<'_>) -> Result) -> Argument<'b> { - // SAFETY: `mem::transmute(x)` is safe because - // 1. `&'b T` keeps the lifetime it originated with `'b` - // (so as to not have an unbounded lifetime) - // 2. `&'b T` and `&'b Opaque` have the same memory layout - // (when `T` is `Sized`, as it is here) - // `mem::transmute(f)` is safe since `fn(&T, &mut Formatter<'_>) -> Result` - // and `fn(&Opaque, &mut Formatter<'_>) -> Result` have the same ABI - // (as long as `T` is `Sized`) - unsafe { - Argument { - ty: ArgumentType::Placeholder { - formatter: mem::transmute(f), - value: mem::transmute(x), - }, - } + Argument { + // INVARIANT: this creates an `ArgumentType<'b>` from a `&'b T` and + // a `fn(&T, ...)`, so the invariant is maintained. + ty: ArgumentType::Placeholder { + value: NonNull::from(x).cast(), + // SAFETY: function pointers always have the same layout. + formatter: unsafe { mem::transmute(f) }, + _lifetime: PhantomData, + }, } } @@ -162,7 +163,14 @@ impl<'a> Argument<'a> { #[inline(always)] pub(super) unsafe fn fmt(&self, f: &mut Formatter<'_>) -> Result { match self.ty { - ArgumentType::Placeholder { formatter, value } => formatter(value, f), + // SAFETY: + // Because of the invariant that if `formatter` had the type + // `fn(&T, _) -> _` then `value` has type `&'b T` where `'b` is + // the lifetime of the `ArgumentType`, and because references + // and `NonNull` are ABI-compatible, this is completely equivalent + // to calling the original function passed to `new` with the + // original reference, which is sound. + ArgumentType::Placeholder { formatter, value, .. } => unsafe { formatter(value, f) }, // SAFETY: the caller promised this. ArgumentType::Count(_) => unsafe { unreachable_unchecked() }, } @@ -208,7 +216,3 @@ impl UnsafeArg { Self { _private: () } } } - -extern "C" { - type Opaque; -} From 7526416ba6bcb3e0b51bfcdc93544a9552f9568e Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 26 Jun 2024 00:06:27 +0200 Subject: [PATCH 14/19] update coverage test --- tests/coverage/issue-83601.cov-map | 6 +++--- tests/coverage/issue-84561.cov-map | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/coverage/issue-83601.cov-map b/tests/coverage/issue-83601.cov-map index ddb4407881aad..f2447e3c92c8a 100644 --- a/tests/coverage/issue-83601.cov-map +++ b/tests/coverage/issue-83601.cov-map @@ -1,12 +1,12 @@ Function name: issue_83601::main -Raw bytes (21): 0x[01, 01, 01, 05, 00, 03, 01, 06, 01, 02, 1c, 05, 03, 09, 01, 1c, 02, 02, 05, 03, 02] +Raw bytes (21): 0x[01, 01, 01, 05, 09, 03, 01, 06, 01, 02, 1c, 05, 03, 09, 01, 1c, 02, 02, 05, 03, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 1 -- expression 0 operands: lhs = Counter(1), rhs = Zero +- expression 0 operands: lhs = Counter(1), rhs = Counter(2) Number of file 0 mappings: 3 - Code(Counter(0)) at (prev + 6, 1) to (start + 2, 28) - Code(Counter(1)) at (prev + 3, 9) to (start + 1, 28) - Code(Expression(0, Sub)) at (prev + 2, 5) to (start + 3, 2) - = (c1 - Zero) + = (c1 - c2) diff --git a/tests/coverage/issue-84561.cov-map b/tests/coverage/issue-84561.cov-map index c4087d9369d87..d3c4671e0ba4c 100644 --- a/tests/coverage/issue-84561.cov-map +++ b/tests/coverage/issue-84561.cov-map @@ -54,15 +54,15 @@ Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 167, 9) to (start + 2, 10) Function name: issue_84561::test3 -Raw bytes (375): 0x[01, 01, 31, 05, 00, 0d, 00, 15, 00, 12, 00, 15, 00, 21, 00, 1e, 00, 21, 00, 31, 00, 3d, 00, 2e, 45, 3d, 00, 42, 49, 45, 00, 3f, 51, 42, 49, 45, 00, 7a, 55, 51, 00, 7a, 55, 51, 00, 77, 5d, 7a, 55, 51, 00, 77, 61, 7a, 55, 51, 00, 72, 65, 77, 61, 7a, 55, 51, 00, 75, be, 01, c2, 01, 79, 69, 6d, 69, 6d, 69, 6d, c2, 01, 00, 69, 6d, c2, 01, 79, 69, 6d, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, b6, 01, 00, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, 33, 01, 08, 01, 03, 1c, 05, 04, 09, 01, 1c, 02, 02, 05, 04, 1f, 0d, 05, 05, 00, 1f, 06, 01, 05, 00, 1f, 15, 01, 09, 01, 1c, 12, 02, 05, 00, 1f, 0e, 01, 05, 00, 0f, 00, 00, 20, 00, 30, 21, 01, 05, 03, 0f, 00, 03, 20, 00, 30, 00, 00, 33, 00, 41, 00, 00, 4b, 00, 5a, 1e, 01, 05, 00, 0f, 00, 05, 09, 03, 10, 00, 05, 0d, 00, 1b, 00, 02, 0d, 00, 1c, 1a, 04, 09, 05, 06, 31, 06, 05, 03, 06, 22, 04, 05, 03, 06, 3d, 04, 09, 04, 06, 2e, 05, 08, 00, 0f, 45, 01, 09, 03, 0a, 2a, 05, 09, 03, 0a, 3f, 05, 08, 00, 0f, 51, 01, 09, 00, 13, 00, 03, 0d, 00, 1d, 3a, 03, 09, 00, 13, 00, 03, 0d, 00, 1d, 77, 03, 05, 00, 0f, 77, 01, 0c, 00, 13, 5d, 01, 0d, 00, 13, 56, 02, 0d, 00, 13, 72, 04, 05, 02, 13, 65, 03, 0d, 00, 13, 6e, 02, 0d, 00, 13, bb, 01, 03, 05, 00, 0f, 69, 01, 0c, 00, 13, 6d, 01, 0d, 03, 0e, 75, 04, 0d, 00, 13, c2, 01, 02, 0d, 00, 17, c2, 01, 01, 14, 00, 1b, 00, 01, 15, 00, 1b, 92, 01, 02, 15, 00, 1b, be, 01, 04, 0d, 00, 13, 7d, 03, 09, 00, 19, b6, 01, 02, 05, 00, 0f, b2, 01, 03, 09, 00, 22, 00, 02, 05, 00, 0f, 00, 03, 09, 00, 2c, 00, 02, 01, 00, 02] +Raw bytes (375): 0x[01, 01, 31, 05, 09, 0d, 00, 15, 19, 12, 00, 15, 19, 21, 00, 1e, 00, 21, 00, 31, 00, 3d, 00, 2e, 45, 3d, 00, 42, 49, 45, 00, 3f, 51, 42, 49, 45, 00, 7a, 55, 51, 00, 7a, 55, 51, 00, 77, 5d, 7a, 55, 51, 00, 77, 61, 7a, 55, 51, 00, 72, 65, 77, 61, 7a, 55, 51, 00, 75, be, 01, c2, 01, 79, 69, 6d, 69, 6d, 69, 6d, c2, 01, 00, 69, 6d, c2, 01, 79, 69, 6d, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, b6, 01, 00, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, 33, 01, 08, 01, 03, 1c, 05, 04, 09, 01, 1c, 02, 02, 05, 04, 1f, 0d, 05, 05, 00, 1f, 06, 01, 05, 00, 1f, 15, 01, 09, 01, 1c, 12, 02, 05, 00, 1f, 0e, 01, 05, 00, 0f, 00, 00, 20, 00, 30, 21, 01, 05, 03, 0f, 00, 03, 20, 00, 30, 00, 00, 33, 00, 41, 00, 00, 4b, 00, 5a, 1e, 01, 05, 00, 0f, 00, 05, 09, 03, 10, 00, 05, 0d, 00, 1b, 00, 02, 0d, 00, 1c, 1a, 04, 09, 05, 06, 31, 06, 05, 03, 06, 22, 04, 05, 03, 06, 3d, 04, 09, 04, 06, 2e, 05, 08, 00, 0f, 45, 01, 09, 03, 0a, 2a, 05, 09, 03, 0a, 3f, 05, 08, 00, 0f, 51, 01, 09, 00, 13, 00, 03, 0d, 00, 1d, 3a, 03, 09, 00, 13, 00, 03, 0d, 00, 1d, 77, 03, 05, 00, 0f, 77, 01, 0c, 00, 13, 5d, 01, 0d, 00, 13, 56, 02, 0d, 00, 13, 72, 04, 05, 02, 13, 65, 03, 0d, 00, 13, 6e, 02, 0d, 00, 13, bb, 01, 03, 05, 00, 0f, 69, 01, 0c, 00, 13, 6d, 01, 0d, 03, 0e, 75, 04, 0d, 00, 13, c2, 01, 02, 0d, 00, 17, c2, 01, 01, 14, 00, 1b, 00, 01, 15, 00, 1b, 92, 01, 02, 15, 00, 1b, be, 01, 04, 0d, 00, 13, 7d, 03, 09, 00, 19, b6, 01, 02, 05, 00, 0f, b2, 01, 03, 09, 00, 22, 00, 02, 05, 00, 0f, 00, 03, 09, 00, 2c, 00, 02, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 49 -- expression 0 operands: lhs = Counter(1), rhs = Zero +- expression 0 operands: lhs = Counter(1), rhs = Counter(2) - expression 1 operands: lhs = Counter(3), rhs = Zero -- expression 2 operands: lhs = Counter(5), rhs = Zero +- expression 2 operands: lhs = Counter(5), rhs = Counter(6) - expression 3 operands: lhs = Expression(4, Sub), rhs = Zero -- expression 4 operands: lhs = Counter(5), rhs = Zero +- expression 4 operands: lhs = Counter(5), rhs = Counter(6) - expression 5 operands: lhs = Counter(8), rhs = Zero - expression 6 operands: lhs = Expression(7, Sub), rhs = Zero - expression 7 operands: lhs = Counter(8), rhs = Zero @@ -111,15 +111,15 @@ Number of file 0 mappings: 51 - Code(Counter(0)) at (prev + 8, 1) to (start + 3, 28) - Code(Counter(1)) at (prev + 4, 9) to (start + 1, 28) - Code(Expression(0, Sub)) at (prev + 2, 5) to (start + 4, 31) - = (c1 - Zero) + = (c1 - c2) - Code(Counter(3)) at (prev + 5, 5) to (start + 0, 31) - Code(Expression(1, Sub)) at (prev + 1, 5) to (start + 0, 31) = (c3 - Zero) - Code(Counter(5)) at (prev + 1, 9) to (start + 1, 28) - Code(Expression(4, Sub)) at (prev + 2, 5) to (start + 0, 31) - = (c5 - Zero) + = (c5 - c6) - Code(Expression(3, Sub)) at (prev + 1, 5) to (start + 0, 15) - = ((c5 - Zero) - Zero) + = ((c5 - c6) - Zero) - Code(Zero) at (prev + 0, 32) to (start + 0, 48) - Code(Counter(8)) at (prev + 1, 5) to (start + 3, 15) - Code(Zero) at (prev + 3, 32) to (start + 0, 48) From 7e7d0a959d9a0450ac3596e842dc49e8e6618bc6 Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 27 Jun 2024 12:16:46 +0200 Subject: [PATCH 15/19] core: improve comment Co-authored-by: Ralf Jung --- library/core/src/fmt/rt.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/fmt/rt.rs b/library/core/src/fmt/rt.rs index 5836ce59ec1dc..65a4d537cc74d 100644 --- a/library/core/src/fmt/rt.rs +++ b/library/core/src/fmt/rt.rs @@ -68,8 +68,8 @@ pub(super) enum Flag { #[derive(Copy, Clone)] enum ArgumentType<'a> { Placeholder { - // INVARIANT: if `formatter` had type `fn(&T, _) -> _` then `value` - // was derived from a `&T` with lifetime `'a`. + // INVARIANT: `formatter` has type `fn(&T, _) -> _` for some `T`, and `value` + // was derived from a `&'a T`. value: NonNull<()>, formatter: unsafe fn(NonNull<()>, &mut Formatter<'_>) -> Result, _lifetime: PhantomData<&'a ()>, From 3457ecc776e87550bc1b1bd4bb686bde8bd5c504 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Thu, 27 Jun 2024 13:06:10 +0300 Subject: [PATCH 16/19] remove unnecessary packages from `metadata::workspace_members` Currently bootstrap doesn't use any inner paths from rust-analyzer and bootstrap with `ShouldRun::create_or_deps`. Signed-off-by: onur-ozkan --- src/bootstrap/src/core/metadata.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/bootstrap/src/core/metadata.rs b/src/bootstrap/src/core/metadata.rs index 12867cb46b811..220eb5ba126e4 100644 --- a/src/bootstrap/src/core/metadata.rs +++ b/src/bootstrap/src/core/metadata.rs @@ -66,7 +66,10 @@ pub fn build(build: &mut Build) { } /// Invokes `cargo metadata` to get package metadata of each workspace member. -fn workspace_members(build: &Build) -> impl Iterator { +/// +/// This is used to resolve specific crate paths in `fn should_run` to compile +/// particular crate (e.g., `x build sysroot` to build library/sysroot). +fn workspace_members(build: &Build) -> Vec { let collect_metadata = |manifest_path| { let mut cargo = Command::new(&build.initial_cargo); cargo @@ -85,9 +88,5 @@ fn workspace_members(build: &Build) -> impl Iterator { }; // Collects `metadata.packages` from all workspaces. - let packages = collect_metadata("Cargo.toml"); - let ra_packages = collect_metadata("src/tools/rust-analyzer/Cargo.toml"); - let bootstrap_packages = collect_metadata("src/bootstrap/Cargo.toml"); - - packages.into_iter().chain(ra_packages).chain(bootstrap_packages) + collect_metadata("Cargo.toml") } From b1b473e23cf3d948eef7ae2d94c284dd383ecc43 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Thu, 27 Jun 2024 14:16:02 +0300 Subject: [PATCH 17/19] add change-tracker entry Signed-off-by: onur-ozkan --- src/bootstrap/src/utils/change_tracker.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index bfe3622e40d37..ccab25e55a9c3 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -195,4 +195,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[ severity: ChangeSeverity::Warning, summary: "Removed `dist.missing-tools` configuration as it was deprecated long time ago.", }, + ChangeInfo { + change_id: 126701, + severity: ChangeSeverity::Warning, + summary: "`llvm.lld` is enabled by default for the dist profile. If set to false, `lld` will not be included in the dist build.", + }, ]; From 789ee88bd015fde4257464d0120ab57b0d744e0b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 27 Jun 2024 14:56:57 -0400 Subject: [PATCH 18/19] Tighten spans for async blocks --- compiler/rustc_ast/src/ast.rs | 5 +- compiler/rustc_ast/src/mut_visit.rs | 3 +- compiler/rustc_ast/src/visit.rs | 2 +- compiler/rustc_ast_lowering/src/expr.rs | 7 ++- compiler/rustc_ast_lowering/src/item.rs | 16 +++--- .../rustc_ast_pretty/src/pprust/state/expr.rs | 2 +- .../src/assert/context.rs | 2 +- compiler/rustc_parse/src/parser/expr.rs | 5 +- compiler/rustc_resolve/src/def_collector.rs | 2 +- .../src/suspicious_operation_groupings.rs | 2 +- .../clippy/clippy_utils/src/ast_utils.rs | 2 +- src/tools/rustfmt/src/expr.rs | 2 +- ...block-control-flow-static-semantics.stderr | 26 +++++----- ...async-borrowck-escaping-block-error.stderr | 10 ++-- .../async-closures/wrong-fn-kind.stderr | 24 ++++----- .../ui/async-await/async-is-unwindsafe.stderr | 25 +++++----- tests/ui/async-await/coroutine-desc.stderr | 4 +- .../async-await/coroutine-not-future.stderr | 4 +- .../issue-67252-unnamed-future.stderr | 2 +- tests/ui/async-await/issue-68112.stderr | 15 ++---- .../issue-70935-complex-spans.stderr | 20 +++----- ...sue-74072-lifetime-name-annotations.stderr | 14 +++--- tests/ui/async-await/issue-86507.stderr | 2 +- .../issues/issue-78938-async-block.stderr | 10 ++-- .../async-closure-gate.afn.stderr | 4 +- .../async-closure-gate.nofeat.stderr | 4 +- .../async-await/try-on-option-in-async.stderr | 12 ++--- .../cloning-in-async-block-121547.stderr | 18 +++---- ...break-inside-coroutine-issue-124495.stderr | 50 ++++++++----------- tests/ui/coroutine/clone-impl-async.stderr | 24 ++++----- tests/ui/coroutine/gen_block_is_coro.stderr | 12 ++--- .../coroutine/gen_block_is_no_future.stderr | 6 +-- tests/ui/coroutine/gen_block_move.stderr | 13 ++--- .../issue-90014-tait.stderr | 2 +- tests/ui/impl-trait/issue-55872-3.stderr | 4 +- .../ui/impl-trait/issues/issue-78722-2.stderr | 4 +- tests/ui/impl-trait/issues/issue-78722.stderr | 2 +- .../ui/impl-trait/nested-return-type4.stderr | 2 +- .../mismatch-sugg-for-shorthand-field.stderr | 4 +- .../pattern/non-structural-match-types.stderr | 2 +- .../expected-boxed-future-isnt-pinned.stderr | 2 +- tests/ui/traits/next-solver/async.fail.stderr | 2 +- .../indirect-recursion-issue-112047.stderr | 4 +- 43 files changed, 173 insertions(+), 204 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 4a3ce0e0c3066..f5e79c04d784f 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1454,7 +1454,10 @@ pub enum ExprKind { Block(P, Option