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

DW_AT_discr_value elided in DWARF for some enum variants #104625

Open
bcantrill opened this issue Nov 20, 2022 · 2 comments
Open

DW_AT_discr_value elided in DWARF for some enum variants #104625

bcantrill opened this issue Nov 20, 2022 · 2 comments
Labels
C-bug Category: This is a bug. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bcantrill
Copy link

Code

There is a regression from 1.64.0 to 1.65.0 in the DWARF emitted for some variants for some enums, as the below program shows:

#[repr(u32)]
pub enum DontKnowWhyThisIsNeededButItIs {
    Blargh,
}

//
// When compiled on 1.65.0 or later, the `ThisIsBad` variant in the below
// `Foo` enum does not have a `DW_AT_discr_value` in the DWARF, as displayed
// by `dwarfdump`:
//
//
// ```
// ...
// < 2><0x00000049>      DW_TAG_structure_type
//                         DW_AT_name                  Foo
//                         DW_AT_byte_size             0x0000000c
//                         DW_AT_alignment             0x00000004
// < 3><0x00000050>        DW_TAG_variant_part
//                           DW_AT_discr                 <0x00000055>
// < 4><0x00000055>          DW_TAG_member
//                             DW_AT_type                  <0x00000180>
//                             DW_AT_alignment             0x00000004
//                             DW_AT_data_member_location  0
//                             DW_AT_artificial            yes(1)
// < 4><0x0000005c>          DW_TAG_variant
//                             DW_AT_discr_value           1
// < 5><0x0000005e>            DW_TAG_member
//                               DW_AT_name                  ThisIsFine
//                               DW_AT_type                  <0x00000094>
//                               DW_AT_alignment             0x00000004
//                               DW_AT_data_member_location  0
// < 4><0x0000006a>          DW_TAG_variant
// < 5><0x0000006b>            DW_TAG_member
//                               DW_AT_name                  ThisIsBad
//                               DW_AT_type                  <0x0000009b>
//                               DW_AT_alignment             0x00000004
//                               DW_AT_data_member_location  0
// < 4><0x00000077>          DW_TAG_variant
//                             DW_AT_discr_value           3
// < 5><0x00000079>            DW_TAG_member
//                               DW_AT_name                  ThisIsAlsoFine
//                               DW_AT_type                  <0x000000b9>
//                               DW_AT_alignment             0x00000004
//                               DW_AT_data_member_location  0
// < 4><0x00000085>          DW_TAG_variant
//                             DW_AT_discr_value           4
// < 5><0x00000087>            DW_TAG_member
//                               DW_AT_name                  ThisIsFineToo
//                               DW_AT_type                  <0x000000cc>
//                               DW_AT_alignment             0x00000004
//                               DW_AT_data_member_location  0
// ...
// ```
//
// By contrast, when compiled on 1.64.0, all variants have a
// `DW_AT_discr_value` as expected:
//
// ```
// ...
// < 2><0x00000049>      DW_TAG_structure_type
//                         DW_AT_name                  Foo
//                         DW_AT_byte_size             0x0000000c
//                         DW_AT_alignment             0x00000004
// < 3><0x00000050>        DW_TAG_variant_part
//                           DW_AT_discr                 <0x00000055>
// < 4><0x00000055>          DW_TAG_member
//                             DW_AT_type                  <0x00000181>
//                             DW_AT_alignment             0x00000001
//                             DW_AT_data_member_location  0
//                             DW_AT_artificial            yes(1)
// < 4><0x0000005c>          DW_TAG_variant
//                             DW_AT_discr_value           0
// < 5><0x0000005e>            DW_TAG_member
//                               DW_AT_name                  ThisIsFine
//                               DW_AT_type                  <0x00000095>
//                               DW_AT_alignment             0x00000004
//                               DW_AT_data_member_location  0
// < 4><0x0000006a>          DW_TAG_variant
//                             DW_AT_discr_value           1
// < 5><0x0000006c>            DW_TAG_member
//                               DW_AT_name                  ThisIsBad
//                               DW_AT_type                  <0x0000009c>
//                               DW_AT_alignment             0x00000004
//                               DW_AT_data_member_location  0
// < 4><0x00000078>          DW_TAG_variant
//                             DW_AT_discr_value           2
// < 5><0x0000007a>            DW_TAG_member
//                               DW_AT_name                  ThisIsAlsoFine
//                               DW_AT_type                  <0x000000ba>
//                               DW_AT_alignment             0x00000004
//                               DW_AT_data_member_location  0
// < 4><0x00000086>          DW_TAG_variant
//                             DW_AT_discr_value           3
// < 5><0x00000088>            DW_TAG_member
//                               DW_AT_name                  ThisIsFineToo
//                               DW_AT_type                  <0x000000cd>
//                               DW_AT_alignment             0x00000004
//                               DW_AT_data_member_location  0
// ...
// ```
// 
// Note that this is pretty squirrely; if the `#[repr(u32)]` is absent on the
// `DontKnowWhyThisIsNeededButItIs` enum (for example), the `ThisIsBad` member
// will correctly have a `DW_AT_discr_value` -- as it will if the
// `DontKnowWhyThisIsNeededButItIs` is absent entirely, or if the first tuple
// has fewer than 5 elements.  Finally (and perhaps most vexing?) it will also
// be chased away if there is a second variant that has the identical signature
// as `ThisIsBad` -- in which case all variants will correctly have a
// `DW_AT_discr_value`.
//
pub enum Foo {
    ThisIsFine,
    ThisIsBad(
        (u8, u8, u8, u8, u8),
        DontKnowWhyThisIsNeededButItIs,
    ),
    ThisIsAlsoFine(
        (u8, u8, u8, u8, u8),
    ),
    ThisIsFineToo(
        (u8, u8, u8, u8),
        DontKnowWhyThisIsNeededButItIs,
    ),
    ThisIsMaybeFine(
        (u8, u8, u8, u8),
        DontKnowWhyThisIsNeededButItIs,
    ),
}

#[allow(dead_code)]
static QUUX: Option<Foo> = None;

fn main() {}

Version it worked on

As the comment in the program indicates, the DWARF for the Foo enum looks like this on 1.64.0 and earlier:

< 2><0x00000049>      DW_TAG_structure_type
                        DW_AT_name                  Foo
                        DW_AT_byte_size             0x0000000c
                        DW_AT_alignment             0x00000004
< 3><0x00000050>        DW_TAG_variant_part
                          DW_AT_discr                 <0x00000055>
< 4><0x00000055>          DW_TAG_member
                            DW_AT_type                  <0x00000181>
                            DW_AT_alignment             0x00000001
                            DW_AT_data_member_location  0
                            DW_AT_artificial            yes(1)
< 4><0x0000005c>          DW_TAG_variant
                            DW_AT_discr_value           0
< 5><0x0000005e>            DW_TAG_member
                              DW_AT_name                  ThisIsFine
                              DW_AT_type                  <0x00000095>
                              DW_AT_alignment             0x00000004
                              DW_AT_data_member_location  0
< 4><0x0000006a>          DW_TAG_variant
                            DW_AT_discr_value           1
< 5><0x0000006c>            DW_TAG_member
                              DW_AT_name                  ThisIsBad
                              DW_AT_type                  <0x0000009c>
                              DW_AT_alignment             0x00000004
                              DW_AT_data_member_location  0
< 4><0x00000078>          DW_TAG_variant
                            DW_AT_discr_value           2
< 5><0x0000007a>            DW_TAG_member
                              DW_AT_name                  ThisIsAlsoFine
                              DW_AT_type                  <0x000000ba>
                              DW_AT_alignment             0x00000004
                              DW_AT_data_member_location  0
< 4><0x00000086>          DW_TAG_variant
                            DW_AT_discr_value           3
< 5><0x00000088>            DW_TAG_member
                              DW_AT_name                  ThisIsFineToo
                              DW_AT_type                  <0x000000cd>
                              DW_AT_alignment             0x00000004
                              DW_AT_data_member_location  0

rustc --version --verbose:

rustc 1.64.0 (a55dd71d5 2022-09-19)
binary: rustc
commit-hash: a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52
commit-date: 2022-09-19
host: x86_64-unknown-linux-gnu
release: 1.64.0
LLVM version: 14.0.6

Version with regression

But on 1.65.0 and later, it looks like this:

< 2><0x00000049>      DW_TAG_structure_type
                        DW_AT_name                  Foo
                        DW_AT_byte_size             0x0000000c
                        DW_AT_alignment             0x00000004
< 3><0x00000050>        DW_TAG_variant_part
                          DW_AT_discr                 <0x00000055>
< 4><0x00000055>          DW_TAG_member
                            DW_AT_type                  <0x00000180>
                            DW_AT_alignment             0x00000004
                            DW_AT_data_member_location  0
                            DW_AT_artificial            yes(1)
< 4><0x0000005c>          DW_TAG_variant
                            DW_AT_discr_value           1
< 5><0x0000005e>            DW_TAG_member
                              DW_AT_name                  ThisIsFine
                              DW_AT_type                  <0x00000094>
                              DW_AT_alignment             0x00000004
                              DW_AT_data_member_location  0
< 4><0x0000006a>          DW_TAG_variant
< 5><0x0000006b>            DW_TAG_member
                              DW_AT_name                  ThisIsBad
                              DW_AT_type                  <0x0000009b>
                              DW_AT_alignment             0x00000004
                              DW_AT_data_member_location  0
< 4><0x00000077>          DW_TAG_variant
                            DW_AT_discr_value           3
< 5><0x00000079>            DW_TAG_member
                              DW_AT_name                  ThisIsAlsoFine
                              DW_AT_type                  <0x000000b9>
                              DW_AT_alignment             0x00000004
                              DW_AT_data_member_location  0
< 4><0x00000085>          DW_TAG_variant
                            DW_AT_discr_value           4
< 5><0x00000087>            DW_TAG_member
                              DW_AT_name                  ThisIsFineToo
                              DW_AT_type                  <0x000000cc>
                              DW_AT_alignment             0x00000004
                              DW_AT_data_member_location  0

rustc --version --verbose:

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: x86_64-unknown-linux-gnu
release: 1.65.0
LLVM version: 15.0.0

Unsurprisingly, there is an LLVM version bump across these two versions.

If it helps to have context about how we hit this, we saw this in the debugger (Humility) for our embedded Rust operating system (Hubris); see oxidecomputer/humility#263 for details.

Thanks in advance to anyone who looks at this -- the DWARF emitted by Rust has been invaluable for our work!

@bcantrill bcantrill added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Nov 20, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Nov 20, 2022
@luqmana
Copy link
Member

luqmana commented Nov 20, 2022

searched nightlies: from nightly-2022-09-01 to nightly-2022-10-01
regressed nightly: nightly-2022-09-09
searched commit range: c2804e6...1120c5e
regressed commit: 512bd84

bisected with cargo-bisect-rustc v0.6.4

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --test-dir=. --start=2022-09-01 --end=2022-10-01 

That points at #94075 which seems reasonable. It looks like it we hit in humility because the enum niche optimizations were expanded in that PR. Before the PR, the optimization would only kick in for enums that had a single "dataful" variant whereas now it can apply in more cases. (I imagine the squirrely-ness of the test case also has to do with the opt kicking in or not)

As for why that inhibits DW_AT_discr_value:

/// The DW_AT_discr_value is optional, and is omitted if
/// - This is the only variant of a univariant enum (i.e. their is no discriminant)
/// - This is the "untagged" variant of a niche-layout enum
/// (where only the other variants are identified by a single value)

----------------+-------------------------------------------------------------------------------------------------------+
ThisIsFine      |                                                                                                       |
ThisIsBad       | __1: DontKnowWhyThisIsNeededButItIs  | __0: (u8, u8, u8, u8, u8)                                      |
ThisIsAlsoFine  |                                      | __0: (u8, u8, u8, u8, u8)                                      |
ThisIsFineToo   |                                      | __1: DontKnowWhyThisIsNeededButItIs    | __0: (u8, u8, u8, u8) |
ThisIsMaybeFine |                                      | __1: DontKnowWhyThisIsNeededButItIs    | __0: (u8, u8, u8, u8) |
----------------+--------------------------------------+----------------------------------------+-----------------------+
Byte Offset     0                                      4                                        8

This is a rough sketch of the layout of Foo for the different variants. The first DW_TAG_member under Foo's DW_TAG_member indicates that the discriminant is at offset 0 (DW_AT_data_member_location = 0). But that's also where the second tuple member of the ThisIsBad variant is. But that's fine because we know DontKnowWhyThisIsNeededButItIs is an enum with a single variant and can be represented with a single value (in fact, the #[repr(u32)] is forcing it to even have a value here). But that's 1 value out of all the possibilities a u32 can hold, i.e. a niche. So that's where rustc decides to stash the discriminant. The discriminant value is thus computed as (d - 0) + 1 for the other variants leading to the values seen in DW_AT_discr_value.

@apiraino apiraino added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed regression-untriaged Untriaged performance or correctness regression. labels Nov 23, 2022
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants