| Message ID | CA+55aFzZtGyz3CLF5Hjyjr0GVkx3SYui8yE9JjO5tzzHZzk78g@mail.gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, 30 Apr 2015, Linus Torvalds wrote: > On Tue, Apr 28, 2015 at 3:48 AM, Heiko Carstens > <heiko.carstens@de.ibm.com> wrote: > > gcc knows about a new "hotpatch" attribute which sparse can safely ignore, > > since it modifies only which code will be generated just like the > > "no_instrument_function" attribute. > > I'm wondering if sparse should just ignore all attributes it doesn't > recognize, so that we could just remove this ever-expanding list of > things that don't actually matter for sparse.. > > The "unrecognized attribute" thing made more sense way back when - > when I wanted to get he basic attributes handled. Now it's just > noise... > The list of attributes does not really exapand quickly so explicitly marking them to be ignored after a new "unrecognized attribute" triggers would have the advantage that it is actually looked at and not silently ignored. thx! hofrat -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 30, 2015 at 08:45:33AM -0700, Linus Torvalds wrote: > On Tue, Apr 28, 2015 at 3:48 AM, Heiko Carstens > <heiko.carstens@de.ibm.com> wrote: > > gcc knows about a new "hotpatch" attribute which sparse can safely ignore, > > since it modifies only which code will be generated just like the > > "no_instrument_function" attribute. > > I'm wondering if sparse should just ignore all attributes it doesn't > recognize, so that we could just remove this ever-expanding list of > things that don't actually matter for sparse.. > > The "unrecognized attribute" thing made more sense way back when - > when I wanted to get he basic attributes handled. Now it's just > noise... Unfortunately, ever so often there's an attribute we *can't* just ignore, and in those cases it's better for us to warn rather than breaking. For instance, we won't just be able to ignore "cleanup"; we need to actually handle it, because it affects code flow. I think part of the problem arises because sparse claims (via the preprocessor symbols we provide) to be whatever version of GCC it was compiled with. I think that's a mistake. We should pick a version of GCC for which we support all the attributes we actually need to do something with, and advertise ourselves specifically as that version. That way, a codebase that includes detection for available GCC features will avoid feeding us attributes and other features that we don't understand. I suspect that 3.2 is probably the version sparse should claim to be, for now. We could then use something like the patch you posted, possibly with the addition of an attribute blacklist for any attributes we know we need to do something with but don't yet, so that we warn about *those* attributes and no others. For anything only supported by a newer version of GCC, it's the code we're scanning's fault for feeding us something we never claimed to understand. We can then update the version of GCC we claim to be once we add support for any features of that version of GCC that absolutely need Sparse support. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 30, 2015 at 10:38 AM, <josh@joshtriplett.org> wrote: > > I think part of the problem arises because sparse claims (via the > preprocessor symbols we provide) to be whatever version of GCC it was > compiled with. I think that's a mistake. We should pick a version of > GCC for which we support all the attributes we actually need to do > something with, and advertise ourselves specifically as that version. Fair enough. Although there may then be headers that are unhappy. > I suspect that 3.2 is probably the version sparse should claim to be, > for now. That may be too old. You can't reliably compile the kernel with gcc that old (some config options will complain). At least CONFIG_GCOV_KERNEL wants 3.4 minimum. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 30, 2015 at 10:51:42AM -0700, Linus Torvalds wrote: > On Thu, Apr 30, 2015 at 10:38 AM, <josh@joshtriplett.org> wrote: > > I think part of the problem arises because sparse claims (via the > > preprocessor symbols we provide) to be whatever version of GCC it was > > compiled with. I think that's a mistake. We should pick a version of > > GCC for which we support all the attributes we actually need to do > > something with, and advertise ourselves specifically as that version. > > Fair enough. Although there may then be headers that are unhappy. If you mean the GCC internal headers, I don't think those have version checks; after all, you'd only use them with the GCC they ship with, right? ;) For any other headers, I think we'll get worse results by claiming to be a version of GCC that has features we don't actually support. > > I suspect that 3.2 is probably the version sparse should claim to be, > > for now. > > That may be too old. You can't reliably compile the kernel with gcc > that old (some config options will complain). > > At least CONFIG_GCOV_KERNEL wants 3.4 minimum. If we want to claim to be 3.4 instead, then there are some attributes we'll need to check for and warn if we find. From a quick read of https://ohse.de/uwe/articles/gcc-attributes.html I think this is a complete list of attributes in 3.4 that sparse has to care about but doesn't currently support: cleanup gcc_struct ms_struct "cleanup" we'll need to handle because it affects control flow; gcc_struct and ms_struct affect structure layout, which we have warnings related to. For the moment, we could just add an explicit warning if we see any of those three attributes, set our GCC version to 3.4, and then drop the "unknown attribute" warning. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/parse.c b/parse.c index b43d6835528b..bab0a8e2e931 100644 --- a/parse.c +++ b/parse.c @@ -501,109 +501,6 @@ static struct init_keyword { { "__word__", NS_KEYWORD, MOD_LONG, .op = &mode_word_op }, }; -const char *ignored_attributes[] = { - "alias", - "__alias__", - "alloc_size", - "__alloc_size__", - "always_inline", - "__always_inline__", - "artificial", - "__artificial__", - "bounded", - "__bounded__", - "cdecl", - "__cdecl__", - "cold", - "__cold__", - "constructor", - "__constructor__", - "deprecated", - "__deprecated__", - "destructor", - "__destructor__", - "dllexport", - "__dllexport__", - "dllimport", - "__dllimport__", - "error", - "__error__", - "externally_visible", - "__externally_visible__", - "fastcall", - "__fastcall__", - "format", - "__format__", - "format_arg", - "__format_arg__", - "gnu_inline", - "__gnu_inline__", - "hot", - "__hot__", - "leaf", - "__leaf__", - "l1_text", - "__l1_text__", - "l1_data", - "__l1_data__", - "l2", - "__l2__", - "malloc", - "__malloc__", - "may_alias", - "__may_alias__", - "model", - "__model__", - "ms_abi", - "__ms_abi__", - "ms_hook_prologue", - "__ms_hook_prologue__", - "naked", - "__naked__", - "no_instrument_function", - "__no_instrument_function__", - "noclone", - "__noclone", - "__noclone__", - "noinline", - "__noinline__", - "nonnull", - "__nonnull", - "__nonnull__", - "nothrow", - "__nothrow", - "__nothrow__", - "regparm", - "__regparm__", - "section", - "__section__", - "sentinel", - "__sentinel__", - "signal", - "__signal__", - "stdcall", - "__stdcall__", - "syscall_linkage", - "__syscall_linkage__", - "sysv_abi", - "__sysv_abi__", - "unused", - "__unused__", - "used", - "__used__", - "vector_size", - "__vector_size__", - "visibility", - "__visibility__", - "warn_unused_result", - "__warn_unused_result__", - "warning", - "__warning__", - "weak", - "__weak__", -}; - - void init_parser(int stream) { int i; @@ -617,14 +514,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); - sym->ident->keyword = 1; - sym->op = &ignore_attr_op; - } } @@ -1222,7 +1111,6 @@ static struct token *recover_unknown_attribute(struct token *token) { struct expression *expr = NULL; - sparse_error(token->pos, "attribute '%s': unknown attribute", show_ident(token->ident)); token = token->next; if (match_op(token, '(')) token = parens_expression(token, &expr, "in attribute");