-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
syz-fuzzer, executor: Add support for blacklisting data race frames #1472
Conversation
lint says:
|
if r.CheckResult.Features[host.FeatureLeakChecking].Enabled { | ||
gateCallback = func() { fuzzer.gateCallback(r.MemoryLeakFrames) } | ||
} | ||
fuzzer.gate = ipc.NewGate(2**flagProcs, gateCallback) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the gate in all cases, even without leak checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean: move this back to main?
The code is still there, just in "useBugFrames". ipc.NewGate does nothing if gateCallback is nil? And it seems only if leak checking is enabled is it set to non-nil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see, it should work then, just somewhat confusing b/c gate is not a no-op even without callback. It has major role even without leak/race checking, so seeing it being setup in useBugFrames is somewhat misleading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved fuzzer.gate assignment.
syz-fuzzer/fuzzer.go
Outdated
} | ||
fuzzer.gate = ipc.NewGate(2*flagProcs, gateCallback) | ||
|
||
if r.CheckResult.Features[host.FeatureKCSAN].Enabled { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to check len(r.DataRaceFrames) != 0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
This adds support to add frames that have already been in data races, to the KCSAN report blacklist.
LGTM |
Codecov Report
@@ Coverage Diff @@
## master #1472 +/- ##
==========================================
- Coverage 56.37% 56.36% -0.02%
==========================================
Files 140 140
Lines 26147 26162 +15
==========================================
+ Hits 14741 14745 +4
- Misses 10701 10712 +11
Partials 705 705
Continue to review full report at Codecov.
|
This adds support to add frames that have already been in data races, to
the KCSAN report blacklist.