Re: Warning message when using sparse

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

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

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