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