From: "Martin Liška" <mliska@suse.cz>
To: Marco Elver <elver@google.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [PATCH] Add no_sanitize_coverage attribute.
Date: Thu, 20 May 2021 11:08:38 +0200 [thread overview]
Message-ID: <7f0a98b5-1c36-d357-8c5e-bd561f029a48@suse.cz> (raw)
In-Reply-To: <CANpmjNN+Zsi=XCxBo2kp8Mo5P3W6ApvcnxtWdn8nSapY8Fff+w@mail.gmail.com>
On 5/20/21 10:45 AM, Marco Elver wrote:
> On Thu, 20 May 2021 at 10:33, Martin Liška <mliska@suse.cz> wrote:
>> Hello.
>>
>> The patch implements one missing attribute which can be used for per-function
>> disabling of coverage sanitization.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Thanks for implementing this so quickly. One thing I just have to
> double check, is if this works with always_inline (not instrumented if
> inlined into no_sanitize),
No, in this case it's instrumented (if caller has not set the attribute).
> and inline (not inlined if inline fn is
> no_sanitize_coverage).
No again, it's not blocking inlining.
> Just like the other no_sanitize* do. The test doesn't mention them.
It's aligned with similar issue that was discussed some time ago:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94722#c9
Martin
>
> Thanks,
> -- Marco
>
>> Ready to be installed?
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> * asan.h (sanitize_coverage_p): New function.
>> * doc/extend.texi: Document it.
>> * fold-const.c (fold_range_test): Use sanitize_flags_p
>> instead of flag_sanitize_coverage.
>> (fold_truth_andor): Likewise.
>> * sancov.c: Likewise.
>> * tree-ssa-ifcombine.c (ifcombine_ifandif): Likewise.
>>
>> gcc/c-family/ChangeLog:
>>
>> * c-attribs.c (handle_no_sanitize_coverage_attribute): New.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.dg/sancov/attribute.c: New test.
>> ---
>> gcc/asan.h | 10 ++++++++++
>> gcc/c-family/c-attribs.c | 20 ++++++++++++++++++++
>> gcc/doc/extend.texi | 6 ++++++
>> gcc/fold-const.c | 4 ++--
>> gcc/sancov.c | 4 ++--
>> gcc/testsuite/gcc.dg/sancov/attribute.c | 15 +++++++++++++++
>> gcc/tree-ssa-ifcombine.c | 4 +++-
>> 7 files changed, 58 insertions(+), 5 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/sancov/attribute.c
>>
>> diff --git a/gcc/asan.h b/gcc/asan.h
>> index f110f1db563..8c0b2baf170 100644
>> --- a/gcc/asan.h
>> +++ b/gcc/asan.h
>> @@ -249,4 +249,14 @@ sanitize_flags_p (unsigned int flag, const_tree fn = current_function_decl)
>> return result_flags;
>> }
>>
>> +/* Return true when coverage sanitization should happend for FN function. */
>> +
>> +static inline bool
>> +sanitize_coverage_p (const_tree fn = current_function_decl)
>> +{
>> + return (flag_sanitize_coverage
>> + && lookup_attribute ("no_sanitize_coverage",
>> + DECL_ATTRIBUTES (fn)) == NULL_TREE);
>> +}
>> +
>> #endif /* TREE_ASAN */
>> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
>> index ccf9e4ccf0b..671b27c3200 100644
>> --- a/gcc/c-family/c-attribs.c
>> +++ b/gcc/c-family/c-attribs.c
>> @@ -62,6 +62,8 @@ static tree handle_no_address_safety_analysis_attribute (tree *, tree, tree,
>> int, bool *);
>> static tree handle_no_sanitize_undefined_attribute (tree *, tree, tree, int,
>> bool *);
>> +static tree handle_no_sanitize_coverage_attribute (tree *, tree, tree, int,
>> + bool *);
>> static tree handle_asan_odr_indicator_attribute (tree *, tree, tree, int,
>> bool *);
>> static tree handle_stack_protect_attribute (tree *, tree, tree, int, bool *);
>> @@ -449,6 +451,8 @@ const struct attribute_spec c_common_attribute_table[] =
>> handle_no_sanitize_thread_attribute, NULL },
>> { "no_sanitize_undefined", 0, 0, true, false, false, false,
>> handle_no_sanitize_undefined_attribute, NULL },
>> + { "no_sanitize_coverage", 0, 0, true, false, false, false,
>> + handle_no_sanitize_coverage_attribute, NULL },
>> { "asan odr indicator", 0, 0, true, false, false, false,
>> handle_asan_odr_indicator_attribute, NULL },
>> { "warning", 1, 1, true, false, false, false,
>> @@ -1211,6 +1215,22 @@ handle_no_sanitize_undefined_attribute (tree *node, tree name, tree, int,
>> return NULL_TREE;
>> }
>>
>> +/* Handle a "no_sanitize_coverage" attribute; arguments as in
>> + struct attribute_spec.handler. */
>> +
>> +static tree
>> +handle_no_sanitize_coverage_attribute (tree *node, tree name, tree, int,
>> + bool *no_add_attrs)
>> +{
>> + if (TREE_CODE (*node) != FUNCTION_DECL)
>> + {
>> + warning (OPT_Wattributes, "%qE attribute ignored", name);
>> + *no_add_attrs = true;
>> + }
>> +
>> + return NULL_TREE;
>> +}
>> +
>> /* Handle an "asan odr indicator" attribute; arguments as in
>> struct attribute_spec.handler. */
>>
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 826804e6149..3ddeb0dee3a 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -3415,6 +3415,12 @@ The @code{no_sanitize_undefined} attribute on functions is used
>> to inform the compiler that it should not check for undefined behavior
>> in the function when compiling with the @option{-fsanitize=undefined} option.
>>
>> +@item no_sanitize_coverage
>> +@cindex @code{no_sanitize_coverage} function attribute
>> +The @code{no_sanitize_coverage} attribute on functions is used
>> +to inform the compiler that it should not do coverage-guided
>> +fuzzing code instrumentation (@option{-fsanitize-coverage}).
>> +
>> @item no_split_stack
>> @cindex @code{no_split_stack} function attribute
>> @opindex fsplit-stack
>> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
>> index 3be9c15e6b2..d088187a057 100644
>> --- a/gcc/fold-const.c
>> +++ b/gcc/fold-const.c
>> @@ -6016,7 +6016,7 @@ fold_range_test (location_t loc, enum tree_code code, tree type,
>> logical_op_non_short_circuit
>> = param_logical_op_non_short_circuit;
>> if (logical_op_non_short_circuit
>> - && !flag_sanitize_coverage
>> + && !sanitize_coverage_p ()
>> && lhs != 0 && rhs != 0
>> && (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR)
>> && operand_equal_p (lhs, rhs, 0))
>> @@ -9652,7 +9652,7 @@ fold_truth_andor (location_t loc, enum tree_code code, tree type,
>> logical_op_non_short_circuit
>> = param_logical_op_non_short_circuit;
>> if (logical_op_non_short_circuit
>> - && !flag_sanitize_coverage
>> + && !sanitize_coverage_p ()
>> && (code == TRUTH_AND_EXPR
>> || code == TRUTH_ANDIF_EXPR
>> || code == TRUTH_OR_EXPR
>> diff --git a/gcc/sancov.c b/gcc/sancov.c
>> index d656c37cae9..9cfbd425def 100644
>> --- a/gcc/sancov.c
>> +++ b/gcc/sancov.c
>> @@ -313,9 +313,9 @@ public:
>> return new pass_sancov<O0> (m_ctxt);
>> }
>> virtual bool
>> - gate (function *)
>> + gate (function *fun)
>> {
>> - return flag_sanitize_coverage && (!O0 || !optimize);
>> + return sanitize_coverage_p (fun->decl) && (!O0 || !optimize);
>> }
>> virtual unsigned int
>> execute (function *fun)
>> diff --git a/gcc/testsuite/gcc.dg/sancov/attribute.c b/gcc/testsuite/gcc.dg/sancov/attribute.c
>> new file mode 100644
>> index 00000000000..bf6dbd4bae7
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/sancov/attribute.c
>> @@ -0,0 +1,15 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-fsanitize-coverage=trace-pc -fdump-tree-optimized" } */
>> +
>> +void foo(void)
>> +{
>> +}
>> +
>> +void
>> +__attribute__((no_sanitize_coverage))
>> +bar(void)
>> +{
>> +}
>> +
>> +
>> +/* { dg-final { scan-tree-dump-times "__builtin___sanitizer_cov_trace_pc \\(\\)" 1 "optimized" } } */
>> diff --git a/gcc/tree-ssa-ifcombine.c b/gcc/tree-ssa-ifcombine.c
>> index 836a12d6b80..f93e04aa4df 100644
>> --- a/gcc/tree-ssa-ifcombine.c
>> +++ b/gcc/tree-ssa-ifcombine.c
>> @@ -40,6 +40,8 @@ along with GCC; see the file COPYING3. If not see
>> #include "gimplify-me.h"
>> #include "tree-cfg.h"
>> #include "tree-ssa.h"
>> +#include "attribs.h"
>> +#include "asan.h"
>>
>> #ifndef LOGICAL_OP_NON_SHORT_CIRCUIT
>> #define LOGICAL_OP_NON_SHORT_CIRCUIT \
>> @@ -567,7 +569,7 @@ ifcombine_ifandif (basic_block inner_cond_bb, bool inner_inv,
>> if (param_logical_op_non_short_circuit != -1)
>> logical_op_non_short_circuit
>> = param_logical_op_non_short_circuit;
>> - if (!logical_op_non_short_circuit || flag_sanitize_coverage)
>> + if (!logical_op_non_short_circuit || sanitize_coverage_p ())
>> return false;
>> /* Only do this optimization if the inner bb contains only the conditional. */
>> if (!gsi_one_before_end_p (gsi_start_nondebug_after_labels_bb (inner_cond_bb)))
>> --
>> 2.31.1
>>
next prev parent reply other threads:[~2021-05-20 9:08 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-20 8:33 Martin Liška
2021-05-20 8:45 ` Marco Elver
2021-05-20 9:08 ` Martin Liška [this message]
2021-05-20 10:55 ` Marco Elver
2021-05-21 8:50 ` Martin Liška
2021-05-21 12:39 ` Marco Elver
2021-05-24 8:03 ` Martin Liška
2021-05-24 8:07 ` Marco Elver
2021-05-25 9:55 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7f0a98b5-1c36-d357-8c5e-bd561f029a48@suse.cz \
--to=mliska@suse.cz \
--cc=elver@google.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ndesaulniers@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).