Make modules case insenstive on windows (#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

Author: tevoinea

src/agent/coverage/src/allowlist.rs

@@ -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)?)
 }

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

@@ -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(())
+}

src/agent/coverage/src/source.rs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 use std::collections::{BTreeMap, BTreeSet};
+
 use std::num::NonZeroU32;
 
 use anyhow::{Context, Result};
@@ -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;
@@ -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)?;
 

src/agent/coverage/tests/snapshot.rs

@@ -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)
@@ -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();

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

@@ -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>
 [ ] 

src/agent/coverage/tests/windows/inlinee.cpp renamed to src/agent/coverage/tests/windows/Inlinee.cpp

@@ -46,7 +46,6 @@ impl Module {
             error!("Error getting path from file handle: {}", e);
             "???".into()
         });
-
         let image_details = get_image_details(&path)?;
 
         Ok(Module {

src/agent/debugger/src/module.rs

@@ -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(),
             )
@@ -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(),
             )
@@ -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(),
             )