Skip to content
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

cmd/compile: excessive instrumentation for libFuzzer #53760

Closed
randall77 opened this issue Jul 8, 2022 · 3 comments
Closed

cmd/compile: excessive instrumentation for libFuzzer #53760

randall77 opened this issue Jul 8, 2022 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge fuzz Issues related to native fuzzing support NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@randall77
Copy link
Contributor

func f() {
}

When compiled with

go tool compile -d=libfuzzer -S f.go

We get a call to runtime.libfuzzerTraceConstCmp1, which seems unnecessary.

This happens because we add (in the order pass):

if counter == 255 {
    counter = 1
} else {
    counter += 1
}

But then we add instrumentation to all integer comparisons (in the walk pass):

runtime.libfuzzerTraceConstCmp1(counter, 255)

I don't think we need to rewrite the internal libfuzzer counter overflow check to use the runtime comparison function. Only user comparisons should be so treated. I guess it technically doesn't hurt, but it will be slow.

@kyakdan

@randall77 randall77 added Performance fuzz Issues related to native fuzzing support labels Jul 8, 2022
@randall77 randall77 added this to the Go1.20 milestone Jul 8, 2022
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 11, 2022
kyakdan added a commit to CodeIntelligenceTesting/go that referenced this issue Jul 11, 2022
Do not intercept integer compares that are used to increment libFuzzer's
8-bit counters. This is unnecessary and has a negative impact on the
fuzzing performance. This fixes golang#53760.
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/416796 mentions this issue: cmd/compile: avoid excessive libfuzzer instrumentation of int compares

@kyakdan
Copy link
Contributor

kyakdan commented Jul 11, 2022

@randall77 Great catch! I've pushed a fix that avoids intercepting integer compares of libFuzzer's 8-bit counters.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
@mdempsky
Copy link
Contributor

mdempsky commented Aug 31, 2022

Would it be more efficient to emit something like:

tmp := int(counter) + 1
counter = uint8(tmp + tmp>>8)

so the counter increment is branchless?

@golang golang locked and limited conversation to collaborators Aug 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge fuzz Issues related to native fuzzing support NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

5 participants