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

Add error type wrapping all possible errors #122

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ gimli = "0.22.0"
capstone = "0.7.0"
addr2line = "0.13.0"
syntect = {version = "4.4.0", optional = true}
thiserror = "1.0.21"

# Dependencies specific to macOS & Linux
[target.'cfg(unix)'.dependencies]
Expand Down
4 changes: 2 additions & 2 deletions examples/gui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ mod example {
};
use headcrab::{
symbol::DisassemblySource, symbol::RelocatedDwarf, target::LinuxTarget, target::UnixTarget,
CrabResult,
CrabError, CrabResult,
};
use imgui::{im_str, ClipboardBackend, Condition, FontSource, ImStr, ImString};
use imgui_glium_renderer::Renderer;
Expand Down Expand Up @@ -135,7 +135,7 @@ mod example {
if let Some(remote) = &self.remote {
Ok(remote)
} else {
Err("No running process".to_string().into())
Err(CrabError::HeadCrab("No running process".to_string()))
}
}

Expand Down
45 changes: 28 additions & 17 deletions examples/repl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ mod example {
use headcrab::{
symbol::{DisassemblySource, RelocatedDwarf, Snippet},
target::{AttachOptions, LinuxTarget, UnixTarget},
CrabResult,
CrabError, CrabResult,
};

#[cfg(target_os = "linux")]
Expand Down Expand Up @@ -147,15 +147,15 @@ mod example {
if let Some(remote) = &self.remote {
Ok(remote)
} else {
Err("No running process".to_string().into())
Err(CrabError::HeadCrab("No running process".to_string()))
}
}

fn mut_remote(&mut self) -> CrabResult<&mut LinuxTarget> {
if let Some(remote) = &mut self.remote {
Ok(remote)
} else {
Err("No running process".to_string().into())
Err(CrabError::HeadCrab("No running process".to_string()))
}
}

Expand Down Expand Up @@ -308,7 +308,8 @@ mod example {
return Ok(());
}

let command = ReplCommand::from_str(command)?;
let command =
ReplCommand::from_str(command).map_err(|e| CrabError::HeadCrab(e.to_string()))?;
match command {
ReplCommand::Help(()) => {
ReplCommand::print_help(std::io::stdout(), color).unwrap();
Expand Down Expand Up @@ -349,11 +350,18 @@ mod example {
return print_source_for_top_of_stack_symbol(context, 3);
}
ReplCommand::Registers(sub_cmd) => match &*sub_cmd {
"" => Err(format!(
"Expected subcommand found nothing. Try `regs read`"
))?,
"" => {
return Err(CrabError::HeadCrab(
"Expected subcommand found nothing. Try `regs read`".into(),
));
}
"read" => println!("{:?}", context.remote()?.read_regs()?),
_ => Err(format!("Unknown `regs` subcommand `{}`", sub_cmd))?,
_ => {
return Err(CrabError::HeadCrab(format!(
"Unknown `regs` subcommand `{}`",
sub_cmd
)));
}
},
ReplCommand::Backtrace(sub_cmd) => {
return show_backtrace(context, &sub_cmd);
Expand Down Expand Up @@ -394,26 +402,28 @@ mod example {
if let Ok(addr) = {
usize::from_str_radix(&location, 10)
.map(|addr| addr as usize)
.map_err(|e| Box::new(e))
.map_err(CrabError::ParseInt)
.or_else(|_e| {
if location.starts_with("0x") {
let raw_num = location.trim_start_matches("0x");
usize::from_str_radix(raw_num, 16)
.map(|addr| addr as usize)
.map_err(|_e| Box::new(format!("Invalid address format.")))
.map_err(|_e| CrabError::HeadCrab("Invalid address format.".into()))
} else {
context
.debuginfo()
.get_symbol_address(&location)
.ok_or(Box::new(format!("No such symbol {}", location)))
.ok_or_else(|| {
CrabError::HeadCrab(format!("No such symbol {}", location))
})
}
})
} {
context.mut_remote()?.set_breakpoint(addr)?;
} else {
Err(format!(
"Breakpoints must be set on a symbol or at a given address. For example `b main` or `b 0x0000555555559394` or even `b 93824992252820`"
))?
return Err(CrabError::HeadCrab(
"Breakpoints must be set on a symbol or at a given address. For example `b main` or `b 0x0000555555559394` or even `b 93824992252820`".into()
));
}
Ok(())
}
Expand Down Expand Up @@ -508,9 +518,10 @@ mod example {
println!(" {} {}", name, location);
}

let (_dwarf, unit, dw_die_offset) = frame
.function_debuginfo()
.ok_or_else(|| "No dwarf debuginfo for function".to_owned())?;
let (_dwarf, unit, dw_die_offset) =
frame.function_debuginfo().ok_or_else(|| {
CrabError::HeadCrab("No dwarf debuginfo for function".to_owned())
})?;

let mut eval_ctx = X86_64EvalContext {
frame_base: None,
Expand Down
41 changes: 41 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
use thiserror::Error;

#[derive(Error, Debug)]
pub enum CrabError {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there should be a separate error type for target and one for symbol.

Copy link
Author

@DevinR528 DevinR528 Oct 11, 2020

Choose a reason for hiding this comment

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

Does there still need to be a top-level wrapper for pub type CrabResult<T> = Result<T, ErrorType>;? Something like

pub enum CrabError {
    Target(crate::target::TargetError),
    Symbol(crate::symbol::SymbolError),
}

Copy link
Author

Choose a reason for hiding this comment

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

Or should each mod (target and symbol) have their own Result type?

Copy link
Contributor

@bjorn3 bjorn3 Oct 12, 2020

Choose a reason for hiding this comment

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

Does there still need to be a top-level wrapper for pub type CrabResult = Result<T, ErrorType>;? Something like

That could be useful.

Or should each mod (target and symbol) have their own Result type?

Yes please.

#[error("Error occurred in headcrab: {0}")]
HeadCrab(String),

#[error("{0}")]
Dwarf(String),

#[error("Hardware breakpoint error: {0}")]
HardwareBP(#[from] crate::target::HardwareBreakpointError),

#[error("Error occurred with breakpoint: {0}")]
Breakpoint(#[from] crate::target::BreakpointError),

#[error("Error occurred during gmili parsing: {0}")]
Symbol(#[from] gimli::Error),

#[error("Error occurred while reading object file: {0}")]
Object(#[from] object::read::Error),

#[error("Error occurred while monitoring process: {0}")]
NixPtrace(#[from] nix::Error),

#[error("Error occurred while reading /proc: {0}")]
// NOTE: If a `ProcError::InternalError` is found this is a bug in the `procfs` crate.
ProcFs(#[from] procfs::ProcError),

#[error("{0}")]
StdIo(#[from] std::io::Error),

#[error("{0}")]
ParseInt(#[from] std::num::ParseIntError),

#[error("{0}")]
Utf8Error(#[from] std::str::Utf8Error),

#[error("{0}")]
FfiNull(#[from] std::ffi::NulError),
}
5 changes: 4 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
//! Headcrab, a modern Rust debugging library.

pub type CrabResult<T> = Result<T, Box<dyn std::error::Error + Send + Sync + Sync>>;
pub type CrabResult<T> = Result<T, error::CrabError>;

pub mod error;
/// Functions to work with target processes: reading & writing memory, process control functions, etc.
pub mod target;

/// Symbolication layer.
#[cfg(unix)]
pub mod symbol;

pub use error::CrabError;
28 changes: 18 additions & 10 deletions src/symbol/dwarf_utils.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use gimli::{DebuggingInformationEntry, Dwarf, Unit, UnitOffset, ValueType};

use super::Reader;
use crate::CrabResult;
use crate::{CrabError, CrabResult};

macro_rules! dwarf_attr {
(str($dwarf:ident,$unit:ident) $entry:ident.$name:ident || $missing:ident) => {
Expand All @@ -18,12 +18,11 @@ macro_rules! dwarf_attr {
dwarf_attr_exists_action_action!($missing, unit_ref)
}
val => {
return Err(format!(
return Err(crate::error::CrabError::Dwarf(format!(
"`{:?}` is not a valid value for a {} attribute",
val,
gimli::$name,
)
.into())
)))
}
}
} else {
Expand All @@ -35,7 +34,10 @@ macro_rules! dwarf_attr {
dwarf_attr_exists_action_action!(
$missing,
attr.udata_value()
.ok_or(concat!("invalid value for", stringify!($name)))?
.ok_or(crate::error::CrabError::Dwarf(format!(
"invalid value for {}",
stringify!($name)
)))?
)
} else {
dwarf_attr_missing_action!($missing, $name)
Expand All @@ -48,9 +50,11 @@ macro_rules! dwarf_attr {
dwarf_attr_exists_action_action!($missing, encoding)
}
encoding => {
return Err(
format!("invalid value for {}: {:?}", gimli::$name, encoding).into(),
)
return Err(crate::error::CrabError::Dwarf(format!(
"invalid value for {}: {:?}",
gimli::$name,
encoding
)))
}
}
} else {
Expand Down Expand Up @@ -83,7 +87,10 @@ macro_rules! dwarf_attr_missing_action {
continue;
};
(error, $name:ident) => {
return Err(concat!("missing ", stringify!($name), " attribute").into());
return Err(crate::error::CrabError::Dwarf(format!(
"missing {} attribute",
stringify!($name),
)));
};
(None, $name:ident) => {
None
Expand Down Expand Up @@ -181,7 +188,8 @@ fn value_type_from_base_type(
Ok(ValueType::Generic)
} else {
let base_type_die = unit.entry(base_type)?;
Ok(ValueType::from_entry(&base_type_die)?.ok_or_else(|| "not a base type".to_owned())?)
Ok(ValueType::from_entry(&base_type_die)?
.ok_or_else(|| CrabError::Dwarf("not a base type".to_owned()))?)
}
}

Expand Down
35 changes: 23 additions & 12 deletions src/symbol/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::fmt;

use super::dwarf_utils::SearchAction;
use super::*;
use crate::{CrabError, CrabResult};

pub struct Frame<'a> {
dwarf: &'a gimli::Dwarf<Reader<'a>>,
Expand Down Expand Up @@ -68,7 +69,7 @@ impl<'a> Frame<'a> {
) -> CrabResult<()> {
let (dwarf, unit, dw_die_offset) = self
.function_debuginfo()
.ok_or_else(|| "No dwarf debuginfo for function".to_owned())?;
.ok_or_else(|| CrabError::Dwarf("No dwarf debuginfo for function".to_owned()))?;

dwarf_utils::search_tree(unit, Some(dw_die_offset), |entry, _indent| {
if entry.offset() == dw_die_offset {
Expand All @@ -95,7 +96,7 @@ impl<'a> Frame<'a> {
) -> CrabResult<()> {
let (dwarf, unit, dw_die_offset) = self
.function_debuginfo()
.ok_or_else(|| "No dwarf debuginfo for function".to_owned())?;
.ok_or_else(|| CrabError::Dwarf("No dwarf debuginfo for function".to_owned()))?;
dwarf_utils::search_tree(unit, Some(dw_die_offset), |entry, _indent| {
if entry.tag() == gimli::DW_TAG_inlined_subroutine && entry.offset() != dw_die_offset {
return Ok(SearchAction::SkipChildren); // Already visited by addr2line frame iter
Expand Down Expand Up @@ -162,21 +163,27 @@ impl<'ctx> LocalValue<'ctx> {
match encoding {
gimli::DW_ATE_unsigned | gimli::DW_ATE_signed => {
let size = u8::try_from(size).map_err(|_| {
format!("`{}` is too big for DW_ATE_unsigned or DW_ATE_signed", size,)
CrabError::Dwarf(format!(
"`{}` is too big for DW_ATE_unsigned or DW_ATE_signed",
size
))
})?;
let data = match self {
LocalValue::Pieces(pieces) => {
if pieces.len() != 1 {
return Err("too many pieces for an integer value".into());
return Err(CrabError::Dwarf(
"too many pieces for an integer value".into(),
));
}

if pieces[0].size_in_bits.is_none()
|| pieces[0].size_in_bits.unwrap() == u64::from(size) * 8
{
} else {
return Err(
format!("wrong size for piece {:?}", pieces[0]).into()
);
return Err(CrabError::Dwarf(format!(
"wrong size for piece {:?}",
pieces[0]
)));
}
// FIXME handle this
assert!(
Expand All @@ -187,7 +194,9 @@ impl<'ctx> LocalValue<'ctx> {

let value = match &pieces[0].location {
gimli::Location::Empty => {
return Err("found empty piece for an integer value".into())
return Err(CrabError::Dwarf(
"found empty piece for an integer value".into(),
))
}
gimli::Location::Register { register } => {
eval_ctx.register(
Expand All @@ -207,9 +216,9 @@ impl<'ctx> LocalValue<'ctx> {
value: _,
byte_offset: _,
} => {
return Err(
"found implicit pointer for an integer value".into()
)
return Err(CrabError::Dwarf(
"found implicit pointer for an integer value".into(),
))
}
};
match value {
Expand All @@ -223,7 +232,9 @@ impl<'ctx> LocalValue<'ctx> {
gimli::Value::I64(data) => data as u64,
gimli::Value::U64(data) => data,
gimli::Value::F32(_) | gimli::Value::F64(_) => {
return Err("found float piece for an integer value".into())
return Err(CrabError::Dwarf(
"found float piece for an integer value".into(),
))
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/symbol/relocate.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::*;
use crate::CrabError;

pub struct RelocatedDwarf(Vec<RelocatedDwarfEntry>);

Expand Down Expand Up @@ -56,11 +57,11 @@ impl RelocatedDwarfEntry {
})
})
.ok_or_else(|| {
format!(
CrabError::HeadCrab(format!(
"Couldn't find segment for `{}`+0x{:x}",
file.display(),
offset
)
))
})?;
Ok(RelocatedDwarfEntry {
address_range: address,
Expand Down
Loading