-
Notifications
You must be signed in to change notification settings - Fork 378
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
Switch mtail to use native go-fuzz for OSS-Fuzz #317
Conversation
This change was tested on gcr.io/oss-fuzz-base/base-builder Docker image where https://github.com/mdempsky/go114-fuzz-build(native go-fuzz) is installed as bin/go-fuzz. Since the CI for this repo uses gcr.io/fuzzit-public/buster-golang12 Docker image, the fuzzing check is failing at the moment. Also, to use github.com/mdempsky/go114-fuzz-build, go1.14 is required, so I guess this change has to be deferred until a fuzzit-public golang1.14 Docker image is released. One other option I see is to switch the CircusCI-fuzzing test to use gcr.io/oss-fuzz-base/base-builder, if that's suitable. |
add3655
to
a2616d2
Compare
$(OUT)/vm-fuzzer: $(GOFILES) | $(GOFUZZBUILD) | ||
$(GOFUZZBUILD) -libfuzzer -o fuzzer.a ./internal/vm | ||
$(OUT)/vm-fuzzer: $(GOFILES) | $(GOFUZZ) | ||
$(GOFUZZ) -o fuzzer.a github.com/google/mtail/internal/vm | ||
$(CXX) $(CXXFLAGS) $(LIB_FUZZING_ENGINE) fuzzer.a -lpthread -o $(OUT)/vm-fuzzer |
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.
With the change above, you won't need to rename GOFUZZBUILD to GOFUZZ.
I am confused why this PR doesn't remove the CXX line though, if this change uses "native go-fuzz"; I don't see where go-fuzz is being invoked.
@@ -72,7 +72,8 @@ $(GOFUZZBUILD): | |||
|
|||
GOFUZZ = $(GOBIN)/go-fuzz | |||
$(GOFUZZ): | |||
go get $(GOGETFLAGS) github.com/dvyukov/go-fuzz/go-fuzz | |||
go get $(GOGETFLAGS) github.com/mdempsky/go114-fuzz-build | |||
ln -s $(GOPATH)/bin/go114-fuzz-build $(GOPATH)/bin/go-fuzz |
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.
This should have changed the GOFUZZBUILD rule above instead of this one, avoiding the symlink and the change of meaning.
going to drop this PR as I've fixed it elsewhere. |
Ref: google/oss-fuzz#3638