From 985d49b9168c74094db994a60e2699e87f09fde7 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 8 Dec 2023 10:46:11 -0500 Subject: [PATCH 1/4] test(trim-paths): use `--config` CLI to change options --- tests/testsuite/profile_trim_paths.rs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/tests/testsuite/profile_trim_paths.rs b/tests/testsuite/profile_trim_paths.rs index 9f96b094613..746d44ed040 100644 --- a/tests/testsuite/profile_trim_paths.rs +++ b/tests/testsuite/profile_trim_paths.rs @@ -523,22 +523,9 @@ fn object_works_helper(run: impl Fn(&std::path::Path) -> Vec) { p.cargo("clean").run(); - p.change_file( - "Cargo.toml", - r#" - [package] - name = "foo" - version = "0.0.1" - - [dependencies] - bar = "0.0.1" - - [profile.dev] - trim-paths = "object" - "#, - ); - p.cargo("build --verbose -Ztrim-paths") + .arg("--config") + .arg(r#"profile.dev.trim-paths="object""#) .masquerade_as_nightly_cargo(&["-Ztrim-paths"]) .with_stderr(&format!( "\ From 7cb7b47fe7581cafbf75be1efb099a6c8f558aa4 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 8 Dec 2023 10:49:06 -0500 Subject: [PATCH 2/4] test(trim-paths): add test for each split-debuginfo options Also demonstarte that on Linux with split-debuginfo on the remap is broken --- tests/testsuite/profile_trim_paths.rs | 110 +++++++++++++++++++------- 1 file changed, 80 insertions(+), 30 deletions(-) diff --git a/tests/testsuite/profile_trim_paths.rs b/tests/testsuite/profile_trim_paths.rs index 746d44ed040..3c3d5b20c4c 100644 --- a/tests/testsuite/profile_trim_paths.rs +++ b/tests/testsuite/profile_trim_paths.rs @@ -453,33 +453,65 @@ fn diagnostics_works() { } #[cfg(target_os = "macos")] -#[cargo_test(requires_nm, nightly, reason = "-Zremap-path-scope is unstable")] -fn object_works() { - object_works_helper(|path| { +mod object_works { + use super::*; + + fn inspect_debuginfo(path: &std::path::Path) -> Vec { std::process::Command::new("nm") .arg("-pa") .arg(path) .output() .expect("nm works") .stdout - }) + } + + #[cargo_test(requires_nm, nightly, reason = "-Zremap-path-scope is unstable")] + fn with_split_debuginfo_off() { + object_works_helper("off", inspect_debuginfo); + } + + #[cargo_test(requires_nm, nightly, reason = "-Zremap-path-scope is unstable")] + fn with_split_debuginfo_packed() { + object_works_helper("packed", inspect_debuginfo); + } + + #[cargo_test(requires_nm, nightly, reason = "-Zremap-path-scope is unstable")] + fn with_split_debuginfo_unpacked() { + object_works_helper("unpacked", inspect_debuginfo); + } } #[cfg(target_os = "linux")] -#[cargo_test(requires_readelf, nightly, reason = "-Zremap-path-scope is unstable")] -fn object_works() { - object_works_helper(|path| { +mod object_works { + use super::*; + + fn inspect_debuginfo(path: &std::path::Path) -> Vec { std::process::Command::new("readelf") .arg("-wi") .arg(path) .output() .expect("readelf works") .stdout - }) + } + + #[cargo_test(requires_readelf, nightly, reason = "-Zremap-path-scope is unstable")] + fn with_split_debuginfo_off() { + object_works_helper("off", inspect_debuginfo); + } + + #[cargo_test(requires_readelf, nightly, reason = "-Zremap-path-scope is unstable")] + fn with_split_debuginfo_packed() { + object_works_helper("packed", inspect_debuginfo); + } + + #[cargo_test(requires_readelf, nightly, reason = "-Zremap-path-scope is unstable")] + fn with_split_debuginfo_unpacked() { + object_works_helper("unpacked", inspect_debuginfo); + } } #[cfg(unix)] -fn object_works_helper(run: impl Fn(&std::path::Path) -> Vec) { +fn object_works_helper(split_debuginfo: &str, run: impl Fn(&std::path::Path) -> Vec) { use std::os::unix::ffi::OsStrExt; let registry_src = paths::home().join(".cargo/registry/src"); @@ -495,14 +527,19 @@ fn object_works_helper(run: impl Fn(&std::path::Path) -> Vec) { let p = project() .file( "Cargo.toml", - r#" + &format!( + r#" [package] name = "foo" version = "0.0.1" [dependencies] bar = "0.0.1" - "#, + + [profile.dev] + split-debuginfo = "{split_debuginfo}" + "# + ), ) .file("src/main.rs", "fn main() { bar::f(); }") .build(); @@ -530,12 +567,12 @@ fn object_works_helper(run: impl Fn(&std::path::Path) -> Vec) { .with_stderr(&format!( "\ [COMPILING] bar v0.0.1 -[RUNNING] `rustc [..]\ +[RUNNING] `rustc [..]-C split-debuginfo={split_debuginfo} [..]\ -Zremap-path-scope=object \ --remap-path-prefix={pkg_remap} \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [COMPILING] foo v0.0.1 ([CWD]) -[RUNNING] `rustc [..]\ +[RUNNING] `rustc [..]-C split-debuginfo={split_debuginfo} [..]\ -Zremap-path-scope=object \ --remap-path-prefix=[CWD]= \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] @@ -547,31 +584,44 @@ fn object_works_helper(run: impl Fn(&std::path::Path) -> Vec) { assert!(bin_path.is_file()); let stdout = run(&bin_path); assert!(memchr::memmem::find(&stdout, rust_src).is_none()); - if cfg!(target_os = "macos") { - for line in stdout.split(|c| c == &b'\n') { - let registry = memchr::memmem::find(line, registry_src_bytes).is_none(); - let local = memchr::memmem::find(line, pkg_root).is_none(); - if registry && local { - continue; - } + for line in stdout.split(|c| c == &b'\n') { + let registry = memchr::memmem::find(line, registry_src_bytes).is_none(); + let local = memchr::memmem::find(line, pkg_root).is_none(); + if registry && local { + continue; + } + #[cfg(target_os = "macos")] + { + // `OSO` symbols can't be trimmed at this moment. + // See if memchr::memmem::find(line, b" OSO ").is_some() { - // `OSO` symbols can't be trimmed at this moment. - // See - // TODO: Change to `is_none()` once the issue is resolved. continue; } // on macOS `SO` symbols are embedded in final binaries and should be trimmed. // See rust-lang/rust#117652. - assert!( - memchr::memmem::find(line, b" SO ").is_some(), - "untrimmed `SO` symbol found" - ) + if memchr::memmem::find(line, b" SO ").is_some() { + continue; + } + } + + #[cfg(target_os = "linux")] + { + // There is a bug in rustc `-Zremap-path-scope`. + // See rust-lang/rust/pull/118518 + if memchr::memmem::find(line, b"DW_AT_comp_dir").is_some() { + continue; + } + if memchr::memmem::find(line, b"DW_AT_GNU_dwo_name").is_some() { + continue; + } } - } else { - assert!(memchr::memmem::find(&stdout, registry_src_bytes).is_none()); - assert!(memchr::memmem::find(&stdout, pkg_root).is_none()); + + panic!( + "unexpected untrimmed symbol: {}", + String::from_utf8(line.into()).unwrap() + ); } } From bb86adfff7052d5e004431b818e8fd1ab74a54ae Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 8 Dec 2023 13:52:40 -0500 Subject: [PATCH 3/4] fix(trim-paths): explicit remap to current dir `.` In https://github.com/rust-lang/rust/blob/87e1447aa/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs#L856 when the remap result of `work_dir` is an empty string, LLVM won't generate some symbols for root debuginfo node. For example, `N_SO` and `N_OSO` on macOS, or `DW_AT_comp_dir` on Linux when debuginfo is splitted. Precisely, it is observed that when the `DIFile` of compile unit was provied with an empty compilation `Directory` string, LLVM would not emit those symbols for the root DI node. This behavior is not desired, resulting in corrupted debuginfo and degrading debugging experience. This is might not be a bug of `--remap-path-prefix` in rustc, since `-fdebug-prefix-map` in clang 16 could have the same result (`DW_AT_comp_dir` is gone when `work_dir` is remapped to an empty string). However, in gcc 12 `fdebug-prefix-map` will return an absolute work_dir when an empty string occurs. To not bother whether this needs to be fixed in rustc or not, let's fix it by always appending an explicit `.` when `--remap-path-prefix` remaps to relative workspace root a.k.a. where rustc is invoking. For more on gcc/clang remap options, see https://reproducible-builds.org/docs/build-path/ --- src/cargo/core/compiler/mod.rs | 2 +- tests/testsuite/profile_trim_paths.rs | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 14aa9814831..4b53a6cb825 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -1230,7 +1230,7 @@ fn trim_paths_args( // * path dependencies outside workspace root directory if is_local && pkg_root.strip_prefix(ws_root).is_ok() { remap.push(ws_root); - remap.push("="); // empty to remap to relative paths. + remap.push("=."); // remap to relative rustc work dir explicitly } else { remap.push(pkg_root); remap.push("="); diff --git a/tests/testsuite/profile_trim_paths.rs b/tests/testsuite/profile_trim_paths.rs index 3c3d5b20c4c..c3bb121da44 100644 --- a/tests/testsuite/profile_trim_paths.rs +++ b/tests/testsuite/profile_trim_paths.rs @@ -83,7 +83,7 @@ fn release_profile_default_to_object() { [COMPILING] foo v0.0.1 ([CWD]) [RUNNING] `rustc [..]\ -Zremap-path-scope=object \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [FINISHED] release [..]", ) @@ -121,7 +121,7 @@ fn one_option() { [COMPILING] foo v0.0.1 ([CWD]) [RUNNING] `rustc [..]\ -Zremap-path-scope={option} \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [FINISHED] dev [..]", )) @@ -158,7 +158,7 @@ fn multiple_options() { [COMPILING] foo v0.0.1 ([CWD]) [RUNNING] `rustc [..]\ -Zremap-path-scope=diagnostics,macro,object \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [FINISHED] dev [..]", ) @@ -193,7 +193,7 @@ fn profile_merge_works() { [COMPILING] foo v0.0.1 ([CWD]) [RUNNING] `rustc [..]\ -Zremap-path-scope=diagnostics \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [FINISHED] custom [..]", ) @@ -243,7 +243,7 @@ fn registry_dependency() { [COMPILING] foo v0.0.1 ([CWD]) [RUNNING] `rustc [..]\ -Zremap-path-scope=object \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [FINISHED] dev [..] [RUNNING] `target/debug/foo[EXE]`" @@ -297,7 +297,7 @@ fn git_dependency() { [COMPILING] foo v0.0.1 ([CWD]) [RUNNING] `rustc [..]\ -Zremap-path-scope=object \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [FINISHED] dev [..] [RUNNING] `target/debug/foo[EXE]`" @@ -338,12 +338,12 @@ fn path_dependency() { [COMPILING] bar v0.0.1 ([..]/cocktail-bar) [RUNNING] `rustc [..]\ -Zremap-path-scope=object \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [COMPILING] foo v0.0.1 ([CWD]) [RUNNING] `rustc [..]\ -Zremap-path-scope=object \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [FINISHED] dev [..] [RUNNING] `target/debug/foo[EXE]`" @@ -392,7 +392,7 @@ fn path_dependency_outside_workspace() { [COMPILING] foo v0.0.1 ([CWD]) [RUNNING] `rustc [..]\ -Zremap-path-scope=object \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [FINISHED] dev [..] [RUNNING] `target/debug/foo[EXE]`" @@ -446,7 +446,7 @@ fn diagnostics_works() { "\ [RUNNING] [..]rustc [..]\ -Zremap-path-scope=diagnostics \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..]", ) .run(); @@ -574,7 +574,7 @@ fn object_works_helper(split_debuginfo: &str, run: impl Fn(&std::path::Path) -> [COMPILING] foo v0.0.1 ([CWD]) [RUNNING] `rustc [..]-C split-debuginfo={split_debuginfo} [..]\ -Zremap-path-scope=object \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..] [FINISHED] dev [..]", )) @@ -737,7 +737,7 @@ fn lldb_works_after_trimmed() { "\ [RUNNING] `rustc [..]\ -Zremap-path-scope=object \ - --remap-path-prefix=[CWD]= \ + --remap-path-prefix=[CWD]=. \ --remap-path-prefix=[..]/lib/rustlib/src/rust=/rustc/[..]", ) .run(); From fcd4221c5aee6bb23a4c6f22624d278775b46144 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 8 Dec 2023 16:34:00 -0500 Subject: [PATCH 4/4] test(trim-paths): don't follow links to separate debuginfo files For refeference: https://sourceware.org/binutils/docs/binutils/readelf.html --- tests/testsuite/profile_trim_paths.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/testsuite/profile_trim_paths.rs b/tests/testsuite/profile_trim_paths.rs index c3bb121da44..8a883a004bf 100644 --- a/tests/testsuite/profile_trim_paths.rs +++ b/tests/testsuite/profile_trim_paths.rs @@ -487,7 +487,8 @@ mod object_works { fn inspect_debuginfo(path: &std::path::Path) -> Vec { std::process::Command::new("readelf") - .arg("-wi") + .arg("--debug-dump=info") + .arg("--debug-dump=no-follow-links") // older version can't recognized but just a warning .arg(path) .output() .expect("readelf works")