Re: Add fuzzing coverage support
On Sat, Dec 5, 2015 at 1:54 AM, Nathan Sidwell wrote: > On 12/04/15 13:28, Dmitry Vyukov wrote: >> >> On Fri, Dec 4, 2015 at 6:39 PM, Jakub Jelinek wrote: >>> >>> On Fri, Dec 04, 2015 at 06:32:38PM +0100, Dmitry Vyukov wrote: +2015-12-04 Dmitry Vyukov + + * sancov.c: New file. + * Makefile.in (OBJS): Add sancov.o. + * invoke.texi (-fsanitize-coverage=trace-pc): Describe. + * passes.def (sancov_pass): Add. + * tree-pass.h (sancov_pass): Add. + * common.opt (-fsanitize-coverage=trace-pc): Add. + * sanitizer.def (BUILT_IN_SANITIZER_COV_TRACE_PC): Add. + * builtins.def (DEF_SANITIZER_BUILTIN): Enable for + flag_sanitize_coverage. >>> >>> >>> This is ok for trunk. >> >> >> >> Committed as 231296 > > > This seems to have changed the testsuite (gcc.dg/sancov) without record in > testsuite/ChangeLog. Further, the tests presume sanitizer coverage is > implemented. I'm seeing asan.c fail with: > > cc1: warning: -fsanitize=address and -fsanitize=kernel-address are not > supported for this target > > cc1: warning: -fsanitize=address not supported for this target > > output is: > cc1: warning: -fsanitize=address and -fsanitize=kernel-address are not > supported for this target > > cc1: warning: -fsanitize=address not supported for this target > > > FAIL: gcc.dg/sancov/asan.c -O0 (test for excess errors) Mailed a fix titled "[PATCH] Fix new sancov tests". Sorry for the breakage.
Re: Add fuzzing coverage support
On 12/04/15 13:28, Dmitry Vyukov wrote: On Fri, Dec 4, 2015 at 6:39 PM, Jakub Jelinek wrote: On Fri, Dec 04, 2015 at 06:32:38PM +0100, Dmitry Vyukov wrote: +2015-12-04 Dmitry Vyukov + + * sancov.c: New file. + * Makefile.in (OBJS): Add sancov.o. + * invoke.texi (-fsanitize-coverage=trace-pc): Describe. + * passes.def (sancov_pass): Add. + * tree-pass.h (sancov_pass): Add. + * common.opt (-fsanitize-coverage=trace-pc): Add. + * sanitizer.def (BUILT_IN_SANITIZER_COV_TRACE_PC): Add. + * builtins.def (DEF_SANITIZER_BUILTIN): Enable for + flag_sanitize_coverage. This is ok for trunk. Committed as 231296 This seems to have changed the testsuite (gcc.dg/sancov) without record in testsuite/ChangeLog. Further, the tests presume sanitizer coverage is implemented. I'm seeing asan.c fail with: cc1: warning: -fsanitize=address and -fsanitize=kernel-address are not supported for this target cc1: warning: -fsanitize=address not supported for this target output is: cc1: warning: -fsanitize=address and -fsanitize=kernel-address are not supported for this target cc1: warning: -fsanitize=address not supported for this target FAIL: gcc.dg/sancov/asan.c -O0 (test for excess errors)
Re: Add fuzzing coverage support
On Fri, Dec 4, 2015 at 6:39 PM, Jakub Jelinek wrote: > On Fri, Dec 04, 2015 at 06:32:38PM +0100, Dmitry Vyukov wrote: >> +2015-12-04 Dmitry Vyukov >> + >> + * sancov.c: New file. >> + * Makefile.in (OBJS): Add sancov.o. >> + * invoke.texi (-fsanitize-coverage=trace-pc): Describe. >> + * passes.def (sancov_pass): Add. >> + * tree-pass.h (sancov_pass): Add. >> + * common.opt (-fsanitize-coverage=trace-pc): Add. >> + * sanitizer.def (BUILT_IN_SANITIZER_COV_TRACE_PC): Add. >> + * builtins.def (DEF_SANITIZER_BUILTIN): Enable for >> + flag_sanitize_coverage. > > This is ok for trunk. Committed as 231296
Re: Add fuzzing coverage support
On Fri, Dec 04, 2015 at 06:32:38PM +0100, Dmitry Vyukov wrote: > +2015-12-04 Dmitry Vyukov > + > + * sancov.c: New file. > + * Makefile.in (OBJS): Add sancov.o. > + * invoke.texi (-fsanitize-coverage=trace-pc): Describe. > + * passes.def (sancov_pass): Add. > + * tree-pass.h (sancov_pass): Add. > + * common.opt (-fsanitize-coverage=trace-pc): Add. > + * sanitizer.def (BUILT_IN_SANITIZER_COV_TRACE_PC): Add. > + * builtins.def (DEF_SANITIZER_BUILTIN): Enable for > + flag_sanitize_coverage. This is ok for trunk. Jakub
Re: Add fuzzing coverage support
On Fri, Dec 4, 2015 at 2:45 PM, Yury Gribov wrote:
> On 12/04/2015 04:41 PM, Jakub Jelinek wrote:
>>
>> Hi!
>>
>> While this has been posted after stage1 closed and I'm not really happy
>> that it missed the deadline, I'm willing to grant an exception, the patch
>> is small enough that it is ok at this point of stage3. That said, next
>> time
>> please try to submit new features in time.
>>
>> Are there any plans for GCC 7 for the other -fsanitize-coverage= options,
>> or are those just LLVM alternatives to GCC's gcov/-fprofile-generate etc.?
>>
>> On Thu, Dec 03, 2015 at 08:17:06PM +0100, Dmitry Vyukov wrote:
>>>
>>> +unsigned sancov_pass (function *fun)
>>
>>
>> Formatting:
>> unsigned
>> sancov_pass (function *fun)
>>
>>> +{
>>> + basic_block bb;
>>> + gimple_stmt_iterator gsi;
>>> + gimple *stmt, *f;
>>> + static bool inited;
>>> +
>>> + if (!inited)
>>> +{
>>> + inited = true;
>>> + initialize_sanitizer_builtins ();
>>> +}
>>
>>
>> You can call this unconditionally, it will return as the first thing
>> if it is already initialized, no need for another guard.
>>
>>> +
>>> + /* Insert callback into beginning of every BB. */
>>> + FOR_EACH_BB_FN (bb, fun)
>>> +{
>>> + gsi = gsi_after_labels (bb);
>>> + if (gsi_end_p (gsi))
>>> +continue;
>>> + stmt = gsi_stmt (gsi);
>>> + f = gimple_build_call (builtin_decl_implicit (
>>> + BUILT_IN_SANITIZER_COV_TRACE_PC), 0);
>>
>>
>> I (personally) prefer no ( at the end of line unless really needed.
>> In this case you can just do:
>>tree fndecl = builtin_decl_implicit
>> (BUILT_IN_SANITIZER_COV_TRACE_PC);
>>gimple *g = gimple_build_call (fndecl, 0);
>> which is same number of lines, but looks nicer.
>> Also, please move also the gsi, stmt and f (better g or gcall)
>> declarations to the first assignment to them, they aren't used outside of
>> the loop.
>
>
> Also FYI clang-format config has been recently added to contrib/
> (https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02214.html).
Rock-n-roll!
Applied to sancov.c
Re: Add fuzzing coverage support
On Fri, Dec 4, 2015 at 2:41 PM, Jakub Jelinek wrote:
> Hi!
>
> While this has been posted after stage1 closed and I'm not really happy
> that it missed the deadline, I'm willing to grant an exception, the patch
> is small enough that it is ok at this point of stage3. That said, next time
> please try to submit new features in time.
Sorry.
Thanks!
> Are there any plans for GCC 7 for the other -fsanitize-coverage= options,
> or are those just LLVM alternatives to GCC's gcov/-fprofile-generate etc.?
No, they are not alternatives to gcov. Other coverage modes are backed
by sanitizer_common runtime and libFuzzer, which together allow to do
efficient in-process fuzzing.
We don't have plans to port other options at the moment per se.
Though, that's doable and we could sync sanitizer library for runtime
support.
> On Thu, Dec 03, 2015 at 08:17:06PM +0100, Dmitry Vyukov wrote:
>> +unsigned sancov_pass (function *fun)
>
> Formatting:
> unsigned
> sancov_pass (function *fun)
Missed this. Done.
>> +{
>> + basic_block bb;
>> + gimple_stmt_iterator gsi;
>> + gimple *stmt, *f;
>> + static bool inited;
>> +
>> + if (!inited)
>> +{
>> + inited = true;
>> + initialize_sanitizer_builtins ();
>> +}
>
> You can call this unconditionally, it will return as the first thing
> if it is already initialized, no need for another guard.
Done
>> +
>> + /* Insert callback into beginning of every BB. */
>> + FOR_EACH_BB_FN (bb, fun)
>> +{
>> + gsi = gsi_after_labels (bb);
>> + if (gsi_end_p (gsi))
>> +continue;
>> + stmt = gsi_stmt (gsi);
>> + f = gimple_build_call (builtin_decl_implicit (
>> + BUILT_IN_SANITIZER_COV_TRACE_PC), 0);
>
> I (personally) prefer no ( at the end of line unless really needed.
> In this case you can just do:
> tree fndecl = builtin_decl_implicit (BUILT_IN_SANITIZER_COV_TRACE_PC);
> gimple *g = gimple_build_call (fndecl, 0);
> which is same number of lines, but looks nicer.
> Also, please move also the gsi, stmt and f (better g or gcall)
> declarations to the first assignment to them, they aren't used outside of
> the loop.
Done
>> --- testsuite/gcc.dg/sancov/asan.c(revision 0)
>> +++ testsuite/gcc.dg/sancov/asan.c(working copy)
>> @@ -0,0 +1,21 @@
>> +/* Test coverage/asan interaction:
>> + - coverage instruments __asan_init ctor (thus 4 covarage callbacks)
>> + - coverage does not instrument asan-emitted basic blocks
>> + - asan considers coverage callback as "nonfreeing" (thus 1 asan store
>> + callback. */
>> +/* { dg-do compile } */
>> +/* { dg-options "-fsanitize-coverage=trace-pc -fsanitize=address" } */
>> +
>> +void notailcall ();
>> +
>> +void foo(volatile int *a, int *b)
>> +{
>> + *a = 1;
>> + if (*b)
>> +*a = 2;
>> + notailcall ();
>> +}
>> +
>> +/* { dg-final { scan-assembler-times "call __sanitizer_cov_trace_pc" 4 }
>> } */
>> +/* { dg-final { scan-assembler-times "call __asan_report_load4" 1 } } */
>> +/* { dg-final { scan-assembler-times "call __asan_report_store4" 1 } } */
>
> I don't like these, we have lots of targets, and different targets have
> different instructions for making calls, different whitespace in between
> the insn name and called function, sometimes some extra decoration on the fn
> name, (say sometimes an extra _ prefix), etc. IMHO much better to add
> -fdump-tree-optimized and scan-tree-dump-times instead for the calls in the
> optimized dump. Affects all tests.
Done
Much better now.
> Please repost a patch with these changes fixed, it will be hopefully ackable
> then.
New patch is attached.
Code review is updated:
https://codereview.appspot.com/280140043
Test are passing. Also compiled and booted kernel with this.
Also applied clang-format to sancov.c.
Please take another look.
Index: ChangeLog
===
--- ChangeLog (revision 231277)
+++ ChangeLog (working copy)
@@ -1,3 +1,15 @@
+2015-12-04 Dmitry Vyukov
+
+ * sancov.c: New file.
+ * Makefile.in (OBJS): Add sancov.o.
+ * invoke.texi (-fsanitize-coverage=trace-pc): Describe.
+ * passes.def (sancov_pass): Add.
+ * tree-pass.h (sancov_pass): Add.
+ * common.opt (-fsanitize-coverage=trace-pc): Add.
+ * sanitizer.def (BUILT_IN_SANITIZER_COV_TRACE_PC): Add.
+ * builtins.def (DEF_SANITIZER_BUILTIN): Enable for
+ flag_sanitize_coverage.
+
2015-12-04 Jeff Law
* tree-ssa-reassoc.c (maybe_optimize_range_tests): Return boolean
@@ -593,7 +605,6 @@
* tree-ssa-structalias.c (find_func_aliases_for_builtin_call)
(find_func_clobbers, ipa_pta_execute): Handle BUILT_IN_GOACC_PARALLEL.
->>> .r231221
2015-12-02 Segher Boessenkool
* config/rs6000/rs6000.md (cstore_si_as_di): New expander.
Index: Makefile.in
===
--- Makefile.in (revision 231277)
+++ Makefile.in (working copy)
@@ -1427,6 +1427,7 @@
tsan.o \
ubsan.o \
sanopt.o \
+ sancov.
Re: Add fuzzing coverage support
On 12/04/2015 04:41 PM, Jakub Jelinek wrote:
Hi!
While this has been posted after stage1 closed and I'm not really happy
that it missed the deadline, I'm willing to grant an exception, the patch
is small enough that it is ok at this point of stage3. That said, next time
please try to submit new features in time.
Are there any plans for GCC 7 for the other -fsanitize-coverage= options,
or are those just LLVM alternatives to GCC's gcov/-fprofile-generate etc.?
On Thu, Dec 03, 2015 at 08:17:06PM +0100, Dmitry Vyukov wrote:
+unsigned sancov_pass (function *fun)
Formatting:
unsigned
sancov_pass (function *fun)
+{
+ basic_block bb;
+ gimple_stmt_iterator gsi;
+ gimple *stmt, *f;
+ static bool inited;
+
+ if (!inited)
+{
+ inited = true;
+ initialize_sanitizer_builtins ();
+}
You can call this unconditionally, it will return as the first thing
if it is already initialized, no need for another guard.
+
+ /* Insert callback into beginning of every BB. */
+ FOR_EACH_BB_FN (bb, fun)
+{
+ gsi = gsi_after_labels (bb);
+ if (gsi_end_p (gsi))
+continue;
+ stmt = gsi_stmt (gsi);
+ f = gimple_build_call (builtin_decl_implicit (
+ BUILT_IN_SANITIZER_COV_TRACE_PC), 0);
I (personally) prefer no ( at the end of line unless really needed.
In this case you can just do:
tree fndecl = builtin_decl_implicit (BUILT_IN_SANITIZER_COV_TRACE_PC);
gimple *g = gimple_build_call (fndecl, 0);
which is same number of lines, but looks nicer.
Also, please move also the gsi, stmt and f (better g or gcall)
declarations to the first assignment to them, they aren't used outside of
the loop.
Also FYI clang-format config has been recently added to contrib/
(https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02214.html).
--- testsuite/gcc.dg/sancov/asan.c (revision 0)
+++ testsuite/gcc.dg/sancov/asan.c (working copy)
@@ -0,0 +1,21 @@
+/* Test coverage/asan interaction:
+ - coverage instruments __asan_init ctor (thus 4 covarage callbacks)
+ - coverage does not instrument asan-emitted basic blocks
+ - asan considers coverage callback as "nonfreeing" (thus 1 asan store
+ callback. */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize-coverage=trace-pc -fsanitize=address" } */
+
+void notailcall ();
+
+void foo(volatile int *a, int *b)
+{
+ *a = 1;
+ if (*b)
+*a = 2;
+ notailcall ();
+}
+
+/* { dg-final { scan-assembler-times "call__sanitizer_cov_trace_pc" 4
} } */
+/* { dg-final { scan-assembler-times "call__asan_report_load4" 1 } } */
+/* { dg-final { scan-assembler-times "call__asan_report_store4" 1 } }
*/
I don't like these, we have lots of targets, and different targets have
different instructions for making calls, different whitespace in between
the insn name and called function, sometimes some extra decoration on the fn
name, (say sometimes an extra _ prefix), etc. IMHO much better to add
-fdump-tree-optimized and scan-tree-dump-times instead for the calls in the
optimized dump. Affects all tests.
Please repost a patch with these changes fixed, it will be hopefully ackable
then.
Jakub
Re: Add fuzzing coverage support
Hi!
While this has been posted after stage1 closed and I'm not really happy
that it missed the deadline, I'm willing to grant an exception, the patch
is small enough that it is ok at this point of stage3. That said, next time
please try to submit new features in time.
Are there any plans for GCC 7 for the other -fsanitize-coverage= options,
or are those just LLVM alternatives to GCC's gcov/-fprofile-generate etc.?
On Thu, Dec 03, 2015 at 08:17:06PM +0100, Dmitry Vyukov wrote:
> +unsigned sancov_pass (function *fun)
Formatting:
unsigned
sancov_pass (function *fun)
> +{
> + basic_block bb;
> + gimple_stmt_iterator gsi;
> + gimple *stmt, *f;
> + static bool inited;
> +
> + if (!inited)
> +{
> + inited = true;
> + initialize_sanitizer_builtins ();
> +}
You can call this unconditionally, it will return as the first thing
if it is already initialized, no need for another guard.
> +
> + /* Insert callback into beginning of every BB. */
> + FOR_EACH_BB_FN (bb, fun)
> +{
> + gsi = gsi_after_labels (bb);
> + if (gsi_end_p (gsi))
> +continue;
> + stmt = gsi_stmt (gsi);
> + f = gimple_build_call (builtin_decl_implicit (
> + BUILT_IN_SANITIZER_COV_TRACE_PC), 0);
I (personally) prefer no ( at the end of line unless really needed.
In this case you can just do:
tree fndecl = builtin_decl_implicit (BUILT_IN_SANITIZER_COV_TRACE_PC);
gimple *g = gimple_build_call (fndecl, 0);
which is same number of lines, but looks nicer.
Also, please move also the gsi, stmt and f (better g or gcall)
declarations to the first assignment to them, they aren't used outside of
the loop.
> --- testsuite/gcc.dg/sancov/asan.c(revision 0)
> +++ testsuite/gcc.dg/sancov/asan.c(working copy)
> @@ -0,0 +1,21 @@
> +/* Test coverage/asan interaction:
> + - coverage instruments __asan_init ctor (thus 4 covarage callbacks)
> + - coverage does not instrument asan-emitted basic blocks
> + - asan considers coverage callback as "nonfreeing" (thus 1 asan store
> + callback. */
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize-coverage=trace-pc -fsanitize=address" } */
> +
> +void notailcall ();
> +
> +void foo(volatile int *a, int *b)
> +{
> + *a = 1;
> + if (*b)
> +*a = 2;
> + notailcall ();
> +}
> +
> +/* { dg-final { scan-assembler-times "call __sanitizer_cov_trace_pc" 4 } }
> */
> +/* { dg-final { scan-assembler-times "call __asan_report_load4" 1 } } */
> +/* { dg-final { scan-assembler-times "call __asan_report_store4" 1 } } */
I don't like these, we have lots of targets, and different targets have
different instructions for making calls, different whitespace in between
the insn name and called function, sometimes some extra decoration on the fn
name, (say sometimes an extra _ prefix), etc. IMHO much better to add
-fdump-tree-optimized and scan-tree-dump-times instead for the calls in the
optimized dump. Affects all tests.
Please repost a patch with these changes fixed, it will be hopefully ackable
then.
Jakub
Re: Add fuzzing coverage support
On Thu, Dec 3, 2015 at 7:34 PM, Dmitry Vyukov wrote:
> I've attached updated patch (also reuploaded
> https://codereview.appspot.com/280140043).
> Fixed ChangeLog.
> Added invoke.texi.
> Fixed style issues.
>
> The function is defined only in kernel at the moment. Here is my patch:
> https://github.com/dvyukov/linux/commit/f86eda0c895c47ea02ee37e981aeade7b03014d7
> It is not mailed yet, for kernel asan people requested submit to gcc
> first, then to kernel.
>
> It will also be supported by libsanitizer later (Kostya?). But it is
> not yet there.
>
> Regarding plugins, we did tsan first as gcc plugin. It was difficult
> to support, difficult to use, difficult to distribute. I maintain this
> patch for a month, two people complained that it does not build
> (because they synched to slightly different revisions).
Added missing:
stmt = gsi_stmt (gsi);
Now actually run tests and compiled kernel with it.
Index: ChangeLog
===
--- ChangeLog (revision 231234)
+++ ChangeLog (working copy)
@@ -1,3 +1,15 @@
+2015-12-03 Dmitry Vyukov
+
+ * sancov.c: New file.
+ * Makefile.in (OBJS): Add sancov.o.
+ * invoke.texi (-fsanitize-coverage=trace-pc): Describe.
+ * passes.def (sancov_pass): Add.
+ * tree-pass.h (sancov_pass): Add.
+ * common.opt (-fsanitize-coverage=trace-pc): Add.
+ * sanitizer.def (BUILT_IN_SANITIZER_COV_TRACE_PC): Add.
+ * builtins.def (DEF_SANITIZER_BUILTIN): Enable for
+ flag_sanitize_coverage.
+
2015-12-03 Evandro Menezes
* config/aarch64/aarch64-cores.def: Use the Exynos M1 cost model.
@@ -360,7 +372,6 @@
* tree-ssa-structalias.c (find_func_aliases_for_builtin_call)
(find_func_clobbers, ipa_pta_execute): Handle BUILT_IN_GOACC_PARALLEL.
->>> .r231221
2015-12-02 Segher Boessenkool
* config/rs6000/rs6000.md (cstore_si_as_di): New expander.
Index: Makefile.in
===
--- Makefile.in (revision 231234)
+++ Makefile.in (working copy)
@@ -1427,6 +1427,7 @@
tsan.o \
ubsan.o \
sanopt.o \
+ sancov.o \
tree-call-cdce.o \
tree-cfg.o \
tree-cfgcleanup.o \
@@ -2400,6 +2401,7 @@
$(srcdir)/ubsan.c \
$(srcdir)/tsan.c \
$(srcdir)/sanopt.c \
+ $(srcdir)/sancov.c \
$(srcdir)/ipa-devirt.c \
$(srcdir)/internal-fn.h \
@all_gtfiles@
Index: builtins.def
===
--- builtins.def (revision 231234)
+++ builtins.def (working copy)
@@ -210,7 +210,8 @@
DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, TYPE,\
true, true, true, ATTRS, true, \
(flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_THREAD \
-| SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT)))
+| SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT) \
+ || flag_sanitize_coverage))
#undef DEF_CILKPLUS_BUILTIN
#define DEF_CILKPLUS_BUILTIN(ENUM, NAME, TYPE, ATTRS) \
Index: common.opt
===
--- common.opt (revision 231234)
+++ common.opt (working copy)
@@ -225,6 +225,11 @@
Variable
unsigned int flag_sanitize_recover = SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT | SANITIZE_KERNEL_ADDRESS
+fsanitize-coverage=trace-pc
+Common Report Var(flag_sanitize_coverage)
+Enable coverage-guided fuzzing code instrumentation.
+Inserts call to __sanitizer_cov_trace_pc into every basic block.
+
; Flag whether a prefix has been added to dump_base_name
Variable
bool dump_base_name_prefixed = false
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 231234)
+++ doc/invoke.texi (working copy)
@@ -6135,6 +6135,11 @@
@code{libubsan} library is not needed and is not linked in, so this
is usable even in freestanding environments.
+@item -fsanitize-coverage=trace-pc
+@opindex fsanitize-coverage=trace-pc
+Enable coverage-guided fuzzing code instrumentation.
+Inserts call to __sanitizer_cov_trace_pc into every basic block.
+
@item -fcheck-pointer-bounds
@opindex fcheck-pointer-bounds
@opindex fno-check-pointer-bounds
Index: passes.def
===
--- passes.def (revision 231234)
+++ passes.def (working copy)
@@ -237,6 +237,7 @@
NEXT_PASS (pass_split_crit_edges);
NEXT_PASS (pass_pre);
NEXT_PASS (pass_sink_code);
+ NEXT_PASS (pass_sancov);
NEXT_PASS (pass_asan);
NEXT_PASS (pass_tsan);
/* Pass group that runs when 1) enabled, 2) there are loops
@@ -346,6 +347,7 @@
to forward object-size and builtin folding results properly. */
NEXT_PASS (pass_copy_prop);
NEXT_PASS (pass_dce);
+ NEXT_PASS (pass_sancov);
NEXT_PASS (pass_asan);
NEXT_PASS (pass_tsan);
/* ??? We do want some kind of loop invariant motion, but we possibly
@@ -369,6 +371,7 @@
NEXT_PASS (pass_lower_vaarg);
NEXT_PASS (pass_lower_vector);
NEXT_PASS (pass_lower_complex_O0);
+ NE
Re: Add fuzzing coverage support
I've attached updated patch (also reuploaded
https://codereview.appspot.com/280140043).
Fixed ChangeLog.
Added invoke.texi.
Fixed style issues.
The function is defined only in kernel at the moment. Here is my patch:
https://github.com/dvyukov/linux/commit/f86eda0c895c47ea02ee37e981aeade7b03014d7
It is not mailed yet, for kernel asan people requested submit to gcc
first, then to kernel.
It will also be supported by libsanitizer later (Kostya?). But it is
not yet there.
Regarding plugins, we did tsan first as gcc plugin. It was difficult
to support, difficult to use, difficult to distribute. I maintain this
patch for a month, two people complained that it does not build
(because they synched to slightly different revisions).
Index: ChangeLog
===
--- ChangeLog (revision 231234)
+++ ChangeLog (working copy)
@@ -1,3 +1,15 @@
+2015-12-03 Dmitry Vyukov
+
+ * sancov.c: New file.
+ * Makefile.in (OBJS): Add sancov.o.
+ * invoke.texi (-fsanitize-coverage=trace-pc): Describe.
+ * passes.def (sancov_pass): Add.
+ * tree-pass.h (sancov_pass): Add.
+ * common.opt (-fsanitize-coverage=trace-pc): Add.
+ * sanitizer.def (BUILT_IN_SANITIZER_COV_TRACE_PC): Add.
+ * builtins.def (DEF_SANITIZER_BUILTIN): Enable for
+ flag_sanitize_coverage.
+
2015-12-03 Evandro Menezes
* config/aarch64/aarch64-cores.def: Use the Exynos M1 cost model.
@@ -360,7 +372,6 @@
* tree-ssa-structalias.c (find_func_aliases_for_builtin_call)
(find_func_clobbers, ipa_pta_execute): Handle BUILT_IN_GOACC_PARALLEL.
->>> .r231221
2015-12-02 Segher Boessenkool
* config/rs6000/rs6000.md (cstore_si_as_di): New expander.
Index: Makefile.in
===
--- Makefile.in (revision 231234)
+++ Makefile.in (working copy)
@@ -1427,6 +1427,7 @@
tsan.o \
ubsan.o \
sanopt.o \
+ sancov.o \
tree-call-cdce.o \
tree-cfg.o \
tree-cfgcleanup.o \
@@ -2400,6 +2401,7 @@
$(srcdir)/ubsan.c \
$(srcdir)/tsan.c \
$(srcdir)/sanopt.c \
+ $(srcdir)/sancov.c \
$(srcdir)/ipa-devirt.c \
$(srcdir)/internal-fn.h \
@all_gtfiles@
Index: builtins.def
===
--- builtins.def (revision 231234)
+++ builtins.def (working copy)
@@ -210,7 +210,8 @@
DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, TYPE,\
true, true, true, ATTRS, true, \
(flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_THREAD \
-| SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT)))
+| SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT) \
+ || flag_sanitize_coverage))
#undef DEF_CILKPLUS_BUILTIN
#define DEF_CILKPLUS_BUILTIN(ENUM, NAME, TYPE, ATTRS) \
Index: common.opt
===
--- common.opt (revision 231234)
+++ common.opt (working copy)
@@ -225,6 +225,11 @@
Variable
unsigned int flag_sanitize_recover = SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT | SANITIZE_KERNEL_ADDRESS
+fsanitize-coverage=trace-pc
+Common Report Var(flag_sanitize_coverage)
+Enable coverage-guided fuzzing code instrumentation.
+Inserts call to __sanitizer_cov_trace_pc into every basic block.
+
; Flag whether a prefix has been added to dump_base_name
Variable
bool dump_base_name_prefixed = false
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 231234)
+++ doc/invoke.texi (working copy)
@@ -6135,6 +6135,11 @@
@code{libubsan} library is not needed and is not linked in, so this
is usable even in freestanding environments.
+@item -fsanitize-coverage=trace-pc
+@opindex fsanitize-coverage=trace-pc
+Enable coverage-guided fuzzing code instrumentation.
+Inserts call to __sanitizer_cov_trace_pc into every basic block.
+
@item -fcheck-pointer-bounds
@opindex fcheck-pointer-bounds
@opindex fno-check-pointer-bounds
Index: passes.def
===
--- passes.def (revision 231234)
+++ passes.def (working copy)
@@ -237,6 +237,7 @@
NEXT_PASS (pass_split_crit_edges);
NEXT_PASS (pass_pre);
NEXT_PASS (pass_sink_code);
+ NEXT_PASS (pass_sancov);
NEXT_PASS (pass_asan);
NEXT_PASS (pass_tsan);
/* Pass group that runs when 1) enabled, 2) there are loops
@@ -346,6 +347,7 @@
to forward object-size and builtin folding results properly. */
NEXT_PASS (pass_copy_prop);
NEXT_PASS (pass_dce);
+ NEXT_PASS (pass_sancov);
NEXT_PASS (pass_asan);
NEXT_PASS (pass_tsan);
/* ??? We do want some kind of loop invariant motion, but we possibly
@@ -369,6 +371,7 @@
NEXT_PASS (pass_lower_vaarg);
NEXT_PASS (pass_lower_vector);
NEXT_PASS (pass_lower_complex_O0);
+ NEXT_PASS (pass_sancov_O0);
NEXT_PASS (pass_asan_O0);
NEXT_PASS (pass_tsan_O0);
NEXT_PASS (pass_sanopt);
Index: sancov.c
=
Re: Add fuzzing coverage support
On 12/02/2015 06:38 PM, Dmitry Vyukov wrote:
One thing to consider would
be whether you really need this split between O0/optimize versions, or
whether you can find a place in the queue where to insert it
unconditionally. Have you considered this at all or did you just follow
asan/tsan?
I inserted the pass just before asan/tsan because it looks like the
right place for it. If we do it after asan, it will insert coverage
for all asan-emited BBs which is highly undesirable. I also think it
is a good idea to run a bunch of optimizations before coverage pass to
not emit too many coverage callbacks (but I can't say that I am very
knowledgeable in this area). FWIW clang does the same: coverage passes
run just before asan/tsan.
There's one other thing I want to put out there. Is this kind of thing
maybe what plugins were invented for? I don't really like the concept of
plugins, but it seems to me that this sort of thing might be an
application for them.
+public:
+ static pass_data pd ()
+ {
+static const pass_data data =
I think a static data member would be better than the unnecessary pd ()
function. This is also unlike existing practice, and I wonder how others
think about it. IMO a fairly strong case could be made that if we're using
C++, then this sort of thing ought to be part of the class definition.
I vary name of the pass depending on the O0 template argument (again
following asan):
O0 ? "sancov_O0" : "sancov", /* name */
If we call it "sancov" always, then I can make it just a global var
(as all other passes in gcc).
Or I can make it a static variable of the template class and move
definition of the class (as you proposed).
What would you prefer?
I think I prefer the static var of the template class. I just wonder why
we don't have the pass_data for all the existing passes as static data
members? I'm sure there's some reason.
asan also distinguishes the name between asan/asan0. I'd either follow
that naming convention, or remove the _O0 variant for all three of them.
I lean towards the latter.
Bernd
Re: Add fuzzing coverage support
On Wed, Dec 2, 2015 at 11:51 AM, Jakub Jelinek wrote:
> On Wed, Dec 02, 2015 at 05:55:29PM +0100, Dmitry Vyukov wrote:
>> Can you point to some concrete coding style violations (besides
>> function comments)?
>>
>>
>> > We seem to have no established process for deciding whether we want a new
>> > feature. I am not sure how to approach such a question, and it comes up
>> > quite often. Somehow the default answer usually seems to be to accept
>> > anything.
>
>> Index: ChangeLog
>> ===
>> --- ChangeLog (revision 231000)
>> +++ ChangeLog (working copy)
>> @@ -1,3 +1,13 @@
>> +2015-11-27 Dmitry Vyukov
>> +
>> + * sancov.c: add file.
>> + * Makefile.in: add sancov.c.
>> + * passes.def: add sancov pass.
>> + * tree-pass.h: add sancov pass.
>> + * common.opt: add -fsanitize-coverage=trace-pc flag.
>> + * sanitizer.def: add __sanitizer_cov_trace_pc builtin.
>> + * builtins.def: enable sanitizer builtins when coverage is enabled.
>
> All the ChangeLog entries should start with upper case letter after :,
> instead of add file. say New file., in most other cases there is missing
> () with what you've actually changed.
> Say
> * Makefile.in (OBJS): Add sancov.o.
> You should not add sancov.c to GTFILES if there are no GTY variables (and if
> there were, you would need to include the generated gt-* header at the end).
> * builtins.def (DEF_SANITIZER_BUILTIN): Enable for
> flag_sanitize_coverage.
> etc.
>
>> --- builtins.def (revision 231000)
>> +++ builtins.def (working copy)
>> @@ -210,7 +210,8 @@
>>DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, TYPE,\
>> true, true, true, ATTRS, true, \
>> (flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_THREAD \
>> - | SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT)))
>> + | SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT) \
>> + || flag_sanitize_coverage))
>
> The || should be indented below flag_.
>
>> + FOR_EACH_BB_FN (bb, fun)
>> +{
>> + gsi = gsi_start_bb (bb);
>> + stmt = gsi_stmt (gsi);
>> + while (stmt && dyn_cast (stmt))
>> +{
>> + gsi_next (&gsi);
>> + stmt = gsi_stmt (gsi);
>> +}
>> + if (!stmt)
>> +continue;
>
> That would be
> gsi = gsi_after_labels (bb);
> If you want to ignore empty basic blocks, then followed by
> if (gsi_end_p (gsi))
> continue;
>
>> + f = gimple_build_call (builtin_decl_implicit (BUILT_IN_SANCOV_TRACE),
>> 0);
>
> Generally, the text after BUILT_IN_ should be uppercase variant of the
> function name, not a shorthand for it.
> So I'd expect BUILT_IN_SANITIZER_COV_TRACE_PC instead.
> If that makes the line too long, just use some temporary variable
> say for builtin_decl_implicit result.
>
Probably I should answer these:
> But I guess the most important question is, where is the
> __sanitizer_cov_trace_pc function defined, I don't see it in libsanitizer
> right now, is it meant for kernel only, something else?
For the kernel, __sanitizer_cov_trace_pc will have to be in the kernel
itself -- we do not use any of our user-space run-times for the
kernel.
For the user space we may add it to
sanitizer_common/sanitizer_coverage_libcdep.cc,
however for userspace the benefit of having __sanitizer_cov_trace_pc
is not so big because we already have __sanitizer_cov_trace_basic_block.
__sanitizer_cov_trace_basic_block has similar functionality but a bit
more hairy implementation in the compiler
and requires an initialization step done at module init, which in the
kernel we don't have.
(Dmitry, correct me if I am wrong).
> If it is going to be in some libsanitizer library (which one),
All of them, the coverage code is in sanitizer_common
> then one also
> needs code to ensure that the library is linked in if -fsanitize-coverage
> is used during linking. Why is it not -fsanitize=coverage, btw?
> Is it incompatible with some other sanitizers, or compatible with all of
> them?
Partially for historical reasons, partially because Clang's
-fsanitize-coverage has many subflags
(today: func, bb, edge, indir-call, 8bit-counters, trace-cmp,
trace-bb) and having them as
-fsanitize=coverage-foo,coverage-bar,coverage-zoo would be more
verbose.
--kcc
>
> Jakub
Re: Add fuzzing coverage support
On Wed, Dec 02, 2015 at 05:55:29PM +0100, Dmitry Vyukov wrote:
> Can you point to some concrete coding style violations (besides
> function comments)?
>
>
> > We seem to have no established process for deciding whether we want a new
> > feature. I am not sure how to approach such a question, and it comes up
> > quite often. Somehow the default answer usually seems to be to accept
> > anything.
> Index: ChangeLog
> ===
> --- ChangeLog (revision 231000)
> +++ ChangeLog (working copy)
> @@ -1,3 +1,13 @@
> +2015-11-27 Dmitry Vyukov
> +
> + * sancov.c: add file.
> + * Makefile.in: add sancov.c.
> + * passes.def: add sancov pass.
> + * tree-pass.h: add sancov pass.
> + * common.opt: add -fsanitize-coverage=trace-pc flag.
> + * sanitizer.def: add __sanitizer_cov_trace_pc builtin.
> + * builtins.def: enable sanitizer builtins when coverage is enabled.
All the ChangeLog entries should start with upper case letter after :,
instead of add file. say New file., in most other cases there is missing
() with what you've actually changed.
Say
* Makefile.in (OBJS): Add sancov.o.
You should not add sancov.c to GTFILES if there are no GTY variables (and if
there were, you would need to include the generated gt-* header at the end).
* builtins.def (DEF_SANITIZER_BUILTIN): Enable for
flag_sanitize_coverage.
etc.
> --- builtins.def (revision 231000)
> +++ builtins.def (working copy)
> @@ -210,7 +210,8 @@
>DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, TYPE,\
> true, true, true, ATTRS, true, \
> (flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_THREAD \
> - | SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT)))
> + | SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT) \
> + || flag_sanitize_coverage))
The || should be indented below flag_.
> + FOR_EACH_BB_FN (bb, fun)
> +{
> + gsi = gsi_start_bb (bb);
> + stmt = gsi_stmt (gsi);
> + while (stmt && dyn_cast (stmt))
> +{
> + gsi_next (&gsi);
> + stmt = gsi_stmt (gsi);
> +}
> + if (!stmt)
> +continue;
That would be
gsi = gsi_after_labels (bb);
If you want to ignore empty basic blocks, then followed by
if (gsi_end_p (gsi))
continue;
> + f = gimple_build_call (builtin_decl_implicit (BUILT_IN_SANCOV_TRACE),
> 0);
Generally, the text after BUILT_IN_ should be uppercase variant of the
function name, not a shorthand for it.
So I'd expect BUILT_IN_SANITIZER_COV_TRACE_PC instead.
If that makes the line too long, just use some temporary variable
say for builtin_decl_implicit result.
But I guess the most important question is, where is the
__sanitizer_cov_trace_pc function defined, I don't see it in libsanitizer
right now, is it meant for kernel only, something else?
If it is going to be in some libsanitizer library (which one), then one also
needs code to ensure that the library is linked in if -fsanitize-coverage
is used during linking. Why is it not -fsanitize=coverage, btw?
Is it incompatible with some other sanitizers, or compatible with all of
them?
Jakub
Re: Add fuzzing coverage support
On Wed, Dec 2, 2015 at 6:11 PM, Bernd Schmidt wrote:
> On 12/02/2015 05:55 PM, Dmitry Vyukov wrote:
>>
>> Can you point to some concrete coding style violations (besides
>> function comments)?
>>
>> (flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_THREAD \
>> - | SANITIZE_UNDEFINED |
>> SANITIZE_NONDEFAULT)))
>> + | SANITIZE_UNDEFINED |
>> SANITIZE_NONDEFAULT) \
>> + || flag_sanitize_coverage))
>
>
> The || should line up with the other condition (i.e. the part starting with
> flag_sanitize).
>
>> +unsigned sancov_pass (function *fun)
>
>
> Split the line after the return type.
>
>> +
>> +template
>> +class pass_sancov : public gimple_opt_pass
>> +{
Thanks for the review!
> This seems to be a new idiom but I find it OK.
Duplicating this code would double patch size :)
Asan pass has this duplication.
FWIW I found this template trick better than duplication of a large
chunk of code
> One thing to consider would
> be whether you really need this split between O0/optimize versions, or
> whether you can find a place in the queue where to insert it
> unconditionally. Have you considered this at all or did you just follow
> asan/tsan?
I inserted the pass just before asan/tsan because it looks like the
right place for it. If we do it after asan, it will insert coverage
for all asan-emited BBs which is highly undesirable. I also think it
is a good idea to run a bunch of optimizations before coverage pass to
not emit too many coverage callbacks (but I can't say that I am very
knowledgeable in this area). FWIW clang does the same: coverage passes
run just before asan/tsan.
>> +public:
>> + static pass_data pd ()
>> + {
>> +static const pass_data data =
>
>
> I think a static data member would be better than the unnecessary pd ()
> function. This is also unlike existing practice, and I wonder how others
> think about it. IMO a fairly strong case could be made that if we're using
> C++, then this sort of thing ought to be part of the class definition.
I vary name of the pass depending on the O0 template argument (again
following asan):
O0 ? "sancov_O0" : "sancov", /* name */
If we call it "sancov" always, then I can make it just a global var
(as all other passes in gcc).
Or I can make it a static variable of the template class and move
definition of the class (as you proposed).
What would you prefer?
Re: Add fuzzing coverage support
On 12/02/2015 05:55 PM, Dmitry Vyukov wrote:
Can you point to some concrete coding style violations (besides
function comments)?
(flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_THREAD \
- | SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT)))
+ | SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT) \
+ || flag_sanitize_coverage))
The || should line up with the other condition (i.e. the part starting
with flag_sanitize).
+unsigned sancov_pass (function *fun)
Split the line after the return type.
+
+template
+class pass_sancov : public gimple_opt_pass
+{
This seems to be a new idiom but I find it OK. One thing to consider
would be whether you really need this split between O0/optimize
versions, or whether you can find a place in the queue where to insert
it unconditionally. Have you considered this at all or did you just
follow asan/tsan?
+public:
+ static pass_data pd ()
+ {
+static const pass_data data =
I think a static data member would be better than the unnecessary pd ()
function. This is also unlike existing practice, and I wonder how others
think about it. IMO a fairly strong case could be made that if we're
using C++, then this sort of thing ought to be part of the class definition.
Bernd
Re: Add fuzzing coverage support
On Wed, Dec 2, 2015 at 5:47 PM, Bernd Schmidt wrote: > On 12/02/2015 05:10 PM, Dmitry Vyukov wrote: >> >> ping > > > I do not see the original submission in my archives. That's strange. I don't see it in gcc-patches archives as well. The original email contained a plain-text patch attachment. Attaching it again. > This one comes too late > to make it into gcc-6. I can make some initial comments. > This patch adds support for coverage-guided fuzzing: https://codereview.appspot.com/280140043 > > > Please send patches with the mail (the best method is usually a text/plain > attachment) so that they can be quoted. > > From what I can tell this is relatively straight-forward. You'll want to fix > a number of issues with coding style (formatting, and lack of function > comments). > > There's no documentation in invoke.texi. Will fix. Can you point to some concrete coding style violations (besides function comments)? > We seem to have no established process for deciding whether we want a new > feature. I am not sure how to approach such a question, and it comes up > quite often. Somehow the default answer usually seems to be to accept > anything. Index: ChangeLog === --- ChangeLog (revision 231000) +++ ChangeLog (working copy) @@ -1,3 +1,13 @@ +2015-11-27 Dmitry Vyukov + + * sancov.c: add file. + * Makefile.in: add sancov.c. + * passes.def: add sancov pass. + * tree-pass.h: add sancov pass. + * common.opt: add -fsanitize-coverage=trace-pc flag. + * sanitizer.def: add __sanitizer_cov_trace_pc builtin. + * builtins.def: enable sanitizer builtins when coverage is enabled. + 2015-11-27 Jakub Jelinek PR tree-optimization/68552 Index: Makefile.in === --- Makefile.in (revision 231000) +++ Makefile.in (working copy) @@ -1426,6 +1426,7 @@ tsan.o \ ubsan.o \ sanopt.o \ + sancov.o \ tree-call-cdce.o \ tree-cfg.o \ tree-cfgcleanup.o \ @@ -2399,6 +2400,7 @@ $(srcdir)/ubsan.c \ $(srcdir)/tsan.c \ $(srcdir)/sanopt.c \ + $(srcdir)/sancov.c \ $(srcdir)/ipa-devirt.c \ $(srcdir)/internal-fn.h \ @all_gtfiles@ Index: builtins.def === --- builtins.def (revision 231000) +++ builtins.def (working copy) @@ -210,7 +210,8 @@ DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, TYPE,\ true, true, true, ATTRS, true, \ (flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_THREAD \ -| SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT))) +| SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT) \ + || flag_sanitize_coverage)) #undef DEF_CILKPLUS_BUILTIN #define DEF_CILKPLUS_BUILTIN(ENUM, NAME, TYPE, ATTRS) \ Index: common.opt === --- common.opt (revision 231000) +++ common.opt (working copy) @@ -225,6 +225,11 @@ Variable unsigned int flag_sanitize_recover = SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT | SANITIZE_KERNEL_ADDRESS +fsanitize-coverage=trace-pc +Common Report Var(flag_sanitize_coverage) +Enable coverage-guided fuzzing support. +Inserts call to __sanitizer_cov_trace_pc into every basic block. + ; Flag whether a prefix has been added to dump_base_name Variable bool dump_base_name_prefixed = false Index: passes.def === --- passes.def (revision 231000) +++ passes.def (working copy) @@ -237,6 +237,7 @@ NEXT_PASS (pass_split_crit_edges); NEXT_PASS (pass_pre); NEXT_PASS (pass_sink_code); + NEXT_PASS (pass_sancov); NEXT_PASS (pass_asan); NEXT_PASS (pass_tsan); /* Pass group that runs when 1) enabled, 2) there are loops @@ -346,6 +347,7 @@ to forward object-size and builtin folding results properly. */ NEXT_PASS (pass_copy_prop); NEXT_PASS (pass_dce); + NEXT_PASS (pass_sancov); NEXT_PASS (pass_asan); NEXT_PASS (pass_tsan); /* ??? We do want some kind of loop invariant motion, but we possibly @@ -369,6 +371,7 @@ NEXT_PASS (pass_lower_vaarg); NEXT_PASS (pass_lower_vector); NEXT_PASS (pass_lower_complex_O0); + NEXT_PASS (pass_sancov_O0); NEXT_PASS (pass_asan_O0); NEXT_PASS (pass_tsan_O0); NEXT_PASS (pass_sanopt); Index: sancov.c === --- sancov.c (revision 0) +++ sancov.c (working copy) @@ -0,0 +1,116 @@ +/* Code coverage instrumentation for fuzzing. + Copyright (C) 2015 Free Software Foundation, Inc. + Contributed by Dmitry Vyukov + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without ev
Re: Add fuzzing coverage support
On 12/02/2015 05:10 PM, Dmitry Vyukov wrote: ping I do not see the original submission in my archives. This one comes too late to make it into gcc-6. I can make some initial comments. This patch adds support for coverage-guided fuzzing: https://codereview.appspot.com/280140043 Please send patches with the mail (the best method is usually a text/plain attachment) so that they can be quoted. From what I can tell this is relatively straight-forward. You'll want to fix a number of issues with coding style (formatting, and lack of function comments). There's no documentation in invoke.texi. We seem to have no established process for deciding whether we want a new feature. I am not sure how to approach such a question, and it comes up quite often. Somehow the default answer usually seems to be to accept anything. Bernd
Re: Add fuzzing coverage support
ping Number of bugs found with this coverage in kernel already crossed 40: https://github.com/google/syzkaller/wiki/Found-Bugs On Fri, Nov 27, 2015 at 3:30 PM, Dmitry Vyukov wrote: > +syzkaller group > > On Fri, Nov 27, 2015 at 3:28 PM, Dmitry Vyukov wrote: >> Hello, >> >> This patch adds support for coverage-guided fuzzing: >> https://codereview.appspot.com/280140043 >> >> Coverage-guided fuzzing is a powerful randomized testing technique >> that uses coverage feedback to determine new interesting inputs to a >> program. Some examples of the systems that use it are AFL >> (http://lcamtuf.coredump.cx/afl/) and LibFuzzer >> (http://llvm.org/docs/LibFuzzer.html). Compiler coverage >> instrumentation allows to make fuzzing more efficient as compared to >> binary instrumentation. >> >> Flag that enables coverage is named -fsanitize-coverage=trace-pc to >> make it consistent with similar functionality in clang, which already >> supports a set of fuzzing coverage modes: >> http://clang.llvm.org/docs/SanitizerCoverage.html >> -fsanitize-coverage=trace-pc is not yet supported in clang, but we >> plan to add it. >> >> This particular coverage mode simply inserts function calls into every >> basic block. >> I've built syzkaller, a Linux system call fuzzer >> (https://github.com/google/syzkaller), using this functionality. The >> fuzzer has found 30+ previously unknown bugs in kernel >> (https://github.com/google/syzkaller/wiki/Found-Bugs) in slightly more >> than a month (while kernel was extensively fuzzed with trinity -- >> non-guided fuzzer). Quentin also built some kernel fuzzer on top of >> it. >> >> Why not gcov. Typical fuzzing loop looks as follows: (1) reset >> coverage, (2) execute a bit of code, (3) collect coverage, repeat. A >> typical coverage can be just a dozen of basic blocks (e.g. invalid >> input). In such context gcov becomes prohibitively expensive as >> reset/collect coverage steps depend on total number of basic >> blocks/edges in program (in case of kernel it is about 2M). Cost of >> this "tracing" mode depends on number of executed basic blocks/edges. >> On top of that, kernel required per-thread coverage because there are >> always background threads and unrelated processes that also produce >> coverage. With inlined gcov instrumentation per-thread coverage is not >> possible. >> Inlined fast paths do not make lots of sense in fuzzing scenario, >> because lots of code is executed just once in between resets. Also it >> is not really possible to inline accesses to per-cpu and per-task data >> structures for kernel. >> >> OK for trunk?
