diff mbox

compile-i386: do not generate an infinite loop

Message ID 200907182334.10900.kdudka@redhat.com (mailing list archive)
State Mainlined, archived
Headers show

Commit Message

Kamil Dudka July 18, 2009, 9:34 p.m. UTC
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

Comments

Christopher Li July 22, 2009, 8:38 a.m. UTC | #1
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
--
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
Kamil Dudka July 22, 2009, 9:24 a.m. UTC | #2
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

--
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
Jeff Garzik July 22, 2009, 12:45 p.m. UTC | #3
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>


--
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
Christopher Li July 22, 2009, 4:34 p.m. UTC | #4
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
--
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
Christopher Li July 22, 2009, 4:39 p.m. UTC | #5
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
>
--
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
Jeff Garzik July 22, 2009, 4:51 p.m. UTC | #6
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




--
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
Kamil Dudka July 22, 2009, 5:21 p.m. UTC | #7
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
--
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
Christopher Li July 22, 2009, 7:18 p.m. UTC | #8
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
Christopher Li July 22, 2009, 7:30 p.m. UTC | #9
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
--
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
diff mbox

Patch

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