Simplify types used in stack walking (#657)

* Fix test build
* Unify stack frame types
* Always include code offset within a function
* Run cargo test --workspace in CI
This commit is contained in:
Jason Shirk 2021-03-10 23:36:03 -08:00 committed by GitHub
parent 277725776d
commit aa83f20ad8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 110 additions and 155 deletions

View File

@ -103,6 +103,18 @@ macro_rules! from_json {
}};
}
#[cfg(target_os = "windows")]
const EXE: &str = r"c:\bin\fuzz.exe";
#[cfg(target_os = "linux")]
const EXE: &str = "/bin/fuzz.exe";
#[cfg(target_os = "windows")]
const LIB: &str = r"c:\lib\libpthread.dll";
#[cfg(target_os = "linux")]
const LIB: &str = "/lib/libpthread.so.0";
fn module(s: &str) -> ModulePath {
ModulePath::new(s.into()).unwrap()
}
@ -112,7 +124,8 @@ fn test_cmd_filter_empty_def() {
let filter = from_json!([]);
// All modules and symbols are included by default.
let exe = module("/bin/fuzz.exe");
let exe = module(EXE);
assert!(filter.includes_module(&exe));
assert!(filter.includes_symbol(&exe, "main"));
assert!(filter.includes_symbol(&exe, "_start"));
@ -120,7 +133,7 @@ fn test_cmd_filter_empty_def() {
assert!(filter.includes_symbol(&exe, "__asan_memcpy"));
assert!(filter.includes_symbol(&exe, "__asan_load8"));
let lib = module("/lib/libpthread.so.0");
let lib = module(LIB);
assert!(filter.includes_module(&lib));
assert!(filter.includes_symbol(&lib, "pthread_join"));
assert!(filter.includes_symbol(&lib, "pthread_yield"));
@ -136,7 +149,7 @@ fn test_cmd_filter_module_include_list() {
]);
// The filtered module and its matching symbols are included.
let exe = module("/bin/fuzz.exe");
let exe = module(EXE);
assert!(filter.includes_module(&exe));
assert!(!filter.includes_symbol(&exe, "_start"));
assert!(filter.includes_symbol(&exe, "main"));
@ -145,7 +158,7 @@ fn test_cmd_filter_module_include_list() {
assert!(!filter.includes_symbol(&exe, "__asan_load8"));
// Other modules and their symbols are included by default.
let lib = module("/lib/libpthread.so.0");
let lib = module(LIB);
assert!(filter.includes_module(&lib));
assert!(filter.includes_symbol(&lib, "pthread_join"));
assert!(filter.includes_symbol(&lib, "pthread_yield"));
@ -163,7 +176,7 @@ fn test_cmd_filter_exclude_list() {
]);
// The filtered module is included, and its matching symbols are excluded.
let exe = module("/bin/fuzz.exe");
let exe = module(EXE);
assert!(filter.includes_module(&exe));
assert!(!filter.includes_symbol(&exe, "_start"));
assert!(filter.includes_symbol(&exe, "main"));
@ -173,7 +186,7 @@ fn test_cmd_filter_exclude_list() {
assert!(!filter.includes_symbol(&exe, "_start"));
// Other modules and their symbols are included by default.
let lib = module("/lib/libpthread.so.0");
let lib = module(LIB);
assert!(filter.includes_module(&lib));
assert!(filter.includes_symbol(&lib, "pthread_join"));
assert!(filter.includes_symbol(&lib, "pthread_yield"));
@ -197,7 +210,7 @@ fn test_cmd_filter_include_list_and_exclude_default() {
]);
// The filtered module is included, and only matching rules are included.
let exe = module("/bin/fuzz.exe");
let exe = module(EXE);
assert!(filter.includes_module(&exe));
assert!(!filter.includes_symbol(&exe, "_start"));
assert!(filter.includes_symbol(&exe, "main"));
@ -206,7 +219,7 @@ fn test_cmd_filter_include_list_and_exclude_default() {
assert!(!filter.includes_symbol(&exe, "__asan_load8"));
// Other modules and their symbols are excluded by default.
let lib = module("/lib/libpthread.so.0");
let lib = module(LIB);
assert!(!filter.includes_module(&lib));
assert!(!filter.includes_symbol(&lib, "pthread_yield"));
assert!(!filter.includes_symbol(&lib, "pthread_join"));

View File

@ -14,12 +14,33 @@ use serde::{Serialize, Serializer};
use win_util::memory;
use winapi::{shared::minwindef::DWORD, um::winnt::HANDLE};
use crate::dbghelp::{self, DebugHelpGuard, ModuleInfo};
use crate::dbghelp::{self, DebugHelpGuard, ModuleInfo, SymLineInfo};
const UNKNOWN_MODULE: &str = "<UnknownModule>";
/// The file and line number for frames in the call stack.
#[derive(Clone, Debug, Hash, PartialEq)]
pub enum DebugFunctionLocation {
pub struct FileInfo {
pub file: String,
pub line: u32,
}
impl Display for FileInfo {
fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result {
write!(formatter, "{}:{}", self.file, self.line)
}
}
impl From<&SymLineInfo> for FileInfo {
fn from(sym_line_info: &SymLineInfo) -> Self {
let file = sym_line_info.filename().to_string_lossy().into();
let line = sym_line_info.line_number();
FileInfo { file, line }
}
}
#[derive(Clone, Debug, Hash, PartialEq)]
pub struct DebugFunctionLocation {
/// File/line if available
///
/// Should be stable - ASLR and JIT should not change source position,
@ -27,18 +48,35 @@ pub enum DebugFunctionLocation {
///
/// We mitigate this loss of precision by collecting multiple samples
/// for the same hash bucket.
Line { file: String, line: u32 },
pub file_info: Option<FileInfo>,
/// Offset if line information not available.
Offset { disp: u64 },
pub displacement: u64,
}
impl DebugFunctionLocation {
pub fn new(displacement: u64) -> Self {
DebugFunctionLocation {
displacement,
file_info: None,
}
}
pub fn new_with_file_info(displacement: u64, file_info: FileInfo) -> Self {
DebugFunctionLocation {
displacement,
file_info: Some(file_info),
}
}
}
impl Display for DebugFunctionLocation {
fn fmt(&self, formatter: &mut Formatter) -> fmt::Result {
match self {
DebugFunctionLocation::Line { file, line } => write!(formatter, "{}:{}", file, line)?,
DebugFunctionLocation::Offset { disp } => write!(formatter, "0x{:x}", disp)?,
};
if let Some(file_info) = &self.file_info {
write!(formatter, "{}", file_info)?;
} else {
write!(formatter, "0x{:x}", self.displacement)?;
}
Ok(())
}
}
@ -72,14 +110,13 @@ impl DebugStackFrame {
impl Display for DebugStackFrame {
fn fmt(&self, formatter: &mut Formatter) -> fmt::Result {
match self {
DebugStackFrame::Frame { function, location } => match location {
DebugFunctionLocation::Line { file, line } => {
write!(formatter, "{} {}:{}", function, file, line)
DebugStackFrame::Frame { function, location } => {
if let Some(file_info) = &location.file_info {
write!(formatter, "{} {}", function, file_info)
} else {
write!(formatter, "{}+0x{:x}", function, location.displacement)
}
DebugFunctionLocation::Offset { disp } => {
write!(formatter, "{}+0x{:x}", function, disp)
}
},
}
DebugStackFrame::CorruptFrame => formatter.write_str("<corrupt frame(s)>"),
}
}
@ -96,7 +133,7 @@ impl Serialize for DebugStackFrame {
#[derive(Debug, PartialEq, Serialize)]
pub struct DebugStack {
frames: Vec<DebugStackFrame>,
pub frames: Vec<DebugStackFrame>,
}
impl DebugStack {
@ -104,10 +141,6 @@ impl DebugStack {
DebugStack { frames }
}
pub fn frames(&self) -> &[DebugStackFrame] {
&self.frames
}
pub fn stable_hash(&self) -> u64 {
// 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.
@ -131,7 +164,7 @@ impl DebugStack {
impl Display for DebugStack {
fn fmt(&self, formatter: &mut Formatter) -> fmt::Result {
let mut first = true;
for frame in self.frames() {
for frame in &self.frames {
if !first {
writeln!(formatter)?;
}
@ -161,27 +194,21 @@ fn get_function_location_in_module(
let sym_line_info =
dbghlp.sym_get_file_and_line(process_handle, program_counter, inline_context);
let displacement = sym_info.displacement();
let location = match sym_line_info {
// Don't use file/line for these magic line numbers.
Ok(ref sym_line_info) if !sym_line_info.is_fake_line_number() => {
DebugFunctionLocation::Line {
file: sym_line_info.filename().to_string_lossy().into(),
line: sym_line_info.line_number(),
}
DebugFunctionLocation::new_with_file_info(displacement, sym_line_info.into())
}
_ => DebugFunctionLocation::Offset {
disp: sym_info.displacement(),
},
_ => DebugFunctionLocation::new(displacement),
};
DebugStackFrame::new(function, location)
} 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::Offset {
disp: program_counter - module_info.base_address(),
};
let location = DebugFunctionLocation::new(program_counter - module_info.base_address());
DebugStackFrame::new(module_info.name().to_string_lossy().into(), location)
}
}
@ -199,7 +226,7 @@ fn get_frame_with_unknown_module(process_handle: HANDLE, program_counter: u64) -
.checked_sub(mi.base_address())
.expect("logic error computing fake rva");
let location = DebugFunctionLocation::Offset { disp: offset };
let location = DebugFunctionLocation::new(offset);
DebugStackFrame::new(UNKNOWN_MODULE.into(), location)
} else {
DebugStackFrame::corrupt_frame()
@ -266,19 +293,19 @@ mod test {
macro_rules! frame {
($name: expr, disp: $disp: expr) => {
DebugStackFrame::new(
$name.to_string(),
DebugFunctionLocation::Offset { disp: $disp },
)
DebugStackFrame::new($name.to_string(), DebugFunctionLocation::new($disp))
};
($name: expr, line: ($file: expr, $line: expr)) => {
($name: expr, disp: $disp: expr, line: ($file: expr, $line: expr)) => {
DebugStackFrame::new(
$name.to_string(),
DebugFunctionLocation::Line {
file: $file.to_string(),
line: $line,
},
DebugFunctionLocation::new_with_file_info(
$disp,
FileInfo {
file: $file.to_string(),
line: $line,
},
),
)
};
}
@ -287,21 +314,22 @@ mod test {
fn stable_stack_hash() {
let frames = vec![
frame!("ntdll", disp: 88442200),
frame!("usage", line: ("foo.c", 88)),
frame!("main", line: ("foo.c", 42)),
frame!("usage", disp: 10, line: ("foo.c", 88)),
frame!("main", disp: 20, line: ("foo.c", 42)),
];
let stack = DebugStack::new(frames);
// Hard coded hash constant is what we want to ensure the hash function is stable.
assert_eq!(stack.stable_hash(), 8083364444338290471);
// Hard coded hash constant is what we want to ensure
// the hash function is relatively stable.
assert_eq!(stack.stable_hash(), 4643290346391834992);
}
#[test]
fn stable_hash_ignore_jit() {
let mut frames = vec![
frame!("ntdll", disp: 88442200),
frame!("usage", line: ("foo.c", 88)),
frame!("main", line: ("foo.c", 42)),
frame!("usage", disp: 10, line: ("foo.c", 88)),
frame!("main", disp: 20, line: ("foo.c", 42)),
];
let base_frames = frames.clone();
@ -318,8 +346,8 @@ mod test {
fn stable_hash_assuming_stack_corrupted() {
let mut frames = vec![
frame!("ntdll", disp: 88442200),
frame!("usage", line: ("foo.c", 88)),
frame!("main", line: ("foo.c", 42)),
frame!("usage", disp: 10, line: ("foo.c", 88)),
frame!("main", disp: 20, line: ("foo.c", 42)),
];
let base_frames = frames.clone();

View File

@ -423,6 +423,7 @@ mod tests {
test_process(
r"C:\windows\system32\WindowsPowerShell\v1.0\powershell.exe",
&vec!["/nop".to_string(), "/c".to_string(), $script.to_string()],
&HashMap::default(),
$timeout,
/*ignore first chance exceptions*/ true,
None,

View File

@ -10,7 +10,7 @@ pub mod verifier_stop;
use std::{fmt, path::Path};
use debugger::stack;
use debugger::stack::{DebugStack, DebugStackFrame};
use log::error;
use win_util::process;
use winapi::um::{
@ -169,14 +169,15 @@ pub fn new_exception_description(
pub fn new_exception(
process_handle: HANDLE,
exception: &EXCEPTION_DEBUG_INFO,
stack: stack::DebugStack,
stack: DebugStack,
) -> Exception {
let stack_hash = stack.stable_hash();
Exception {
exception_code: exception.ExceptionRecord.ExceptionCode,
description: new_exception_description(process_handle, &exception.ExceptionRecord),
stack_hash: stack.stable_hash(),
stack_hash,
first_chance: exception.dwFirstChance != 0,
stack_frames: stack.frames().iter().map(|f| f.into()).collect(),
stack_frames: stack.frames,
}
}
@ -196,94 +197,6 @@ pub fn new_test_result(
}
}
/// The file and line number for frame in the calls stack.
#[derive(Clone)]
pub struct FileInfo {
pub file: String,
pub line: u32,
}
/// The location within a function for a call stack entry.
#[derive(Clone)]
pub enum DebugFunctionLocation {
/// If symbol information is available, we use the file/line numbers for stability across builds.
FileInfo(FileInfo),
/// If no symbol information is available, the offset within the function is used.
Displacement(u64),
}
impl fmt::Display for DebugFunctionLocation {
fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
match self {
DebugFunctionLocation::FileInfo(file_info) => {
write!(formatter, "{}:{}", file_info.file, file_info.line)
}
DebugFunctionLocation::Displacement(disp) => write!(formatter, "0x{:x}", disp),
}
}
}
impl<'a> From<&'a stack::DebugFunctionLocation> for DebugFunctionLocation {
fn from(location: &'a stack::DebugFunctionLocation) -> Self {
match location {
stack::DebugFunctionLocation::Line { file, line } => {
DebugFunctionLocation::FileInfo(FileInfo {
file: file.to_string(),
line: *line,
})
}
stack::DebugFunctionLocation::Offset { disp } => {
DebugFunctionLocation::Displacement(*disp)
}
}
}
}
/// A stack frame for reporting where an exception or other bug occurs.
#[derive(Clone)]
pub enum DebugStackFrame {
Frame {
/// The name of the function (if available via symbols or exports) or possibly something else like a
/// (possibly synthetic) module name.
function: String,
/// Location details such as file/line (if symbols are available) or offset
location: DebugFunctionLocation,
},
CorruptFrame,
}
impl fmt::Display for DebugStackFrame {
fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
match self {
DebugStackFrame::Frame { function, location } => {
formatter.write_str(function)?;
match location {
DebugFunctionLocation::FileInfo(file_info) => {
write!(formatter, " {}:{}", file_info.file, file_info.line)
}
DebugFunctionLocation::Displacement(disp) => write!(formatter, "+0x{:x}", disp),
}
}
DebugStackFrame::CorruptFrame => formatter.write_str("<corrupt frame(s)>"),
}
}
}
impl<'a> From<&'a stack::DebugStackFrame> for DebugStackFrame {
fn from(frame: &'a stack::DebugStackFrame) -> Self {
match frame {
stack::DebugStackFrame::Frame { function, location } => DebugStackFrame::Frame {
function: function.to_string(),
location: location.into(),
},
stack::DebugStackFrame::CorruptFrame => DebugStackFrame::CorruptFrame,
}
}
}
/// The details of an exception observed by the execution engine.
#[derive(Clone)]
pub struct Exception {

View File

@ -205,9 +205,12 @@ mod test {
if let Err(err) = &resp {
let as_text = format!("{}", err);
assert!(as_text.contains("Maximum number of attempts reached for this request"));
assert!(
as_text.contains("Maximum number of attempts reached for this request")
|| as_text.contains("os error 10061")
);
} else {
bail!("response to {} was expected to fail", invalid_url);
anyhow::bail!("response to {} was expected to fail", invalid_url);
}
Ok(())

View File

@ -38,13 +38,10 @@ cargo build --release --locked
cargo clippy --release -- -D warnings
# export RUST_LOG=trace
export RUST_BACKTRACE=full
cargo test --release --manifest-path ./onefuzz-supervisor/Cargo.toml
cargo test --release --workspace
# TODO: re-enable integration tests.
# cargo test --release --manifest-path ./onefuzz-agent/Cargo.toml --features integration_test -- --nocapture
cargo test --release --manifest-path ./onefuzz-agent/Cargo.toml
cargo test --release --manifest-path ./onefuzz/Cargo.toml
cp target/release/onefuzz-agent* ../../artifacts/agent
cp target/release/onefuzz-supervisor* ../../artifacts/agent