rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.22k stars 12.56k forks source link

debugger_visualizer: natvis files are accumulated instead of replaced on incremental builds #120913

Open MaulingMonkey opened 7 months ago

MaulingMonkey commented 7 months ago

Changes to my natvis files (e.g. replacing the DisplayString 1 → 2 below) aren't always being reflected in my debugger.

// src/lib.rs
#![debugger_visualizer(natvis_file = "my.natvis")]
#[derive(Debug)] pub struct Type(u32);
<!-- src/my.natvis -->
<?xml version="1.0" encoding="utf-8"?>
<AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
    <Type Name="my::Type"><DisplayString>1</DisplayString></Type>
</AutoVisualizer>
// examples/basic.rs
#[no_mangle] #[inline(never)] fn __break() {}
fn main() {
    let my = my::Type(42);
    __break();
    dbg!(my);
}

After enabling Visual Studio 2017's natvis diagnostic messages and stepping out of __break, I've started to accumulate many warnings, with one warning per build after the 1st (starting with no warnings for the first build after a cargo clean):

Natvis: basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6): Warning: Conflicting <Type> entries detected for type 'my::Type' at 'basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6)' and 'basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6)'.  The <Type> entry at 'basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6)' will have priority.
Natvis: basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6): Warning: Conflicting <Type> entries detected for type 'my::Type' at 'basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6)' and 'basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6)'.  The <Type> entry at 'basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6)' will have priority.
Natvis: basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6): Warning: Conflicting <Type> entries detected for type 'my::Type' at 'basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6)' and 'basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6)'.  The <Type> entry at 'basic-0.natvis (from ...\debug\examples\basic.pdb)(4,6)' will have priority.

I believe that for every build, the new contents of my.natvis are accumulated... and then conflict with the stale contents of previous my.natvis contents. These stale contents then, presumably, frequently win / take priority, resulting in not seeing the updated visualizer. Adding insult to injury, switching from stable to nightly, I ended up getting warnings for what I believe to be every type in {intrinsic, libcore, liballoc, libstd}.natvis until I cargo cleaned - you might have stale stdlib visualizers after a toolchain upgrade.

Meta

rustc --version --verbose:

rustc 1.76.0 (07dca489a 2024-02-04)
binary: rustc
commit-hash: 07dca489ac2d933c78d3c5158e3f43beefeb02ce
commit-date: 2024-02-04
host: x86_64-pc-windows-msvc
release: 1.76.0
LLVM version: 17.0.6
cargo 1.78.0-nightly (ccc84ccec 2024-02-07)
release: 1.78.0-nightly
commit-hash: ccc84ccec4b7340eb916aefda1cb3e2fe17d8e7b
commit-date: 2024-02-07
host: x86_64-pc-windows-msvc
libgit2: 1.7.2 (sys:0.18.2 vendored)
libcurl: 8.6.0-DEV (sys:0.4.71+curl-8.6.0 vendored ssl:Schannel)
os: Windows 10.0.19045 (Windows 10 Pro) [64-bit]
bjorn3 commented 7 months ago

Every compilation should write the natvis files to a unique temporary directory to pass to the linker: https://github.com/rust-lang/rust/blob/9aa232ecc7bb006a1fad404f437b049482021a3a/compiler/rustc_codegen_ssa/src/back/link.rs#L2426 And only the natvis files in this directory should be passed to the linker with /NATVIS:. It is possible that we are forgetting to mention the natvis files in the dep-info file, which could explain stale results due to the crate not getting rebuilt when the natvis files change, but that wouldn't explain why the debugger complains about duplicate files.

MaulingMonkey commented 7 months ago

Every compilation should write the natvis files to a unique temporary directory

That might be the problem, actually. If link.exe /NATIVS:... updates the pdb in-place, it might be attempting to replace the embedded natvis files by full path, not just by filename... and if each build gets a unique temporary directory, that gives each natvis a unique path. This also explains:

  1. Why I couldn't repro this with natvis-pdbs - it passes absolute canonicalized paths to the original stable *.natvis paths.
  2. Why I didn't see stdlib natvis warnings until I switched channels - they're passed by stable paths, so it wasn't until:
-/NATVIS:%USERPROFILE%\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\etc\*.natvis
+/NATVIS:%USERPROFILE%\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\etc\*.natvis

That I had nightly's natvis files living alongside stable's in my .pdbs.


Tired of wrangling Visual Studio, I created a ≈15MB .natvis file (through recursive copy+paste abuse to create a very large XML comment), and alternated between touching examples\basic.rs and cargo b --example basic. The only thing that noticably grew in my target folder is debug\examples\basic.pdb (which grew by about ≈15MB each build - no compression apparently.) Deleting it basic.pdb reset the size of the to ≈16MB when regenerated in the next build.


If using the original natvis locations is undesirable (user might modify mid-build?), using a stable subdirectory of target instead of a unique temp dir would help a lot... although you'll still end up with stale natvis data:

bjorn3 commented 7 months ago

If link.exe /NATIVS:... updates the pdb in-place

If that is indeed the case, that is an issue. That would mean that reproducible builds would require rustc to remove the pdb file before linking.

If using the original natvis locations is undesirable (user might modify mid-build?),

Not just modify. It might not even exist on the build system if the rlib was moved to a different machine.

using a stable subdirectory of target instead of a unique temp dir would help a lot...

Cargo wouldn't know how to remove that directory for cargo clean -p my_package. I think removing the pdb is the best option, also for reproducibility.