Fuzzer: Log locals and values referred to from locals #6913
+128
−65
Conversation
This file contains 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
tlively
reviewed
Sep 7, 2024
| return makeLoggingCall(builder.makeLocalGet(index, type)); | ||
| } | ||
|
|
||
| if (type.isRef()) { |
There was a problem hiding this comment.
How about inverting this condition to save a level of nesting on most of the following code?
| auto& fields = heapType.getStruct().fields; | ||
| if (!fields.empty()) { | ||
| auto fieldIndex = upTo(fields.size()); | ||
| auto fieldType = fields[fieldIndex].type; |
There was a problem hiding this comment.
Do you think it's worth linearly searching from this random starting index looking for the first loggable field? We could also do a breadth-first search on the type graph looking for a loggable type.
Comment on lines
+748
to
+752
| // If the ref is null, log a random integer. The randomness is to | ||
| // avoid the risk of colliding with the value logged in the other | ||
| // arm. | ||
| auto* whenNull = makeLoggingCall(makeConst(Type::i32)); | ||
| return builder.makeIf(isNullCheck, whenNull, whenNonNull); |
There was a problem hiding this comment.
We could also unconditionally log the result of the null check, followed by a conditional log of the field without an else arm in the if.
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.
Logging values is important in the fuzzer as the loggings are observable
effects that we can then compare to other VMs and after optimizations.
Previously we logged random things, which had some chance to pick
useful data, but it makes sense to focus on sensitive values such as locals.
By logging locals, we get a higher chance to notice when a bad change to a
local happens.
If the local is a reference then we can't log its value, but we can log if it is
null at least. We can also try to find a field that is loggable, if it is a reference
to a struct.