Skip to content

runtime: Improve Map instrumentation #845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Sep 10, 2023
Merged

runtime: Improve Map instrumentation #845

merged 3 commits into from
Sep 10, 2023

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Sep 6, 2023

No description provided.

@fmeum
Copy link
Contributor Author

fmeum commented Sep 6, 2023 •
edited
Loading

Stacked on #843, otherwise ready for review.

@fmeum fmeum changed the title Improve Map instrumentation runtime: Improve Map instrumentation Sep 6, 2023
@fmeum fmeum requested a review from a team September 6, 2023 11:11
bertschneider
bertschneider approved these changes Sep 6, 2023
View reviewed changes
Copy link
Contributor

@bertschneider bertschneider left a comment

Choose a reason for hiding this comment

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

Nice addition!

As we're at it, should we also hook remove and replace to potentially mutate maps in another interesting ways?

@fmeum fmeum force-pushed the FUZZ-793-better-map branch from 99840fd to 5c10d89 Compare September 6, 2023 12:13
@fmeum
Copy link
Contributor Author

fmeum commented Sep 6, 2023

As we're at it, should we also hook remove and replace to potentially mutate maps in another interesting ways?

I thought about that but couldn't come up with common cases in which this would meaningfully influence control flow. For example, most of the time the return value of remove isn't used and the function is called on something known to be contained in the map. I'm undecided.

@fmeum fmeum force-pushed the FUZZ-793-better-map branch from 5c10d89 to 8f0a4f8 Compare September 6, 2023 12:50
@fmeum fmeum marked this pull request as ready for review September 6, 2023 12:50
@bertschneider
Copy link
Contributor

Then let's think about this a little longer. Perhaps we find a proper use-case.

@fmeum fmeum force-pushed the FUZZ-793-better-map branch from 8f0a4f8 to 7f6663c Compare September 6, 2023 13:12
@fmeum
Copy link
Contributor Author

fmeum commented Sep 6, 2023

Then let's think about this a little longer. Perhaps we find a proper use-case.

Just to understand your point better: Would you prefer to think about this more after or before we consider merging this PR? I'm fine with both :-)

@bertschneider
Copy link
Contributor

bertschneider commented Sep 6, 2023 •
edited
Loading

Ah, afterwards! Let's get this merged and perhaps our subconscious comes up with a valid use-case 😄

@fmeum
Copy link
Contributor Author

fmeum commented Sep 6, 2023 •
edited
Loading

Ah, afterwards! Let's get this merged and perhaps our subconscious comes up with a valid use-case 😄

*hits auto-merge* :-)

@fmeum fmeum force-pushed the FUZZ-793-better-map branch 2 times, most recently from 90baa56 to a569c8a Compare September 6, 2023 18:08
@fmeum
examples: Fix Log4jFuzzer definition
420df15 
The fuzzer still finds the issue in ~2-3 minutes as verified manually.
@fmeum
runtime: Do not track map key comparisons per key
7fa727d 
With a dynamic map, this very quickly overflows the feature table and
stalls fuzzer progress due to the rapidly increasing feature count for
distinct keys.

In the future, we may be able to detect static keys and reenable this
more fine-grained tracking.
@fmeum fmeum force-pushed the FUZZ-793-better-map branch 2 times, most recently from 7df641f to 9f156e8 Compare September 10, 2023 09:40
@fmeum fmeum enabled auto-merge (rebase) September 10, 2023 09:41
@fmeum fmeum force-pushed the FUZZ-793-better-map branch from 9f156e8 to 84671c9 Compare September 10, 2023 11:36
@fmeum fmeum force-pushed the FUZZ-793-better-map branch from 84671c9 to 9940157 Compare September 10, 2023 11:37
@fmeum fmeum merged commit 0f295f0 into main Sep 10, 2023
@fmeum fmeum deleted the FUZZ-793-better-map branch September 10, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@bertschneider bertschneider bertschneider approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fmeum @bertschneider