[RFC PATCH 0/2] drop the concept of 'known-but-ignored' attributes

[RFC PATCH 0/2] drop the concept of 'known-but-ignored' attributes

[Luc Van Oostenryck]

The goal of this series is to drop the concept of
'known-but-ignored' attributes which, for a semantic
checker like sparse, adds absolutely no added value
but bring endless annoyances.

Luc Van Oostenryck (2):
  By default disable the warning flag '-Wunknown-attribute'
  drop the concept of 'known-but-ignored' attributes

 gcc-attr-list.h                     | 210 ------------------------------------
 lib.c                               |   2 +-
 parse.c                             |  42 +-------
 validation/Wunknown-attribute-def.c |   1 -
 4 files changed, 2 insertions(+), 253 deletions(-)
 delete mode 100644 gcc-attr-list.h

-- 
2.16.0

[RFC PATCH 1/2] By default disable the warning flag '-Wunknown-attribute'

[Luc Van Oostenryck]

Generally, we're not interested by those warnings, but we
can always explicitly ask for them if needed.

So make the flag '-Wunknown-attribute' off by default.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 lib.c                               | 2 +-
 validation/Wunknown-attribute-def.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib.c b/lib.c
index fcd31e84c..aec52a6fa 100644
--- a/lib.c
+++ b/lib.c
@@ -251,7 +251,7 @@ int Wtransparent_union = 0;
 int Wtypesign = 0;
 int Wundef = 0;
 int Wuninitialized = 1;
-int Wunknown_attribute = 1;
+int Wunknown_attribute = 0;
 int Wvla = 1;
 
 int dump_macro_defs = 0;
diff --git a/validation/Wunknown-attribute-def.c b/validation/Wunknown-attribute-def.c
index 0c0868d6b..defe643db 100644
--- a/validation/Wunknown-attribute-def.c
+++ b/validation/Wunknown-attribute-def.c
@@ -4,6 +4,5 @@ static int foo(void) __attribute__((unknown_attribute));
  * check-name: warn-unknown-attribute
  *
  * check-error-start
-Wunknown-attribute-def.c:1:37: warning: attribute 'unknown_attribute': unknown attribute
  * check-error-end
  */
-- 
2.16.0

[RFC PATCH 2/2] drop the concept of 'known-but-ignored' attributes

[Luc Van Oostenryck]

Since, by definition, we ignore ignored attributes, it's completely
useless to keep a list of those (which needs to be updated each time
gcc, clang, ... add new attributes, wich is quite often).

Let's just keep things simple and stop to try to make a distinction
between 'unknown' and 'ignored' attributes. We can then only care
about the attributes we (need to) handle and simply ignore the others
ones since we don't do anything with them anyway.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 gcc-attr-list.h | 210 --------------------------------------------------------
 parse.c         |  42 +-----------
 2 files changed, 1 insertion(+), 251 deletions(-)
 delete mode 100644 gcc-attr-list.h

diff --git a/gcc-attr-list.h b/gcc-attr-list.h
deleted file mode 100644
index e21bec4f7..000000000
--- a/gcc-attr-list.h
+++ /dev/null
@@ -1,210 +0,0 @@
-GCC_ATTR(BELOW100)
-GCC_ATTR(OS_Task)
-GCC_ATTR(OS_main)
-GCC_ATTR(OS_task)
-GCC_ATTR(abi_tag)
-GCC_ATTR(absdata)
-GCC_ATTR(address)
-GCC_ATTR(alias)
-GCC_ATTR(aligned)
-GCC_ATTR(alloc_align)
-GCC_ATTR(alloc_size)
-GCC_ATTR(altivec)
-GCC_ATTR(always_inline)
-GCC_ATTR(artificial)
-GCC_ATTR(assume_aligned)
-GCC_ATTR(bank_switch)
-GCC_ATTR(based)
-GCC_ATTR(below100)
-GCC_ATTR(bnd_instrument)
-GCC_ATTR(bnd_legacy)
-GCC_ATTR(bnd_variable_size)
-GCC_ATTR(break_handler)
-GCC_ATTR(brk_interrupt)
-GCC_ATTR(callee_pop_aggregate_return)
-GCC_ATTR(cb)
-GCC_ATTR(cdecl)
-GCC_ATTR(cleanup)
-GCC_ATTR(cmse_nonsecure_call)
-GCC_ATTR(cmse_nonsecure_entry)
-GCC_ATTR(cold)
-GCC_ATTR(common)
-GCC_ATTR(common_object)
-GCC_ATTR(const)
-GCC_ATTR(constructor)
-GCC_ATTR(critical)
-GCC_ATTR(default)
-GCC_ATTR(deprecated)
-GCC_ATTR(designated_init)
-GCC_ATTR(destructor)
-GCC_ATTR(disinterrupt)
-GCC_ATTR(dllexport)
-GCC_ATTR(dllimport)
-GCC_ATTR(eightbit_data)
-GCC_ATTR(either)
-GCC_ATTR(error)
-GCC_ATTR(exception)
-GCC_ATTR(exception_handler)
-GCC_ATTR(externally_visible)
-GCC_ATTR(fallthrough)
-GCC_ATTR(far)
-GCC_ATTR(fast_interrupt)
-GCC_ATTR(fastcall)
-GCC_ATTR(flatten)
-GCC_ATTR(force_align_arg_pointer)
-GCC_ATTR(format)
-GCC_ATTR(format_arg)
-GCC_ATTR(forwarder_section)
-GCC_ATTR(function_vector)
-GCC_ATTR(gcc_struct)
-GCC_ATTR(gnu_inline)
-GCC_ATTR(hidden)
-GCC_ATTR(hot)
-GCC_ATTR(hotpatch)
-GCC_ATTR(ifunc)
-GCC_ATTR(indirect_branch)
-GCC_ATTR(init_priority)
-GCC_ATTR(interfacearm)
-GCC_ATTR(internal)
-GCC_ATTR(interrupt)
-GCC_ATTR(interrupt_handler)
-GCC_ATTR(interrupt_thread)
-GCC_ATTR(io)
-GCC_ATTR(io_low)
-GCC_ATTR(isr)
-GCC_ATTR(keep_interrupts_masked)
-GCC_ATTR(kernel)
-GCC_ATTR(kspisusp)
-GCC_ATTR(l1_data)
-GCC_ATTR(l1_data_A)
-GCC_ATTR(l1_data_B)
-GCC_ATTR(l1_text)
-GCC_ATTR(l2)
-GCC_ATTR(leaf)
-GCC_ATTR(long_call)
-GCC_ATTR(longcall)
-GCC_ATTR(lower)
-GCC_ATTR(malloc)
-GCC_ATTR(may_alias)
-GCC_ATTR(maybe_unused)
-GCC_ATTR(medium_call)
-GCC_ATTR(micromips)
-GCC_ATTR(mips16)
-GCC_ATTR(mode)
-GCC_ATTR(model)
-GCC_ATTR(monitor)
-GCC_ATTR(ms_abi)
-GCC_ATTR(ms_hook_prologue)
-GCC_ATTR(ms_struct)
-GCC_ATTR(naked)
-GCC_ATTR(near)
-GCC_ATTR(nested)
-GCC_ATTR(nested_ready)
-GCC_ATTR(nesting)
-GCC_ATTR(nmi)
-GCC_ATTR(nmi_handler)
-GCC_ATTR(no_address_safety_analysis)
-GCC_ATTR(no_caller_saved_registers)
-GCC_ATTR(no_gccisr)
-GCC_ATTR(no_icf)
-GCC_ATTR(no_instrument_function)
-GCC_ATTR(no_profile_instrument_function)
-GCC_ATTR(no_reorder)
-GCC_ATTR(no_sanitize)
-GCC_ATTR(no_sanitize_address)
-GCC_ATTR(no_sanitize_thread)
-GCC_ATTR(no_sanitize_undefined)
-GCC_ATTR(no_split_stack)
-GCC_ATTR(no_stack_limit)
-GCC_ATTR(noclone)
-GCC_ATTR(nocommon)
-GCC_ATTR(nocompression)
-GCC_ATTR(nodiscard)
-GCC_ATTR(noinit)
-GCC_ATTR(noinline)
-GCC_ATTR(noipa)
-GCC_ATTR(nomicromips)
-GCC_ATTR(nomips16)
-GCC_ATTR(nonnull)
-GCC_ATTR(nonstring)
-GCC_ATTR(noplt)
-GCC_ATTR(noreturn)
-GCC_ATTR(nosave_low_regs)
-GCC_ATTR(not_nested)
-GCC_ATTR(nothrow)
-GCC_ATTR(notshared)
-GCC_ATTR(optimize)
-GCC_ATTR(packed)
-GCC_ATTR(partial_save)
-GCC_ATTR(patchable_function_entry)
-GCC_ATTR(pcs)
-GCC_ATTR(persistent)
-GCC_ATTR(progmem)
-GCC_ATTR(protected)
-GCC_ATTR(pure)
-GCC_ATTR(reentrant)
-GCC_ATTR(regparm)
-GCC_ATTR(renesas)
-GCC_ATTR(resbank)
-GCC_ATTR(reset)
-GCC_ATTR(returns_nonnull)
-GCC_ATTR(returns_twice)
-GCC_ATTR(s390_vector_bool)
-GCC_ATTR(saddr)
-GCC_ATTR(save_all)
-GCC_ATTR(save_volatiles)
-GCC_ATTR(saveall)
-GCC_ATTR(scalar_storage_order)
-GCC_ATTR(sda)
-GCC_ATTR(section)
-GCC_ATTR(selectany)
-GCC_ATTR(sentinel)
-GCC_ATTR(shared)
-GCC_ATTR(short_call)
-GCC_ATTR(shortcall)
-GCC_ATTR(signal)
-GCC_ATTR(simd)
-GCC_ATTR(sp_switch)
-GCC_ATTR(spu_vector)
-GCC_ATTR(sseregparm)
-GCC_ATTR(stack_protect)
-GCC_ATTR(stdcall)
-GCC_ATTR(syscall_linkage)
-GCC_ATTR(sysv_abi)
-GCC_ATTR(target)
-GCC_ATTR(target_clones)
-GCC_ATTR(tda)
-GCC_ATTR(thiscall)
-GCC_ATTR(tiny)
-GCC_ATTR(tiny_data)
-GCC_ATTR(tls_model)
-GCC_ATTR(transaction_callable)
-GCC_ATTR(transaction_may_cancel_outer)
-GCC_ATTR(transaction_pure)
-GCC_ATTR(transaction_safe)
-GCC_ATTR(transaction_safe_dynamic)
-GCC_ATTR(transaction_unsafe)
-GCC_ATTR(transaction_wrap)
-GCC_ATTR(transparent_union)
-GCC_ATTR(trap_exit)
-GCC_ATTR(trapa_handler)
-GCC_ATTR(unused)
-GCC_ATTR(upper)
-GCC_ATTR(use_debug_exception_return)
-GCC_ATTR(use_shadow_register_set)
-GCC_ATTR(used)
-GCC_ATTR(vector)
-GCC_ATTR(vector_size)
-GCC_ATTR(version_id)
-GCC_ATTR(visibility)
-GCC_ATTR(vliw)
-GCC_ATTR(volatile)
-GCC_ATTR(wakeup)
-GCC_ATTR(warm)
-GCC_ATTR(warn_if_not_aligned)
-GCC_ATTR(warn_unused)
-GCC_ATTR(warn_unused_result)
-GCC_ATTR(warning)
-GCC_ATTR(weak)
-GCC_ATTR(weakref)
-GCC_ATTR(zda)
diff --git a/parse.c b/parse.c
index e255345fd..fa6f0ffc7 100644
--- a/parse.c
+++ b/parse.c
@@ -85,7 +85,7 @@ static attr_t
 	attribute_bitwise,
 	attribute_address_space, attribute_context,
 	attribute_designated_init,
-	attribute_transparent_union, ignore_attribute,
+	attribute_transparent_union,
 	attribute_mode, attribute_force;
 
 typedef struct symbol *to_mode_t(struct symbol *);
@@ -381,10 +381,6 @@ static struct symbol_op transparent_union_op = {
 	.attribute = attribute_transparent_union,
 };
 
-static struct symbol_op ignore_attr_op = {
-	.attribute = ignore_attribute,
-};
-
 static struct symbol_op mode_QI_op = {
 	.type = KW_MODE,
 	.to_mode = to_QI_mode
@@ -548,24 +544,6 @@ static struct init_keyword {
 };
 
 
-static const char *ignored_attributes[] = {
-
-#define GCC_ATTR(x)		\
-	STRINGIFY(x), 		\
-	STRINGIFY(__##x##__),
-
-#include "gcc-attr-list.h"
-
-#undef GCC_ATTR
-
-	"bounded",
-	"__bounded__",
-	"__noclone",
-	"__nonnull",
-	"__nothrow",
-};
-
-
 void init_parser(int stream)
 {
 	int i;
@@ -579,16 +557,6 @@ void init_parser(int stream)
 		sym->ctype.base_type = ptr->type;
 		sym->op = ptr->op;
 	}
-
-	for (i = 0; i < ARRAY_SIZE(ignored_attributes); i++) {
-		const char * name = ignored_attributes[i];
-		struct symbol *sym = create_symbol(stream, name, SYM_KEYWORD,
-						   NS_KEYWORD);
-		if (!sym->op) {
-			sym->ident->keyword = 1;
-			sym->op = &ignore_attr_op;
-		}
-	}
 }
 
 
@@ -990,14 +958,6 @@ static struct token *typeof_specifier(struct token *token, struct decl_state *ct
 	return expect(token, ')', "after typeof");
 }
 
-static struct token *ignore_attribute(struct token *token, struct symbol *attr, struct decl_state *ctx)
-{
-	struct expression *expr = NULL;
-	if (match_op(token, '('))
-		token = parens_expression(token, &expr, "in attribute");
-	return token;
-}

Re: [RFC PATCH 0/2] drop the concept of 'known-but-ignored' attributes

[Josh Triplett]

On Wed, Feb 14, 2018 at 12:21:07AM +0100, Luc Van Oostenryck wrote:
> The goal of this series is to drop the concept of
> 'known-but-ignored' attributes which, for a semantic
> checker like sparse, adds absolutely no added value
> but bring endless annoyances.

I absolutely agree that these have been annoying. However, they've also
been useful in the past to identify new flags used in the kernel and
other projects that we may need to actually act on, rather than just
ignore.

Re: [RFC PATCH 0/2] drop the concept of 'known-but-ignored' attributes

[Linus Torvalds]

On Thu, Feb 15, 2018 at 7:57 AM, Josh Triplett <josh@joshtriplett.org> wrote:
>
> I absolutely agree that these have been annoying. However, they've also
> been useful in the past to identify new flags used in the kernel and
> other projects that we may need to actually act on, rather than just
> ignore.

I think the issue is that the warning is useful for _sparse_
developers, but not to actual users.

So I do think the warning itself should be off by default - but maybe
the "known but ignored" table should exist so that sparse people can
say "is there a new attribute that I need to look at"?

                   Linus

Re: [RFC PATCH 0/2] drop the concept of 'known-but-ignored' attributes

[Josh Triplett]

On Thu, Feb 15, 2018 at 09:33:14AM -0800, Linus Torvalds wrote:
> On Thu, Feb 15, 2018 at 7:57 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> >
> > I absolutely agree that these have been annoying. However, they've also
> > been useful in the past to identify new flags used in the kernel and
> > other projects that we may need to actually act on, rather than just
> > ignore.
> 
> I think the issue is that the warning is useful for _sparse_
> developers, but not to actual users.

Unless they're debugging why Sparse doesn't fully understand their code,
yes.

> So I do think the warning itself should be off by default - but maybe
> the "known but ignored" table should exist so that sparse people can
> say "is there a new attribute that I need to look at"?

Agreed on both counts.

I also wonder if it might make sense to make "unknown attribute" a
warning that automatically gets displayed *if* displaying other warnings
at the same time, to help with debugging the other warnings. That way,
if you run Sparse on a file that would otherwise produce no warnings, an
unknown attribute doesn't create noise; however, if you run Sparse on a
file that would give warnings anyway, you get the unknown attribute
warning as well, which might make it obvious why the other warnings
arise.

For instance, if you see Sparse warn about an unknown attribute on a
type, followed by a pile of type mismatches involving that type, perhaps
you'd want to investigate that attribute and its use.

Re: [RFC PATCH 0/2] drop the concept of 'known-but-ignored' attributes

[Luc Van Oostenryck]

On Thu, Feb 15, 2018 at 09:33:14AM -0800, Linus Torvalds wrote:
> On Thu, Feb 15, 2018 at 7:57 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> >
> > I absolutely agree that these have been annoying. However, they've also
> > been useful in the past to identify new flags used in the kernel and
> > other projects that we may need to actually act on, rather than just
> > ignore.
> 
> I think the issue is that the warning is useful for _sparse_
> developers, but not to actual users.
> 
> So I do think the warning itself should be off by default - but maybe
> the "known but ignored" table should exist so that sparse people can
> say "is there a new attribute that I need to look at"?

Yes, I think that the off by default is a clear thing. 
For the table, yes, I sort of agree with you and Josh at some level
but in practice I don't think it will be of any help. There is
already a number of attributes (and builtins) sparse *should* act on
(for attributes, I'm thinking to 'noreturn', for example) but haven't
and this for a long time already. I also think that once the warning
will be off by default, the table won't be updated anymore.

So, I'm not sure. I can't say I disagree but I can't say I'm
convinced it will be of any help.

-- Luc

Re: [RFC PATCH 0/2] drop the concept of 'known-but-ignored' attributes

[Luc Van Oostenryck]

On Thu, Feb 15, 2018 at 09:54:02AM -0800, Josh Triplett wrote:
> On Thu, Feb 15, 2018 at 09:33:14AM -0800, Linus Torvalds wrote:
> > On Thu, Feb 15, 2018 at 7:57 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> > >
> > > I absolutely agree that these have been annoying. However, they've also
> > > been useful in the past to identify new flags used in the kernel and
> > > other projects that we may need to actually act on, rather than just
> > > ignore.
> > 
> > I think the issue is that the warning is useful for _sparse_
> > developers, but not to actual users.
> 
> Unless they're debugging why Sparse doesn't fully understand their code,
> yes.
> 
> > So I do think the warning itself should be off by default - but maybe
> > the "known but ignored" table should exist so that sparse people can
> > say "is there a new attribute that I need to look at"?
> 
> Agreed on both counts.
> 
> I also wonder if it might make sense to make "unknown attribute" a
> warning that automatically gets displayed *if* displaying other warnings
> at the same time, to help with debugging the other warnings. That way,
> if you run Sparse on a file that would otherwise produce no warnings, an
> unknown attribute doesn't create noise; however, if you run Sparse on a
> file that would give warnings anyway, you get the unknown attribute
> warning as well, which might make it obvious why the other warnings
> arise.
> 
> For instance, if you see Sparse warn about an unknown attribute on a
> type, followed by a pile of type mismatches involving that type, perhaps
> you'd want to investigate that attribute and its use.

Yes, that would be an idea. It's a bit annoying to implement though,
unknown attributes is something detected at parsing time, while type
errors are done later.

OTOH, there are other situations where postponing a warning would
also be something useful, so ...

-- Luc

Re: [RFC PATCH 0/2] drop the concept of 'known-but-ignored' attributes

[Christopher Li]

On Thu, Feb 15, 2018 at 11:53 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>> So I do think the warning itself should be off by default - but maybe
>> the "known but ignored" table should exist so that sparse people can
>> say "is there a new attribute that I need to look at"?
>
> Yes, I think that the off by default is a clear thing.
> For the table, yes, I sort of agree with you and Josh at some level
> but in practice I don't think it will be of any help. There is
> already a number of attributes (and builtins) sparse *should* act on
> (for attributes, I'm thinking to 'noreturn', for example) but haven't
> and this for a long time already. I also think that once the warning
> will be off by default, the table won't be updated anymore.

I agree with the default should be turn off for kernel checking
to reduce the noise.

The known attribute table does have some usage. If the developer are
writing sparse related checking, he might want to know if he misspell
some sparse specific attribute. The other case is that, some sparse
specific attribute is not implement in this version of sparse. It might
be useful to know.

In this case, the developer can turn this warning on to catch if
there is misspell of the "adress_space" attribute for example.

I suggest change default on off, but keep the warning option if
some one want to turn it on.

Chris