|
|
Chromium Code Reviews|
Created:
4 years, 9 months ago by Oliver Chang Modified:
4 years, 8 months ago CC:
chromium-reviews, kcc2, inferno, mmoroz Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionPull in libFuzzer as a third party dep.
This makes it so that developers no longer have to build
clang locally to get libFuzzer.
R=thakis@chromium.org,aizatsky@chromium.org
BUG=591248, 598448
# Previous patchset passed CQ bots, but had a trivial conflict when
# trying to apply because of catapult roll.
NOTRY=true
Committed: https://crrev.com/cbd1cf202d289ccaf76aa67828e9425e5a061d6b
Cr-Commit-Position: refs/heads/master@{#384692}
Patch Set 1 #Patch Set 2 : DEPS #Patch Set 3 : Update readme. #
Total comments: 3
Patch Set 4 : move negative config #
Total comments: 2
Patch Set 5 : add svn revision comment #Patch Set 6 : checklicenses #Patch Set 7 : rolling libfuzzer to pick up license header changes #Patch Set 8 : rebase #Patch Set 9 : rebase again because catapalt #
Messages
Total messages: 54 (23 generated)
Nico, Mike, mind taking a look at this?
Do we really want to just check in these files? Don't we'd rather want to set up something like https://chromium.googlesource.com/external/llvm.org/llvm/ (except for libfuzzer) and pull that in via DEPS? (ask on http://crbug.com/587658 to get that mirror) That way, rolling libfuzzer only requires changing a single number in deps
On 2016/03/02 01:12:24, Nico wrote: > Do we really want to just check in these files? Don't we'd rather want to set up > something like https://chromium.googlesource.com/external/llvm.org/llvm/ (except > for libfuzzer) and pull that in via DEPS? (ask on http://crbug.com/587658 to get > that mirror) > > That way, rolling libfuzzer only requires changing a single number in deps I think libfuzzer is part of the llvm tree under lib/Fuzzer? Is there a way to only pull that directory via DEPS without pulling the rest of LLVM?
On 2016/03/02 01:19:13, Oliver Chang wrote: > On 2016/03/02 01:12:24, Nico wrote: > > Do we really want to just check in these files? Don't we'd rather want to set > up > > something like https://chromium.googlesource.com/external/llvm.org/llvm/ > (except > > for libfuzzer) and pull that in via DEPS? (ask on http://crbug.com/587658 to > get > > that mirror) > > > > That way, rolling libfuzzer only requires changing a single number in deps > > I think libfuzzer is part of the llvm tree under lib/Fuzzer? Is there a way to > only pull that directory via DEPS without pulling the rest of LLVM? Not with git; you'd have to set up a separate git mirror. Maybe you could pull it via svn from the https://src.chromium.org/viewvc/llvm-project/llvm/trunk/lib/Fuzzer/ mirror, but I think infra would prefer if you ask them for a mirror for that subdirectory.
On 2016/03/02 01:44:35, Nico wrote: > On 2016/03/02 01:19:13, Oliver Chang wrote: > > On 2016/03/02 01:12:24, Nico wrote: > > > Do we really want to just check in these files? Don't we'd rather want to > set > > up > > > something like https://chromium.googlesource.com/external/llvm.org/llvm/ > > (except > > > for libfuzzer) and pull that in via DEPS? (ask on http://crbug.com/587658 to > > get > > > that mirror) > > > > > > That way, rolling libfuzzer only requires changing a single number in deps > > > > I think libfuzzer is part of the llvm tree under lib/Fuzzer? Is there a way to > > only pull that directory via DEPS without pulling the rest of LLVM? > > Not with git; you'd have to set up a separate git mirror. > > Maybe you could pull it via svn from the > https://src.chromium.org/viewvc/llvm-project/llvm/trunk/lib/Fuzzer/ mirror, but > I think infra would prefer if you ask them for a mirror for that subdirectory. Cool, filed a bug (https://crbug.com/591259). Will update this CL once that's set up, thanks!
Description was changed from ========== Setup a mirror of libFuzzer in third_party. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248 ========== to ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248 ==========
Nico, Mike, I've updated this CL to pull in libFuzzer via DEPS. Please take a look.
https://codereview.chromium.org/1752183002/diff/40001/testing/libfuzzer/BUILD.gn File testing/libfuzzer/BUILD.gn (right): https://codereview.chromium.org/1752183002/diff/40001/testing/libfuzzer/BUILD... testing/libfuzzer/BUILD.gn:15: configs -= [ "//build/config/sanitizers:default_sanitizer_coverage_flags" ] Will this negative config propagate into source_set?
https://codereview.chromium.org/1752183002/diff/40001/testing/libfuzzer/BUILD.gn File testing/libfuzzer/BUILD.gn (right): https://codereview.chromium.org/1752183002/diff/40001/testing/libfuzzer/BUILD... testing/libfuzzer/BUILD.gn:15: configs -= [ "//build/config/sanitizers:default_sanitizer_coverage_flags" ] On 2016/03/31 22:14:43, aizatsky wrote: > Will this negative config propagate into source_set? No.
https://codereview.chromium.org/1752183002/diff/40001/testing/libfuzzer/BUILD.gn File testing/libfuzzer/BUILD.gn (right): https://codereview.chromium.org/1752183002/diff/40001/testing/libfuzzer/BUILD... testing/libfuzzer/BUILD.gn:15: configs -= [ "//build/config/sanitizers:default_sanitizer_coverage_flags" ] On 2016/03/31 22:24:01, Nico wrote: > On 2016/03/31 22:14:43, aizatsky wrote: > > Will this negative config propagate into source_set? > > No. Moved to third_party/libFuzzer/BUILD.gn
lgtm! https://codereview.chromium.org/1752183002/diff/60001/DEPS File DEPS (right): https://codereview.chromium.org/1752183002/diff/60001/DEPS#newcode107 DEPS:107: 'libfuzzer_revision': '7af9c54c44b8ff3beed8456689d04628a3a8a58a', should this have a comment which llvm svn revision it corresponds too?
(does this work on non-linux? if so, consider also adding a gyp file)
On 2016/03/31 22:35:04, Nico wrote: > (does this work on non-linux? if so, consider also adding a gyp file) It is our decision to support only gn.
lgtm
ok On Thu, Mar 31, 2016 at 6:39 PM, <aizatsky@chromium.org> wrote: > On 2016/03/31 22:35:04, Nico wrote: > > (does this work on non-linux? if so, consider also adding a gyp file) > > It is our decision to support only gn. > > https://codereview.chromium.org/1752183002/ > -- 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.
thanks for reviewing! https://codereview.chromium.org/1752183002/diff/60001/DEPS File DEPS (right): https://codereview.chromium.org/1752183002/diff/60001/DEPS#newcode107 DEPS:107: 'libfuzzer_revision': '7af9c54c44b8ff3beed8456689d04628a3a8a58a', On 2016/03/31 22:34:36, Nico wrote: > should this have a comment which llvm svn revision it corresponds too? Added.
The CQ bit was checked by ochang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org, aizatsky@chromium.org Link to the patchset: https://codereview.chromium.org/1752183002/#ps80001 (title: "add svn revision comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1752183002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1752183002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by ochang@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1752183002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1752183002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
That's https://bugs.chromium.org/p/chromium/issues/detail?id=599692&q=%22file%20is%2... apparently On Thu, Mar 31, 2016 at 8:48 PM, commit-bot@chromium.org via codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote: > Try jobs failed on following builders: > chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, > > http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub... > ) > > https://codereview.chromium.org/1752183002/ > -- 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.
ochang@chromium.org changed reviewers: + thestig@chromium.org
Oops, didn't notice the checklicenses failure. +Lei for checklicenses.py change for FuzzerSHA1.cpp Mike, it looks like some libFuzzer test files are missing license headers. Could you please them? See https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...: 'third_party/libFuzzer/src/test/SimpleTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/CounterTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/BufferOverflowOnInput.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/SwitchTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/StrncmpTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/TimeoutTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/CallerCalleeTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/StrcmpTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/LeakTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/NullDerefTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/SpamyTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/FourIndependentBranchesTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/LeakTimeoutTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/UninstrumentedTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/MemcmpTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/SimpleHashTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/NthRunCrashTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/SimpleDictionaryTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/FullCoverageSetTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/ThreadedTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/RepeatedMemcmp.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/CustomMutatorTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/InitializeTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/FuzzerFnAdapterUnittest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/SimpleFnAdapterTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/FuzzerUnittest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/test/SimpleCmpTest.cpp' has non-whitelisted license 'UNKNOWN' 'third_party/libFuzzer/src/FuzzerSHA1.cpp' has non-whitelisted license 'Public domain University of Illinois/NCSA Open Source License (BSD like)'
On 2016/04/01 01:55:34, Oliver Chang wrote: > Oops, didn't notice the checklicenses failure. +Lei for checklicenses.py change > for FuzzerSHA1.cpp lgtm > Mike, it looks like some libFuzzer test files are missing license headers. Could > you please them? s/please/please add/ ?
On 2016/04/01 04:28:57, Lei Zhang wrote: > On 2016/04/01 01:55:34, Oliver Chang wrote: > > Oops, didn't notice the checklicenses failure. +Lei for checklicenses.py > change > > for FuzzerSHA1.cpp > > lgtm > > > Mike, it looks like some libFuzzer test files are missing license headers. > Could > > you please them? > > s/please/please add/ ? What is the minimum I could add? No other test sources in llvm have licenses. I wouldn't want to add full multi-line headers.
On 2016/04/01 17:50:10, aizatsky wrote: > On 2016/04/01 04:28:57, Lei Zhang wrote: > > On 2016/04/01 01:55:34, Oliver Chang wrote: > > > Oops, didn't notice the checklicenses failure. +Lei for checklicenses.py > > change > > > for FuzzerSHA1.cpp > > > > lgtm > > > > > Mike, it looks like some libFuzzer test files are missing license headers. > > Could > > > you please them? > > > > s/please/please add/ ? > > What is the minimum I could add? No other test sources in llvm have licenses. I > wouldn't want to add full multi-line headers. A: // This file is distributed under the University of Illinois Open Source // License. See LICENSE.TXT for details. At the start of each .cpp file seems to do the trick.
The CQ bit was checked by ochang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org, aizatsky@chromium.org, thestig@chromium.org Link to the patchset: https://codereview.chromium.org/1752183002/#ps120001 (title: "rolling libfuzzer to pick up license header changes")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1752183002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1752183002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was unchecked by ochang@chromium.org
The CQ bit was checked by ochang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org, thakis@chromium.org, aizatsky@chromium.org Link to the patchset: https://codereview.chromium.org/1752183002/#ps140001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1752183002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1752183002/140001
Description was changed from ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248 ========== to ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for DEPS:
While running git apply --index -3 -p1;
error: patch failed: DEPS:101
Falling back to three-way merge...
Applied patch to 'DEPS' with conflicts.
U DEPS
Patch: DEPS
Index: DEPS
diff --git a/DEPS b/DEPS
index
1593b91763f133db477d5393d56382242d88ccb7..8f0d575d0ec80050c3e76aa7468f2e14bffb79f3
100644
--- a/DEPS
+++ b/DEPS
@@ -101,6 +101,10 @@ vars = {
# the commit queue can handle CLs rolling catapult
# and whatever else without interference from each other.
'catapult_revision': 'df46fb3507ac4715b8dd3c1dbc7a3413beed3e58',
+ # Three lines of non-changing comments so that
+ # the commit queue can handle CLs rolling libFuzzer
+ # and whatever else without interference from each other.
+ 'libfuzzer_revision': '1d3e15006639ad8c1aadd2d7bac4dab9a6be7da6', # from svn
revision 265174
}
# Only these hosts are allowed for dependencies in this DEPS file.
@@ -430,6 +434,10 @@ deps_os = {
# Wireless Display Software. Used on Chrome OS.
'src/third_party/wds/src':
Var('chromium_git') + '/external/github.com/01org/wds' + '@' +
'f187dda5fccaad08e168dc6657109325f42c648e',
+
+ # Used for building libFuzzers (only supports Linux).
+ 'src/third_party/libFuzzer/src':
+ Var('chromium_git') + '/chromium/llvm-project/llvm/lib/Fuzzer.git' + '@' +
Var('libfuzzer_revision'),
},
'android': {
'src/third_party/android_protobuf/src':
The CQ bit was unchecked by commit-bot@chromium.org
Description was changed from ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 ========== to ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 # Previous patchset passed CQ, but had a trivial conflict when # trying to apply because of catapult roll. NOTRY=true ==========
Description was changed from ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 # Previous patchset passed CQ, but had a trivial conflict when # trying to apply because of catapult roll. NOTRY=true ========== to ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 # Previous patchset passed CQ, but had a trivial conflict when # trying to apply because of catapult roll. NOTRY=true ==========
The CQ bit was checked by ochang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org, thakis@chromium.org, aizatsky@chromium.org Link to the patchset: https://codereview.chromium.org/1752183002/#ps160001 (title: "rebase again because catapalt")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1752183002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1752183002/160001
Description was changed from ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 # Previous patchset passed CQ, but had a trivial conflict when # trying to apply because of catapult roll. NOTRY=true ========== to ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 # Previous patchset passed CQ bots, but had a trivial conflict when # trying to apply because of catapult roll. NOTRY=true ==========
Message was sent while issue was closed.
Description was changed from ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 # Previous patchset passed CQ bots, but had a trivial conflict when # trying to apply because of catapult roll. NOTRY=true ========== to ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 # Previous patchset passed CQ bots, but had a trivial conflict when # trying to apply because of catapult roll. NOTRY=true ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 # Previous patchset passed CQ bots, but had a trivial conflict when # trying to apply because of catapult roll. NOTRY=true ========== to ========== Pull in libFuzzer as a third party dep. This makes it so that developers no longer have to build clang locally to get libFuzzer. R=thakis@chromium.org,aizatsky@chromium.org BUG=591248,598448 # Previous patchset passed CQ bots, but had a trivial conflict when # trying to apply because of catapult roll. NOTRY=true Committed: https://crrev.com/cbd1cf202d289ccaf76aa67828e9425e5a061d6b Cr-Commit-Position: refs/heads/master@{#384692} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/cbd1cf202d289ccaf76aa67828e9425e5a061d6b Cr-Commit-Position: refs/heads/master@{#384692} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
