1481237 - automate updating internal libFuzzer code

Status: RESOLVED FIXED

(Firefox :: Untriaged, enhancement)

Description

[u473386]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

Steps to reproduce:

oss-fuzz always uses very recent libFuzzer versions, and sometimes new and experimental flags. While unknown flags are ignored, I think it's still a good idea to sync the internal libFuzzer revision to get the full benefit. For that I'd like to either change clone_libfuzzer.sh to take an SVN revision and to automatically re-write moz.build accordingly, or add a new script to do that. What do you think?

Comment 1

[Christian Holler (:decoder)]

Do you want to make these updates on-the-fly when building on oss-fuzz?

If so, that might work, but in general an automated update of our libfuzzer version is not possible (because every new libfuzzer version can regress the tree).

Comment 2

[u473386]

> Do you want to make these updates on-the-fly when building on oss-fuzz?

Yes, I'll get the LLVM revision from its clang, and pass it to the script to sync to that revision. There will be no changes to mozilla-central other than a changed (or additional) script.

Comment 3

[u473386]

Attachment: libfuzzer

Please take a look. I used it to auto-sync to HEAD, and then built Firefox. Fuzzing works.

Comment 4

[u473386]

This doesn't necessarily need to be in mozilla-central. It could also be done on the oss-fuzz side. Less preferable I think.

Comment 5

[Christian Holler (:decoder)]

Comment on attachment 8998672 [details] [diff] [review]
libfuzzer

We need to do this a little differently (though not much):

The default behavior of ./clone_libfuzzer.sh should remain the same and the revision that we currently have in the tree *must* be in the script. It is required by all upstream scripts to have that information.

I would suggest to check for an optional argument, e.g. "update" and if that argument is specified, you pull HEAD instead and do the moz.build changes, then output the revision you updated to. This will also be really handy in case we want to update our in-tree version, because we can simply use that script instead of manually messing with moz.build.

The moz.build changes should also only be run when that "update" parameter is specified.

Apart from those remarks I see no reason to not have this in the tree.

Comment 6

[u473386]

> The default behavior of ./clone_libfuzzer.sh should remain the same and the revision that we currently have in the tree *must* be in the script. It is required by all upstream scripts to have that information.

Does it have to be the git revision from the chromium tree, or can it be the equivalent svn revision?

Comment 7

[Christian Holler (:decoder)]

I don't really care if it is coming from a git mirror or the svn repository, but why do you want to change the pull source?

Comment 8

[u473386]

clang only gives an svn revision I can sync too. I'm a bit reluctant to sync to HEAD, rather than the version oss-fuzz actually uses (which is usually close to HEAD, but still).

Comment 9

[Christian Holler (:decoder)]

Ah I see! I suggest you change the source to the original repository then and find out what the equivalent is for the version we are using right now.

Comment 10

[u473386]

Attachment: libFuzzer-2

With no argument it works as before, otherwise it updates to that revision.

Comment 11

[Christian Holler (:decoder)]

Comment on attachment 8999835 [details] [diff] [review]
libFuzzer-2

Review of attachment 8999835 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, clear to land, r=me.

Comment 12

[u473386]

Attachment: libFuzzer-3

(Commitified.)

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f578ca2a361e
Automate updating internal libFuzzer code. r=decoder

Comment 14

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/f578ca2a361e

Comment 15

[u473386]

Attachment: svn

Minor improvement. What do you think? It's useful for updates to HEAD, or as confirmation.

Comment 16

[Christian Holler (:decoder)]

Comment on attachment 9015153 [details] [diff] [review]
svn

I think it is ok to make this change but this bug is closed so you will have to file a new bug to make this change. It might also make sense for you to look into setting up moz-phab as described here: https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html# so you can push patches directly to our phabricator instance, making things a lot easier once setup correctly.

Comment 17

[u473386]

What's not completely clear to me is if I still need to file (and reference) bug reports then. It appears yes.

Comment 18

[Christian Holler (:decoder)]

(In reply to pdknsk from comment #17)
> What's not completely clear to me is if I still need to file (and reference)
> bug reports then. It appears yes.

Yes, a bug always needs to be filed, but having the Phabricator setup working makes it much more convenient to work with the changes. You can push the patch directly from command line to review using moz-phab submit, make changes and repush if necessary, and also trigger landing directly from Phabricator. You don't need to update the bug anymore yourself with patches, review requests, etc. So this certainly makes sense if you want to continue to contribute patches, and setup is fairly easy. Send me an email if you have more questions about this :)