* [PATCH] sparse/parse.c: ignore hotpatch attribute @ 2015-04-28 10:48 Heiko Carstens 2015-04-29 23:22 ` Christopher Li 2015-04-30 15:45 ` Linus Torvalds 0 siblings, 2 replies; 8+ messages in thread From: Heiko Carstens @ 2015-04-28 10:48 UTC (permalink / raw) To: Christopher Li; +Cc: linux-sparse, Heiko Carstens 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. The gcc hotpatch feature patch: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=11762b8363737591bfb9c66093bc2edf289b917f Currently the Linux kernel makes use of this attribute: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=61f552141c9c0e88b3fdc7046265781ffd8fa68a Without this patch sparse will emit warnings like "error: attribute 'hotpatch': unknown attribute" Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com> --- parse.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/parse.c b/parse.c index b43d6835528b..8afae73d5325 100644 --- a/parse.c +++ b/parse.c @@ -540,6 +540,8 @@ const char *ignored_attributes[] = { "__gnu_inline__", "hot", "__hot__", + "hotpatch", + "__hotpatch__", "leaf", "__leaf__", "l1_text", -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] sparse/parse.c: ignore hotpatch attribute 2015-04-28 10:48 [PATCH] sparse/parse.c: ignore hotpatch attribute Heiko Carstens @ 2015-04-29 23:22 ` Christopher Li 2015-04-30 11:47 ` Heiko Carstens 2015-04-30 15:45 ` Linus Torvalds 1 sibling, 1 reply; 8+ messages in thread From: Christopher Li @ 2015-04-29 23:22 UTC (permalink / raw) To: Heiko Carstens; +Cc: Linux-Sparse That patch looks fine. Can you add some test case for the hot patch as well? It need to cover the case you run into. Preferably cover each of the variation of hot patch. It will be useful when we parse these attributes properly. Chris 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. > > The gcc hotpatch feature patch: > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=11762b8363737591bfb9c66093bc2edf289b917f > > Currently the Linux kernel makes use of this attribute: > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=61f552141c9c0e88b3fdc7046265781ffd8fa68a > > Without this patch sparse will emit warnings like > "error: attribute 'hotpatch': unknown attribute" > > Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com> > --- > parse.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/parse.c b/parse.c > index b43d6835528b..8afae73d5325 100644 > --- a/parse.c > +++ b/parse.c > @@ -540,6 +540,8 @@ const char *ignored_attributes[] = { > "__gnu_inline__", > "hot", > "__hot__", > + "hotpatch", > + "__hotpatch__", > "leaf", > "__leaf__", > "l1_text", > -- > 2.1.4 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sparse/parse.c: ignore hotpatch attribute 2015-04-29 23:22 ` Christopher Li @ 2015-04-30 11:47 ` Heiko Carstens 0 siblings, 0 replies; 8+ messages in thread From: Heiko Carstens @ 2015-04-30 11:47 UTC (permalink / raw) To: Christopher Li; +Cc: Linux-Sparse On Wed, Apr 29, 2015 at 04:22:13PM -0700, Christopher Li wrote: > That patch looks fine. > > Can you add some test case for the hot patch as well? > It need to cover the case you run into. > > Preferably cover each of the variation of hot patch. > > It will be useful when we parse these attributes properly. > > Chris Something like below? I have to admit that I have not much of a clue of what I'm doing ;) From 03da0b28318100cc9a1389c81920654b29405d73 Mon Sep 17 00:00:00 2001 From: Heiko Carstens <heiko.carstens@de.ibm.com> Date: Thu, 30 Apr 2015 12:53:28 +0200 Subject: [PATCH] sparse/parse.c: ignore hotpatch attribute 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. The gcc hotpatch feature patch: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=11762b8363737591bfb9c66093bc2edf289b917f Currently the Linux kernel makes use of this attribute: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=61f552141c9c0e88b3fdc7046265781ffd8fa68a Without this patch sparse will emit warnings like "error: attribute 'hotpatch': unknown attribute" Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com> --- ident-list.h | 1 + parse.c | 2 ++ validation/attr-hotpatch.c | 7 +++++++ 3 files changed, 10 insertions(+) create mode 100644 validation/attr-hotpatch.c diff --git a/ident-list.h b/ident-list.h index b65b667da720..1849ea613b47 100644 --- a/ident-list.h +++ b/ident-list.h @@ -81,6 +81,7 @@ IDENT(constructor); IDENT(__constructor__); IDENT(destructor); IDENT(__destructor__); IDENT(cold); IDENT(__cold__); IDENT(hot); IDENT(__hot__); +IDENT(hotpatch); IDENT(__hotpatch__); IDENT(cdecl); IDENT(__cdecl__); IDENT(stdcall); IDENT(__stdcall__); IDENT(fastcall); IDENT(__fastcall__); diff --git a/parse.c b/parse.c index b43d6835528b..8afae73d5325 100644 --- a/parse.c +++ b/parse.c @@ -540,6 +540,8 @@ const char *ignored_attributes[] = { "__gnu_inline__", "hot", "__hot__", + "hotpatch", + "__hotpatch__", "leaf", "__leaf__", "l1_text", diff --git a/validation/attr-hotpatch.c b/validation/attr-hotpatch.c new file mode 100644 index 000000000000..205169389956 --- /dev/null +++ b/validation/attr-hotpatch.c @@ -0,0 +1,7 @@ +static void __attribute__((hotpatch(0,3))) bar(void) +{ +} + +/* + * check-name: attribute hotpatch + */ -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] sparse/parse.c: ignore hotpatch attribute 2015-04-28 10:48 [PATCH] sparse/parse.c: ignore hotpatch attribute Heiko Carstens 2015-04-29 23:22 ` Christopher Li @ 2015-04-30 15:45 ` Linus Torvalds 2015-04-30 15:50 ` Nicholas Mc Guire 2015-04-30 17:38 ` josh 1 sibling, 2 replies; 8+ messages in thread From: Linus Torvalds @ 2015-04-30 15:45 UTC (permalink / raw) To: Heiko Carstens; +Cc: Christopher Li, Sparse Mailing-list [-- Attachment #1: Type: text/plain, Size: 679 bytes --] 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... Something like the attached (mostly untested) patch.. Linus [-- Attachment #2: patch.diff --] [-- Type: text/plain, Size: 2751 bytes --] parse.c | 112 ---------------------------------------------------------------- 1 file changed, 112 deletions(-) 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"); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] sparse/parse.c: ignore hotpatch attribute 2015-04-30 15:45 ` Linus Torvalds @ 2015-04-30 15:50 ` Nicholas Mc Guire 2015-04-30 17:38 ` josh 1 sibling, 0 replies; 8+ messages in thread From: Nicholas Mc Guire @ 2015-04-30 15:50 UTC (permalink / raw) To: Linus Torvalds; +Cc: Heiko Carstens, Christopher Li, Sparse Mailing-list 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sparse/parse.c: ignore hotpatch attribute 2015-04-30 15:45 ` Linus Torvalds 2015-04-30 15:50 ` Nicholas Mc Guire @ 2015-04-30 17:38 ` josh 2015-04-30 17:51 ` Linus Torvalds 1 sibling, 1 reply; 8+ messages in thread From: josh @ 2015-04-30 17:38 UTC (permalink / raw) To: Linus Torvalds; +Cc: Heiko Carstens, Christopher Li, Sparse Mailing-list 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sparse/parse.c: ignore hotpatch attribute 2015-04-30 17:38 ` josh @ 2015-04-30 17:51 ` Linus Torvalds 2015-04-30 19:41 ` josh 0 siblings, 1 reply; 8+ messages in thread From: Linus Torvalds @ 2015-04-30 17:51 UTC (permalink / raw) To: Josh Triplett; +Cc: Heiko Carstens, Christopher Li, Sparse Mailing-list 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sparse/parse.c: ignore hotpatch attribute 2015-04-30 17:51 ` Linus Torvalds @ 2015-04-30 19:41 ` josh 0 siblings, 0 replies; 8+ messages in thread From: josh @ 2015-04-30 19:41 UTC (permalink / raw) To: Linus Torvalds; +Cc: Heiko Carstens, Christopher Li, Sparse Mailing-list 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-04-30 19:41 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-28 10:48 [PATCH] sparse/parse.c: ignore hotpatch attribute Heiko Carstens 2015-04-29 23:22 ` Christopher Li 2015-04-30 11:47 ` Heiko Carstens 2015-04-30 15:45 ` Linus Torvalds 2015-04-30 15:50 ` Nicholas Mc Guire 2015-04-30 17:38 ` josh 2015-04-30 17:51 ` Linus Torvalds 2015-04-30 19:41 ` josh
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.