On 06/09/2017 12:12 PM, Richard Biener wrote:
On Fri, Jun 9, 2017 at 11:29 AM, Martin Liška <mli...@suse.cz> wrote:
On 06/08/2017 03:47 PM, Jakub Jelinek wrote:
Hi!
I'd still prefer to handle it with the flags infrastructure instead, but
if
Richard wants to do it this way, then at least:
On Thu, Jun 08, 2017 at 03:30:49PM +0200, Martin Liška wrote:
+/* Return true when flag_sanitize & FLAG is non-zero. If FN is
non-null,
+ remove all flags mentioned in "no_sanitize_flags" of DECL_ATTRIBUTES.
*/
+
+bool
+sanitize_flags_p (unsigned int flag, const_tree fn)
+{
+ unsigned int result_flags = flag_sanitize & flag;
This function really should be either inline, or partly inline, partly out
of line, to handle the common case (sanitization of something not enabled)
in the fast path.
Hello.
Having that inlined would be great, however we'll need to put it to tree.h
and thus we have to include "options.h" before tree.h in multiple source
files.
Please take a look at partial patch.
And, it should have an early out,
if (result_flags == 0)
return false;
Good idea!
+
+ if (fn != NULL_TREE)
+ {
+ tree value = lookup_attribute ("no_sanitize_flags",
DECL_ATTRIBUTES (fn));
The attribute, if it is internal only, should have spaces or similar
characters in its name, like "fn spec", "omp declare target" and many
others.
Done that.
Whoo, wait -- this is for internal use only? Can you step back and explain
why we need this? We do, after all, have -fsanitize options already.
Can be seen here:
__attribute__((no_sanitize_thread, no_sanitize ("null"), no_sanitize ("address"), no_sanitize
("undefined"), no_sanitize ("address"), sanitize no_flags (16777195)))
fn1 ()
{
...
}
where no_sanitize_thread and no_sanitize are normal attributes used by users.
But we want to aggregate all there attributes and build one integer mask that
will drive sanitize_flags_p. And that's why we introduced 'sanitize no_flags'
attribute, so that we don't have to iterate all attrs every time in
anitize_flags_p.
Hope it explains situation?
Martin
Richard.
+add_no_sanitize_value (tree node, unsigned int flags)
+{
+ tree attr = lookup_attribute ("no_sanitize_flags", DECL_ATTRIBUTES
(node));
+ if (attr)
+ {
+ unsigned int old_value = tree_to_uhwi (TREE_VALUE (attr));
+ flags |= old_value;
+ }
+
+ DECL_ATTRIBUTES (node)
+ = tree_cons (get_identifier ("no_sanitize_flags"),
+ build_int_cst (unsigned_type_node, flags),
+ DECL_ATTRIBUTES (node));
If there is a previous attribute already, can't you modify it in
place? If not, as it could be perhaps shared? with other functions
somehow, at least you should avoid adding a new attribute if
(old_value | flags) == old_value.
Yep, we should definitely share, I'll add test-case for that.
I'm currently testing the incremental patch, may I install it
after regression tests?
Martin
Jakub
