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

compile_native_go_fuzzer: only search for fuzz function in .go files #7903

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kmoe
Copy link

@kmoe kmoe commented Jun 22, 2022

Found this issue while updating HCL fuzzer to Go 1.18 native fuzzing.

Issue

Prior to this change, $fuzzer_filename is populated with a list of all files containing the fuzz $function string, whether they be git index file, README, Go file, or anything else.

These multiple files are passed to rewrite_go_fuzz_harness on line 86 as separate arguments:

+ rewrite_go_fuzz_harness /src/hcl/hclsyntax/fuzz/README.md /src/hcl/hclsyntax/fuzz/fuzz_test.go FuzzParseTemplate

This leads to an error on line 31 as the arguments to rewrite_go_fuzz_harness are now incorrect.

+ fuzzer_filename=/src/hcl/hclsyntax/fuzz/README.md
+ fuzz_function=/src/hcl/hclsyntax/fuzz/fuzz_test.go
[...]
+ fuzzer_fn=/src/hcl/hclsyntax/fuzz/README.md_fuzz_.go
+ echo 'replacing *testing.F'
replacing *testing.F
+ sed -i 's/func /src/hcl/hclsyntax/fuzz/fuzz_test.go(\([a-zA-Z0-9]*\) \*testing\.F)/func /src/hcl/hclsyntax/fuzz/fuzz_test.go(\1 \*go118fuzzbuildutils\.F)/g' /src/hcl/hclsyntax/fuzz/README.md_fuzz_.go
sed: -e expression #1, char 13: unknown option to `s'

Solution

Limiting the grep to .go files (a47f2e0) fixes the case in which the fuzz $function string is present in one Go file and one or more non-Go files.

Subsequent lines of the script will still break if $function is present in more than one Go file. ca6e880 suggests erroring out in this case.

@google-cla
Copy link

google-cla bot commented Jun 22, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@AdamKorcz
Copy link
Collaborator

Thanks a lot for this. Is there any chance you could also get rid of the -r flag?

@kmoe
Copy link
Author

kmoe commented Jun 22, 2022

Sure, is there any particular reason that was left as a TODO - anything to look out for here?

@AdamKorcz
Copy link
Collaborator

Sure, is there any particular reason that was left as a TODO - anything to look out for here?

Not really. There shouldn't be anything contextual in OSS-Fuzz that caused the -r flag to be in that line.

@Navidem
Copy link
Contributor

Navidem commented Aug 9, 2022

@kmoe friendly ping :)

@kmoe kmoe force-pushed the compile-native-go-fuzzer-go-files-only branch from ca6e880 to 32e52b2 Compare September 21, 2022 22:32
@kmoe
Copy link
Author

kmoe commented Sep 21, 2022

The first commit was superseded by #8238, so I've rebased.

Also removed the redundant grep -r flags.

@AdamKorcz @Navidem, thanks for your patience. This is ready for review.

@jonathanmetzman
Copy link
Contributor

/gcbrun trial_build.py go --engine libfuzzer --sanitizer coverage address

@jonathanmetzman
Copy link
Contributor

Are we still interested in merging this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants