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 11 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
66 changes: 43 additions & 23 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 @@ -84,15 +83,24 @@ impl Display for DebugFunctionLocation {
#[derive(Clone, Debug, Hash, PartialEq)]
pub enum DebugStackFrame {
Frame {
function: String,
module_name: String,
location: DebugFunctionLocation,
bmc-msft marked this conversation as resolved.
Show resolved Hide resolved
symbol: Option<String>,
},
CorruptFrame,
}

impl DebugStackFrame {
pub fn new(function: String, location: DebugFunctionLocation) -> DebugStackFrame {
DebugStackFrame::Frame { function, location }
pub fn new(
module_name: String,
location: DebugFunctionLocation,
symbol: Option<String>,
) -> DebugStackFrame {
DebugStackFrame::Frame {
module_name,
location,
symbol,
}
}

pub fn corrupt_frame() -> DebugStackFrame {
Expand All @@ -110,11 +118,18 @@ impl DebugStackFrame {
impl Display for DebugStackFrame {
fn fmt(&self, formatter: &mut Formatter) -> fmt::Result {
match self {
DebugStackFrame::Frame { function, location } => {
DebugStackFrame::Frame {
module_name, location, symbol
} => {
if let Some(symbol) = symbol {
write!(formatter, "{}!{}", module_name, symbol)?;
} else {
write!(formatter, "{}", module_name)?;
}
if let Some(file_info) = &location.file_info {
write!(formatter, "{} {}", function, file_info)
write!(formatter, " {}", file_info)
} else {
write!(formatter, "{}+0x{:x}", function, location.displacement)
write!(formatter, "+0x{:x}", location.displacement)
}
}
DebugStackFrame::CorruptFrame => formatter.write_str("<corrupt frame(s)>"),
Expand Down Expand Up @@ -145,7 +160,7 @@ impl DebugStack {
// Corrupted stacks and jit can result in stacks that vary from run to run, so we exclude
// those frames and anything below them for a more stable hash.
let first_unstable_frame = self.frames.iter().position(|f| match f {
DebugStackFrame::Frame { function, .. } => function == UNKNOWN_MODULE,
DebugStackFrame::Frame { module_name , .. } => module_name == UNKNOWN_MODULE,
DebugStackFrame::CorruptFrame => true,
});

Expand Down Expand Up @@ -182,15 +197,11 @@ 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 sym_line_info =
dbghlp.sym_get_file_and_line(process_handle, program_counter, inline_context);

Expand All @@ -204,12 +215,16 @@ fn get_function_location_in_module(
_ => DebugFunctionLocation::new(displacement),
};

DebugStackFrame::new(function, location)
DebugStackFrame::new(
module_name,
location,
Some(sym_info.symbol().to_owned()),
)
} 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, location, None)
}
}

Expand All @@ -227,7 +242,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)
} else {
DebugStackFrame::corrupt_frame()
}
Expand Down Expand Up @@ -292,20 +307,25 @@ mod test {
use super::*;

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

($name: expr, disp: $disp: expr, line: ($file: expr, $line: expr)) => {
($module: expr, disp: $location: expr, line: ($file: expr, $line: expr)) => {
DebugStackFrame::new(
$name.to_string(),
$module.to_string(),
DebugFunctionLocation::new_with_file_info(
$disp,
$location,
FileInfo {
file: $file.to_string(),
line: $line,
},
),
None,
)
};
}
Expand All @@ -321,7 +341,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(), 13075142555051297168);
}

#[test]
Expand Down
18 changes: 13 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,23 @@ impl<'a> Tester<'a> {
line: f.to_string(),
..Default::default()
},
debugger::stack::DebugStackFrame::Frame { function, location } => StackEntry {
debugger::stack::DebugStackFrame::Frame {
module_name,
location,
symbol,
} => StackEntry {
line: f.to_string(),
function_name: Some(function.to_owned()), // TODO: this includes both the module & symbol
function_name: symbol.to_owned(),
address: Some(location.displacement),
module_offset: None,
module_path: None,
module_path: Some(module_name.to_owned()),
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