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 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
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() {
let allowlist = AllowList::parse("vccrt")?;
assert!(!allowlist.is_allowed("VCCRT"));

Ok(())
}
11 changes: 8 additions & 3 deletions src/agent/coverage/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,18 @@ pub fn binary_to_source_coverage(
};

if let Some(file) = location.file() {
let file_path = if cfg!(windows) {
// Windows paths are case insensitive.
FilePath::new(file.full_path().to_ascii_lowercase())?
} else {
FilePath::new(file.full_path())?
};
tevoinea marked this conversation as resolved.
Show resolved Hide resolved

// Only include relevant inlinees.
if !source_allowlist.is_allowed(&file.full_path()) {
if !source_allowlist.is_allowed(&file_path) {
continue;
}

let file_path = FilePath::new(file.full_path())?;

// We have a hit.
let file_coverage = source.files.entry(file_path).or_default();
let line = Line(line_number);
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