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

allow for more coverage data in total #519

Merged
merged 5 commits into from
Feb 9, 2021

Conversation

bmc-msft
Copy link
Contributor

@bmc-msft bmc-msft commented Feb 8, 2021

No description provided.

@bmc-msft bmc-msft requested a review from ranweiler February 9, 2021 02:18
@@ -46,6 +46,9 @@ impl TotalCoverage {
pub async fn update_bytes(&self, new_data: &[u8]) -> Result<()> {
match self.data().await {
Ok(Some(mut total_data)) => {
if total_data.len() < new_data.len() {
total_data.resize_with(new_data.len(), || 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The total table is the concatenation of the modules in filesystem-iteration order. This is not defined, but we assume it is stable. I don't think this assumption is actually valid.

Even if it held, the resizing of the total (on input coverage analysis where we discover a new module) assumes the new coverage data is appended to the old, but this isn't necessarily true. Suppose we always iterated files in a lexicographic ordering. If our total coverage only included fuzz.exe before, and then we discover core.dll, the new total.cov will have us merge core.dll.cov over the existing fuzz.exe.cov, then extend total.cov to accomodate the difference in size.

Stability of coverage map size does hold for each individual module. So I think the real fix is to just create and rewrite the total.cov on each input, instead of merging over the last one. In this way, we can just piggyback on the correct merging of the individual modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the combined total to use the per-module totals.

Note, the per-module total is now a BTreeMap, which means they will be consistently sorted based on the filenames. Let's call that... eventual consistency? ;)

@bmc-msft bmc-msft requested a review from ranweiler February 9, 2021 18:28
@bmc-msft bmc-msft merged commit 6d4f456 into microsoft:main Feb 9, 2021
@bmc-msft bmc-msft deleted the allow-for-more-data branch February 9, 2021 21:47
@ghost ghost locked as resolved and limited conversation to collaborators Apr 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants