Add randomized fuzzer selection to Fuchsia runs. by flowerhack · Pull Request #471 · google/clusterfuzz

flowerhack · 2019-05-21T04:58:30Z

Apologies for the length of the CL; I promise it's shorter than it looks
(though I can chop it up further).

Also apologies for the delay; there was a Fuchsia hack-it week, and some
instability/thrashing on the Fuchsia side of things.

This PR does the following:

Adds the fuchsia_utils directory, which contains a (hopefully) stable
interface for working with Fuchsia. This includes logic for extracting
Fuzzers from a Fuchsia build.
Moves the Fuchsia build logic into build_manager. While it's a very
tiny class right now, and doesn't take much advantage of build_manager
features, this should allow it to e.g. use the weighted random selection
in future CLs, handle revision # logic more gracefully, etc.
Adds the ability to choose a fuzzer at random, and run it. (Left
for a future CL is extracting the crash log from the device and placing
it someplace useful, though writing the log should be handled
automagically by our wrapper class.)

My only question is the license headers: I added the standard Google LLC
license header to the files we're yoinking from Fuchsia, but also left
the Fuchsia license header in place. I do not know whether we can just
merge these, if there's an elegant way to handle it, etc--I would've
just left the files totally intact, but the linter complained about the
header not looking right :) So yeah, let me know.

flowerhack · 2019-05-21T04:58:56Z

@oliverchang @nopsledder

flowerhack · 2019-05-21T18:14:14Z

/gcbrun

flowerhack · 2019-05-21T18:25:13Z

/gcbrun

oliverchang

Nice! some initial comments

oliverchang · 2019-05-21T18:17:44Z

+    self.host = Host.from_dir(
+        os.path.join(fuchsia_resources_dir, 'build', 'out', 'default'))
+    self.device = Device(self.host, 'localhost', fuchsia_portnum)
+    pkg, tgt = environment.get_value('FUZZ_TARGET').split('/')


s/pkg/package/ and s/tgt/target/

Also, what's the format of "FUZZ_TARGET" ? I'm assuming package_name/binary_name ?

@inferno-chromium Seems like another case where we need to support / in a fuzz target name. Seems like we need to normalize the binary name too (and not just project).

Yep, that's the expected format of FUZZ_TARGET.

Completed s/pkg/package/ and s/tgt/target/.

(Leaving comment open--I don't have context on the question of "supporting slashes in target names" but I assume @inferno-chromium may need to peek :) )

oliverchang · 2019-05-21T18:30:38Z

@@ -0,0 +1,13 @@
+# Copyright 2019 Google LLC


For discussion:

It seems better to include these scripts with the build, rather than copying them to CF as-is since the Fuchsia SDK is unstable. That way, for each build we should be guaranteed to have a set of scripts that work. This is important for regression task etc where we do bisects.

We're also in the midst of doing PY2->PY3, and copying things here may complicate things. Do these scripts work with PY3 already?

If we go with that, we also can't import the scripts as-is, because importing is fairly permanent and we shouldn't try to reimport if we set up a build for another revision. We may need something like a stable CLI interface here for the necessary functionality.

Discussed during meeting today.

Consensus seems to be:

we're hard-forking these scripts from Fuchsia, feel free to request changes s.t. they meet ClusterFuzz's style guide

we agreed that the Fuchsia team will be responsible for validating that "whatever is placed in the build bucket" is run-able via these scripts

we accept the risk that, should we need to change/modify these scripts (which we're hoping is rather unlikely!), performing bisects for older builds will become either impossible or require a cumbersome hack

Finally, I think these scripts work with Python 3, but I'm not sure how to test such a thing. Guidance would be very appreciated! :D

we accept the risk that, should we need to change/modify these scripts (which we're hoping is rather unlikely!), performing bisects for older builds will become either impossible or require a cumbersome hack

We also brought up the idea of having a minimal "input pusher" utility with every build for this purpose, since that's the part that is most likely to change. Did we reach a decision on whether or not to do this? @nopsledder

flowerhack · 2019-05-21T18:32:33Z

/gcbrun

Apologies for the length of the CL; I promise it's shorter than it looks (though I can chop it up further). Also apologies for the delay; there was a Fuchsia hack-it week, and some instability/thrashing on the Fuchsia side of things. This PR does the following: * Adds the fuchsia_utils directory, which contains a (hopefully) stable interface for working with Fuchsia. This includes logic for extracting Fuzzers from a Fuchsia build. * Moves the Fuchsia build logic into build_manager. While it's a very tiny class right now, and doesn't take much advantage of build_manager features, this should allow it to e.g. use the weighted random selection in future CLs, handle revision # logic more gracefully, etc. * Adds the ability to *choose* a fuzzer at random, and run it. (Left for a future CL is extracting the crash log from the device and placing it someplace useful, though *writing* the log should be handled automagically by our wrapper class.) My only question is the license headers: I added the standard Google LLC license header to the files we're yoinking from Fuchsia, but *also* left the Fuchsia license header in place. I do not know whether we can just merge these, if there's an elegant way to handle it, etc--I would've just left the files totally intact, but the linter complained about the header not looking right :) So yeah, let me know.

flowerhack · 2019-05-22T00:02:53Z

/gcbrun

flowerhack · 2019-05-22T00:46:58Z

/gcbrun

oliverchang

nice! sorry for the delay, but here some additional comments.

flowerhack · 2019-05-23T22:37:47Z

/gcbrun

inferno-chromium · 2019-05-24T03:31:13Z

/gcbrun

oliverchang

Looking great! Thanks for addressing all the comments.

flowerhack · 2019-05-24T21:00:48Z

/gcbrun

flowerhack · 2019-05-24T21:16:11Z

/gcbrun

flowerhack · 2019-05-24T22:40:49Z

/gcbrun

flowerhack · 2019-05-25T00:27:17Z

/gcbrun

flowerhack · 2019-05-25T00:40:56Z

/gcbrun

oliverchang · 2019-05-27T01:15:07Z

Everything seems to work on App Engine, merging.

googlebot added the cla: yes CLA signed. label May 21, 2019

oliverchang reviewed May 21, 2019

View reviewed changes

flowerhack added 4 commits May 21, 2019 17:00

fixed test

a954fe8

lint

71e994f

oops lint

53d482a

flowerhack force-pushed the int-build-manager2 branch from 5824cee to 53d482a Compare May 22, 2019 00:00

flowerhack added 3 commits May 21, 2019 17:00

local fix

395a9e3

lint

3853622

remove bot env

65ec1d9

flowerhack added 4 commits May 21, 2019 17:18

Hard-fork Fuchsia utilities for use in CF.

3074682

update license

63f0b9d

initial code review response

9fc21fb

more code review tweaks

3715218

oliverchang reviewed May 22, 2019

View reviewed changes

flowerhack added 3 commits May 23, 2019 14:42

code review

617b69c

code review

f072956

code review

2ad2869

inferno-chromium mentioned this pull request May 24, 2019

Remove README from Fuchsia resources dir. #486

Merged

oliverchang approved these changes May 24, 2019

View reviewed changes

Comment thread src/python/tests/core/build_management/build_manager_test.py Outdated

Comment thread src/python/bot/fuzzers/libfuzzer.py

code review

833b2c0

lint

12aa43f

jonathanmetzman reviewed May 24, 2019

View reviewed changes

Comment thread src/python/platforms/fuchsia/device.py Outdated

Fix gsutil imports wrt appengine

e7d42d9

jonathanmetzman reviewed May 24, 2019

View reviewed changes

Comment thread src/python/platforms/fuchsia/util/host.py Outdated

flowerhack added 2 commits May 24, 2019 17:25

Fix more appengine import issues

51e06fa

fixed more imports

616d755

lint

1d8f8c4

oliverchang merged commit df8a187 into google:master May 27, 2019

Conversation

flowerhack commented May 21, 2019

Uh oh!

flowerhack commented May 21, 2019

Uh oh!

flowerhack commented May 21, 2019

Uh oh!

flowerhack commented May 21, 2019

Uh oh!

oliverchang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

oliverchang May 21, 2019

Choose a reason for hiding this comment

Uh oh!

oliverchang May 21, 2019

Choose a reason for hiding this comment

Uh oh!

flowerhack May 22, 2019

Choose a reason for hiding this comment

Uh oh!

oliverchang May 21, 2019

Choose a reason for hiding this comment

Uh oh!

flowerhack May 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oliverchang May 22, 2019

Choose a reason for hiding this comment

Uh oh!

flowerhack commented May 21, 2019

Uh oh!

flowerhack commented May 22, 2019

Uh oh!

flowerhack commented May 22, 2019

Uh oh!

oliverchang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

flowerhack commented May 23, 2019

Uh oh!

inferno-chromium commented May 24, 2019

Uh oh!

oliverchang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

flowerhack commented May 24, 2019

Uh oh!

flowerhack commented May 24, 2019

Uh oh!

Uh oh!

flowerhack commented May 24, 2019

Uh oh!

Uh oh!

flowerhack commented May 25, 2019

Uh oh!

flowerhack commented May 25, 2019

Uh oh!

oliverchang commented May 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

flowerhack May 22, 2019 •

edited

Loading