Auto merge of #147022 - Zalathar:no-args, r=wesleywiser
Remove current code for embedding command-line args in PDB The compiler currently has code that will obtain a list of quoted command-line arguments, and pass it through to TargetMachine creation, so that the command-line args can be embedded in PDB output. This PR removes that code, due to subtle concerns that might not have been apparent when it was originally added. --- Those concerns include: - The entire command-line quoting process is repeated every time a target-machine-factory is created. In incremental builds this typically occurs 500+ times, instead of happening only once. The repeated quoting constitutes a large chunk of instructions executed in the `large-workspace` benchmark. - See https://github.com/rust-lang/rust/pull/146804#issuecomment-3317322958 for an example of the perf consequences of skipping all that work. - This overhead occurs even when building for targets or configurations that don't emit PDB output. - Command-line arguments are obtained in a way that completely bypasses the query system, which is a problem for the integrity of incremental compilation. - Fixing this alone is likely to inhibit incremental rebuilds for most or all CGUs, even in builds that don't emit PDB output. - Command-line arguments and the executable path are obtained in a way that completely bypasses the compiler's path-remapping system, which is a reproducibility hazard. - https://github.com/rust-lang/rust/issues/128842 --- Relevant PRs: - https://github.com/rust-lang/rust/pull/113492 - https://github.com/rust-lang/rust/pull/130446 - https://github.com/rust-lang/rust/pull/131805 - https://github.com/rust-lang/rust/pull/146700 - https://github.com/rust-lang/rust/pull/146973 Zulip thread: - https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Some.20PDB.20info.20bypasses.20the.20query.20system.20and.20path.20remapping/with/541432211 --- According to rust-lang/rust#96475, one of the big motivations for embedding the command-line arguments was to enable tools like Live++. [It appears that Live++ doesn't actually support Rust yet](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/embeded.20compiler.20args.20and.20--remap-path-prefix/near/523800010), so it's possible that there aren't any existing workflows for this removal to break. In the future, there could be a case for reintroducing some or all of this functionality, guarded behind an opt-in flag so that it doesn't cause problems for other users. But as it stands, the current implementation puts a disproportionate burden on other users and on compiler maintainers.
This commit is contained in:
@@ -1,37 +0,0 @@
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
/// Joins command-line arguments into a single space-separated string, quoting
|
||||
/// and escaping individual arguments as necessary.
|
||||
///
|
||||
/// The result is intended to be informational, for embedding in debug metadata,
|
||||
/// and might not be properly quoted/escaped for actual command-line use.
|
||||
pub(crate) fn quote_command_line_args(args: &[String]) -> String {
|
||||
// Start with a decent-sized buffer, since rustc invocations tend to be long.
|
||||
let mut buf = String::with_capacity(128);
|
||||
|
||||
for arg in args {
|
||||
if !buf.is_empty() {
|
||||
buf.push(' ');
|
||||
}
|
||||
|
||||
print_arg_quoted(&mut buf, arg);
|
||||
}
|
||||
|
||||
buf
|
||||
}
|
||||
|
||||
/// Equivalent to LLVM's `sys::printArg` with quoting always enabled
|
||||
/// (see llvm/lib/Support/Program.cpp).
|
||||
fn print_arg_quoted(buf: &mut String, arg: &str) {
|
||||
buf.reserve(arg.len() + 2);
|
||||
|
||||
buf.push('"');
|
||||
for ch in arg.chars() {
|
||||
if matches!(ch, '"' | '\\' | '$') {
|
||||
buf.push('\\');
|
||||
}
|
||||
buf.push(ch);
|
||||
}
|
||||
buf.push('"');
|
||||
}
|
||||
@@ -1,25 +0,0 @@
|
||||
#[test]
|
||||
fn quote_command_line_args() {
|
||||
use super::quote_command_line_args;
|
||||
|
||||
struct Case<'a> {
|
||||
args: &'a [&'a str],
|
||||
expected: &'a str,
|
||||
}
|
||||
|
||||
let cases = &[
|
||||
Case { args: &[], expected: "" },
|
||||
Case { args: &["--hello", "world"], expected: r#""--hello" "world""# },
|
||||
Case { args: &["--hello world"], expected: r#""--hello world""# },
|
||||
Case {
|
||||
args: &["plain", "$dollar", "spa ce", r"back\slash", r#""quote""#, "plain"],
|
||||
expected: r#""plain" "\$dollar" "spa ce" "back\\slash" "\"quote\"" "plain""#,
|
||||
},
|
||||
];
|
||||
|
||||
for &Case { args, expected } in cases {
|
||||
let args = args.iter().copied().map(str::to_owned).collect::<Vec<_>>();
|
||||
let actual = quote_command_line_args(&args);
|
||||
assert_eq!(actual, expected, "args {args:?}");
|
||||
}
|
||||
}
|
||||
@@ -1,5 +1,4 @@
|
||||
pub(crate) mod archive;
|
||||
mod command_line_args;
|
||||
pub(crate) mod lto;
|
||||
pub(crate) mod owned_target_machine;
|
||||
mod profiling;
|
||||
|
||||
@@ -38,8 +38,6 @@ impl OwnedTargetMachine {
|
||||
output_obj_file: &CStr,
|
||||
debug_info_compression: &CStr,
|
||||
use_emulated_tls: bool,
|
||||
argv0: &str,
|
||||
command_line_args: &str,
|
||||
use_wasm_eh: bool,
|
||||
) -> Result<Self, LlvmError<'static>> {
|
||||
// SAFETY: llvm::LLVMRustCreateTargetMachine copies pointed to data
|
||||
@@ -66,10 +64,6 @@ impl OwnedTargetMachine {
|
||||
output_obj_file.as_ptr(),
|
||||
debug_info_compression.as_ptr(),
|
||||
use_emulated_tls,
|
||||
argv0.as_ptr(),
|
||||
argv0.len(),
|
||||
command_line_args.as_ptr(),
|
||||
command_line_args.len(),
|
||||
use_wasm_eh,
|
||||
)
|
||||
};
|
||||
|
||||
@@ -31,7 +31,6 @@ use rustc_span::{BytePos, InnerSpan, Pos, SpanData, SyntaxContext, sym};
|
||||
use rustc_target::spec::{CodeModel, FloatAbi, RelocModel, SanitizerSet, SplitDebuginfo, TlsModel};
|
||||
use tracing::{debug, trace};
|
||||
|
||||
use crate::back::command_line_args::quote_command_line_args;
|
||||
use crate::back::lto::ThinBuffer;
|
||||
use crate::back::owned_target_machine::OwnedTargetMachine;
|
||||
use crate::back::profiling::{
|
||||
@@ -253,19 +252,6 @@ pub(crate) fn target_machine_factory(
|
||||
|
||||
let use_emulated_tls = matches!(sess.tls_model(), TlsModel::Emulated);
|
||||
|
||||
// Command-line information to be included in the target machine.
|
||||
// This seems to only be used for embedding in PDB debuginfo files.
|
||||
// FIXME(Zalathar): Maybe skip this for non-PDB targets?
|
||||
let argv0 = std::env::current_exe()
|
||||
.unwrap_or_default()
|
||||
.into_os_string()
|
||||
.into_string()
|
||||
.unwrap_or_default();
|
||||
let command_line_args = quote_command_line_args(&sess.expanded_args);
|
||||
// Self-profile counter for the number of bytes produced by command-line quoting.
|
||||
// Values are summed, so the summary result is cumulative across all TM factories.
|
||||
sess.prof.artifact_size("quoted_command_line_args", "-", command_line_args.len() as u64);
|
||||
|
||||
let debuginfo_compression = sess.opts.debuginfo_compression.to_string();
|
||||
match sess.opts.debuginfo_compression {
|
||||
rustc_session::config::DebugInfoCompression::Zlib => {
|
||||
@@ -326,8 +312,6 @@ pub(crate) fn target_machine_factory(
|
||||
&output_obj_file,
|
||||
&debuginfo_compression,
|
||||
use_emulated_tls,
|
||||
&argv0,
|
||||
&command_line_args,
|
||||
use_wasm_eh,
|
||||
)
|
||||
})
|
||||
|
||||
@@ -2330,10 +2330,6 @@ unsafe extern "C" {
|
||||
OutputObjFile: *const c_char,
|
||||
DebugInfoCompression: *const c_char,
|
||||
UseEmulatedTls: bool,
|
||||
Argv0: *const c_uchar, // See "PTR_LEN_STR".
|
||||
Argv0Len: size_t,
|
||||
CommandLineArgs: *const c_uchar, // See "PTR_LEN_STR".
|
||||
CommandLineArgsLen: size_t,
|
||||
UseWasmEH: bool,
|
||||
) -> *mut TargetMachine;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user