[PATCH] compile-i386: do not generate an infinite loop

[PATCH] compile-i386: do not generate an infinite loop

[Kamil Dudka]

[-- Attachment #1: Type: text/plain, Size: 175 bytes --]

Hello,

I've probably encountered a bug within compile-i386.c. It generates
an infinite loop for 'while' statement. My testing example and proposed
patch are enclosed.

Kamil

[-- Attachment #2: list.c --]
[-- Type: text/x-csrc, Size: 183 bytes --]

#include <stdlib.h>

typedef void *TItem;

void dispose_list(TItem *list) {
    while (list) {
        TItem *item = list;
        list = (TItem *) *list;
        free(item);
    }
}

[-- Attachment #3: 0001-compile-i386-do-not-generate-an-infinite-loop.patch --]
[-- Type: text/x-diff, Size: 1251 bytes --]

From 60c47d120b577092f0d8fe9001ca6753706dcdbc Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Sat, 18 Jul 2009 23:24:38 +0200
Subject: [PATCH] compile-i386: do not generate an infinite loop

---
 compile-i386.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/compile-i386.c b/compile-i386.c
index 37ea52e..abe9313 100644
--- a/compile-i386.c
+++ b/compile-i386.c
@@ -1913,6 +1913,10 @@ static void emit_loop(struct statement *stmt)
 
 	x86_symbol_decl(stmt->iterator_syms);
 	x86_statement(pre_statement);
+	if (!post_condition || post_condition->type != EXPR_VALUE || post_condition->value) {
+		loop_top = new_label();
+		emit_label(loop_top, "loop top");
+	}
 	if (pre_condition) {
 		if (pre_condition->type == EXPR_VALUE) {
 			if (!pre_condition->value) {
@@ -1936,10 +1940,6 @@ static void emit_loop(struct statement *stmt)
 			insn("jz", lbv, NULL, NULL);
 		}
 	}
-	if (!post_condition || post_condition->type != EXPR_VALUE || post_condition->value) {
-		loop_top = new_label();
-		emit_label(loop_top, "loop top");
-	}
 	x86_statement(statement);
 	if (stmt->iterator_continue->used)
 		emit_label(loop_continue, "'continue' iterator");
-- 
1.6.3.3

Re: [PATCH] compile-i386: do not generate an infinite loop

[Christopher Li]

On Sat, Jul 18, 2009 at 2:34 PM, Kamil Dudka<kdudka@redhat.com> wrote:
> Hello,
>
> I've probably encountered a bug within compile-i386.c. It generates
> an infinite loop for 'while' statement. My testing example and proposed
> patch are enclosed.

Adding Jeff to the CC list. The compile-i386.c is Jeff's pet project.
The change looks good to me. I would like to give Jeff some time
to comment it before I apply the patch.

BTW, have you take a look at Linus's example.c? It is based on the
linearized byte code and It does more advance stuff like simple
register allocation. In my opinion example.c is a better place to start
hacking compiler back end  than compile.c

Chris

Re: [PATCH] compile-i386: do not generate an infinite loop

[Kamil Dudka]

On Wed July 22 2009 10:38:43 Christopher Li wrote:
> Adding Jeff to the CC list. The compile-i386.c is Jeff's pet project.
> The change looks good to me. I would like to give Jeff some time
> to comment it before I apply the patch.

Thanks!

> BTW, have you take a look at Linus's example.c? It is based on the
> linearized byte code and It does more advance stuff like simple
> register allocation. In my opinion example.c is a better place to start
> hacking compiler back end  than compile.c

Linus's example.c works fine with the same test-case. I decided to take 
compile.c as template just because it doesn't use the linearized code.
The tree structure better accomplishes my requirements for now. Maybe I'll 
turn to byte code in future. I've just started to play with it.

Anyway SPARSE seems to be quite helpful. It's easy to read and can save
a lot of development time while processing C sources.

Kamil

Re: [PATCH] compile-i386: do not generate an infinite loop

[Jeff Garzik]

Christopher Li wrote:
> On Sat, Jul 18, 2009 at 2:34 PM, Kamil Dudka<kdudka@redhat.com> wrote:
>> Hello,
>>
>> I've probably encountered a bug within compile-i386.c. It generates
>> an infinite loop for 'while' statement. My testing example and proposed
>> patch are enclosed.
> 
> Adding Jeff to the CC list. The compile-i386.c is Jeff's pet project.
> The change looks good to me. I would like to give Jeff some time
> to comment it before I apply the patch.

Acked-by: Jeff Garzik <jgarzik@redhat.com>

Re: [PATCH] compile-i386: do not generate an infinite loop

[Christopher Li]

On Wed, Jul 22, 2009 at 2:24 AM, Kamil Dudka<kdudka@redhat.com> wrote:
> Anyway SPARSE seems to be quite helpful. It's easy to read and can save
> a lot of development time while processing C sources.

Just curious, what are you trying to build with sparse? A Linus filter would be
pretty cool.

BTW, I want to start some hacking guide for sparse. I am particular interested
in what is the common pain when a new hacker try to work on sparse. Do you
find sparse pretty easy to learn on?

Chris

Re: [PATCH] compile-i386: do not generate an infinite loop

[Christopher Li]

Oh one last thing. I need a signed-off line from you.

Chris

On Sat, Jul 18, 2009 at 2:34 PM, Kamil Dudka<kdudka@redhat.com> wrote:
> Hello,
>
> I've probably encountered a bug within compile-i386.c. It generates
> an infinite loop for 'while' statement. My testing example and proposed
> patch are enclosed.
>
> Kamil
>

Re: [PATCH] compile-i386: do not generate an infinite loop

[Jeff Garzik]

Christopher Li wrote:
> Oh one last thing. I need a signed-off line from you.

I think sparse needs some text document, describing what Signed-off-by 
means.  I did this in one of my own projects by copying the relevant 
"DCO" from kernel tree's Documentation/SubmittingPatches.

	Jeff

Re: [PATCH] compile-i386: do not generate an infinite loop

[Kamil Dudka]

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

On Wednesday 22 of July 2009 18:51:14 Jeff Garzik wrote:
> Christopher Li wrote:
> > Oh one last thing. I need a signed-off line from you.
>
> I think sparse needs some text document, describing what Signed-off-by
> means.  I did this in one of my own projects by copying the relevant
> "DCO" from kernel tree's Documentation/SubmittingPatches.

Thanks! I was just looking for some info about "Signed-off-by"...

The signed-off patch is attached.

Kamil

[-- Attachment #2: 0001-compile-i386-do-not-generate-infinite-loop.patch --]
[-- Type: text/x-diff, Size: 1296 bytes --]

From 984e2612d2174390a54a36a805051c3f66cf6250 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Sat, 18 Jul 2009 23:16:53 +0200
Subject: [PATCH] compile-i386: do not generate infinite loop

Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
 compile-i386.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/compile-i386.c b/compile-i386.c
index 37ea52e..abe9313 100644
--- a/compile-i386.c
+++ b/compile-i386.c
@@ -1913,6 +1913,10 @@ static void emit_loop(struct statement *stmt)
 
 	x86_symbol_decl(stmt->iterator_syms);
 	x86_statement(pre_statement);
+	if (!post_condition || post_condition->type != EXPR_VALUE || post_condition->value) {
+		loop_top = new_label();
+		emit_label(loop_top, "loop top");
+	}
 	if (pre_condition) {
 		if (pre_condition->type == EXPR_VALUE) {
 			if (!pre_condition->value) {
@@ -1936,10 +1940,6 @@ static void emit_loop(struct statement *stmt)
 			insn("jz", lbv, NULL, NULL);
 		}
 	}
-	if (!post_condition || post_condition->type != EXPR_VALUE || post_condition->value) {
-		loop_top = new_label();
-		emit_label(loop_top, "loop top");
-	}
 	x86_statement(statement);
 	if (stmt->iterator_continue->used)
 		emit_label(loop_continue, "'continue' iterator");
-- 
1.6.3.3

Re: [PATCH] compile-i386: do not generate an infinite loop

[Kamil Dudka]

On Wednesday 22 of July 2009 18:34:13 Christopher Li wrote:
> Just curious, what are you trying to build with sparse? A Linus filter
> would be pretty cool.

Sorry for my ignorance, I've never heard about the Linus filter. Could you 
please point me to some relevant info? For now we only want to play with 
separation logic and use it for static analysis of code as part of our 
research at FIT BUT.

> BTW, I want to start some hacking guide for sparse. I am particular
> interested in what is the common pain when a new hacker try to work on
> sparse. Do you find sparse pretty easy to learn on?

I've just written my first "hello world" SPARSE client (nothing useful yet) 
and I am going to write the same as gcc-4.5 plug-in and compare it with each 
other - what is similar, what is different etc.

I haven't encountered any problem while learning SPARSE yet. The code is easy 
to read and the examples sufficient. Maybe worth to write some brief 
description of the particular examples within README? I'll come with some 
ideas later if any.

Kamil

Re: [PATCH] compile-i386: do not generate an infinite loop

[Christopher Li]

On Wed, Jul 22, 2009 at 9:51 AM, Jeff Garzik<jeff@garzik.org> wrote:
> Christopher Li wrote:
>
> I think sparse needs some text document, describing what Signed-off-by
> means.  I did this in one of my own projects by copying the relevant "DCO"
> from kernel tree's Documentation/SubmittingPatches.

Good idea.

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] compile-i386: do not generate an infinite loop

[Christopher Li]

On Wed, Jul 22, 2009 at 10:21 AM, Kamil Dudka<kdudka@redhat.com> wrote:
> Sorry for my ignorance, I've never heard about the Linus filter. Could you
> please point me to some relevant info? For now we only want to play with
> separation logic and use it for static analysis of code as part of our
> research at FIT BUT.

Of course you haven't heard of it. I just make it up myself.  A Linus filter
is a program that take bad C code as input and output good C code as if
it is written by Linus himself. We just need to hook it to LKML.

Chris