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

Commit

Permalink
PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tevoinea committed Sep 25, 2023
1 parent cc551a6 commit ab75983
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 22 deletions.
16 changes: 8 additions & 8 deletions src/agent/coverage/src/allowlist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the MIT License.

use anyhow::Result;
use regex::{Regex, RegexBuilder, RegexSet};
use regex::{Regex, RegexSet};
use std::path::Path;

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -142,15 +142,15 @@ fn glob_to_regex(expr: &str) -> Result<Regex> {
let expr = expr.replace(r"\*", ".*");

// Anchor to line start and end.
let expr = format!("^{expr}$");

let mut rb = RegexBuilder::new(&expr);

if cfg!(windows) {
rb.case_insensitive(true);
// On Windows we should also ignore case.

Check failure on line 145 in src/agent/coverage/src/allowlist.rs

View workflow job for this annotation

GitHub Actions / agent (self-hosted, 1ES.Pool=onefuzz-ci, 1ES.ImageOverride=github-runner-image-ubuntu-20.04)

Diff in /usr/local/vss-agent/2.309.0/_work/onefuzz/onefuzz/src/agent/coverage/src/allowlist.rs
let expr = if cfg!(windows) {
format!("(?i)^{expr}$")
}
else {
format!("^{expr}$")
};

Ok(rb.build()?)
Ok(Regex::new(&expr)?)
}

#[cfg(test)]
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())?
};

// 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
16 changes: 8 additions & 8 deletions src/agent/coverage/tests/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,21 @@ fn windows_snapshot_tests() {
.record()
.unwrap();

// Windows modules should be case insensitive
recorded
.coverage
.modules
.keys()
.for_each(|k| assert_eq!(k.to_string().to_ascii_lowercase(), k.to_string()));

// generate source-line coverage info
let source =
coverage::source::binary_to_source_coverage(&recorded.coverage, &source_allowlist)
.expect("binary_to_source_coverage");

// 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
4 changes: 1 addition & 3 deletions src/agent/debugger/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ impl Module {
error!("Error getting path from file handle: {}", e);

Check failure on line 46 in src/agent/debugger/src/module.rs

View workflow job for this annotation

GitHub Actions / agent (self-hosted, 1ES.Pool=onefuzz-ci, 1ES.ImageOverride=github-runner-image-ubuntu-20.04)

Diff in /usr/local/vss-agent/2.309.0/_work/onefuzz/onefuzz/src/agent/debugger/src/module.rs
"???".into()
});

let path = PathBuf::from(path.as_os_str().to_ascii_lowercase());


let image_details = get_image_details(&path)?;

Ok(Module {
Expand Down

0 comments on commit ab75983

Please sign in to comment.