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

Make modules case insenstive on windows #3527

Merged
Merged
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
7 changes: 6 additions & 1 deletion src/agent/coverage/src/allowlist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,12 @@ fn glob_to_regex(expr: &str) -> Result<Regex> {
let expr = expr.replace(r"\*", ".*");

// Anchor to line start and end.
let expr = format!("^{expr}$");
// On Windows we should also ignore case.
let expr = if cfg!(windows) {
format!("(?i)^{expr}$")
} else {
format!("^{expr}$")
};

Ok(Regex::new(&expr)?)
}
Expand Down
18 changes: 18 additions & 0 deletions src/agent/coverage/src/allowlist/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,21 @@ fn test_allowlist_escape() -> Result<()> {

Ok(())
}

#[cfg(target_os = "windows")]
#[test]
fn test_windows_allowlists_are_not_case_sensitive() -> Result<()> {
let allowlist = AllowList::parse("vccrt")?;
assert!(allowlist.is_allowed("VCCRT"));

Ok(())
}

#[cfg(not(target_os = "windows"))]
#[test]
fn test_linux_allowlists_are_case_sensitive() -> Result<()> {
let allowlist = AllowList::parse("vccrt")?;
assert!(!allowlist.is_allowed("VCCRT"));

Ok(())
}
26 changes: 26 additions & 0 deletions src/agent/coverage/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

use std::collections::{BTreeMap, BTreeSet};

use std::num::NonZeroU32;

use anyhow::{Context, Result};
Expand All @@ -11,6 +12,7 @@ use debuggable_module::load_module::LoadModule;
use debuggable_module::loader::Loader;
use debuggable_module::path::FilePath;
use debuggable_module::{Module, Offset};
use symbolic::symcache::transform::{SourceLocation, Transformer};

use crate::allowlist::AllowList;
use crate::binary::BinaryCoverage;
Expand Down Expand Up @@ -69,6 +71,30 @@ pub fn binary_to_source_coverage(
let mut symcache = vec![];
let mut converter = SymCacheConverter::new();

if cfg!(windows) {
use symbolic::symcache::transform::Function;
struct CaseInsensitive {}
impl Transformer for CaseInsensitive {
fn transform_function<'f>(&'f mut self, f: Function<'f>) -> Function<'f> {
f
}

fn transform_source_location<'f>(
&'f mut self,
mut sl: SourceLocation<'f>,
) -> SourceLocation<'f> {
sl.file.name = sl.file.name.to_ascii_lowercase().into();
sl.file.directory = sl.file.directory.map(|d| d.to_ascii_lowercase().into());
sl.file.comp_dir = sl.file.comp_dir.map(|d| d.to_ascii_lowercase().into());
sl
}
}

let case_insensitive_transformer = CaseInsensitive {};

converter.add_transformer(case_insensitive_transformer);
}

let exe = Object::parse(module.executable_data())?;
converter.process_object(&exe)?;

Expand Down
14 changes: 12 additions & 2 deletions src/agent/coverage/tests/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ fn windows_snapshot_tests() {
};

// filter to just the input test file:
let source_allowlist = AllowList::parse(&input_path.to_string_lossy()).unwrap();
let source_allowlist =
AllowList::parse(&input_path.to_string_lossy().to_ascii_lowercase()).unwrap();

let exe_cmd = std::process::Command::new(&exe_name);
let recorded = coverage::CoverageRecorder::new(exe_cmd)
Expand All @@ -57,9 +58,18 @@ fn windows_snapshot_tests() {
coverage::source::binary_to_source_coverage(&recorded.coverage, &source_allowlist)
.expect("binary_to_source_coverage");

println!("{:?}", source.files.keys());

// For Windows, the source coverage is tracked using case-insensitive paths.
// The conversion from case-sensitive to insensitive is done when converting from binary to source coverage.
// By naming our test file with a capital letter, we can ensure that the case-insensitive conversion is working.
source.files.keys().for_each(|k| {
assert_eq!(k.to_string().to_ascii_lowercase(), k.to_string());
});

let file_coverage = source
.files
.get(&FilePath::new(input_path.to_string_lossy()).unwrap())
.get(&FilePath::new(input_path.to_string_lossy().to_ascii_lowercase()).unwrap())
.expect("coverage for input");

let mut result = String::new();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: coverage/tests/snapshot.rs
expression: result
input_file: coverage/tests/windows/inlinee.cpp
input_file: coverage/tests/windows/Inlinee.cpp
---
[ ] #include <iostream>
[ ]
Expand Down
1 change: 0 additions & 1 deletion src/agent/debugger/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ impl Module {
error!("Error getting path from file handle: {}", e);
"???".into()
});

let image_details = get_image_details(&path)?;

Ok(Module {
Expand Down
9 changes: 6 additions & 3 deletions src/agent/onefuzz/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ impl<'a> Expand<'a> {

fn input_file_sha256(&self) -> Result<ExpandedValue<'a>> {
let Some(val) = self.values.get(PlaceHolder::Input.get_string()) else {
bail!("no value found for {}, unable to evaluate {}",
bail!(
"no value found for {}, unable to evaluate {}",
PlaceHolder::Input.get_string(),
PlaceHolder::InputFileSha256.get_string(),
)
Expand All @@ -149,7 +150,8 @@ impl<'a> Expand<'a> {

fn extract_file_name_no_ext(&self) -> Result<ExpandedValue<'a>> {
let Some(val) = self.values.get(PlaceHolder::Input.get_string()) else {
bail!("no value found for {}, unable to evaluate {}",
bail!(
"no value found for {}, unable to evaluate {}",
PlaceHolder::Input.get_string(),
PlaceHolder::InputFileNameNoExt.get_string(),
)
Expand All @@ -173,7 +175,8 @@ impl<'a> Expand<'a> {

fn extract_file_name(&self) -> Result<ExpandedValue<'a>> {
let Some(val) = self.values.get(PlaceHolder::Input.get_string()) else {
bail!("no value found for {}, unable to evaluate {}",
bail!(
"no value found for {}, unable to evaluate {}",
PlaceHolder::Input.get_string(),
PlaceHolder::InputFileName.get_string(),
)
Expand Down
Loading