Fix invalid DWARF for enums when using ThinLTO #59380
Conversation
We were setting the same identifier for both the DW_TAG_structure_type and the DW_TAG_variant_part. This becomes a problem when using thinlto becauses it uses the identifier as a key for a map of types that is used to delete duplicates based on the ODR, so one of them is deleted as a duplicate, resulting in invalid DWARF. The DW_TAG_variant_part isn't a standalone type, so it doesn't need an identifier. Fix by omitting its identifier.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
Thanks for the PR, @philipc! I'll try to review shortly. |
|
This seems to make the infrastructure around composite types more complicated. Wouldn't it be easier to keep the ODR identifier for the |
|
Omitting the identifier seemed more correct to me, but yes that'll be simpler. I'll update the PR later. |
and bump llvm version in test
|
I've given the variant parts their own identifier. |
|
Thanks, @philipc! As far as I can tell these identifiers are solely used for LLVM uniquing and don't make it to actual debuginfo, so I don't think that giving an identifier to the variant part is especially wrong. That being said, this code is very old and was initially written against LLVM 3.x. It could probably do with a more fundamental overhaul. @bors r+ |
|
📌 Commit 3a5a8a5 has been approved by |
bors
added
S-waiting-on-bors
…oerister Fix invalid DWARF for enums when using ThinLTO We were setting the same identifier for both the DW_TAG_structure_type and the DW_TAG_variant_part. This becomes a problem when using ThinLTO becauses it uses the identifier as a key for a map of types that is used to delete duplicates based on the ODR, so one of them is deleted as a duplicate, resulting in invalid DWARF. The DW_TAG_variant_part isn't a standalone type, so it doesn't need an identifier. Fix by omitting its identifier. ODR uniquing is [enabled here](https://github.com/rust-lang/rust/blob/f21dee2c6179276321a88a63300dce74ff707e92/src/rustllvm/PassWrapper.cpp#L1101).
Rollup of 5 pull requests Successful merges: - #59343 (rustc(codegen): uncache `def_symbol_name` prefix from `symbol_name`.) - #59380 (Fix invalid DWARF for enums when using ThinLTO) - #59463 (skip dyn keyword lint under macros) - #59539 (Fix infinite recursion) - #59544 (manifest: only include miri on the nightly channel) Failed merges: r? @ghost
We were setting the same identifier for both the DW_TAG_structure_type
and the DW_TAG_variant_part. This becomes a problem when using ThinLTO
becauses it uses the identifier as a key for a map of types that is used
to delete duplicates based on the ODR, so one of them is deleted as a
duplicate, resulting in invalid DWARF.
The DW_TAG_variant_part isn't a standalone type, so it doesn't need
an identifier. Fix by omitting its identifier.
ODR uniquing is enabled here.