Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

add symbol and module names to StackFrame #723

Merged
21 commits merged into from
Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from 6 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
51 changes: 37 additions & 14 deletions src/agent/debugger/src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
use std::{
fmt::{self, Display, Formatter},
hash::{Hash, Hasher},
path::Path,
};

use anyhow::Result;
Expand Down Expand Up @@ -86,13 +85,25 @@ pub enum DebugStackFrame {
Frame {
function: String,
location: DebugFunctionLocation,
bmc-msft marked this conversation as resolved.
Show resolved Hide resolved
symbol: Option<String>,
module_name: Option<String>,
},
CorruptFrame,
}

impl DebugStackFrame {
pub fn new(function: String, location: DebugFunctionLocation) -> DebugStackFrame {
DebugStackFrame::Frame { function, location }
pub fn new(
function: String,
bmc-msft marked this conversation as resolved.
Show resolved Hide resolved
location: DebugFunctionLocation,
symbol: Option<String>,
module_name: Option<String>,
) -> DebugStackFrame {
DebugStackFrame::Frame {
function,
location,
symbol,
module_name,
}
}

pub fn corrupt_frame() -> DebugStackFrame {
Expand All @@ -110,7 +121,9 @@ impl DebugStackFrame {
impl Display for DebugStackFrame {
fn fmt(&self, formatter: &mut Formatter) -> fmt::Result {
match self {
DebugStackFrame::Frame { function, location } => {
DebugStackFrame::Frame {
function, location, ..
} => {
if let Some(file_info) = &location.file_info {
write!(formatter, "{} {}", function, file_info)
} else {
Expand Down Expand Up @@ -182,14 +195,12 @@ fn get_function_location_in_module(
program_counter: u64,
inline_context: DWORD,
) -> DebugStackFrame {
let module_name = module_info.name().to_string_lossy().to_string();

if let Ok(sym_info) =
dbghlp.sym_from_inline_context(process_handle, program_counter, inline_context)
{
let function = format!(
"{}!{}",
Path::new(module_info.name()).display(),
sym_info.symbol()
);
let function = format!("{}!{}", module_name, sym_info.symbol());

let sym_line_info =
dbghlp.sym_get_file_and_line(process_handle, program_counter, inline_context);
Expand All @@ -204,12 +215,17 @@ fn get_function_location_in_module(
_ => DebugFunctionLocation::new(displacement),
};

DebugStackFrame::new(function, location)
DebugStackFrame::new(
function,
location,
Some(sym_info.symbol().to_owned()),
Some(module_name),
)
} else {
// No function - assume we have an exe with no pdb (so no exports). This should be
// common, so we won't report an error. We do want a nice(ish) location though.
let location = DebugFunctionLocation::new(program_counter - module_info.base_address());
DebugStackFrame::new(module_info.name().to_string_lossy().into(), location)
DebugStackFrame::new(module_name.clone(), location, None, Some(module_name))
}
}

Expand All @@ -227,7 +243,7 @@ fn get_frame_with_unknown_module(process_handle: HANDLE, program_counter: u64) -
.expect("logic error computing fake rva");

let location = DebugFunctionLocation::new(offset);
DebugStackFrame::new(UNKNOWN_MODULE.into(), location)
DebugStackFrame::new(UNKNOWN_MODULE.into(), location, None, None)
} else {
DebugStackFrame::corrupt_frame()
}
Expand Down Expand Up @@ -293,7 +309,12 @@ mod test {

macro_rules! frame {
($name: expr, disp: $disp: expr) => {
DebugStackFrame::new($name.to_string(), DebugFunctionLocation::new($disp))
DebugStackFrame::new(
$name.to_string(),
DebugFunctionLocation::new($disp),
None,
None,
)
};

($name: expr, disp: $disp: expr, line: ($file: expr, $line: expr)) => {
Expand All @@ -306,6 +327,8 @@ mod test {
line: $line,
},
),
None,
None,
)
};
}
Expand All @@ -321,7 +344,7 @@ mod test {

// Hard coded hash constant is what we want to ensure
// the hash function is relatively stable.
assert_eq!(stack.stable_hash(), 4643290346391834992);
assert_eq!(stack.stable_hash(), 7581155446307191472);
}

#[test]
Expand Down
19 changes: 14 additions & 5 deletions src/agent/onefuzz/src/input_tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,24 @@ impl<'a> Tester<'a> {
line: f.to_string(),
..Default::default()
},
debugger::stack::DebugStackFrame::Frame { function, location } => StackEntry {
debugger::stack::DebugStackFrame::Frame {
function,
location,
symbol,
module_name,
} => StackEntry {
line: f.to_string(),
function_name: Some(function.to_owned()), // TODO: this includes both the module & symbol
function_name: symbol.to_owned().or_else(|| Some(function.to_owned())),
address: Some(location.displacement),
module_offset: None,
module_path: None,
module_path: module_name.to_owned(),
bmc-msft marked this conversation as resolved.
Show resolved Hide resolved
source_file_line: location.file_info.as_ref().map(|x| x.line.into()),
source_file_name: location.file_info.as_ref().map(|x| x.file.to_string()),
source_file_path: None,
source_file_name: location
.file_info
.as_ref()
.map(|x| x.file.rsplit_terminator('\\').next().map(|x| x.to_owned()))
.flatten(),
source_file_path: location.file_info.as_ref().map(|x| x.file.to_string()),
function_offset: None,
},
})
Expand Down