bad asm goto ?

bad asm goto ?

[Dave Jones]

I just ran sparse on a kernel tree for the first time in ages, and got
hundreds of warnings, many of which related to the tracepoint macros
in the kernel, which use the new asm goto feature.

Looking at the git log of sparse, I see there was some support added
for asm goto, but it seems to choke every time, printing ..

include/trace/events/kmem.h:45:1: error: bad asm output
include/trace/events/kmem.h:45:1: error: incompatible types in comparison expression (different address spaces)


(this is with a sparse built from git head from a few minutes ago)

	Dave

Re: bad asm goto ?

[Christopher Li]

On Mon, Oct 25, 2010 at 2:31 PM, Dave Jones <davej@redhat.com> wrote:
> I just ran sparse on a kernel tree for the first time in ages, and got
> hundreds of warnings, many of which related to the tracepoint macros
> in the kernel, which use the new asm goto feature.
>
> Looking at the git log of sparse, I see there was some support added
> for asm goto, but it seems to choke every time, printing ..
>
> include/trace/events/kmem.h:45:1: error: bad asm output
> include/trace/events/kmem.h:45:1: error: incompatible types in comparison expression (different address spaces)
>
>
> (this is with a sparse built from git head from a few minutes ago)

Do you have a smaller test case to reproduce the error?

Chris

Re: bad asm goto ?

[Dave Jones]

On Mon, Oct 25, 2010 at 03:10:29PM -0700, Christopher Li wrote:
 > On Mon, Oct 25, 2010 at 2:31 PM, Dave Jones <davej@redhat.com> wrote:
 > > I just ran sparse on a kernel tree for the first time in ages, and got
 > > hundreds of warnings, many of which related to the tracepoint macros
 > > in the kernel, which use the new asm goto feature.
 > >
 > > Looking at the git log of sparse, I see there was some support added
 > > for asm goto, but it seems to choke every time, printing ..
 > >
 > > include/trace/events/kmem.h:45:1: error: bad asm output
 > > include/trace/events/kmem.h:45:1: error: incompatible types in comparison expression (different address spaces)
 > >
 > >
 > > (this is with a sparse built from git head from a few minutes ago)
 > 
 > Do you have a smaller test case to reproduce the error?

I tried unwinding some of the nested macros, and came up with ..

# define __ASM_FORM(x)  " " #x " "
# define JUMP_LABEL_INITIAL_NOP ".byte 0xe9 \n\t .long 0\n\t"
# define __ASM_SEL(a,b) __ASM_FORM(b)
#define _ASM_PTR        __ASM_SEL(.long, .quad)

# define JUMP_LABEL(key, label)                                 \
        do {                                                    \
                asm goto("1:"                                   \
                        JUMP_LABEL_INITIAL_NOP                  \
                        ".pushsection __jump_table,  \"a\" \n\t"\
                        _ASM_PTR "1b, %l[" #label "], %c0 \n\t" \
                        ".popsection \n\t"                      \
                        : :  "i" (key) :  : label);             \
        } while (0)

int main(int argc, char *argv[])
{
        JUMP_LABEL("1", do_trace );
        return 1;
do_trace:
        return 0;
}

gcc seems to not choke on it, but sparse still does, so I think I managed
to untangle things correctly..

	Dave

Re: bad asm goto ?

[Christopher Li]

On Mon, Oct 25, 2010 at 3:39 PM, Dave Jones <davej@redhat.com> wrote:
> I tried unwinding some of the nested macros, and came up with ..
>
> # define __ASM_FORM(x)  " " #x " "
> # define JUMP_LABEL_INITIAL_NOP ".byte 0xe9 \n\t .long 0\n\t"
> # define __ASM_SEL(a,b) __ASM_FORM(b)
> #define _ASM_PTR        __ASM_SEL(.long, .quad)
>
> # define JUMP_LABEL(key, label)                                 \
>        do {                                                    \
>                asm goto("1:"                                   \
>                        JUMP_LABEL_INITIAL_NOP                  \
>                        ".pushsection __jump_table,  \"a\" \n\t"\
>                        _ASM_PTR "1b, %l[" #label "], %c0 \n\t" \
>                        ".popsection \n\t"                      \
>                        : :  "i" (key) :  : label);             \
>        } while (0)
>
> int main(int argc, char *argv[])
> {
>        JUMP_LABEL("1", do_trace );
>        return 1;
> do_trace:
>        return 0;
> }
>

Thanks, I got this example working for me too. You are right, sparse is
choking on it. Let me take a look tonight.

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: bad asm goto ?

[Christopher Li]

It seems that the error is not from the goto. It is from the malformed
asm_clobber.
For empty asm_clobber list, sparse currently add nil expr into the
list. Evaluate will
complain about the nil pointer thinking it has a bad type.

Any way, this patch should fix it.

Please let me know that help you or not. I am going to apply it after
the 0.4.3 release.

Chris

diff --git a/evaluate.c b/evaluate.c
index f8343c2..1fb03ed 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3221,7 +3221,7 @@ static void evaluate_asm_statement(struct statement *stmt)

 	FOR_EACH_PTR(stmt->asm_clobbers, expr) {
 		if (!expr) {
-			sparse_error(stmt->pos, "bad asm output");
+			sparse_error(stmt->pos, "bad asm clobbers");
 			return;
 		}
 		if (expr->type == EXPR_STRING)
diff --git a/parse.c b/parse.c
index 537055f..6b7d656 100644
--- a/parse.c
+++ b/parse.c
@@ -1894,7 +1894,8 @@ static struct token *parse_asm_clobbers(struct
token *token, struct statement *s

 	do {
 		token = primary_expression(token->next, &expr);
-		add_expression(clobbers, expr);
+		if (expr)
+			add_expression(clobbers, expr);
 	} while (match_op(token, ','));
 	return token;
 }

Re: bad asm goto ?

[Harvey Harrison]

On Mon, Oct 25, 2010 at 8:10 PM, Christopher Li <sparse@chrisli.org> wrote:
> It seems that the error is not from the goto. It is from the malformed
> asm_clobber.
> For empty asm_clobber list, sparse currently add nil expr into the
> list. Evaluate will
> complain about the nil pointer thinking it has a bad type.
>
> Any way, this patch should fix it.
>
> Please let me know that help you or not. I am going to apply it after
> the 0.4.3 release.
>

I'm afraid this didn't solve it for me when applying on top of v0.4.3,
if you have any other patches to try,
I'd be glad to test them out.

Harvey

Re: bad asm goto ?

[Christopher Li]

On Wed, Oct 27, 2010 at 7:45 PM, Harvey Harrison
<harvey.harrison@gmail.com> wrote:
\>
> I'm afraid this didn't solve it for me when applying on top of v0.4.3,
> if you have any other patches to try,
> I'd be glad to test them out.

What is your test case? The patch does work for me on the small test
case provide by
Dave. I am just wondering if you are running the same thing.
If not, please let me know what you run and what is the error message.

Chris

Re: bad asm goto ?

[Harvey Harrison]

On Wed, Oct 27, 2010 at 11:12 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Wed, Oct 27, 2010 at 7:45 PM, Harvey Harrison
> <harvey.harrison@gmail.com> wrote:
> \>
>> I'm afraid this didn't solve it for me when applying on top of v0.4.3,
>> if you have any other patches to try,
>> I'd be glad to test them out.
>
> What is your test case? The patch does work for me on the small test
> case provide by
> Dave. I am just wondering if you are running the same thing.
> If not, please let me know what you run and what is the error message.
>

Sorry about that, just ran again and had a leftover binary elsewhere
in my path, your patch does fix
the error, all that's left is the:

include/trace/events/kmem.h:45:1: error: incompatible types in
comparison expression (different address spaces)

Sorry for the noise.

Harvey

Re: bad asm goto ?

[Christopher Li]

On Wed, Oct 27, 2010 at 11:33 PM, Harvey Harrison
<harvey.harrison@gmail.com> wrote:
> Sorry about that, just ran again and had a leftover binary elsewhere
> in my path, your patch does fix
> the error, all that's left is the:

No problem. Thanks for the testing.


Chris

Re: bad asm goto ?

[Harvey Harrison]

Ping, noticed you moving towards a 0.4.4 release and this was still not applied.

Harvey

On Mon, Oct 25, 2010 at 8:10 PM, Christopher Li <sparse@chrisli.org> wrote:
> It seems that the error is not from the goto. It is from the malformed
> asm_clobber.
> For empty asm_clobber list, sparse currently add nil expr into the
> list. Evaluate will
> complain about the nil pointer thinking it has a bad type.
>
> Any way, this patch should fix it.
>
> Please let me know that help you or not. I am going to apply it after
> the 0.4.3 release.
>
> Chris
>
> diff --git a/evaluate.c b/evaluate.c
> index f8343c2..1fb03ed 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -3221,7 +3221,7 @@ static void evaluate_asm_statement(struct statement *stmt)
>
>        FOR_EACH_PTR(stmt->asm_clobbers, expr) {
>                if (!expr) {
> -                       sparse_error(stmt->pos, "bad asm output");
> +                       sparse_error(stmt->pos, "bad asm clobbers");
>                        return;
>                }
>                if (expr->type == EXPR_STRING)
> diff --git a/parse.c b/parse.c
> index 537055f..6b7d656 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -1894,7 +1894,8 @@ static struct token *parse_asm_clobbers(struct
> token *token, struct statement *s
>
>        do {
>                token = primary_expression(token->next, &expr);
> -               add_expression(clobbers, expr);
> +               if (expr)
> +                       add_expression(clobbers, expr);
>        } while (match_op(token, ','));
>        return token;
>  }
> --
> 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
>
--
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: bad asm goto ?

[Christopher Li]

On Sun, Aug 21, 2011 at 9:01 PM, Harvey Harrison
<harvey.harrison@gmail.com> wrote:
> Ping, noticed you moving towards a 0.4.4 release and this was still not applied.

It falls through the cracks. I just add a test case and commit the fix.

Thanks for the reminder.

Chris