Skip to content

Commit 2eed350

Browse files
tevoineachkeita
authored andcommitted
Make modules case insenstive on windows (microsoft#3527)
* Make modules and coverage allowlist case insensitive on Windows * Tests and fmt * PR comments * fmt * Debugging missing file coverage * fmt * Broken linux test * Add a case insensitive transformer for better perf * cargo fix
1 parent ce83822 commit 2eed350

File tree

8 files changed

+69
-8
lines changed

8 files changed

+69
-8
lines changed

src/agent/coverage/src/allowlist.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,12 @@ fn glob_to_regex(expr: &str) -> Result<Regex> {
142142
let expr = expr.replace(r"\*", ".*");
143143

144144
// Anchor to line start and end.
145-
let expr = format!("^{expr}$");
145+
// On Windows we should also ignore case.
146+
let expr = if cfg!(windows) {
147+
format!("(?i)^{expr}$")
148+
} else {
149+
format!("^{expr}$")
150+
};
146151

147152
Ok(Regex::new(&expr)?)
148153
}

src/agent/coverage/src/allowlist/tests.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,21 @@ fn test_allowlist_escape() -> Result<()> {
175175

176176
Ok(())
177177
}
178+
179+
#[cfg(target_os = "windows")]
180+
#[test]
181+
fn test_windows_allowlists_are_not_case_sensitive() -> Result<()> {
182+
let allowlist = AllowList::parse("vccrt")?;
183+
assert!(allowlist.is_allowed("VCCRT"));
184+
185+
Ok(())
186+
}
187+
188+
#[cfg(not(target_os = "windows"))]
189+
#[test]
190+
fn test_linux_allowlists_are_case_sensitive() -> Result<()> {
191+
let allowlist = AllowList::parse("vccrt")?;
192+
assert!(!allowlist.is_allowed("VCCRT"));
193+
194+
Ok(())
195+
}

src/agent/coverage/src/source.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
use std::collections::{BTreeMap, BTreeSet};
5+
56
use std::num::NonZeroU32;
67

78
use anyhow::{Context, Result};
@@ -11,6 +12,7 @@ use debuggable_module::load_module::LoadModule;
1112
use debuggable_module::loader::Loader;
1213
use debuggable_module::path::FilePath;
1314
use debuggable_module::{Module, Offset};
15+
use symbolic::symcache::transform::{SourceLocation, Transformer};
1416

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

74+
if cfg!(windows) {
75+
use symbolic::symcache::transform::Function;
76+
struct CaseInsensitive {}
77+
impl Transformer for CaseInsensitive {
78+
fn transform_function<'f>(&'f mut self, f: Function<'f>) -> Function<'f> {
79+
f
80+
}
81+
82+
fn transform_source_location<'f>(
83+
&'f mut self,
84+
mut sl: SourceLocation<'f>,
85+
) -> SourceLocation<'f> {
86+
sl.file.name = sl.file.name.to_ascii_lowercase().into();
87+
sl.file.directory = sl.file.directory.map(|d| d.to_ascii_lowercase().into());
88+
sl.file.comp_dir = sl.file.comp_dir.map(|d| d.to_ascii_lowercase().into());
89+
sl
90+
}
91+
}
92+
93+
let case_insensitive_transformer = CaseInsensitive {};
94+
95+
converter.add_transformer(case_insensitive_transformer);
96+
}
97+
7298
let exe = Object::parse(module.executable_data())?;
7399
converter.process_object(&exe)?;
74100

src/agent/coverage/tests/snapshot.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ fn windows_snapshot_tests() {
4343
};
4444

4545
// filter to just the input test file:
46-
let source_allowlist = AllowList::parse(&input_path.to_string_lossy()).unwrap();
46+
let source_allowlist =
47+
AllowList::parse(&input_path.to_string_lossy().to_ascii_lowercase()).unwrap();
4748

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

61+
println!("{:?}", source.files.keys());
62+
63+
// For Windows, the source coverage is tracked using case-insensitive paths.
64+
// The conversion from case-sensitive to insensitive is done when converting from binary to source coverage.
65+
// By naming our test file with a capital letter, we can ensure that the case-insensitive conversion is working.
66+
source.files.keys().for_each(|k| {
67+
assert_eq!(k.to_string().to_ascii_lowercase(), k.to_string());
68+
});
69+
6070
let file_coverage = source
6171
.files
62-
.get(&FilePath::new(input_path.to_string_lossy()).unwrap())
72+
.get(&FilePath::new(input_path.to_string_lossy().to_ascii_lowercase()).unwrap())
6373
.expect("coverage for input");
6474

6575
let mut result = String::new();

src/agent/coverage/tests/snapshots/snapshot__windows_snapshot_tests.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
source: coverage/tests/snapshot.rs
33
expression: result
4-
input_file: coverage/tests/windows/inlinee.cpp
4+
input_file: coverage/tests/windows/Inlinee.cpp
55
---
66
[ ] #include <iostream>
77
[ ]

src/agent/debugger/src/module.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ impl Module {
4646
error!("Error getting path from file handle: {}", e);
4747
"???".into()
4848
});
49-
5049
let image_details = get_image_details(&path)?;
5150

5251
Ok(Module {

src/agent/onefuzz/src/expand.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,8 @@ impl<'a> Expand<'a> {
128128

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

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

174176
fn extract_file_name(&self) -> Result<ExpandedValue<'a>> {
175177
let Some(val) = self.values.get(PlaceHolder::Input.get_string()) else {
176-
bail!("no value found for {}, unable to evaluate {}",
178+
bail!(
179+
"no value found for {}, unable to evaluate {}",
177180
PlaceHolder::Input.get_string(),
178181
PlaceHolder::InputFileName.get_string(),
179182
)

0 commit comments

Comments
 (0)