Repository: microsoft/onefuzz
File: src/agent/onefuzz-task/src/tasks/coverage/generic.rs (Rust)

Code before:
        AllowList::parse(&text)
    }
}

struct TaskContext<'a> {
    config: &'a Config,
    coverage: BinaryCoverage,
    allowlist: TargetAllowList,
    heartbeat: Option<TaskHeartbeatClient>,
    cache: Arc<DebugInfoCache>,
}

impl<'a> TaskContext<'a> {
    pub fn new(
        config: &'a Config,
        coverage: BinaryCoverage,
        allowlist: TargetAllowList,
        heartbeat: Option<TaskHeartbeatClient>,
        target_exe: String,
    ) -> Result<Self> {
        let cache = DebugInfoCache::new(allowlist.source_files.clone());
        let loader = Loader::new();

        let module: Box<dyn Module> = LoadModule::load(&loader, FilePath::new(target_exe)?)?;

        cache.get_or_insert(&*module)?;

        let arccache = Arc::new(cache);

        Ok(Self {
            config,
            coverage,
            allowlist,
            heartbeat,
            cache: arccache,
        })
    }

    pub async fn record_input(&mut self, input: &Path) -> Result<()> {
        debug!("recording coverage for {}", input.display());
        let attempts = MAX_COVERAGE_RECORDING_ATTEMPTS;

        for attempt in 1..=attempts {
            let result = self.try_record_input(input).await;

            if let Err(err) = &result {
                // Recording failed, check if we can retry.
                if attempt < attempts {
                    // We will retry, but warn to capture the error if we succeed.
                    warn!(
                        "error recording coverage for input = {}: {:?}",

Review comment (bug):
By using `?` here, we bail on error. One type of error is that the module simply has no debuginfo. In almost all real life cases, this would signal an error in job submission. However, in general, we could imagine that the `target_exe` is a libfuzzer binary whose true target code is in some DLL that gets dynamically linked or loaded.

Question for the OneFuzz team to consider: is it a hard requirement that the `target_exe` include debuginfo?

I think it is probably fair to say yes, and it may even already be de facto true (I don't remember). But that should be understood and agreed upon.

Code after:
        AllowList::parse(&text)
    }
}

struct TaskContext<'a> {
    config: &'a Config,
    coverage: BinaryCoverage,
    allowlist: TargetAllowList,
    heartbeat: Option<TaskHeartbeatClient>,
    cache: Arc<DebugInfoCache>,
}

impl<'a> TaskContext<'a> {
    pub fn new(
        config: &'a Config,
        coverage: BinaryCoverage,
        allowlist: TargetAllowList,
        heartbeat: Option<TaskHeartbeatClient>,
        target_exe: String,
    ) -> Result<Self> {
        let cache = DebugInfoCache::new(allowlist.source_files.clone());
        let loader = Loader::new();

        // Preload the cache with the target executable, to avoid counting debuginfo analysis
        // time against the exeuction timeout for the first iteration.
        let module: Box<dyn Module> = LoadModule::load(&loader, FilePath::new(target_exe)?)?;

        cache
            .get_or_insert(&*module)
            .context("Failed to load debuginfo for target_exe when populating DebugInfoCache")?;

        Ok(Self {
            config,
            coverage,
            allowlist,
            heartbeat,
            cache: Arc::new(cache),
        })
    }

    pub async fn record_input(&mut self, input: &Path) -> Result<()> {
        debug!("recording coverage for {}", input.display());
        let attempts = MAX_COVERAGE_RECORDING_ATTEMPTS;

        for attempt in 1..=attempts {
            let result = self.try_record_input(input).await;

            if let Err(err) = &result {
                // Recording failed, check if we can retry.
                if attempt < attempts {
                    // We will retry, but warn to capture the error if we succeed.