Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2274563002:
Add fuzzer infra to blink and fuzz CSS parser (Closed)
|
Created:
4 years, 4 months ago by Charlie Harrison Modified:
4 years, 3 months ago CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-style_chromium.org, chromium-reviews, dglazkov+blink, rwlbuis Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd fuzzer infra to blink and fuzz CSS parser
This patch adds initialization for blink fuzzers that need the
full unit test environment, as well as a simple fuzzer for the
CSS tokenizer + parser.
BUG=642570, 642572
Committed: https://crrev.com/b85a5c4053a3d53331834b6f533196a6a11ee41c
Cr-Commit-Position: refs/heads/master@{#415634}
Patch Set 1 #Patch Set 2 : remove files #Patch Set 3 : Add configs #Patch Set 4 : use explicit deps #
Total comments: 3
Patch Set 5 : fix up seed corpus #Patch Set 6 : rebase #
Messages
Total messages: 39 (20 generated)
Description was changed from ========== Add fuzzer infra to blink and fuzz CSS parser BUG= ========== to ========== Add fuzzer infra to blink and fuzz CSS parser This patch adds proper initialization for blink fuzzers, as well as a simple fuzzer for the CSS tokenizer + parser. BUG= ==========
Description was changed from ========== Add fuzzer infra to blink and fuzz CSS parser This patch adds proper initialization for blink fuzzers, as well as a simple fuzzer for the CSS tokenizer + parser. BUG= ========== to ========== Add fuzzer infra to blink and fuzz CSS parser This patch adds initialization for blink fuzzers that need the full unit test environment, as well as a simple fuzzer for the CSS tokenizer + parser. BUG= ==========
The CQ bit was checked by csharrison@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
csharrison@chromium.org changed reviewers: + esprehn@chromium.org
Elliott, could you take a look at this? I noticed a few of my fuzzers failing due to assuming a unit test like environment, so I wrote up a function that will initialize fuzzers with a similar environment (as we talked about offline). This also adds a dead-simple css parser fuzzer.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Looks like there are some dependency issues (they should probably surface with local building). I'll try to address those now.
Looks like there are some dependency issues (they should probably surface with local building). I'll try to address those now.
The CQ bit was checked by csharrison@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Issues are fixed. Ready for a first look!
csharrison@chromium.org changed reviewers: + mmoroz@chromium.org
+mmoroz@ for new fuzzer review.
LGTM after addressing a comment on seed_corpus path. https://codereview.chromium.org/2274563002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/BUILD.gn (right): https://codereview.chromium.org/2274563002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/BUILD.gn:1139: seed_corpus = "//third_party/WebKit/LayoutTests/fast/css/resources/*.css" I'm afraid this wildcard won't work because we expect that value to be a path to corpus directory. It might be a good thing to have an opportunity to filter files like "*.css", but right now we don't have it. It would be fine to pass the whole directory (there are not too many files, and html/js ones also have some CSS-strings, so it won't harm).
Thanks! https://codereview.chromium.org/2274563002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/BUILD.gn (right): https://codereview.chromium.org/2274563002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/BUILD.gn:1139: seed_corpus = "//third_party/WebKit/LayoutTests/fast/css/resources/*.css" On 2016/08/25 08:00:47, mmoroz wrote: > I'm afraid this wildcard won't work because we expect that value to be a path to > corpus directory. It might be a good thing to have an opportunity to filter > files like "*.css", but right now we don't have it. It would be fine to pass the > whole directory (there are not too many files, and html/js ones also have some > CSS-strings, so it won't harm). My main concern was files that were too big (e.g. binaries). Will they override the max_len option or should it be fine?
https://codereview.chromium.org/2274563002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/BUILD.gn (right): https://codereview.chromium.org/2274563002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/BUILD.gn:1139: seed_corpus = "//third_party/WebKit/LayoutTests/fast/css/resources/*.css" On 2016/08/25 13:01:14, Charlie Harrison wrote: > On 2016/08/25 08:00:47, mmoroz wrote: > > I'm afraid this wildcard won't work because we expect that value to be a path > to > > corpus directory. It might be a good thing to have an opportunity to filter > > files like "*.css", but right now we don't have it. It would be fine to pass > the > > whole directory (there are not too many files, and html/js ones also have some > > CSS-strings, so it won't harm). > > My main concern was files that were too big (e.g. binaries). Will they override > the max_len option or should it be fine? It should be fine. LibFuzzer will use only first |max_len| bytes of those files.
Thanks for the clarification, I updated the GN file. esprehn@ could you ptal?
esprehn@ friendly ping. Feel free to punt if this review should go to someone else.
Is there a BUG this should be associated with? lgtm
Description was changed from ========== Add fuzzer infra to blink and fuzz CSS parser This patch adds initialization for blink fuzzers that need the full unit test environment, as well as a simple fuzzer for the CSS tokenizer + parser. BUG= ========== to ========== Add fuzzer infra to blink and fuzz CSS parser This patch adds initialization for blink fuzzers that need the full unit test environment, as well as a simple fuzzer for the CSS tokenizer + parser. BUG=642570,642572 ==========
On 2016/08/30 21:33:54, esprehn wrote: > Is there a BUG this should be associated with? > > lgtm Yeah, I added an issue for both fuzzer infra and css fuzzer.
The CQ bit was checked by csharrison@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mmoroz@chromium.org Link to the patchset: https://codereview.chromium.org/2274563002/#ps80001 (title: "fix up seed corpus")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
mmoroz@ quick question: Does clusterfuzz run with odr violations on? I am
getting odr errors on some generated mojo code:
==10957==ERROR: AddressSanitizer: odr-violation (0x7ff063a1aa20):
[1] size=50 'memory_coordinator::mojom::ChildMemoryCoordinator::Name_'
gen/components/memory_coordinator/public/interfaces/child_memory_coordinator.mojom.cc:31:36
[2] size=50 'memory_coordinator::mojom::ChildMemoryCoordinator::Name_'
gen/components/memory_coordinator/public/interfaces/child_memory_coordinator.mojom.cc:31:36
These globals were registered at these points:
[1]:
#0 0x4821d7
(/usr/local/google/home/csharrison/chromium/src/out/libfuzzer/stylesheet_contents_fuzzer+0x4821d7)
#1 0x7ff0629682cd
(/usr/local/google/home/csharrison/chromium/src/out/libfuzzer/./libmemory_coordinator_child.so+0x432cd)
[2]:
#0 0x4821d7
(/usr/local/google/home/csharrison/chromium/src/out/libfuzzer/stylesheet_contents_fuzzer+0x4821d7)
#1 0x7ff0636ca71d
(/usr/local/google/home/csharrison/chromium/src/out/libfuzzer/./libmemory_coordinator_common.so+0x3371d)
==10957==HINT: if you don't care about these errors you may set
ASAN_OPTIONS=detect_odr_violation=0
SUMMARY: AddressSanitizer: odr-violation: global
'memory_coordinator::mojom::ChildMemoryCoordinator::Name_' at
gen/components/memory_coordinator/public/interfaces/child_memory_coordinator.mojom.cc:31:36
==10957==ABORTING
I think this is crbug.com/636563 but I'm not sure (just same symptoms). I'm
happy landing this if the fuzzer will still work though.
On 2016/08/31 03:50:49, Charlie Harrison wrote: > mmoroz@ quick question: Does clusterfuzz run with odr violations on? I am > getting odr errors on some generated mojo code: > > ==10957==ERROR: AddressSanitizer: odr-violation (0x7ff063a1aa20): > [1] size=50 'memory_coordinator::mojom::ChildMemoryCoordinator::Name_' > gen/components/memory_coordinator/public/interfaces/child_memory_coordinator.mojom.cc:31:36 > [2] size=50 'memory_coordinator::mojom::ChildMemoryCoordinator::Name_' > gen/components/memory_coordinator/public/interfaces/child_memory_coordinator.mojom.cc:31:36 > These globals were registered at these points: > [1]: > #0 0x4821d7 > (/usr/local/google/home/csharrison/chromium/src/out/libfuzzer/stylesheet_contents_fuzzer+0x4821d7) > #1 0x7ff0629682cd > (/usr/local/google/home/csharrison/chromium/src/out/libfuzzer/./libmemory_coordinator_child.so+0x432cd) > > [2]: > #0 0x4821d7 > (/usr/local/google/home/csharrison/chromium/src/out/libfuzzer/stylesheet_contents_fuzzer+0x4821d7) > #1 0x7ff0636ca71d > (/usr/local/google/home/csharrison/chromium/src/out/libfuzzer/./libmemory_coordinator_common.so+0x3371d) > > ==10957==HINT: if you don't care about these errors you may set > ASAN_OPTIONS=detect_odr_violation=0 > SUMMARY: AddressSanitizer: odr-violation: global > 'memory_coordinator::mojom::ChildMemoryCoordinator::Name_' at > gen/components/memory_coordinator/public/interfaces/child_memory_coordinator.mojom.cc:31:36 > ==10957==ABORTING > > I think this is crbug.com/636563 but I'm not sure (just same symptoms). I'm > happy landing this if the fuzzer will still work though. As I see, we do not disable it by default on linux. But we can easily do that, feel free to land the fuzzer!
The CQ bit was checked by csharrison@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mmoroz@chromium.org, esprehn@chromium.org Link to the patchset: https://codereview.chromium.org/2274563002/#ps100001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add fuzzer infra to blink and fuzz CSS parser This patch adds initialization for blink fuzzers that need the full unit test environment, as well as a simple fuzzer for the CSS tokenizer + parser. BUG=642570,642572 ========== to ========== Add fuzzer infra to blink and fuzz CSS parser This patch adds initialization for blink fuzzers that need the full unit test environment, as well as a simple fuzzer for the CSS tokenizer + parser. BUG=642570,642572 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Add fuzzer infra to blink and fuzz CSS parser This patch adds initialization for blink fuzzers that need the full unit test environment, as well as a simple fuzzer for the CSS tokenizer + parser. BUG=642570,642572 ========== to ========== Add fuzzer infra to blink and fuzz CSS parser This patch adds initialization for blink fuzzers that need the full unit test environment, as well as a simple fuzzer for the CSS tokenizer + parser. BUG=642570,642572 Committed: https://crrev.com/b85a5c4053a3d53331834b6f533196a6a11ee41c Cr-Commit-Position: refs/heads/master@{#415634} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/b85a5c4053a3d53331834b6f533196a6a11ee41c Cr-Commit-Position: refs/heads/master@{#415634} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
