It is incorrect to push the same instance of temporary expression twice
onto mimic-stack because some expressions may use that temporary to
store the result and thus unexpectedly (and incorrectly) change the input of 
another
expression. Below is a bytecode sequence that exposes the bug:

    iconst_2
    dup

    iconst_1
    iadd
    istore_1

    iconst_2
    iadd
    istore_2

If dup pushes the same temporary expression twice then the same
register will be used as left operand (and the result also) for both
ADD operations. This will cause that the result of the first ADD
operation will be stored to the register which is input to the second
ADD operation.

Solution for this is to push different temporaries onto mimic-stack
holding the same initial value.

Signed-off-by: Tomek Grabiec <tgrab...@gmail.com>
---
 jit/ostack-bc.c           |   41 ++++++++++++-----------
 test/jit/ostack-bc-test.c |   80 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 84 insertions(+), 37 deletions(-)

diff --git a/jit/ostack-bc.c b/jit/ostack-bc.c
index 04aef36..87ace8a 100644
--- a/jit/ostack-bc.c
+++ b/jit/ostack-bc.c
@@ -19,13 +19,13 @@
 int convert_pop(struct parse_context *ctx)
 {
        struct expression *expr = stack_pop(ctx->bb->mimic_stack);
-       
+
        if (is_invoke_expr(expr)) {
                struct statement *expr_stmt = alloc_statement(STMT_EXPRESSION);
 
                if (!expr_stmt)
                        return -ENOMEM;
-                       
+
                expr_stmt->expression = &expr->node;
                convert_statement(ctx, expr_stmt);
        } else
@@ -48,6 +48,7 @@ static struct expression *dup_expr(struct parse_context *ctx, 
struct expression
 
        dest = temporary_expr(expr->vm_type, tmp_high, tmp_low);
 
+       expr_get(dest);
        stmt = alloc_statement(STMT_STORE);
        stmt->store_dest = &dest->node;
        stmt->store_src  = &expr->node;
@@ -62,8 +63,8 @@ static int __convert_dup(struct parse_context *ctx, struct 
expression *value)
 
        dup = dup_expr(ctx, value);
 
-       convert_expression(ctx, expr_get(dup));
-       convert_expression(ctx, expr_get(dup));
+       convert_expression(ctx, dup);
+       convert_expression(ctx, dup_expr(ctx, expr_get(dup)));
 
        return 0;
 }
@@ -83,9 +84,9 @@ static int __convert_dup_x1(struct parse_context *ctx, struct 
expression *value1
 
        dup = dup_expr(ctx, value1);
 
-       convert_expression(ctx, expr_get(dup));
+       convert_expression(ctx, dup);
        convert_expression(ctx, value2);
-       convert_expression(ctx, expr_get(dup));
+       convert_expression(ctx, dup_expr(ctx, expr_get(dup)));
 
        return 0;
 }
@@ -106,10 +107,10 @@ static int __convert_dup_x2(struct parse_context *ctx, 
struct expression *value1
 
        dup = dup_expr(ctx, value1);
 
-       convert_expression(ctx, expr_get(dup));
+       convert_expression(ctx, dup);
        convert_expression(ctx, value3);
        convert_expression(ctx, value2);
-       convert_expression(ctx, expr_get(dup));
+       convert_expression(ctx, dup_expr(ctx, expr_get(dup)));
 
        return 0;
 }
@@ -132,10 +133,10 @@ static int __convert_dup2(struct parse_context *ctx, 
struct expression *value1,
        dup = dup_expr(ctx, value1);
        dup2 = dup_expr(ctx, value2);
 
-       convert_expression(ctx, expr_get(dup2));
-       convert_expression(ctx, expr_get(dup));
-       convert_expression(ctx, expr_get(dup2));
-       convert_expression(ctx, expr_get(dup));
+       convert_expression(ctx, dup2);
+       convert_expression(ctx, dup);
+       convert_expression(ctx, dup_expr(ctx, expr_get(dup2)));
+       convert_expression(ctx, dup_expr(ctx, expr_get(dup)));
 
        return 0;
 }
@@ -163,11 +164,11 @@ static int __convert_dup2_x1(struct parse_context * ctx, 
struct expression * val
        dup = dup_expr(ctx, value1);
        dup2 = dup_expr(ctx, value2);
 
-       convert_expression(ctx, expr_get(dup2));
-       convert_expression(ctx, expr_get(dup));
+       convert_expression(ctx, dup2);
+       convert_expression(ctx, dup);
        convert_expression(ctx, value3);
-       convert_expression(ctx, expr_get(dup2));
-       convert_expression(ctx, expr_get(dup));
+       convert_expression(ctx, dup_expr(ctx, expr_get(dup2)));
+       convert_expression(ctx, dup_expr(ctx, expr_get(dup)));
 
        return 0;
 }
@@ -196,12 +197,12 @@ static int __convert_dup2_x2(struct parse_context *ctx, 
struct expression *value
        dup = dup_expr(ctx, value1);
        dup2 = dup_expr(ctx, value2);
 
-       convert_expression(ctx, expr_get(dup2));
-       convert_expression(ctx, expr_get(dup));
+       convert_expression(ctx, dup2);
+       convert_expression(ctx, dup);
        convert_expression(ctx, value4);
        convert_expression(ctx, value3);
-       convert_expression(ctx, expr_get(dup2));
-       convert_expression(ctx, expr_get(dup));
+       convert_expression(ctx, dup_expr(ctx, expr_get(dup2)));
+       convert_expression(ctx, dup_expr(ctx, expr_get(dup)));
 
        return 0;
 }
diff --git a/test/jit/ostack-bc-test.c b/test/jit/ostack-bc-test.c
index a9ec4ef..534ac87 100644
--- a/test/jit/ostack-bc-test.c
+++ b/test/jit/ostack-bc-test.c
@@ -48,18 +48,24 @@ static void assert_dup_stack(unsigned char opc, struct 
expression *value)
 {
        struct basic_block *bb;
        struct statement *stmt;
+       struct statement *stmt2;
 
        bb = alloc_simple_bb(&opc, 1);
        stack_push(bb->mimic_stack, expr_get(value));
 
        convert_to_ir(bb->b_parent);
         stmt = stmt_entry(bb->stmt_list.next);
+        stmt2 = stmt_entry(bb->stmt_list.next->next);
 
        assert_store_stmt(stmt);
        assert_ptr_equals(value, to_expr(stmt->store_src));
        assert_temporary_expr(stmt->store_dest);
 
-       assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
+       assert_store_stmt(stmt2);
+       assert_ptr_equals(stmt->store_dest, stmt2->store_src);
+       assert_temporary_expr(stmt2->store_dest);
+
+       assert_ptr_equals(to_expr(stmt2->store_dest), 
pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
 
        assert_true(stack_is_empty(bb->mimic_stack));
@@ -69,7 +75,7 @@ static void assert_dup_stack(unsigned char opc, struct 
expression *value)
 
 static void assert_dup2_stack(unsigned char opc, struct expression *value, 
struct expression *value2)
 {
-       struct statement *stmt, *stmt2;
+       struct statement *stmt, *stmt2, *stmt3, *stmt4;
        struct basic_block *bb;
 
        if (value->vm_type == J_LONG || value->vm_type == J_DOUBLE) {
@@ -84,7 +90,9 @@ static void assert_dup2_stack(unsigned char opc, struct 
expression *value, struc
 
        convert_to_ir(bb->b_parent);
        stmt = stmt_entry(bb->stmt_list.next);
-       stmt2 = stmt_entry(bb->stmt_list.next->next);
+       stmt2 = stmt_entry(stmt->stmt_list_node.next);
+       stmt3 = stmt_entry(stmt2->stmt_list_node.next);
+       stmt4 = stmt_entry(stmt3->stmt_list_node.next);
 
        assert_store_stmt(stmt);
        assert_ptr_equals(value, to_expr(stmt->store_src));
@@ -94,8 +102,16 @@ static void assert_dup2_stack(unsigned char opc, struct 
expression *value, struc
        assert_ptr_equals(value2, to_expr(stmt2->store_src));
        assert_temporary_expr(stmt->store_dest);
 
-       assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
-       assert_ptr_equals(to_expr(stmt2->store_dest), 
pop_and_put_expr(bb->mimic_stack));
+       assert_store_stmt(stmt3);
+       assert_ptr_equals(stmt2->store_dest, stmt3->store_src);
+       assert_temporary_expr(stmt3->store_dest);
+
+       assert_store_stmt(stmt4);
+       assert_ptr_equals(stmt->store_dest, stmt4->store_src);
+       assert_temporary_expr(stmt4->store_dest);
+
+       assert_ptr_equals(to_expr(stmt4->store_dest), 
pop_and_put_expr(bb->mimic_stack));
+       assert_ptr_equals(to_expr(stmt3->store_dest), 
pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(to_expr(stmt2->store_dest), 
pop_and_put_expr(bb->mimic_stack));
 
@@ -125,7 +141,7 @@ static void assert_dup_x1_stack(unsigned char opc, struct 
expression *value1,
                                struct expression *value2)
 {
        struct basic_block *bb;
-       struct statement *stmt;
+       struct statement *stmt, *stmt2;
 
        bb = alloc_simple_bb(&opc, 1);
 
@@ -134,12 +150,17 @@ static void assert_dup_x1_stack(unsigned char opc, struct 
expression *value1,
 
        convert_to_ir(bb->b_parent);
         stmt = stmt_entry(bb->stmt_list.next);
+       stmt2 = stmt_entry(stmt->stmt_list_node.next);
 
        assert_store_stmt(stmt);
        assert_ptr_equals(value1, to_expr(stmt->store_src));
        assert_temporary_expr(stmt->store_dest);
 
-       assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
+       assert_store_stmt(stmt2);
+       assert_ptr_equals(stmt->store_dest, stmt2->store_src);
+       assert_temporary_expr(stmt2->store_dest);
+
+       assert_ptr_equals(to_expr(stmt2->store_dest), 
pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(value2, pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
 
@@ -151,7 +172,7 @@ static void assert_dup_x1_stack(unsigned char opc, struct 
expression *value1,
 static void assert_dup2_x1_stack(unsigned char opc, struct expression *value1,
                                struct expression *value2, struct expression 
*value3)
 {
-       struct statement *stmt, *stmt2;
+       struct statement *stmt, *stmt2, *stmt3, *stmt4;
        struct basic_block *bb;
 
        if (value1->vm_type == J_LONG || value2->vm_type == J_DOUBLE) {
@@ -167,7 +188,9 @@ static void assert_dup2_x1_stack(unsigned char opc, struct 
expression *value1,
 
        convert_to_ir(bb->b_parent);
         stmt = stmt_entry(bb->stmt_list.next);
-       stmt2 = stmt_entry(bb->stmt_list.next->next);
+       stmt2 = stmt_entry(stmt->stmt_list_node.next);
+       stmt3 = stmt_entry(stmt2->stmt_list_node.next);
+       stmt4 = stmt_entry(stmt3->stmt_list_node.next);
 
        assert_store_stmt(stmt);
        assert_ptr_equals(value1, to_expr(stmt->store_src));
@@ -177,8 +200,16 @@ static void assert_dup2_x1_stack(unsigned char opc, struct 
expression *value1,
        assert_ptr_equals(value2, to_expr(stmt2->store_src));
        assert_temporary_expr(stmt2->store_dest);
 
-       assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
-       assert_ptr_equals(to_expr(stmt2->store_dest), 
pop_and_put_expr(bb->mimic_stack));
+       assert_store_stmt(stmt3);
+       assert_ptr_equals(stmt2->store_dest, stmt3->store_src);
+       assert_temporary_expr(stmt3->store_dest);
+
+       assert_store_stmt(stmt4);
+       assert_ptr_equals(stmt->store_dest, stmt4->store_src);
+       assert_temporary_expr(stmt4->store_dest);
+
+       assert_ptr_equals(to_expr(stmt4->store_dest), 
pop_and_put_expr(bb->mimic_stack));
+       assert_ptr_equals(to_expr(stmt3->store_dest), 
pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(value3, pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(to_expr(stmt2->store_dest), 
pop_and_put_expr(bb->mimic_stack));
@@ -209,7 +240,7 @@ static void assert_dup_x2_stack(unsigned char opc, struct 
expression *value1,
                                struct expression *value2, struct expression 
*value3)
 {
        struct basic_block *bb;
-       struct statement *stmt;
+       struct statement *stmt, *stmt2;
 
        bb = alloc_simple_bb(&opc, 1);
 
@@ -219,12 +250,17 @@ static void assert_dup_x2_stack(unsigned char opc, struct 
expression *value1,
 
        convert_to_ir(bb->b_parent);
         stmt = stmt_entry(bb->stmt_list.next);
+       stmt2 = stmt_entry(stmt->stmt_list_node.next);
 
        assert_store_stmt(stmt);
        assert_ptr_equals(value1, to_expr(stmt->store_src));
        assert_temporary_expr(stmt->store_dest);
 
-       assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
+       assert_store_stmt(stmt2);
+       assert_ptr_equals(stmt->store_dest, stmt2->store_src);
+       assert_temporary_expr(stmt2->store_dest);
+
+       assert_ptr_equals(to_expr(stmt2->store_dest), 
pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(value2, pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(value3, pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
@@ -238,7 +274,7 @@ static void assert_dup2_x2_stack(unsigned char opc, struct 
expression *value1,
                                struct expression *value2, struct expression 
*value3,
                                struct expression *value4)
 {
-       struct statement *stmt, *stmt2;
+       struct statement *stmt, *stmt2, *stmt3, *stmt4;
        struct basic_block *bb;
 
        if (value1->vm_type == J_LONG || value1->vm_type == J_DOUBLE) {
@@ -265,7 +301,9 @@ static void assert_dup2_x2_stack(unsigned char opc, struct 
expression *value1,
 
        convert_to_ir(bb->b_parent);
         stmt = stmt_entry(bb->stmt_list.next);
-       stmt2 = stmt_entry(bb->stmt_list.next->next);
+       stmt2 = stmt_entry(stmt->stmt_list_node.next);
+       stmt3 = stmt_entry(stmt2->stmt_list_node.next);
+       stmt4 = stmt_entry(stmt3->stmt_list_node.next);
 
        assert_store_stmt(stmt);
        assert_ptr_equals(value1, to_expr(stmt->store_src));
@@ -275,8 +313,16 @@ static void assert_dup2_x2_stack(unsigned char opc, struct 
expression *value1,
        assert_ptr_equals(value2, to_expr(stmt2->store_src));
        assert_temporary_expr(stmt2->store_dest);
 
-       assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
-       assert_ptr_equals(to_expr(stmt2->store_dest), 
pop_and_put_expr(bb->mimic_stack));
+       assert_store_stmt(stmt3);
+       assert_ptr_equals(stmt2->store_dest, stmt3->store_src);
+       assert_temporary_expr(stmt3->store_dest);
+
+       assert_store_stmt(stmt4);
+       assert_ptr_equals(stmt->store_dest, stmt4->store_src);
+       assert_temporary_expr(stmt4->store_dest);
+
+       assert_ptr_equals(to_expr(stmt4->store_dest), 
pop_and_put_expr(bb->mimic_stack));
+       assert_ptr_equals(to_expr(stmt3->store_dest), 
pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(value3, pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(value4, pop_and_put_expr(bb->mimic_stack));
        assert_ptr_equals(to_expr(stmt->store_dest), 
pop_and_put_expr(bb->mimic_stack));
-- 
1.6.0.6


------------------------------------------------------------------------------
_______________________________________________
Jatovm-devel mailing list
Jatovm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jatovm-devel

Reply via email to