From 6f37204f821c115e215629f4d47b360bc04530cf Mon Sep 17 00:00:00 2001 From: Yury Delendik Date: Thu, 4 Jun 2020 14:34:05 -0500 Subject: [PATCH] Upgrade gimli to 0.21 (#1819) * Use gimli 0.21 * rm CFI w Expression * Don't write .debug_frame twice --- Cargo.lock | 7 ++---- cranelift/codegen/Cargo.toml | 2 +- cranelift/codegen/src/isa/unwind/systemv.rs | 27 +++++++------------- cranelift/filetests/Cargo.toml | 2 +- crates/debug/Cargo.toml | 2 +- crates/debug/src/transform/attr.rs | 2 +- crates/debug/src/transform/expression.rs | 11 ++++---- crates/debug/src/transform/refs.rs | 6 +++-- crates/debug/src/transform/simulate.rs | 2 +- crates/debug/src/transform/unit.rs | 16 ++++++------ crates/debug/src/transform/utils.rs | 10 ++++---- crates/debug/src/write_debuginfo.rs | 28 +++------------------ crates/jit/Cargo.toml | 2 +- crates/profiling/Cargo.toml | 2 +- 14 files changed, 45 insertions(+), 74 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b11298f3d3..8d73830f4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -923,15 +923,12 @@ dependencies = [ [[package]] name = "gimli" -version = "0.20.0" +version = "0.21.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81dd6190aad0f05ddbbf3245c54ed14ca4aa6dd32f22312b70d8f168c3e3e633" +checksum = "bcc8e0c9bce37868955864dbecd2b1ab2bdf967e6f28066d65aaac620444b65c" dependencies = [ - "arrayvec", - "byteorder", "fallible-iterator", "indexmap", - "smallvec", "stable_deref_trait", ] diff --git a/cranelift/codegen/Cargo.toml b/cranelift/codegen/Cargo.toml index e40baa0e2b..0e2f5a20f3 100644 --- a/cranelift/codegen/Cargo.toml +++ b/cranelift/codegen/Cargo.toml @@ -20,7 +20,7 @@ hashbrown = { version = "0.7", optional = true } target-lexicon = "0.10" log = { version = "0.4.6", default-features = false } serde = { version = "1.0.94", features = ["derive"], optional = true } -gimli = { version = "0.20.0", default-features = false, features = ["write"], optional = true } +gimli = { version = "0.21.0", default-features = false, features = ["write"], optional = true } smallvec = { version = "1.0.0" } thiserror = "1.0.4" byteorder = { version = "1.3.2", default-features = false } diff --git a/cranelift/codegen/src/isa/unwind/systemv.rs b/cranelift/codegen/src/isa/unwind/systemv.rs index 5985082f71..190226e8ce 100644 --- a/cranelift/codegen/src/isa/unwind/systemv.rs +++ b/cranelift/codegen/src/isa/unwind/systemv.rs @@ -8,7 +8,6 @@ use thiserror::Error; use serde::{Deserialize, Serialize}; type Register = u16; -type Expression = Vec; /// Enumerate the errors possible in mapping Cranelift registers to their DWARF equivalent. #[allow(missing_docs)] @@ -23,6 +22,8 @@ pub enum RegisterMappingError { } // This mirrors gimli's CallFrameInstruction, but is serializable +// This excludes CfaExpression, Expression, ValExpression due to +// https://github.com/gimli-rs/gimli/issues/513. // TODO: if gimli ever adds serialization support, remove this type #[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] @@ -30,15 +31,12 @@ pub(crate) enum CallFrameInstruction { Cfa(Register, i32), CfaRegister(Register), CfaOffset(i32), - CfaExpression(Expression), Restore(Register), Undefined(Register), SameValue(Register), Offset(Register, i32), ValOffset(Register, i32), Register(Register, Register), - Expression(Register, Expression), - ValExpression(Register, Expression), RememberState, RestoreState, ArgsSize(u32), @@ -52,34 +50,33 @@ impl From for CallFrameInstruction { CallFrameInstruction::Cfa(reg, offset) => Self::Cfa(reg.0, offset), CallFrameInstruction::CfaRegister(reg) => Self::CfaRegister(reg.0), CallFrameInstruction::CfaOffset(offset) => Self::CfaOffset(offset), - CallFrameInstruction::CfaExpression(expr) => Self::CfaExpression(expr.0), CallFrameInstruction::Restore(reg) => Self::Restore(reg.0), CallFrameInstruction::Undefined(reg) => Self::Undefined(reg.0), CallFrameInstruction::SameValue(reg) => Self::SameValue(reg.0), CallFrameInstruction::Offset(reg, offset) => Self::Offset(reg.0, offset), CallFrameInstruction::ValOffset(reg, offset) => Self::ValOffset(reg.0, offset), CallFrameInstruction::Register(reg1, reg2) => Self::Register(reg1.0, reg2.0), - CallFrameInstruction::Expression(reg, expr) => Self::Expression(reg.0, expr.0), - CallFrameInstruction::ValExpression(reg, expr) => Self::ValExpression(reg.0, expr.0), CallFrameInstruction::RememberState => Self::RememberState, CallFrameInstruction::RestoreState => Self::RestoreState, CallFrameInstruction::ArgsSize(size) => Self::ArgsSize(size), + _ => { + // Cranelift's unwind support does not generate `CallFrameInstruction`s with + // Expression at this moment, and it is not trivial to + // serialize such instructions. + panic!("CallFrameInstruction with Expression not supported"); + } } } } impl Into for CallFrameInstruction { fn into(self) -> gimli::write::CallFrameInstruction { - use gimli::{ - write::{CallFrameInstruction, Expression}, - Register, - }; + use gimli::{write::CallFrameInstruction, Register}; match self { Self::Cfa(reg, offset) => CallFrameInstruction::Cfa(Register(reg), offset), Self::CfaRegister(reg) => CallFrameInstruction::CfaRegister(Register(reg)), Self::CfaOffset(offset) => CallFrameInstruction::CfaOffset(offset), - Self::CfaExpression(expr) => CallFrameInstruction::CfaExpression(Expression(expr)), Self::Restore(reg) => CallFrameInstruction::Restore(Register(reg)), Self::Undefined(reg) => CallFrameInstruction::Undefined(Register(reg)), Self::SameValue(reg) => CallFrameInstruction::SameValue(Register(reg)), @@ -88,12 +85,6 @@ impl Into for CallFrameInstruction { Self::Register(reg1, reg2) => { CallFrameInstruction::Register(Register(reg1), Register(reg2)) } - Self::Expression(reg, expr) => { - CallFrameInstruction::Expression(Register(reg), Expression(expr)) - } - Self::ValExpression(reg, expr) => { - CallFrameInstruction::ValExpression(Register(reg), Expression(expr)) - } Self::RememberState => CallFrameInstruction::RememberState, Self::RestoreState => CallFrameInstruction::RestoreState, Self::ArgsSize(size) => CallFrameInstruction::ArgsSize(size), diff --git a/cranelift/filetests/Cargo.toml b/cranelift/filetests/Cargo.toml index 3fc5cb8df5..974c8fbee0 100644 --- a/cranelift/filetests/Cargo.toml +++ b/cranelift/filetests/Cargo.toml @@ -19,7 +19,7 @@ cranelift-preopt = { path = "../preopt", version = "0.64.0" } byteorder = { version = "1.3.2", default-features = false } file-per-thread-logger = "0.1.2" filecheck = "0.5.0" -gimli = { version = "0.20.0", default-features = false, features = ["read"] } +gimli = { version = "0.21.0", default-features = false, features = ["read"] } log = "0.4.6" memmap = "0.7.0" num_cpus = "1.8.0" diff --git a/crates/debug/Cargo.toml b/crates/debug/Cargo.toml index 8cbae2cc83..acd417add4 100644 --- a/crates/debug/Cargo.toml +++ b/crates/debug/Cargo.toml @@ -12,7 +12,7 @@ readme = "README.md" edition = "2018" [dependencies] -gimli = "0.20.0" +gimli = "0.21.0" wasmparser = "0.57.0" faerie = "0.15.0" wasmtime-environ = { path = "../environ", version = "0.17.0" } diff --git a/crates/debug/src/transform/attr.rs b/crates/debug/src/transform/attr.rs index 1e74ca9a57..cb654fe790 100644 --- a/crates/debug/src/transform/attr.rs +++ b/crates/debug/src/transform/attr.rs @@ -228,7 +228,7 @@ where let mut found_expr: Option = None; for (_, _, expr) in &exprs { if let Some(ref prev_expr) = found_expr { - if expr.0.eq(&prev_expr.0) { + if expr == prev_expr { continue; // the same expression } found_expr = None; diff --git a/crates/debug/src/transform/expression.rs b/crates/debug/src/transform/expression.rs index c7b71c0030..8bd3a76302 100644 --- a/crates/debug/src/transform/expression.rs +++ b/crates/debug/src/transform/expression.rs @@ -215,7 +215,7 @@ impl CompiledExpression { pub fn build(&self) -> Option { if let [CompiledExpressionPart::Code(code)] = self.parts.as_slice() { - return Some(write::Expression(code.to_vec())); + return Some(write::Expression::raw(code.to_vec())); } // locals found, not supported None @@ -245,7 +245,7 @@ impl CompiledExpression { BuildWithLocalsResult::Empty => None, BuildWithLocalsResult::Simple(it, code) => it .next() - .map(|(addr, len)| Ok((addr, len, write::Expression(code.to_vec())))), + .map(|(addr, len)| Ok((addr, len, write::Expression::raw(code.to_vec())))), BuildWithLocalsResult::Ranges(it) => it.next().map(|r| { r.map(|(func_index, start, end, code_buf)| { ( @@ -254,7 +254,7 @@ impl CompiledExpression { addend: start as i64, }, (end - start) as u64, - write::Expression(code_buf), + write::Expression::raw(code_buf), ) }) }), @@ -415,7 +415,7 @@ where }); } else { let pos = pc.offset_from(&expr.0).into_u64() as usize; - let op = Operation::parse(&mut pc, &expr.0, encoding)?; + let op = Operation::parse(&mut pc, encoding)?; match op { Operation::FrameOffset { offset } => { // Expand DW_OP_fpreg into frame location and DW_OP_plus_uconst. @@ -435,7 +435,8 @@ where code_chunk.extend(writer.into_vec()); continue; } - Operation::Literal { .. } + Operation::UnsignedConstant { .. } + | Operation::SignedConstant { .. } | Operation::PlusConstant { .. } | Operation::Piece { .. } => (), Operation::StackValue => { diff --git a/crates/debug/src/transform/refs.rs b/crates/debug/src/transform/refs.rs index 1780f06495..4fa0ce99dd 100644 --- a/crates/debug/src/transform/refs.rs +++ b/crates/debug/src/transform/refs.rs @@ -58,7 +58,7 @@ impl UnitRefsMap { for (die_id, attr_name, offset) in refs.refs { let die = comp_unit.get_mut(die_id); if let Some(unit_id) = self.map.get(&offset) { - die.set(attr_name, write::AttributeValue::ThisUnitEntryRef(*unit_id)); + die.set(attr_name, write::AttributeValue::UnitRef(*unit_id)); } } } @@ -102,7 +102,9 @@ impl DebugInfoRefsMap { if let Some((id, entry_id)) = self.map.get(&offset) { die.set( attr_name, - write::AttributeValue::AnyUnitEntryRef((*id, *entry_id)), + write::AttributeValue::DebugInfoRef(write::Reference::Entry( + *id, *entry_id, + )), ); } } diff --git a/crates/debug/src/transform/simulate.rs b/crates/debug/src/transform/simulate.rs index cb8b38c9ed..4f06ba8aa3 100644 --- a/crates/debug/src/transform/simulate.rs +++ b/crates/debug/src/transform/simulate.rs @@ -262,7 +262,7 @@ fn generate_vars( var.set(gimli::DW_AT_name, write::AttributeValue::StringRef(name_id)); var.set( gimli::DW_AT_type, - write::AttributeValue::ThisUnitEntryRef(type_die_id), + write::AttributeValue::UnitRef(type_die_id), ); var.set( gimli::DW_AT_location, diff --git a/crates/debug/src/transform/unit.rs b/crates/debug/src/transform/unit.rs index a04f11b4ec..01835ccdf7 100644 --- a/crates/debug/src/transform/unit.rs +++ b/crates/debug/src/transform/unit.rs @@ -132,7 +132,7 @@ where // Build DW_TAG_pointer_type for `WebAssemblyPtrWrapper*`: // .. DW_AT_type = add_tag!(parent_id, gimli::DW_TAG_pointer_type => wrapper_ptr_type as wrapper_ptr_type_id { - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(wrapper_die_id) + gimli::DW_AT_type = write::AttributeValue::UnitRef(wrapper_die_id) }); let base_type_id = pointer_type_entry.attr_value(gimli::DW_AT_type)?; @@ -166,7 +166,7 @@ where // .. DW_AT_location = 0 add_tag!(wrapper_die_id, gimli::DW_TAG_member => m_die as m_die_id { gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("__ptr")), - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(wp_die_id), + gimli::DW_AT_type = write::AttributeValue::UnitRef(wp_die_id), gimli::DW_AT_data_member_location = write::AttributeValue::Data1(0) }); @@ -180,10 +180,10 @@ where add_tag!(wrapper_die_id, gimli::DW_TAG_subprogram => deref_op_die as deref_op_die_id { gimli::DW_AT_linkage_name = write::AttributeValue::StringRef(out_strings.add("resolve_vmctx_memory_ptr")), gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("ptr")), - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(ptr_type_id) + gimli::DW_AT_type = write::AttributeValue::UnitRef(ptr_type_id) }); add_tag!(deref_op_die_id, gimli::DW_TAG_formal_parameter => deref_op_this_param as deref_op_this_param_id { - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(wrapper_ptr_type_id), + gimli::DW_AT_type = write::AttributeValue::UnitRef(wrapper_ptr_type_id), gimli::DW_AT_artificial = write::AttributeValue::Flag(true) }); @@ -197,10 +197,10 @@ where add_tag!(wrapper_die_id, gimli::DW_TAG_subprogram => deref_op_die as deref_op_die_id { gimli::DW_AT_linkage_name = write::AttributeValue::StringRef(out_strings.add("resolve_vmctx_memory_ptr")), gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("operator*")), - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(ref_type_id) + gimli::DW_AT_type = write::AttributeValue::UnitRef(ref_type_id) }); add_tag!(deref_op_die_id, gimli::DW_TAG_formal_parameter => deref_op_this_param as deref_op_this_param_id { - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(wrapper_ptr_type_id), + gimli::DW_AT_type = write::AttributeValue::UnitRef(wrapper_ptr_type_id), gimli::DW_AT_artificial = write::AttributeValue::Flag(true) }); @@ -214,10 +214,10 @@ where add_tag!(wrapper_die_id, gimli::DW_TAG_subprogram => deref_op_die as deref_op_die_id { gimli::DW_AT_linkage_name = write::AttributeValue::StringRef(out_strings.add("resolve_vmctx_memory_ptr")), gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("operator->")), - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(ptr_type_id) + gimli::DW_AT_type = write::AttributeValue::UnitRef(ptr_type_id) }); add_tag!(deref_op_die_id, gimli::DW_TAG_formal_parameter => deref_op_this_param as deref_op_this_param_id { - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(wrapper_ptr_type_id), + gimli::DW_AT_type = write::AttributeValue::UnitRef(wrapper_ptr_type_id), gimli::DW_AT_artificial = write::AttributeValue::Flag(true) }); diff --git a/crates/debug/src/transform/utils.rs b/crates/debug/src/transform/utils.rs index f6a30ab21a..d8ad914290 100644 --- a/crates/debug/src/transform/utils.rs +++ b/crates/debug/src/transform/utils.rs @@ -56,7 +56,7 @@ pub(crate) fn add_internal_types( // .. DW_AT_type = add_tag!(root_id, gimli::DW_TAG_pointer_type => memory_bytes_die as memory_bytes_die_id { gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("u8*")), - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(memory_byte_die_id) + gimli::DW_AT_type = write::AttributeValue::UnitRef(memory_byte_die_id) }); // Create artificial VMContext type and its reference for convinience viewing @@ -87,7 +87,7 @@ pub(crate) fn add_internal_types( // .. DW_AT_data_member_location = `memory_offset` add_tag!(vmctx_die_id, gimli::DW_TAG_member => m_die as m_die_id { gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("memory")), - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(memory_bytes_die_id), + gimli::DW_AT_type = write::AttributeValue::UnitRef(memory_bytes_die_id), gimli::DW_AT_data_member_location = write::AttributeValue::Udata(memory_offset as u64) }); } @@ -102,7 +102,7 @@ pub(crate) fn add_internal_types( // .. DW_AT_type = add_tag!(root_id, gimli::DW_TAG_pointer_type => vmctx_ptr_die as vmctx_ptr_die_id { gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("WasmtimeVMContext*")), - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(vmctx_die_id) + gimli::DW_AT_type = write::AttributeValue::UnitRef(vmctx_die_id) }); // Build vmctx_die's DW_TAG_subprogram for `set` method: @@ -116,7 +116,7 @@ pub(crate) fn add_internal_types( gimli::DW_AT_name = write::AttributeValue::StringRef(out_strings.add("set")) }); add_tag!(vmctx_set_id, gimli::DW_TAG_formal_parameter => vmctx_set_this_param as vmctx_set_this_param_id { - gimli::DW_AT_type = write::AttributeValue::ThisUnitEntryRef(vmctx_ptr_die_id), + gimli::DW_AT_type = write::AttributeValue::UnitRef(vmctx_ptr_die_id), gimli::DW_AT_artificial = write::AttributeValue::Flag(true) }); @@ -157,7 +157,7 @@ pub(crate) fn append_vmctx_info( ); var_die.set( gimli::DW_AT_type, - write::AttributeValue::ThisUnitEntryRef(vmctx_die_id), + write::AttributeValue::UnitRef(vmctx_die_id), ); var_die.set(gimli::DW_AT_location, loc); diff --git a/crates/debug/src/write_debuginfo.rs b/crates/debug/src/write_debuginfo.rs index accdb17d40..8fe80f0ceb 100644 --- a/crates/debug/src/write_debuginfo.rs +++ b/crates/debug/src/write_debuginfo.rs @@ -1,6 +1,6 @@ use faerie::artifact::{Decl, SectionKind}; use faerie::*; -use gimli::write::{Address, DebugFrame, Dwarf, EndianVec, FrameTable, Result, Sections, Writer}; +use gimli::write::{Address, Dwarf, EndianVec, FrameTable, Result, Sections, Writer}; use gimli::{RunTimeEndian, SectionId}; #[derive(Clone)] @@ -30,6 +30,9 @@ pub fn emit_dwarf( let mut sections = Sections::new(WriterRelocate::new(endian, symbol_resolver)); dwarf.write(&mut sections)?; + if let Some(frames) = frames { + frames.write_debug_frame(&mut sections.debug_frame)?; + } sections.for_each_mut(|id, s| -> anyhow::Result<()> { artifact.declare_with( id.name(), @@ -55,29 +58,6 @@ pub fn emit_dwarf( Ok(()) })?; - if let Some(frames) = frames { - let mut debug_frame = DebugFrame::from(WriterRelocate::new(endian, symbol_resolver)); - frames.write_debug_frame(&mut debug_frame).unwrap(); - artifact.declare_with( - SectionId::DebugFrame.name(), - Decl::section(SectionKind::Debug), - debug_frame.writer.take(), - )?; - for reloc in &debug_frame.relocs { - artifact.link_with( - faerie::Link { - from: SectionId::DebugFrame.name(), - to: &reloc.name, - at: u64::from(reloc.offset), - }, - faerie::Reloc::Debug { - size: reloc.size, - addend: reloc.addend as i32, - }, - )?; - } - } - Ok(()) } diff --git a/crates/jit/Cargo.toml b/crates/jit/Cargo.toml index 822522da27..c7c38facd3 100644 --- a/crates/jit/Cargo.toml +++ b/crates/jit/Cargo.toml @@ -29,7 +29,7 @@ more-asserts = "0.2.1" anyhow = "1.0" cfg-if = "0.1.9" log = "0.4" -gimli = { version = "0.20.0", default-features = false, features = ["write"] } +gimli = { version = "0.21.0", default-features = false, features = ["write"] } [target.'cfg(target_os = "windows")'.dependencies] winapi = { version = "0.3.8", features = ["winnt", "impl-default"] } diff --git a/crates/profiling/Cargo.toml b/crates/profiling/Cargo.toml index 79b9d3f325..c18935ecde 100644 --- a/crates/profiling/Cargo.toml +++ b/crates/profiling/Cargo.toml @@ -13,7 +13,7 @@ edition = "2018" [dependencies] anyhow = "1.0" cfg-if = "0.1" -gimli = { version = "0.20.0", optional = true } +gimli = { version = "0.21.0", optional = true } lazy_static = "1.4" libc = { version = "0.2.60", default-features = false } scroll = { version = "0.10.1", optional = true }