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

Debuginfo for niche-layout enums can be corrupted due to duplicate type names #84670

Closed
wesleywiser opened this issue Apr 28, 2021 · 3 comments · Fixed by #85292
Closed

Debuginfo for niche-layout enums can be corrupted due to duplicate type names #84670

wesleywiser opened this issue Apr 28, 2021 · 3 comments · Fixed by #85292
Assignees
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) A-layout Area: Memory layout of types C-bug Category: This is a bug. O-windows-msvc Toolchain: MSVC, Operating system: Windows

Comments

@wesleywiser
Copy link
Member

wesleywiser commented Apr 28, 2021

Niche layout enum variants do not get distinct type names that include type parameters which can cause duplicate type names for variants like Option::Some. This in turn will causes debugger to randomly pick one of the various type layouts when inspecting values which can cause invalid results.

fn main() {
    let a = Some(&1u8);
    let b = Some(&2u16);
    let c = Some(&3u32);
    let d = Some(&4u64);
    let e = Some(&5i8);
    let f = Some(&6i64);
    
    std::process::exit(0); // bp here
}

image

Notice the values for a, b and c are totally wrong because they are using the layout for Option<&i64>::Some. The types for the other variables are also wrong but that doesn't happen to cause issues in this particular instance.

@wesleywiser wesleywiser added A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) O-windows-msvc Toolchain: MSVC, Operating system: Windows C-bug Category: This is a bug. A-layout Area: Memory layout of types labels Apr 28, 2021
@wesleywiser wesleywiser self-assigned this Apr 28, 2021
@vadimcn
Copy link
Contributor

vadimcn commented May 8, 2021

@wesleywiser Could you please explain why duplicate names cause this confusion? Aren't PDB types identified by record ids?

@wesleywiser
Copy link
Member Author

@vadimcn I thought that as well but experimentally, the debugger seems to look up the types by name not record id. I've also seen warnings from cvdump related "duplicate/conflicting type definitions" even though the ids are different.

I'll post the cvdump results when I get back into the office on Monday.

@wesleywiser
Copy link
Member Author

For example, cvdump outputs this:

0x1015 : Length = 18, Leaf = 0x1203 LF_FIELDLIST
    list[0] = LF_MEMBER, public, type = T_64PUSHORT(0621), offset = 0
        member name = '__0'

0x1016 : Length = 78, Leaf = 0x1505 LF_STRUCTURE
    # members = 1,  field list type 0x1015, 
    Derivation list type 0x0000, VT shape type 0x0000
    Size = 8, class name = core::option::Some, unique name = 614d2e1de3c5e373cf909c161e5f6384::Some, UDT(0x00001037)

0x1035 : Length = 18, Leaf = 0x1203 LF_FIELDLIST
    list[0] = LF_MEMBER, public, type = T_64PQUAD(0613), offset = 0
        member name = '__0'

0x1037 : Length = 78, Leaf = 0x1505 LF_STRUCTURE
    # members = 1,  field list type 0x1035, 
    Derivation list type 0x0000, VT shape type 0x0000
    Size = 8, class name = core::option::Some, unique name = 2dfef4176b9038163574c2ab2dbdc08f::Some, UDT(0x00001037)

WARNING: UDT mismatch for core::option::Some
<<<<<<
0x1016 : Length = 78, Leaf = 0x1505 LF_STRUCTURE
    # members = 1,  field list type 0x1015, 
    Derivation list type 0x0000, VT shape type 0x0000
    Size = 8, class name = core::option::Some, unique name = 614d2e1de3c5e373cf909c161e5f6384::Some, UDT(0x00001037)

******
0x1037 : Length = 78, Leaf = 0x1505 LF_STRUCTURE
    # members = 1,  field list type 0x1035, 
    Derivation list type 0x0000, VT shape type 0x0000
    Size = 8, class name = core::option::Some, unique name = 2dfef4176b9038163574c2ab2dbdc08f::Some, UDT(0x00001037)

>>>>>>

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 3, 2021
…elwoerister

Improve debugging experience for enums on windows-msvc

This PR makes significant improvements over the status quo of debugging enums on the windows-msvc platform with either WinDbg or Visual Studio in three ways:

1. Improves the debugger experience for directly tagged enums.
2. Fixes a bug which caused the debugger to sometimes show the wrong debug info for niche layout enums. For example, `Option<&u32>` could sometimes use the debug info for `Option<&f64>` instead leading to nonsensical variable values in the debugger.
3. Significantly improves the debugger experience for niche-layout enums.

Let's look at a few examples:

```rust
pub enum CStyleEnum {
    Base = 2,
    Exponent = 16,
}

pub enum NicheLayoutEnum {
    Tag1,
    Data { my_data: CStyleEnum },
    Tag2,
    Tag3,
    Tag4,
}

pub enum OtherEnum<T> {
    Case1(T),
    Case2(T),
}

fn main() {
    let a = Some(CStyleEnum::Base);
    let b = Option::<CStyleEnum>::None;
    let c = NicheLayoutEnum::Tag1;
    let d = NicheLayoutEnum::Data { my_data: CStyleEnum::Exponent };
    let e = NicheLayoutEnum::Tag2;
    let f = Some(&1u32);
    let g = Option::<&'static u32>::None;
    let h = Some(&2u64);
    let i = Option::<&'static u64>::None;
    let j = Some(12u32);
    let k = Option::<u32>::None;
    let l = Some(12.34f64);
    let m = Option::<f64>::None;
    let n = CStyleEnum::Base;
    let o = CStyleEnum::Exponent;
    let p = Some("IAMA optional string!".to_string());
    let q = OtherEnum::Case1(42u32);
}
```

This is what WinDbg Preview shows using the latest rustc nightly:

![image](https://user-images.githubusercontent.com/831192/118285353-57c10780-b49f-11eb-97aa-db3abfc09508.png)

Most of the variables don't show a meaningful value expect for a few cases that we have targeted natvis definitions covering. Even worse, drilling into many of these variables shows information that can be difficult to interpret without an understanding of the layout of Rust types:

![image](https://user-images.githubusercontent.com/831192/118285609-a1a9ed80-b49f-11eb-9c29-b14576984647.png)

With the changes in this PR, we're able to write two natvis definitions that cover all enum cases generally. After building with these changes, WinDbg now shows this instead:

![image](https://user-images.githubusercontent.com/831192/118287730-be472500-b4a1-11eb-8cad-8f6a91c7516b.png)

Drilling into the same variables, we can see much more useful information:

![image](https://user-images.githubusercontent.com/831192/118287888-e20a6b00-b4a1-11eb-927f-32cf33a31c16.png)

Fixes rust-lang#84670
Fixes rust-lang#84671
@bors bors closed this as completed in 2577825 Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) A-layout Area: Memory layout of types C-bug Category: This is a bug. O-windows-msvc Toolchain: MSVC, Operating system: Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants