automate updating internal libFuzzer code

RESOLVED FIXED in Firefox 63

Status

()

enhancement
RESOLVED FIXED
9 months ago
6 months ago

People

(Reporter: pdknsk, Assigned: pdknsk)

Tracking

Trunk
Firefox 63
Points:
---

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

9 months ago
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?
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).
(Assignee)

Comment 2

9 months ago
> 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.
(Assignee)

Comment 3

9 months ago
Posted patch libfuzzer (obsolete) — Splinter Review
Please take a look. I used it to auto-sync to HEAD, and then built Firefox. Fuzzing works.
Attachment #8998672 - Flags: review?(choller)
(Assignee)

Comment 4

9 months ago
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 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.
Attachment #8998672 - Flags: review?(choller)
(Assignee)

Comment 6

8 months ago
> 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?
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?
(Assignee)

Comment 8

8 months ago
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).
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.
(Assignee)

Comment 10

8 months ago
Posted patch libFuzzer-2Splinter Review
With no argument it works as before, otherwise it updates to that revision.
Attachment #8998672 - Attachment is obsolete: true
Attachment #8999835 - Flags: review?(choller)
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.
Attachment #8999835 - Flags: review?(choller) → review+
(Assignee)

Comment 12

8 months ago
Posted patch libFuzzer-3Splinter Review
(Commitified.)
(Assignee)

Updated

8 months ago
Keywords: checkin-needed
Assignee: nobody → pdknsk
Attachment #8999835 - Attachment is obsolete: true

Comment 13

8 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f578ca2a361e
Automate updating internal libFuzzer code. r=decoder
Keywords: checkin-needed

Comment 14

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f578ca2a361e
Status: UNCONFIRMED → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
(Assignee)

Comment 15

7 months ago
Posted patch svn (obsolete) — Splinter Review
Minor improvement. What do you think? It's useful for updates to HEAD, or as confirmation.
Attachment #9015153 - Flags: review?(choller)
(Assignee)

Updated

7 months ago
Attachment #9001412 - Attachment is obsolete: true
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.
Attachment #9015153 - Flags: review?(choller)
(Assignee)

Comment 17

7 months ago
What's not completely clear to me is if I still need to file (and reference) bug reports then. It appears yes.
Flags: needinfo?(choller)
(Assignee)

Updated

7 months ago
Attachment #9015153 - Attachment is obsolete: true
(Assignee)

Updated

7 months ago
Attachment #9001412 - Attachment is obsolete: false
(Assignee)

Updated

7 months ago
Attachment #8999835 - Attachment is obsolete: false
(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 :)
Flags: needinfo?(choller)
You need to log in before you can comment on or make changes to this bug.