2010-07-09 01:51:10

by Larry Finger

[permalink] [raw]
Subject: Warning message when using sparse

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


2010-07-09 20:47:32

by Jiri Slaby

[permalink] [raw]
Subject: Re: Warning message when using sparse

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

2010-07-09 21:22:50

by Larry Finger

[permalink] [raw]
Subject: Re: Warning message when using sparse

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

2010-07-09 21:26:25

by Jiri Slaby

[permalink] [raw]
Subject: Re: Warning message when using sparse

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

2010-07-09 22:04:24

by Christopher Li

[permalink] [raw]
Subject: Re: Warning message when using sparse

On Fri, Jul 9, 2010 at 2:26 PM, Jiri Slaby <[email protected]> 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

2010-07-10 08:39:33

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 2/2] parser: define __builtin_unreachable

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

Signed-off-by: Jiri Slaby <[email protected]>
---
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

2010-07-10 08:39:32

by Jiri Slaby

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

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 <[email protected]>
---
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

2010-07-10 09:07:25

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH 2/2] parser: define __builtin_unreachable

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 <[email protected]>
> ---
> 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

2010-07-10 18:36:14

by Chris Li

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

On Sat, Jul 10, 2010 at 1:39 AM, Jiri Slaby <[email protected]> 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

2010-07-13 07:52:51

by Chris Li

[permalink] [raw]
Subject: Re: [PATCH 2/2] parser: define __builtin_unreachable

On Sat, Jul 10, 2010 at 2:07 AM, Josh Triplett <[email protected]> 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

2010-07-13 18:12:28

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH 2/2] parser: define __builtin_unreachable

On Tue, Jul 13, 2010 at 12:52:48AM -0700, Chris Li wrote:
> On Sat, Jul 10, 2010 at 2:07 AM, Josh Triplett <[email protected]> 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

2010-07-13 18:49:16

by Chris Li

[permalink] [raw]
Subject: Re: [PATCH 2/2] parser: define __builtin_unreachable

On Tue, Jul 13, 2010 at 11:12 AM, Josh Triplett <[email protected]> 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