Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2029323005:
[libfuzzer] content/renderer fuzzer. (Closed)
Description[libfuzzer] content/renderer fuzzer.
BUG=539572
Committed: https://crrev.com/e62e3168e0521b0d1ba70cd6dbbf6e5daa763301
Cr-Commit-Position: refs/heads/master@{#397797}
Patch Set 1 #Patch Set 2 : nits #
Total comments: 11
Patch Set 3 : addressing comments #
Total comments: 4
Messages
Total messages: 18 (5 generated)
aizatsky@chromium.org changed reviewers: + mmoroz@chromium.org, ochang@chromium.org, sky@chromium.org
This is a first attempt at creating content/renderer fuzzer. This version uses data URLs which I plan to change later. I'd really like //content folks to tell me if I'm setting things up right.
https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... File content/test/renderer_fuzzer.cc (right): https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:19: using namespace content; Why not put this in the content namespace? https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:27: RenderViewImpl* view() { return static_cast<RenderViewImpl*>(view_); } Can you use the public types? https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:32: }; private: DISALLOW.. (here and below). https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:50: std::unique_ptr<FuzzerRenderTest> test_; structs don't use '_' in the name. https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:53: static Env* env = new Env(); Is there no setup/tear down type equivalent?
All done. PTAL. https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... File content/test/renderer_fuzzer.cc (right): https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:19: using namespace content; On 2016/06/02 at 21:57:13, sky wrote: > Why not put this in the content namespace? Done. https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:19: using namespace content; On 2016/06/02 at 21:57:13, sky wrote: > Why not put this in the content namespace? Done. https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:27: RenderViewImpl* view() { return static_cast<RenderViewImpl*>(view_); } On 2016/06/02 at 21:57:13, sky wrote: > Can you use the public types? Done. https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:32: }; On 2016/06/02 at 21:57:13, sky wrote: > private: DISALLOW.. (here and below). Done. https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:50: std::unique_ptr<FuzzerRenderTest> test_; On 2016/06/02 at 21:57:13, sky wrote: > structs don't use '_' in the name. Done https://codereview.chromium.org/2029323005/diff/20001/content/test/renderer_f... content/test/renderer_fuzzer.cc:53: static Env* env = new Env(); On 2016/06/02 at 21:57:13, sky wrote: > Is there no setup/tear down type equivalent? No. Fuzzers do not do teardowns. There's only implicit setup at module initialization time. We run them to crash.
LGTM
LGTM https://codereview.chromium.org/2029323005/diff/40001/content/test/renderer_f... File content/test/renderer_fuzzer.cc (right): https://codereview.chromium.org/2029323005/diff/40001/content/test/renderer_f... content/test/renderer_fuzzer.cc:65: common_params.url = GURL("data:text/html," + input); I guess we need a dictionary for HTML? I will try to generate it :) https://codereview.chromium.org/2029323005/diff/40001/content/test/renderer_f... content/test/renderer_fuzzer.cc:72: } // namespace content Does it make sense to have 'extern "C" int LLVMFuzzerTestOneInput()' inside of a namespace?
The CQ bit was checked by aizatsky@chromium.org
https://codereview.chromium.org/2029323005/diff/40001/content/test/renderer_f... File content/test/renderer_fuzzer.cc (right): https://codereview.chromium.org/2029323005/diff/40001/content/test/renderer_f... content/test/renderer_fuzzer.cc:65: common_params.url = GURL("data:text/html," + input); On 2016/06/03 at 14:32:46, mmoroz wrote: > I guess we need a dictionary for HTML? I will try to generate it :) Sure. Though my long-term plan is to explore structure-based generational fuzzing with this one. https://codereview.chromium.org/2029323005/diff/40001/content/test/renderer_f... content/test/renderer_fuzzer.cc:72: } // namespace content On 2016/06/03 at 14:32:46, mmoroz wrote: > Does it make sense to have 'extern "C" int LLVMFuzzerTestOneInput()' inside of a namespace? Yes. It let's you not to write "content::" all the time.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2029323005/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by aizatsky@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2029323005/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== [libfuzzer] content/renderer fuzzer. BUG=539572 ========== to ========== [libfuzzer] content/renderer fuzzer. BUG=539572 Committed: https://crrev.com/e62e3168e0521b0d1ba70cd6dbbf6e5daa763301 Cr-Commit-Position: refs/heads/master@{#397797} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/e62e3168e0521b0d1ba70cd6dbbf6e5daa763301 Cr-Commit-Position: refs/heads/master@{#397797}
Message was sent while issue was closed.
On 2016/06/02 at 23:12:54, sky wrote: > LGTM Scott, I'd like to change this fuzzer to render the html string directly rather than stuff it into data url. Do you have any code pointers on how to do it?
Message was sent while issue was closed.
Not sure. I would have to dig myself. On Thu, Jun 16, 2016 at 12:01 PM, <aizatsky@chromium.org> wrote: > On 2016/06/02 at 23:12:54, sky wrote: >> LGTM > > Scott, I'd like to change this fuzzer to render the html string directly > rather > than stuff it into data url. Do you have any code pointers on how to do it? > > https://codereview.chromium.org/2029323005/ -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
