Skip to content

fuzz: link libfuzzer engine to fuzz with bazel!#7805

Merged
lizan merged 11 commits intoenvoyproxy:masterfrom
asraa:adddictionarytoroutefuzz
Aug 7, 2019
Merged

fuzz: link libfuzzer engine to fuzz with bazel!#7805
lizan merged 11 commits intoenvoyproxy:masterfrom
asraa:adddictionarytoroutefuzz

Conversation

@asraa
Copy link
Copy Markdown
Contributor

@asraa asraa commented Aug 1, 2019 •
edited
Loading

This makes a new config "asan-fuzzer" that links to the libfuzzer engine.

To run a fuzz target with asan and libfuzzer, do
bazel build --config=asan-fuzzer test/common/router:route_fuzz_test_with_libfuzzer --config=asan-fuzzer
bazel-bin/test/common/router/route_fuzz_test_driver test/common/router/route_corpus -runs=-1

(-runs=-1 for indefinite runs, -runs=100 for bounded).

Testing: works locally
Risk: Low

asraa added 3 commits July 31, 2019 11:04
@asraa
bazel link ot libfuzzer
aae9db9 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
update comment
7d90e57 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
clean fuzz build, dont rely on local path
e01c7fa 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa asraa mentioned this pull request Aug 1, 2019
Merged
@asraa asraa changed the title fuzz: link libfuzzer engine to fuzz with blaze! fuzz: link libfuzzer engine to fuzz with bazel! Aug 1, 2019
@asraa
fix linkopts
e862c3d 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa asraa marked this pull request as ready for review August 2, 2019 15:47
@asraa
Copy link
Copy Markdown
Contributor Author

asraa commented Aug 2, 2019

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Aug 2, 2019

🔨 rebuilding ci/circleci: clang_tidy (failed build)

🐱

Caused by: a #7805 (comment) was created by @asraa.

see: more, trace.

htuch
htuch suggested changes Aug 2, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is super awesome, nice to see that it's relatively few lines of code and not too much hackery.

Can you update https://github.com/envoyproxy/envoy/blob/master/test/fuzz/README.md with a section on how to run this on CLI. I think the best guidance is to tell folks to do something like:

bazel run //test/common/common:base64_fuzz_test_driver --config asan-fuzzer

/wait

@htuch htuch self-assigned this Aug 2, 2019
@asraa
add readme
8bb6280 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
fix typo
ce84292 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
Copy link
Copy Markdown
Contributor Author

asraa commented Aug 2, 2019

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Aug 2, 2019

🔨 rebuilding ci/circleci: clang_tidy (failed build)

🐱

Caused by: a #7805 (comment) was created by @asraa.

see: more, trace.

htuch
htuch previously approved these changes Aug 5, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is huge improvement to fuzzing development productivity, LGTM. I like the ability to work directly with corpus.

@yevgenypats this is probably the right basis for doing fuzzing outside of oss-fuzz when working directly under CI.

@yevgenypats
Copy link
Copy Markdown
Contributor

yevgenypats commented Aug 5, 2019

This is awesome, I'll rebase my PR on top of this one, once it is merged!.

@asraa
Copy link
Copy Markdown
Contributor Author

asraa commented Aug 5, 2019

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Aug 5, 2019

🔨 rebuilding ci/circleci: clang_tidy (failed build)

🐱

Caused by: a #7805 (comment) was created by @asraa.

see: more, trace.

@yevgenypats
Copy link
Copy Markdown
Contributor

yevgenypats commented Aug 5, 2019

looks like one of tests here fails. maybe it's out of memory?

@lizan
Copy link
Copy Markdown
Member

lizan commented Aug 5, 2019

Can you try applying this? I think it will fix clang_tidy:

diff --git a/tools/gen_compilation_database.py b/tools/gen_compilation_database.py
index 94869312b..4090b0505 100755
--- a/tools/gen_compilation_database.py
+++ b/tools/gen_compilation_database.py
@@ -8,7 +8,7 @@ import subprocess

 def generateCompilationDatabase(args):
   if args.run_bazel_build:
-    subprocess.check_call(["bazel", "build"] + args.bazel_targets)
+    subprocess.check_call(["bazel", "build", "--build_tag_filters=-manual"] + args.bazel_targets)

   gen_compilation_database_sh = os.path.join(
       os.path.realpath(os.path.dirname(__file__)), "../bazel/gen_compilation_database.sh")

Also merge master to pick up CI changes.

@htuch htuch added the waiting label Aug 6, 2019
asraa added 2 commits August 6, 2019 07:44
@asraa
Merge remote-tracking branch 'upstream/master' into adddictionarytoro…
6aa7944 
…utefuzz

Signed-off-by: Asra Ali <asraa@google.com>
@asraa
clang tidy
e8a3fc9 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
skip tests with manual tag
b6333c8 
Signed-off-by: Asra Ali <asraa@google.com>
@yevgenypats
Copy link
Copy Markdown
Contributor

yevgenypats commented Aug 6, 2019

This is still gets killed due to oom. I suspect that you need to add bazel build --jobs 8 or otherwise it will use 200 workers and will overuse the memory and will get killed by Circle.

@asraa
temp (?) jobs add to clang-tidy
d413d5d 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
Copy link
Copy Markdown
Contributor Author

asraa commented Aug 6, 2019

Thank you! Yes, let's see. I'm not sure what the implications are later, but I did add the jobs flag to see what happens.

@yevgenypats
Copy link
Copy Markdown
Contributor

yevgenypats commented Aug 6, 2019

horray! it worked:) #why_bazel_spins_up_200_workers_by_default:)

@asraa
Copy link
Copy Markdown
Contributor Author

asraa commented Aug 6, 2019

Yes!

@htuch, @lizan -- what do you think about this? (if this merges, probably everyone will OOM CI without the --jobs flag. how many jobs are appropriate, or is there a better way to mitigate this?)

@lizan
Copy link
Copy Markdown
Member

lizan commented Aug 6, 2019

Can you try to fix the jobs number to env ${NUM_CPUS} (which is set to 8 in CI)?

@yevgenypats bazel automatically spins up job to the number of CPU cores, which ignores cgroup, so in a docker environment in large host, it spins job more than cgroup restriction.

@asraa
use env variable to set num cpus
f2ceeea 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
Copy link
Copy Markdown
Contributor Author

asraa commented Aug 7, 2019

Thanks everyone. clang-tidy runs with NUM_CPUS jobs now.

@yevgenypats
Copy link
Copy Markdown
Contributor

yevgenypats commented Aug 7, 2019

horray! lets merge!

@lizan lizan merged commit 6db95bd into envoyproxy:master Aug 7, 2019
@yevgenypats yevgenypats mentioned this pull request Aug 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@lizan lizan lizan approved these changes

@htuch htuch htuch left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@htuch htuch

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@asraa @yevgenypats @lizan @htuch