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

ref(debuginfo): Switch to error with kind #299

Merged
merged 11 commits into from
Nov 30, 2020
34 changes: 3 additions & 31 deletions symbolic-cabi/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,42 +247,14 @@ impl SymbolicErrorCode {
}

use symbolic::debuginfo::{
dwarf::DwarfError, ObjectError, UnknownFileFormatError, UnknownObjectKindError,
ObjectError, UnknownFileFormatError, UnknownObjectKindError,
};
if error.downcast_ref::<UnknownObjectKindError>().is_some() {
return SymbolicErrorCode::UnknownObjectKindError;
} else if error.downcast_ref::<UnknownFileFormatError>().is_some() {
return SymbolicErrorCode::UnknownFileFormatError;
} else if let Some(error) = error.downcast_ref::<ObjectError>() {
return match error {
ObjectError::UnsupportedObject => {
SymbolicErrorCode::ObjectErrorUnsupportedObject
}
ObjectError::Breakpad(_) => SymbolicErrorCode::ObjectErrorBadBreakpadObject,
ObjectError::Elf(_) => SymbolicErrorCode::ObjectErrorBadElfObject,
ObjectError::MachO(_) => SymbolicErrorCode::ObjectErrorBadMachOObject,
ObjectError::Pdb(_) => SymbolicErrorCode::ObjectErrorBadPdbObject,
ObjectError::Pe(_) => SymbolicErrorCode::ObjectErrorBadPeObject,
ObjectError::Wasm(_) => SymbolicErrorCode::ObjectErrorBadWasmObject,
ObjectError::Dwarf(ref e) => match e {
DwarfError::InvalidUnitRef(_) => {
SymbolicErrorCode::DwarfErrorInvalidUnitRef
}
DwarfError::InvalidFileRef(_) => {
SymbolicErrorCode::DwarfErrorInvalidFileRef
}
DwarfError::UnexpectedInline => {
SymbolicErrorCode::DwarfErrorUnexpectedInline
}
DwarfError::InvertedFunctionRange => {
SymbolicErrorCode::DwarfErrorInvertedFunctionRange
}
DwarfError::CorruptedData(_) => SymbolicErrorCode::DwarfErrorCorruptedData,
_ => SymbolicErrorCode::DwarfErrorUnknown,
},
ObjectError::SourceBundle(_) => SymbolicErrorCode::ObjectErrorBadSourceBundle,
_ => SymbolicErrorCode::ObjectErrorUnknown,
};
} else if error.downcast_ref::<ObjectError>().is_some() {
return SymbolicErrorCode::ObjectErrorUnknown;
}

use symbolic::minidump::cfi::{CfiError, CfiErrorKind};
Expand Down
96 changes: 74 additions & 22 deletions symbolic-debuginfo/src/breakpad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use std::borrow::Cow;
use std::collections::BTreeMap;
use std::error::Error;
use std::fmt;
use std::str;

Expand Down Expand Up @@ -32,27 +33,78 @@ const BREAKPAD_HEADER_CAP: usize = 320;
/// Placeholder used for missing function or symbol names.
const UNKNOWN_NAME: &str = "<unknown>";

/// An error when dealing with [`BreakpadObject`](struct.BreakpadObject.html).
/// The error type for [`BreakpadError`].
#[non_exhaustive]
#[derive(Debug, Error)]
pub enum BreakpadError {
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum BreakpadErrorKind {
/// The symbol header (`MODULE` record) is missing.
#[error("missing breakpad symbol header")]
InvalidMagic,

/// A part of the file is not encoded in valid UTF-8.
#[error("bad utf-8 sequence")]
BadEncoding(#[from] str::Utf8Error),
BadEncoding,

/// A record violates the Breakpad symbol syntax.
#[error(transparent)]
BadSyntax(#[from] pest::error::Error<Rule>),
BadSyntax,

/// Parsing of a record failed.
#[error("{0}")]
Parse(&'static str),
}

impl fmt::Display for BreakpadErrorKind {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::InvalidMagic => write!(f, "missing breakpad symbol header"),
Self::BadEncoding => write!(f, "bad utf-8 sequence"),
Self::BadSyntax => write!(f, "invalid syntax"),
Self::Parse(field) => write!(f, "{}", field),
}
}
}

/// An error when dealing with [`BreakpadObject`](struct.BreakpadObject.html).
#[derive(Debug, Error)]
#[error("{kind}")]
pub struct BreakpadError {
kind: BreakpadErrorKind,
#[source]
source: Option<Box<dyn Error + Send + Sync + 'static>>,
}

impl BreakpadError {
/// Creates a new Breakpad error from a known kind of error as well as an arbitrary error
/// payload.
fn new<E>(kind: BreakpadErrorKind, source: E) -> Self
where
E: Into<Box<dyn Error + Send + Sync>>,
{
let source = Some(source.into());
Self { kind, source }
}

/// Returns the corresponding [`BreakpadErrorKind`] for this error.
pub fn kind(&self) -> BreakpadErrorKind {
self.kind
}
}

impl From<BreakpadErrorKind> for BreakpadError {
fn from(kind: BreakpadErrorKind) -> Self {
Self { kind, source: None }
}
}

impl From<str::Utf8Error> for BreakpadError {
fn from(e: str::Utf8Error) -> Self {
Self::new(BreakpadErrorKind::BadEncoding, e)
}
}

impl From<pest::error::Error<Rule>> for BreakpadError {
fn from(e: pest::error::Error<Rule>) -> Self {
Self::new(BreakpadErrorKind::BadSyntax, e)
}
}

// TODO(ja): Test the parser

/// A [module record], constituting the header of a Breakpad file.
Expand Down Expand Up @@ -134,7 +186,7 @@ impl<'d> BreakpadInfoRecord<'d> {
}
}

Err(BreakpadError::Parse("unknown INFO record"))
Err(BreakpadErrorKind::Parse("unknown INFO record").into())
}

fn code_info_from_pair(pair: pest::iterators::Pair<'d, Rule>) -> Result<Self, BreakpadError> {
Expand Down Expand Up @@ -230,7 +282,7 @@ impl<'d> BreakpadFileRecord<'d> {
match pair.as_rule() {
Rule::file_id => {
record.id = u64::from_str_radix(pair.as_str(), 10)
.map_err(|_| BreakpadError::Parse("file identifier"))?;
.map_err(|_| BreakpadErrorKind::Parse("file identifier"))?;
}
Rule::name => record.name = pair.as_str(),
_ => unreachable!(),
Expand Down Expand Up @@ -306,11 +358,11 @@ impl<'d> BreakpadPublicRecord<'d> {
Rule::multiple => record.multiple = true,
Rule::addr => {
record.address = u64::from_str_radix(pair.as_str(), 16)
.map_err(|_| BreakpadError::Parse("symbol address"))?;
.map_err(|_| BreakpadErrorKind::Parse("symbol address"))?;
}
Rule::param_size => {
record.parameter_size = u64::from_str_radix(pair.as_str(), 16)
.map_err(|_| BreakpadError::Parse("symbol parameter size"))?;
.map_err(|_| BreakpadErrorKind::Parse("symbol parameter size"))?;
}
Rule::name => record.name = pair.as_str(),
_ => unreachable!(),
Expand Down Expand Up @@ -395,15 +447,15 @@ impl<'d> BreakpadFuncRecord<'d> {
Rule::multiple => record.multiple = true,
Rule::addr => {
record.address = u64::from_str_radix(pair.as_str(), 16)
.map_err(|_| BreakpadError::Parse("function address"))?;
.map_err(|_| BreakpadErrorKind::Parse("function address"))?;
}
Rule::size => {
record.size = u64::from_str_radix(pair.as_str(), 16)
.map_err(|_| BreakpadError::Parse("function size"))?;
.map_err(|_| BreakpadErrorKind::Parse("function size"))?;
}
Rule::param_size => {
record.parameter_size = u64::from_str_radix(pair.as_str(), 16)
.map_err(|_| BreakpadError::Parse("function parameter size"))?;
.map_err(|_| BreakpadErrorKind::Parse("function parameter size"))?;
}
Rule::name => record.name = pair.as_str(),
_ => unreachable!(),
Expand Down Expand Up @@ -518,23 +570,23 @@ impl BreakpadLineRecord {
match pair.as_rule() {
Rule::addr => {
record.address = u64::from_str_radix(pair.as_str(), 16)
.map_err(|_| BreakpadError::Parse("line address"))?;
.map_err(|_| BreakpadErrorKind::Parse("line address"))?;
}
Rule::size => {
record.size = u64::from_str_radix(pair.as_str(), 16)
.map_err(|_| BreakpadError::Parse("line size"))?;
.map_err(|_| BreakpadErrorKind::Parse("line size"))?;
}
Rule::line_num => {
// NB: Breakpad does not allow negative line numbers and even tests that the
// symbol parser rejects such line records. However, negative line numbers have
// been observed at least for ELF files, so handle them gracefully.
record.line = i32::from_str_radix(pair.as_str(), 10)
.map(|line| u64::from(line as u32))
.map_err(|_| BreakpadError::Parse("line number"))?;
.map_err(|_| BreakpadErrorKind::Parse("line number"))?;
}
Rule::file_id => {
record.file_id = u64::from_str_radix(pair.as_str(), 10)
.map_err(|_| BreakpadError::Parse("file number"))?;
.map_err(|_| BreakpadErrorKind::Parse("file number"))?;
}
_ => unreachable!(),
}
Expand Down Expand Up @@ -768,11 +820,11 @@ impl<'data> BreakpadObject<'data> {
id: module
.id
.parse()
.map_err(|_| BreakpadError::Parse("module id"))?,
.map_err(|_| BreakpadErrorKind::Parse("module id"))?,
arch: module
.arch
.parse()
.map_err(|_| BreakpadError::Parse("module architecture"))?,
.map_err(|_| BreakpadErrorKind::Parse("module architecture"))?,
module,
data,
})
Expand Down
74 changes: 61 additions & 13 deletions symbolic-debuginfo/src/dwarf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//! [`MachObject`]: ../macho/struct.MachObject.html

use std::borrow::Cow;
use std::error::Error;
use std::fmt;
use std::marker::PhantomData;
use std::ops::{Deref, RangeBounds};
Expand Down Expand Up @@ -51,29 +52,76 @@ fn offset(addr: u64, offset: i64) -> u64 {
(addr as i64).wrapping_sub(offset as i64) as u64
}

/// An error handling [`DWARF`](trait.Dwarf.html) debugging information.
/// The error type for [`DwarfError`].
#[non_exhaustive]
#[derive(Debug, Error)]
pub enum DwarfError {
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum DwarfErrorKind {
/// A compilation unit referenced by index does not exist.
#[error("compilation unit for offset {0} does not exist")]
InvalidUnitRef(usize),

/// A file record referenced by index does not exist.
#[error("referenced file {0} does not exist")]
InvalidFileRef(u64),

/// An inline record was encountered without an inlining parent.
#[error("unexpected inline function without parent")]
UnexpectedInline,

/// The debug_ranges of a function are invalid.
#[error("function with inverted address range")]
InvertedFunctionRange,

/// The DWARF file is corrupted. See the cause for more information.
#[error("corrupted dwarf debug data")]
CorruptedData(#[from] GimliError),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GimliError was exposed because of the pub usage here. Maybe we can hide all the gimli re-exports again?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I'll create a follow-up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CorruptedData,
}

impl fmt::Display for DwarfErrorKind {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::InvalidUnitRef(offset) => {
write!(f, "compilation unit for offset {} does not exist", offset)
}
Self::InvalidFileRef(id) => write!(f, "referenced file {} does not exist", id),
Self::UnexpectedInline => write!(f, "unexpected inline function without parent"),
Self::InvertedFunctionRange => write!(f, "function with inverted address range"),
Self::CorruptedData => write!(f, "corrupted dwarf debug data"),
}
}
}

/// An error handling [`DWARF`](trait.Dwarf.html) debugging information.
#[derive(Debug, Error)]
#[error("{kind}")]
pub struct DwarfError {
kind: DwarfErrorKind,
#[source]
source: Option<Box<dyn Error + Send + Sync + 'static>>,
}

impl DwarfError {
/// Creates a new DWARF error from a known kind of error as well as an arbitrary error
/// payload.
fn new<E>(kind: DwarfErrorKind, source: E) -> Self
where
E: Into<Box<dyn Error + Send + Sync>>,
{
let source = Some(source.into());
Self { kind, source }
}

/// Returns the corresponding [`DwarfErrorKind`] for this error.
pub fn kind(&self) -> DwarfErrorKind {
self.kind
}
}

impl From<DwarfErrorKind> for DwarfError {
fn from(kind: DwarfErrorKind) -> Self {
Self { kind, source: None }
}
}

impl From<GimliError> for DwarfError {
fn from(e: GimliError) -> Self {
Self::new(DwarfErrorKind::CorruptedData, e)
}
}

/// DWARF section information including its data.
Expand Down Expand Up @@ -525,7 +573,7 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {

if low_pc > high_pc {
// TODO: consider swallowing errors here?
return Err(DwarfError::InvertedFunctionRange);
return Err(DwarfErrorKind::InvertedFunctionRange.into());
}

range_buf.push(Range {
Expand Down Expand Up @@ -737,7 +785,7 @@ impl<'d, 'a> DwarfUnit<'d, 'a> {
// indicates invalid debug information.
let parent = match stack.peek_mut() {
Some(parent) => parent,
None => return Err(DwarfError::UnexpectedInline),
None => return Err(DwarfErrorKind::UnexpectedInline.into()),
};

// Make sure there is correct line information for the call site of this inlined
Expand Down Expand Up @@ -1076,7 +1124,7 @@ impl<'d> DwarfInfo<'d> {

let index = match search_result {
Ok(index) => index,
Err(0) => return Err(DwarfError::InvalidUnitRef(offset.0)),
Err(0) => return Err(DwarfErrorKind::InvalidUnitRef(offset.0).into()),
Err(next_index) => next_index - 1,
};

Expand All @@ -1087,7 +1135,7 @@ impl<'d> DwarfInfo<'d> {
}
}

Err(DwarfError::InvalidUnitRef(offset.0))
Err(DwarfErrorKind::InvalidUnitRef(offset.0).into())
}

/// Returns an iterator over all compilation units.
Expand Down