Closed
Conversation
Contributor
|
At first glance, it looks like this is actually detecting a real issue, so I'd prefer to either fix it or leave it failing rather than change the test in this way. It's important that afl instrumentation output does not depend on when GC runs, and it looks like GC is actually affecting it here. |
Contributor
Author
|
OK, I'll bisect at some point. |
This was referenced Sep 12, 2023
Contributor
Author
|
Closing in favor of #1824 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix an AFL instrumentation test, which is failing locally and skipped in CI. I believe it's failing for an uninteresting reason, so this PR is just a band-aid to get it passing again.
Review: @mshinwell says that @stedolan is familiar with this test — could you take a look?
What is this test anyway?
The AFL testsuite consists of:
unit -> unit)A run of a test consists of:
I'm not very familiar with afl-show-map but it looks like it prints some stats about which basic blocks are explored by the run of the instrumented program. The intuition is: if you run the same (deterministic) code twice, if a basic block is explored once in (I), it should be explored twice in (II).
What test is failing and why?
The
lazinesstest is failing:The reason the test fails is that the count of basic blocks explored for the
lazinessdoes not exactly double from 1 invocation to 2 invocations.For 1 invocation:
For 2 invocations:
I suspect that the first call to
Gc.major ()is doing something "different enough" to later calls (maybe just more work?), and that's why051424is hit in the first call and053443and040923are hit in the second call. A magic trace suggests that the first call is doing a lot more work incaml_empty_minor_heap, probably collecting the other garbage generated by top-level startup code. Indeed, if I callGc.minor ()once at top-level beforelazinessruns, then the output doubles as expected: