Warning message when using sparse

Warning message when using sparse

[Larry Finger]

On recent kernels, sparse reports the following message for every source 
file that is processed. The warning does not seem to affect the results. 
I'm not sure when the problem started, but I think it was with 2.6.35-rc1.


   CHECK   drivers/staging/rtl8712/rtl871x_rf.c
/home/finger/linux-2.6/arch/x86/include/asm/cpufeature.h:297:21: error: 
Expected ( after asm
/home/finger/linux-2.6/arch/x86/include/asm/cpufeature.h:297:21: error: 
got goto


Larry

Re: Warning message when using sparse

[Jiri Slaby]

On 07/09/2010 03:52 AM, Larry Finger wrote:
> On recent kernels, sparse reports the following message for every source
> file that is processed. The warning does not seem to affect the results.
> I'm not sure when the problem started, but I think it was with 2.6.35-rc1.
> 
> 
>   CHECK   drivers/staging/rtl8712/rtl871x_rf.c
> /home/finger/linux-2.6/arch/x86/include/asm/cpufeature.h:297:21: error:
> Expected ( after asm
> /home/finger/linux-2.6/arch/x86/include/asm/cpufeature.h:297:21: error:
> got goto
> 

Yeah, I posted a support for that already:
http://git.kernel.org/?p=devel/sparse/chrisl/sparse.git;a=summary

The patch is not 100% correct, it doesn't support asm volatile goto and
similar.

What distro you use? I will fix it in opensuse (which provide gcc 4.5
already), if there will be no new release of sparse.

regards,
-- 
js

Re: Warning message when using sparse

[Larry Finger]

Obviously, I pulled the wrong git tree. I got the one from the wiki. When 
I built the one from the kernel.org tree, all is well.

Larry

Re: Warning message when using sparse

[Jiri Slaby]

On 07/09/2010 11:22 PM, Larry Finger wrote:
> Obviously, I pulled the wrong git tree. I got the one from the wiki.
> When I built the one from the kernel.org tree, all is well.

Both are from kernel.org. But the one I sent is a Chris'es "fork" with
my and other fixes.

-- 
js

Re: Warning message when using sparse

[Christopher Li]

On Fri, Jul 9, 2010 at 2:26 PM, Jiri Slaby <jirislaby@gmail.com> wrote:
> On 07/09/2010 11:22 PM, Larry Finger wrote:
>> Obviously, I pulled the wrong git tree. I got the one from the wiki.
>> When I built the one from the kernel.org tree, all is well.
>
> Both are from kernel.org. But the one I sent is a Chris'es "fork" with
> my and other fixes.
>

Yes, you caught me slacking off. It is time to cut a new release
and flush all the bits into the official tree.

Chris

[PATCH 1/2] parser: fix and simplify support of asm goto

[Jiri Slaby]

Christopher Li wrote:
> Yes, you caught me slacking off. It is time to cut a new release
> and flush all the bits into the official tree.

But include the patch below first, please. And maybe the second
one...

---

1) We now handle only "asm (volatile|goto)?", whereas
   "asm volatile? goto?" is correct.
2) We need to match only goto_ident, so do it explicitly against
   token->ident without match_idents.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 parse.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/parse.c b/parse.c
index caf10b9..dd7b1a4 100644
--- a/parse.c
+++ b/parse.c
@@ -1915,7 +1915,8 @@ static struct token *parse_asm_statement(struct token *token, struct statement *
 	stmt->type = STMT_ASM;
 	if (match_idents(token, &__volatile___ident, &__volatile_ident, &volatile_ident, NULL)) {
 		token = token->next;
-	} else if (match_idents(token, &goto_ident, NULL)) {
+	}
+	if (token->ident == &goto_ident) {
 		is_goto = 1;
 		token = token->next;
 	}
-- 
1.7.1

Re: [PATCH 1/2] parser: fix and simplify support of asm goto

[Chris Li]

On Sat, Jul 10, 2010 at 1:39 AM, Jiri Slaby <jslaby@suse.cz> wrote:
> +       if (token->ident == &goto_ident) {

You need to test the token type is TOKEN_IDENT before using token->ident.
I will apply your patch with the fix up.

Chris

Re: [PATCH 1/2] parser: fix and simplify support of asm goto

[Chris Li]

On Sat, Jul 10, 2010 at 1:39 AM, Jiri Slaby <jslaby@suse.cz> wrote:
> +       if (token->ident == &goto_ident) {

You need to test the token type is TOKEN_IDENT before using token->ident.
I will apply your patch with the fix up.

Chris
--
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

[PATCH 2/2] parser: define __builtin_unreachable

[Jiri Slaby]

Gcc 4.5 defines
extern void __builtin_unreachable(void);
so, add it also to sparse.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 lib.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib.c b/lib.c
index a218bfc..ae6a20c 100644
--- a/lib.c
+++ b/lib.c
@@ -740,6 +740,7 @@ void declare_builtin_functions(void)
 	add_pre_buffer ("extern char * __builtin___strncpy_chk(char *, const char *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
 	add_pre_buffer ("extern int __builtin___vsprintf_chk(char *, int, __SIZE_TYPE__, const char *, __builtin_va_list);\n");
 	add_pre_buffer ("extern int __builtin___vsnprintf_chk(char *, __SIZE_TYPE__, int, __SIZE_TYPE__, const char *, __builtin_va_list ap);\n");
+	add_pre_buffer ("extern void __builtin_unreachable(void);\n");
 }
 
 void create_builtin_stream(void)
-- 
1.7.1

Re: [PATCH 2/2] parser: define __builtin_unreachable

[Josh Triplett]

On Sat, Jul 10, 2010 at 10:39:22AM +0200, Jiri Slaby wrote:
> Gcc 4.5 defines
> extern void __builtin_unreachable(void);
> so, add it also to sparse.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  lib.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/lib.c b/lib.c
> index a218bfc..ae6a20c 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -740,6 +740,7 @@ void declare_builtin_functions(void)
>  	add_pre_buffer ("extern char * __builtin___strncpy_chk(char *, const char *, __SIZE_TYPE__, __SIZE_TYPE__);\n");
>  	add_pre_buffer ("extern int __builtin___vsprintf_chk(char *, int, __SIZE_TYPE__, const char *, __builtin_va_list);\n");
>  	add_pre_buffer ("extern int __builtin___vsnprintf_chk(char *, __SIZE_TYPE__, int, __SIZE_TYPE__, const char *, __builtin_va_list ap);\n");
> +	add_pre_buffer ("extern void __builtin_unreachable(void);\n");

__builtin_unreachable has special semantics beyond just a function.
This definition will suffice to allow compilation, but
__builtin_unreachable should have the same effect in sparse that it does
in GCC: mark the point (and the remainder of the basic block) as
unreachable.  Something like the mechanism used for handling noreturn
would work here as well; declaring the function to have attribute
noreturn would probably have almost the right semantics.

- Josh Triplett

Re: [PATCH 2/2] parser: define __builtin_unreachable

[Chris Li]

On Sat, Jul 10, 2010 at 2:07 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> __builtin_unreachable has special semantics beyond just a function.
> This definition will suffice to allow compilation, but
> __builtin_unreachable should have the same effect in sparse that it does
> in GCC: mark the point (and the remainder of the basic block) as
> unreachable.  Something like the mechanism used for handling noreturn
> would work here as well; declaring the function to have attribute
> noreturn would probably have almost the right semantics.
>

The attribute noreturn will apply to the whole function. The function
NEVER returns.
__builtin_unreachable only apply to current basic block. e.g. some
error handling path like panic. The function can still return a value on the
normal path. It has different meaning than attribute noreturn. So I don't think
automatically give the function noreturn attribute is the right thing to do.

I will apply the patch until we got better way to handle this.

Chris

Re: [PATCH 2/2] parser: define __builtin_unreachable

[Chris Li]

On Sat, Jul 10, 2010 at 2:07 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> __builtin_unreachable has special semantics beyond just a function.
> This definition will suffice to allow compilation, but
> __builtin_unreachable should have the same effect in sparse that it does
> in GCC: mark the point (and the remainder of the basic block) as
> unreachable.  Something like the mechanism used for handling noreturn
> would work here as well; declaring the function to have attribute
> noreturn would probably have almost the right semantics.
>

The attribute noreturn will apply to the whole function. The function
NEVER returns.
__builtin_unreachable only apply to current basic block. e.g. some
error handling path like panic. The function can still return a value on the
normal path. It has different meaning than attribute noreturn. So I don't think
automatically give the function noreturn attribute is the right thing to do.

I will apply the patch until we got better way to handle this.

Chris
--
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

Re: [PATCH 2/2] parser: define __builtin_unreachable

[Josh Triplett]

On Tue, Jul 13, 2010 at 12:52:48AM -0700, Chris Li wrote:
> On Sat, Jul 10, 2010 at 2:07 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> > __builtin_unreachable has special semantics beyond just a function.
> > This definition will suffice to allow compilation, but
> > __builtin_unreachable should have the same effect in sparse that it does
> > in GCC: mark the point (and the remainder of the basic block) as
> > unreachable.  Something like the mechanism used for handling noreturn
> > would work here as well; declaring the function to have attribute
> > noreturn would probably have almost the right semantics.
> >
> 
> The attribute noreturn will apply to the whole function. The function
> NEVER returns.
> __builtin_unreachable only apply to current basic block. e.g. some
> error handling path like panic. The function can still return a value on the
> normal path. It has different meaning than attribute noreturn. So I don't think
> automatically give the function noreturn attribute is the right thing to do.

No, I didn't mean that using __builtin_unreachable should mark the
function calling it as noreturn.  I meant that as an approximation to
the right behavior, __builtin_unreachable *itself* could have attribute
noreturn.

- Josh Triplett

Re: [PATCH 2/2] parser: define __builtin_unreachable

[Chris Li]

On Tue, Jul 13, 2010 at 11:12 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> No, I didn't mean that using __builtin_unreachable should mark the
> function calling it as noreturn.  I meant that as an approximation to
> the right behavior, __builtin_unreachable *itself* could have attribute
> noreturn.

Ah, that make sense.

Chris

Re: [PATCH 2/2] parser: define __builtin_unreachable

[Chris Li]

On Tue, Jul 13, 2010 at 11:12 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> No, I didn't mean that using __builtin_unreachable should mark the
> function calling it as noreturn.  I meant that as an approximation to
> the right behavior, __builtin_unreachable *itself* could have attribute
> noreturn.

Ah, that make sense.

Chris
--
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