debuginfo: Set bitwidth appropriately in enum variant tags

[maurer]

Previously, we unconditionally set the bitwidth to 128-bits, the largest an enum would possibly be. Then, LLVM would cut down the constant by chopping off leading zeroes before emitting the DWARF. LLVM only supported 64-bit enumerators, so this would also have occasionally resulted in truncated data.

LLVM added support for 128-bit enumerators in llvm/llvm-project#125578

That patchset trusts the constant to describe how wide the variant tag is, so the high 64-bits of zeros are considered potentially load-bearing.

As a result, we went from emitting tags that looked like:
DW_AT_discr_value (0xfe)

(because dwarf::BestForm selected data1)

to emitting tags that looked like:
DW_AT_discr_value (<0x10> fe ff ff ff 00 00 00 00 00 00 00 00 00 00 00 00 )

This makes the DW_AT_discr_value encode at the bitwidth of the tag, which:

1. Is probably closer to our intentions in terms of describing the data.
2. Doesn't invoke the 128-bit support which may not be supported by all debuggers / downstream tools.
3. Will result in smaller debug information.

[rustbot]

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[maurer]

cc @tromey for expertise on DWARF encoding and whether I've adjusted this correctly

[maurer]

@rustbot author

Looks like I have a few things to fix still, will poke it tomorrow, removing from review queue until then.

[tromey]

cc @tromey for expertise on DWARF encoding and whether I've adjusted this correctly

I think the idea is fine. I don't remember enough about rustc to really comment on the implementation.

I stumbled over an unexpected gdb/llvm/gcc discrepancy in the handle of DW_FORM_data* recently. See this gdb bug. However, assuming the discriminants are unsigned, I think this shouldn't pose a problem -- since it only affects the situation where sign-extension is needed.

[maurer]

The discriminants are not always unsigned unfortunately.

#[repr(i32)]
pub enum Foo {
    Unit = -2,
    TwoInts(u32, u32) = -1,
    ThreeShorts { x: u16, y: u16, z: u16 } = 0,
}

pub fn use_foo(_foo: Foo) {}

will produce via dwarfdump DW_AT_discr_value 254 (-2), and via llvm-dwarfdump DW_AT_discr_value (0xfe) for the first variant.

This happens because LLVM uses the discriminator type to determine the signedness when encoding, but then invokes BestForm because nullopt is passed as the form.

The sign data is available to the debugger through variant_part -> member -> type -> encoding, but based on your gdb bug, it sounds like debuggers might not use this.

@tromey: Based on your gdb bug, do you think pre-selecting DW_FORM_udata and DW_FORM_sdata in addInt for bittage of 64 or lower would be an improvement? Or are you suggesting on the gdb bug that GDB has the erroneous behavior, and emitting a data1 with the assumption that the debugger will use the encoding attribute of the discriminant type to expand it is correct? The other option I could see would be to select the data-width of the form based on the bitwidth of the discriminator.

I think we still want to make a change like this Rust side, because if LLVM is conditioning on the bitwidth, we should pass the right bitwidth.

[tromey]

The sign data is available to the debugger through variant_part -> member -> type -> encoding, but based on your gdb bug, it sounds like debuggers might not use this.

Well, gdb doesn't. It probably should, though the spec isn't entirely clear and the thread from ages ago was also pretty unhelpful.

@tromey: Based on your gdb bug, do you think pre-selecting DW_FORM_udata and DW_FORM_sdata in addInt for bittage of 64 or lower would be an improvement? Or are you suggesting on the gdb bug that GDB has the erroneous behavior, and emitting a data1 with the assumption that the debugger will use the encoding attribute of the discriminant type to expand it is correct? The other option I could see would be to select the data-width of the form based on the bitwidth of the discriminator.

I think it's unclear what should be done. Currently I tend to agree with the LLVM interpretation here, and think that gdb is wrong. At the same time DWARF does recommend this:

If one of the DW_FORM_dataforms is used to represent a signed or unsigned integer, it can be hard for a consumer to discover the context necessary to determine which interpretation is intended. Producers are therefore strongly encouraged to use DW_FORM_sdata or DW_FORM_udata for signed and unsigned integers respectively, rather than DW_FORM_data.

For gdb I am hoping to get some comments on the corresponding GCC bug before taking action. And of course a fix won't appear until the next release anyway...

[maurer]

@rustbot ready

[nikic]

@bors r+ rollup

[bors]

📌 Commit d82219a has been approved by nikic

It is now in the queue for this repository.

[myxoid]

Hi. I originated this bug report within Google. I maintain an open source tool that ingests DWARF, STG, and have previously contributed to libabigail. The latter doesn't yet support variants and discriminants but would run into exactly the same problem.

Making DWARF consumers do type inspection to interpret constant values is a significant burden. In the STG case, it would require either local reference chasing within the DWARF processor (something we avoid wherever possible, for performance reasons) or a post-processing pass to fix up enumerator / discriminator values whose sign was ambiguous during DIE traversal.

This text was referred to in earlier comment:

If one of the DW_FORM_dataforms is used to represent a signed or unsigned integer, it can be hard for a consumer to discover the context necessary to determine which interpretation is intended. Producers are therefore strongly encouraged to use DW_FORM_sdata or DW_FORM_udata for signed and unsigned integers respectively, rather than DW_FORM_data.

It was added to the DWARF standard in response to a very similar issue to this one.

https://dwarfstd.org/issues/020702.1.html

LLVM appears to br emitting enumerator constants unambiguously. I would very much to see discriminator constants emitted the same way (and parsed in STG with the same code).

Aside: I wonder if this also affects gendwarfksyms.