Skip to content

Commit

Permalink
Rollup merge of rust-lang#59380 - philipc:thinlto-variant, r=michaelw…
Browse files Browse the repository at this point in the history
…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).
  • Loading branch information
Centril authored Mar 30, 2019
2 parents be6b4c0 + 3a5a8a5 commit ae25518
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 1 deletion.
18 changes: 17 additions & 1 deletion src/librustc_codegen_llvm/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,17 @@ impl TypeMap<'ll, 'tcx> {
let interner_key = self.unique_id_interner.intern(&enum_variant_type_id);
UniqueTypeId(interner_key)
}

// Get the unique type id string for an enum variant part.
// Variant parts are not types and shouldn't really have their own id,
// but it makes set_members_of_composite_type() simpler.
fn get_unique_type_id_str_of_enum_variant_part<'a>(&mut self,
enum_type_id: UniqueTypeId) -> &str {
let variant_part_type_id = format!("{}_variant_part",
self.get_unique_type_id_as_string(enum_type_id));
let interner_key = self.unique_id_interner.intern(&variant_part_type_id);
self.unique_id_interner.get(interner_key)
}
}

// A description of some recursive type. It can either be already finished (as
Expand Down Expand Up @@ -1689,6 +1700,11 @@ fn prepare_enum_metadata(
},
};

let variant_part_unique_type_id_str = SmallCStr::new(
debug_context(cx).type_map
.borrow_mut()
.get_unique_type_id_str_of_enum_variant_part(unique_type_id)
);
let empty_array = create_DIArray(DIB(cx), &[]);
let variant_part = unsafe {
llvm::LLVMRustDIBuilderCreateVariantPart(
Expand All @@ -1702,7 +1718,7 @@ fn prepare_enum_metadata(
DIFlags::FlagZero,
discriminator_metadata,
empty_array,
unique_type_id_str.as_ptr())
variant_part_unique_type_id_str.as_ptr())
};

// The variant part must be wrapped in a struct according to DWARF.
Expand Down
48 changes: 48 additions & 0 deletions src/test/debuginfo/enum-thinlto.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// ignore-tidy-linelength

// Require LLVM with DW_TAG_variant_part and a gdb that can read it.
// min-system-llvm-version: 8.0
// min-gdb-version: 8.2

// compile-flags:-g -Z thinlto

// === GDB TESTS ===================================================================================

// gdb-command:run

// gdb-command:print *abc
// gdbr-check:$1 = enum_thinlto::ABC::TheA{x: 0, y: 8970181431921507452}

// === LLDB TESTS ==================================================================================

// lldb-command:run

// lldb-command:print *abc
// lldbg-check:(enum_thinlto::ABC) $0 = ABC { }

#![allow(unused_variables)]
#![feature(omit_gdb_pretty_printer_section)]
#![omit_gdb_pretty_printer_section]

// The first element is to ensure proper alignment, irrespective of the machines word size. Since
// the size of the discriminant value is machine dependent, this has be taken into account when
// datatype layout should be predictable as in this case.
#[derive(Debug)]
enum ABC {
TheA { x: i64, y: i64 },
TheB (i64, i32, i32),
}

fn main() {
let abc = ABC::TheA { x: 0, y: 0x7c7c_7c7c_7c7c_7c7c };

f(&abc);
}

fn f(abc: &ABC) {
zzz(); // #break

println!("{:?}", abc);
}

fn zzz() {()}

0 comments on commit ae25518

Please sign in to comment.