Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Natvis definitions for types defined in the core module of the windows crate #2023

Merged
merged 2 commits into from Sep 21, 2022

Conversation

ridwanabdillahi
Copy link
Contributor

This change adds Natvis visualizations for types in the windows crate to help improve the debugging experience on Windows.

Natvis is a framework that can be used to specify how types should be viewed under a supported debugger, such as the Windows debugger (WinDbg) and the Visual Studio debugger.

The Rust compiler does have Natvis support for some types, but this is limited to some of the core libraries and not supported for external crates.

rust-lang/rfcs#3191 proposes adding support for embedding debugging visualizations such as Natvis in a Rust crate. This RFC has been approved, merged and implemented.

This PR adds:

  • Natvis visualizations for types defined in the windows::core module.
  • Tests for testing visualizers embedded in the windows crate.
  • Updates to the CI pipeline to ensure tests for visualizers are run so they do not break silently.
  • A new debugger_visualizer feature for the windows crate to enable the unstable debugger_visualizer Rust feature.

Fixes: #2022

@ridwanabdillahi ridwanabdillahi changed the title Add Natvis definitions for core types defined in the Windows crate Add Natvis definitions for types defined in the windows::core module of the windows crate Sep 15, 2022
@ridwanabdillahi ridwanabdillahi changed the title Add Natvis definitions for types defined in the windows::core module of the windows crate Add Natvis definitions for types defined in the core module of the windows crate Sep 15, 2022
@kennykerr
Copy link
Collaborator

Thanks Ridwan, this looks exciting! I'll take it for a spin when I get a moment.

Can the same natvis be applied to both windows and windows-sys crates?

@ridwanabdillahi
Copy link
Contributor Author

@kennykerr

Can the same natvis be applied to both windows and windows-sys crates?

No, I don't believe they can. For starters, the Natvis definitions depend on the fully qualified type name which includes the crate name. This means for the Natvis definitions I added here, they would only apply to the windows crate.

Also, looking at the definitions for the core types in windows-sys, besides GUID they are all defined as type definitions. From my experience, the debugger does not have access to type definitions and replaces them with the underlying type. With these two types having different layouts, it would also make it difficult to reuse the same Natvis definitions as their counterparts defined in the windows crate.

@kennykerr
Copy link
Collaborator

With these two types having different layouts

They have the exact same memory layouts, but if the debugger just looks at the underlying types then you may have a problem.

@ridwanabdillahi
Copy link
Contributor Author

but if the debugger just looks at the underlying types then you may have a problem

Sorry, I meant from the debuggers aspect in terms of which type is being rendered. The debugger uses the underlying type for type definitions which makes creating specific Natvis for them difficult.

@kennykerr
Copy link
Collaborator

Hi Ridwan! Sorry for the delay on that PR. Been a little swamped. I took it for a quick spin with the VS debugger and it looks great.

Any ideas when it will be stabilized? I'd like to avoid carrying unstable features indefinitely.

Other than that, I'd probably just recommend moving the windows.natvis into the crates/libs/windows folder as its specific to that crate and doesn't need its own top-level folder. The readme could go into the docs folder as it seems very useful.

@ridwanabdillahi
Copy link
Contributor Author

Any ideas when it will be stabilized? I'd like to avoid carrying unstable features indefinitely.

Honestly speaking I was trying to show the value of this unstable feature by applying it directly to crates. I plan on trying to get this stabilized in the near future but that also depends on the responses from the Rust team.

Other than that, I'd probably just recommend moving the windows.natvis into the crates/libs/windows folder as its specific to that crate and doesn't need its own top-level folder. The readme could go into the docs folder as it seems very useful.

Sounds good. I'll make those changes and push out an update.

@MaulingMonkey
Copy link

MaulingMonkey commented Sep 21, 2022

I recently debugged some BSTRs. A windows 0.40-friendly .natvis that could be merged into this or a future PR:

<?xml version="1.0" encoding="utf-8"?>
<AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">

    <!--
        This could be simplified with the `x,bstr` format specifier.
        However, that doesn't appear to be supported by VS Code?
    -->
    <Type Name="windows::core::strings::bstr::BSTR">
        <DisplayString Condition="__0">{(wchar_t*)__0,[((unsigned int *)__0)[-1] / 2]su}</DisplayString>
        <DisplayString Condition="!__0">{(wchar_t*)__0,su}</DisplayString>
        <Expand>
            <Item Condition="__0" Name="[len]"          >((unsigned int *)__0)[-1] / 2</Item>
            <Item Condition="__0" Name="[bytes]"        >((unsigned int *)__0)[-1]</Item>
            <Item Condition="__0" Name="[code units]"   >(wchar_t*)__0,[((unsigned int *)__0)[-1] / 2]su</Item>

            <Item Condition="!__0" Name="[len]"          >0</Item>
            <Item Condition="!__0" Name="[bytes]"        >0</Item>
            <Item Condition="!__0" Name="[code units]"   >(wchar_t*)0,su</Item>
        </Expand>
    </Type>

</AutoVisualizer>

I'm finding it awkward to build windows-rs from repository or I'd just wait to PR this myself. I similar natvis files for my winstr::BString (which unlike window's BSTR is NonNull) @ MaulingMonkey/winstr@47e41bf .

Might drop the [bytes] items.

Any ideas when it will be stabilized? I'd like to avoid carrying unstable features indefinitely.

I'll take this opportunity to plug my natvis-pdbs crate, which should let users pick up these natvis files with a small build.rs script in their apps on stable, even without the nightly feature.

Copy link
Collaborator

@kennykerr kennykerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I still need to get my head around how this is tested but this looks like a great start. I'd love to expand on it once we have it merged in.

@kennykerr kennykerr merged commit 29e5510 into microsoft:master Sep 21, 2022
@kennykerr
Copy link
Collaborator

@MaulingMonkey feel free to contribute something for BSTR! Yes, we should strive for something that works across the WinDbg/VS/Code debuggers.

@tim-weis
Copy link
Contributor

tim-weis commented Feb 4, 2024

Any ideas when it will be stabilized? I'd like to avoid carrying unstable features indefinitely.

The #[debug_visualizer(...)] attribute stabilized in 1.71.0. That's still a ways away from 1.56 as the MSRV for the current 0.52.0 release of windows-core, but at least the feature has landed so this is no longer an exercise in chasing a moving target.

I still need to get my head around how this is tested [...]

This one has me confused, too. While I do not doubt that the tests initially succeeded, they should have started failing when #2078 changed HSTRING from

struct HSTRING(*mut Header);

to

struct HSTRING(Option<std::ptr::NonNull<Header>>);

The tests, however, continue to succeed to this day. I may be misunderstanding how CDB or GitHub workflows or debugger_test work, but WinDbg produces the following output for dx empty:

empty                 [Type: windows_core::strings::hstring::HSTRING]
    [<Raw View>]     [Type: windows_core::strings::hstring::HSTRING]
    [len]            : Unexpected failure to dereference object

which is (meaningfully) distinct from the expected output:

empty            : "" [Type: windows::core::strings::hstring::HSTRING]
    [<Raw View>]     [Type: windows::core::strings::hstring::HSTRING]
    [len]            : 0x0 [Type: unsigned int]

My verification was run against this binary crate:

main.rs:

use windows::core::HSTRING;

fn main() {
    let empty = HSTRING::new();
    println!("{empty}");
}

Cargo.toml:

[package]
name = "windows_natvis"
version = "0.0.0"
edition = "2021"

[dependencies.windows]
version = "0.52.0"
features = []

.cargo/config.toml:

[build]
rustflags = ["--cfg=windows_debugger_visualizer"]

I'd love to continue on my week-long Yak Shave, though I believe we should get rigorous testing set up before moving forward.

Thoughts? (@ridwanabdillahi @kennykerr @MaulingMonkey @riverar @TimMisiak @wesleywiser)

@MaulingMonkey
Copy link

MaulingMonkey commented Feb 4, 2024

which is (meaningfully) distinct from the expected output: [...windows::core...]

Test-expected output involves windows_core, not windows::core, as of ≈10 months ago, so you're looking at stale code. It also involves some horrific casting meant explicitly to decouple HSTRING's definition from it's debug interpretation.

they should have started failing when #2078

Are you sure it didn't? Actions/debugger_visualizer doesn't retain history that far back, (edit:) and it appears to be catching recent visualizer breakage in pull requests just fine.

@riverar
Copy link
Collaborator

riverar commented Feb 4, 2024

Not seeing any test issues here; you can harvest cdb output from %temp%.

(cdb 10.0.22621.2428)

Snippet:

start_debugger_command_6
0:004> dx -r2 pcwstr
pcwstr           : "This is a PCWSTR" [Type: windows_core::strings::pcwstr::PCWSTR]
    [<Raw View>]     [Type: windows_core::strings::pcwstr::PCWSTR]
    [len]            : 0x10
    [chars]         
        [0]              : 0x54 'T' [Type: char16_t]
        [1]              : 0x68 'h' [Type: char16_t]
        [2]              : 0x69 'i' [Type: char16_t]
        [3]              : 0x73 's' [Type: char16_t]
        [4]              : 0x20 ' ' [Type: char16_t]
        [5]              : 0x69 'i' [Type: char16_t]
        [6]              : 0x73 's' [Type: char16_t]
        [7]              : 0x20 ' ' [Type: char16_t]
        [8]              : 0x61 'a' [Type: char16_t]
        [9]              : 0x20 ' ' [Type: char16_t]
        [10]             : 0x50 'P' [Type: char16_t]
        [11]             : 0x43 'C' [Type: char16_t]
        [12]             : 0x57 'W' [Type: char16_t]
        [13]             : 0x53 'S' [Type: char16_t]
        [14]             : 0x54 'T' [Type: char16_t]
        [15]             : 0x52 'R' [Type: char16_t]
0:004> .echo end_debugger_command_6
end_debugger_command_6
0:004> .echo start_debugger_command_7
start_debugger_command_7
0:004> 
start_debugger_command_7
0:004> .echo end_debugger_command_7
end_debugger_command_7
0:004> .echo start_debugger_command_8
start_debugger_command_8
0:004> dx empty
empty            : "" [Type: windows_core::strings::hstring::HSTRING]
    [<Raw View>]     [Type: windows_core::strings::hstring::HSTRING]
    [len]            : 0x0 [Type: unsigned int]
0:004> .echo end_debugger_command_8
end_debugger_command_8

@ridwanabdillahi
Copy link
Contributor Author

ridwanabdillahi commented Feb 4, 2024

This one has me confused, too. While I do not doubt that the tests initially succeeded, they should have started failing when #2078 changed HSTRING from

Hello @tim-weis, the debugger_visualizer tests did indeed fail after PR #2078 was opened. Since the debugger_visualizer tests were part of the github workflows, it actually stopped Kenny from being able to commit his change.

I was able to publish two additional PRs #2077 to unblock his changes and ensure the HSTRING natvis was representative of the memory layout of the HSTRING type as opposed to relying on internal fields.

Once #2077 was merged directly into master Kenny was able to update his branch and the tests succeeded. I hope this clarifies how the tests work and why they were still passing after the changes made in the PR you mentioned.

@kennykerr
Copy link
Collaborator

As recently as #2819 I have been keeping those tests working.

@tim-weis
Copy link
Contributor

tim-weis commented Feb 6, 2024

Thanks for the feedback. This has been rather insightful.

I went ahead and did some more research. The TL;DR is: The HSTRING visualizer works with CDB but not with other debuggers I tried (WinDbg, Visual Studio).

First, though, the windows::core vs. windows_core path discrepancy was down to me copy-pasting outdated expected test output from #2023. This has since been changed, and all debuggers (and visualizers) agree on windows_core as the package-relative path prefix. That turned out to be a red herring.

For reference, here's the full repro (slightly modified from my previous comment).

Cargo.toml

[package]
name = "win_nv"
version = "0.0.0"
edition = "2021"

[dependencies]
windows = { version = "0.52.0", features = [] }

.cargo/config.toml

[build]
# Request that visualizers are embedded into the PDB
rustflags = ["--cfg=windows_debugger_visualizer"]

src/main.rs

use windows::core::HSTRING;

#[inline(never)]
fn __break() {}

fn main() {
    let empty = HSTRING::new();
    println!("{empty}");

    let hstring = HSTRING::from("This is an HSTRING");
    println!("{hstring}");

    __break();
}

This is following a pattern I first discovered in Ridwan's debugger_test crate: It introduces a function (__break()) for the sole purpose of having a symbol to set a breakpoint on, providing a convenient way to insert checkpoints at which execution pauses. This works in combination with the bm command (bm *!*::__break "gu") instructing the debugger to "Go Up" ("gu") whenever the function is hit, taking us back to the scope of interest.

With the crate set up the following command line launches right into the CDB debugger:

cargo b && cd target\debug && "%WindowsSdkDir%Debuggers\x64\cdb.exe" -o win_nv.exe

Once in the debugger, we can set the breakpoint, run to the checkpoint and inspect the HSTRINGs:

0:000> bm *!*::__break "gu"
*** WARNING: Unable to verify checksum for win_nv.exe
  1: 00007ff6`e62d1010 @!"win_nv!win_nv::__break"
0:000> g

This is an HSTRING
win_nv!win_nv::main+0x104:
00007ff6`e62d1124 eb00            jmp     win_nv!win_nv::main+0x106 (00007ff6`e62d1126)
0:000> dx empty
empty            : "" [Type: windows_core::strings::hstring::HSTRING]
    [<Raw View>]     [Type: windows_core::strings::hstring::HSTRING]
    [len]            : 0x0 [Type: unsigned int]
0:000> dx hstring
hstring          : "This is an HSTRING" [Type: windows_core::strings::hstring::HSTRING]
    [<Raw View>]     [Type: windows_core::strings::hstring::HSTRING]
    [len]            : 0x12 [Type: unsigned int]
    [ref_count]      : 1 [Type: windows_core::imp::ref_count::RefCount]
    [flags]          : 0x0 [Type: unsigned int]
    [chars]
0:000> q

That explains why the tests are succeeding. Moving to WinDbg had some surprises, though: Setting the breakpoint the same way as in CDB behaved differently. The "gu" command string went up (at least) one stack frame more than expected. I'm not sure what's up with that, but I just replaced the command string with "pt" ("Step to Next Return") which seemingly worked. For completeness: bm *!*::__break "pt".

Once there, the debugger produced unexpected results for the HSTRING variables:

0:000> dx empty
empty                 [Type: windows_core::strings::hstring::HSTRING]
    [<Raw View>]     [Type: windows_core::strings::hstring::HSTRING]
    [len]            : Unexpected failure to dereference object
0:000> dx hstring
hstring                 [Type: windows_core::strings::hstring::HSTRING]
    [<Raw View>]     [Type: windows_core::strings::hstring::HSTRING]
    [len]            : Unexpected failure to dereference object

Can anyone please verify my observations?


Rust: 1.75.0 (stable)
CDB: cdb version 10.0.22621.382
WinDbg: Debugger client version: 1.2308.2002.0; Debugger engine version: 10.0.25921.1001
Host OS: Windows 10 19045.3930


While this is starting to feel like I'm losing my mind, here are a few more things I tried to make sure I'm looking at the same thing the debugger is:

  • Renamed the executable from windows_natvis to win_nv to prevent any sort of name clashes in .natvis lookup.
  • .nvunloadall followed by an explicit .nvload with a copy of windows.natvis from this repo.
  • Verified that the .natvis was loaded using .nvlist.
  • Enabled natvis diagnostics in Visual Studio, which didn't uncover any obvious issues.
  • Dumped all visualizers in the PDB and compared them against the respective sources (4 standard Rust visualizers plus windows.natvis from here).
  • Replaced all reserved XML tokens with XML entities (e.g., > -> &gt;) and reloaded the modified .natvis, just to be sure.
  • Probably a few other things I forget...

None of the above had any observable effect so I'm confident that windows.natvis is actually loaded and evaluated.

@kennykerr
Copy link
Collaborator

If you think there's an issue here that needs addressing, please hit the "reference in new issue" so we can track it.

@tim-weis
Copy link
Contributor

tim-weis commented Feb 6, 2024

I am fighting GitHub's UI now. Apparently, I cannot create a new issue using the "Reference in new issue" option (the "Create issue" button is grayed out...). I'll just go through the manual process then.

@kennykerr
Copy link
Collaborator

Done: #2836

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Natvis definitions for types defined in windows::core module
5 participants