The 'this' pointer is not always the first call argument. For JNI
function invocations it is the second argument. EXPR_ARG_THIS is
introduced to mark the argument holding object reference so that
the register which is assigned to it can be easily propagated up
during instruction selection.

Before that JNI invocations via invokevirtual were ignored which
leads to incorrect behavior.

Signed-off-by: Tomek Grabiec <tgrab...@gmail.com>
---
 arch/x86/insn-selector.brg |  111 ++++++++++++++++++++++++++++---------------
 include/jit/expression.h   |   16 ++++--
 jit/args.c                 |   16 ++++++
 jit/expression.c           |   10 ++++
 jit/invoke-bc.c            |   40 ++++++++--------
 jit/tree-printer.c         |   17 +++++++
 test/jit/invoke-bc-test.c  |    2 +-
 7 files changed, 148 insertions(+), 64 deletions(-)

diff --git a/arch/x86/insn-selector.brg b/arch/x86/insn-selector.brg
index 231bf57..6cad013 100644
--- a/arch/x86/insn-selector.brg
+++ b/arch/x86/insn-selector.brg
@@ -82,20 +82,6 @@ static void method_args_cleanup(struct basic_block *bb, 
struct tree_node *tree,
        select_insn(bb, tree, imm_reg_insn(INSN_ADD_IMM_REG, args_size, 
stack_ptr));
 }
 
-/*
- * Returns the right-most expression in argument list. It's the first
- * argument passed to method.
- */
-static struct expression *get_first_arg(struct tree_node *args)
-{
-       while (expr_type(to_expr(args)) == EXPR_ARGS_LIST)
-             args = to_expr(args)->args_right;
-
-       assert(expr_type(to_expr(args)) == EXPR_ARG);
-
-       return to_expr(to_expr(args)->arg_expression);
-}
-
 struct _MBState;
 
 static void __binop_reg_local(struct _MBState *, struct basic_block *, struct 
tree_node *, enum insn_type, struct var_info *, long);
@@ -633,12 +619,12 @@ reg:      EXPR_INVOKE(arg) 1
 reg:   EXPR_INVOKEINTERFACE(arg) 1
 {
        struct var_info *eax, *edx = NULL;
-       struct expression *objectref_expr;
        struct var_info *call_target;
        struct compilation_unit *cu;
        unsigned long method_offset;
        struct vm_method *method;
        struct expression *expr;
+       struct insn *call_insn;
        int nr_stack_args;
 
        expr    = to_expr(tree);
@@ -656,14 +642,7 @@ reg:       EXPR_INVOKEINTERFACE(arg) 1
        }
 
        /* object reference */
-       objectref_expr = get_first_arg(expr->args_list);
-
-       if (expr_type(objectref_expr) == EXPR_VALUE) {
-               call_target = get_var(s->b_parent, J_INT);
-               select_insn(s, tree, imm_reg_insn(INSN_MOV_IMM_REG, 
objectref_expr->value, call_target));
-       } else {
-               call_target = state->left->reg1;
-       }
+       call_target = state->left->reg1;
 
        /* object class */
        select_insn(s, tree, membase_reg_insn(INSN_MOV_MEMBASE_REG,
@@ -679,7 +658,12 @@ reg:       EXPR_INVOKEINTERFACE(arg) 1
                (unsigned long) method, eax));
 
        /* invoke method */
-       select_insn(s, tree, reg_insn(INSN_CALL_REG, call_target));
+       call_insn = reg_insn(INSN_CALL_REG, call_target);
+
+       if (vm_method_is_jni(method))
+               select_jni_call(s, tree, call_insn, method);
+       else
+               select_insn(s, tree, call_insn);
 
        nr_stack_args = get_stack_args_count(method);
        if (nr_stack_args)
@@ -1269,6 +1253,46 @@ arg:     EXPR_NO_ARGS
 }
 
 %ifdef CONFIG_X86_32
+arg:   EXPR_ARG_THIS(reg) 1
+{
+       struct var_info *src;
+       struct expression *expr, *arg_expr;
+
+       expr = to_expr(tree);
+       arg_expr = to_expr(expr->arg_expression);
+
+       assert(arg_expr->vm_type == J_REFERENCE);
+
+       src = state->left->reg1;
+
+       select_insn(s, tree, reg_insn(INSN_PUSH_REG, src));
+       state->reg1 = src;
+}
+%else
+arg:   EXPR_ARG_THIS(reg) 1
+{
+       struct var_info *src, *dst;
+       struct expression *expr, *arg_expr;
+
+       expr = to_expr(tree);
+       arg_expr = to_expr(expr->arg_expression);
+
+       assert(arg_expr->vm_type == J_REFERENCE);
+
+       src = state->left->reg1;
+
+       if (expr->arg_reg != REG_UNASSIGNED) {
+               dst = get_fixed_var(s->b_parent, expr->arg_reg);
+               select_insn(s, tree, reg_reg_insn(INSN_MOV_REG_REG, src, dst));
+               state->reg1 = dst;
+       } else {
+               select_insn(s, tree, reg_insn(INSN_PUSH_REG, src));
+               state->reg1 = src;
+       }
+}
+%endif
+
+%ifdef CONFIG_X86_32
 arg:   EXPR_ARG(EXPR_VALUE) 1
 {
        struct expression *expr, *arg_expr;
@@ -1283,6 +1307,8 @@ arg:      EXPR_ARG(EXPR_VALUE) 1
        }
 
        select_insn(s, tree, imm_insn(INSN_PUSH_IMM, imm & ~0UL));
+
+       state->reg1 = NULL;
 }
 %else
 arg:   EXPR_ARG(EXPR_VALUE) 1
@@ -1300,6 +1326,8 @@ arg:      EXPR_ARG(EXPR_VALUE) 1
                select_insn(s, tree, imm_reg_insn(INSN_MOV_IMM_REG, imm, dst));
        } else
                select_insn(s, tree, imm_insn(INSN_PUSH_IMM, imm));
+
+       state->reg1 = NULL;
 }
 %endif
 
@@ -1313,6 +1341,8 @@ arg:      EXPR_ARG(EXPR_FVALUE) 1
        imm  = float_to_uint32(arg_expr->fvalue);
 
        select_insn(s, tree, imm_insn(INSN_PUSH_IMM, imm & ~0UL));
+
+       state->reg1 = NULL;
 }
 
 %ifdef CONFIG_X86_32
@@ -1332,7 +1362,7 @@ arg:      EXPR_ARG(reg)
        src = state->left->reg1;
        select_insn(s, tree, reg_insn(INSN_PUSH_REG, src));
 
-       state->reg1 = src;
+       state->reg1 = NULL;
 }
 %else
 arg:   EXPR_ARG(reg) 1
@@ -1348,17 +1378,22 @@ arg:    EXPR_ARG(reg) 1
        if (expr->arg_reg != REG_UNASSIGNED) {
                dst = get_fixed_var(s->b_parent, expr->arg_reg);
                select_insn(s, tree, reg_reg_insn(INSN_MOV_REG_REG, src, dst));
-               state->reg1 = dst;
        } else {
                select_insn(s, tree, reg_insn(INSN_PUSH_REG, src));
-               state->reg1 = src;
        }
+
+       state->reg1 = NULL;
 }
 %endif
 
 arg:   EXPR_ARGS_LIST(arg, arg)
 {
-       state->reg1 = state->right->reg1;
+       /* We propagate the value from EXPR_ARG_THIS. EXPR_ARG should set
+          state->reg1 to NULL. */
+       if (state->left->reg1)
+               state->reg1 = state->left->reg1;
+       else
+               state->reg1 = state->right->reg1;
 }
 
 reg:   EXPR_EXCEPTION_REF
@@ -2286,24 +2321,19 @@ static void invoke(struct basic_block *s, struct 
tree_node *tree, struct compila
 static void invokevirtual(struct _MBState *state, struct basic_block *s, 
struct tree_node *tree)
 {
        struct expression *expr;
-       struct expression *objectref_expr;
        struct var_info *call_target;
        unsigned long method_offset;
        struct vm_method *method;
+       struct insn *call_insn;
        int nr_stack_args;
 
        expr    = to_expr(tree);
        method_offset = expr_method_index(expr) * sizeof(void *);
 
-       /* object reference */
-       objectref_expr = get_first_arg(expr->args_list);
+       method  = expr->target_method;
 
-       if (expr_type(objectref_expr) == EXPR_VALUE) {
-               call_target = get_var(s->b_parent, J_INT);
-               select_insn(s, tree, imm_reg_insn(INSN_MOV_IMM_REG, 
objectref_expr->value, call_target));
-       } else {
-               call_target = state->left->reg1;
-       }
+       /* object reference */
+       call_target = state->left->reg1;
 
        /* object class */
        select_insn(s, tree, membase_reg_insn(INSN_MOV_MEMBASE_REG, 
call_target, offsetof(struct vm_object, class), call_target));
@@ -2315,9 +2345,12 @@ static void invokevirtual(struct _MBState *state, struct 
basic_block *s, struct
        select_insn(s, tree, imm_reg_insn(INSN_ADD_IMM_REG, method_offset, 
call_target));
 
        /* invoke method */
-       select_insn(s, tree, reg_insn(INSN_CALL_REG, call_target));
+       call_insn = reg_insn(INSN_CALL_REG, call_target);
 
-       method  = expr->target_method;
+       if (vm_method_is_jni(method))
+               select_jni_call(s, tree, call_insn, method);
+       else
+               select_insn(s, tree, call_insn);
 
        nr_stack_args = get_stack_args_count(method);
        if (nr_stack_args)
diff --git a/include/jit/expression.h b/include/jit/expression.h
index 21d0f5d..3198d4f 100644
--- a/include/jit/expression.h
+++ b/include/jit/expression.h
@@ -34,6 +34,7 @@ enum expression_type {
        EXPR_FINVOKEVIRTUAL,
        EXPR_ARGS_LIST,
        EXPR_ARG,
+       EXPR_ARG_THIS,
        EXPR_NO_ARGS,
        EXPR_NEW,
        EXPR_NEWARRAY,
@@ -170,7 +171,7 @@ struct expression {
                        struct tree_node *objectref_expression;
                        struct vm_field *instance_field;
                };
-               
+
                /*  EXPR_INVOKE represents a method invocation expression (see
                    JLS 15.12.) for which the target method can be determined
                    statically.  This expression type can contain side-effects
@@ -198,9 +199,13 @@ struct expression {
                        struct tree_node *args_right;
                };
 
-               /*  EXPR_ARG represents an argument passed to method. This
-                   expression does not evaluate to a value and is used for
-                   instruction selection only.  */
+               /*  EXPR_ARG represents an argument passed to
+                   method. This expression does not evaluate to a
+                   value and is used for instruction selection
+                   only. The same fields are used by EXPR_ARG_THIS
+                   which represents the object reference on which the
+                   invocation is done. It evaluates to J_REFERENCE
+                   holding the object pointer.  */
                struct {
                        struct tree_node *arg_expression;
                        enum machine_reg arg_reg;
@@ -211,7 +216,7 @@ struct expression {
                struct {
                        /* Nothing. */
                };
-               
+
                /*  EXPR_NEW represents creation of a new instance that is
                    unitialized .  */
                struct {
@@ -318,6 +323,7 @@ struct expression *finvoke_expr(struct vm_method *);
 struct expression *finvokevirtual_expr(struct vm_method *);
 struct expression *args_list_expr(struct expression *, struct expression *);
 struct expression *arg_expr(struct expression *);
+struct expression *arg_this_expr(struct expression *);
 struct expression *no_args_expr(void);
 struct expression *new_expr(struct vm_class *);
 struct expression *newarray_expr(unsigned long, struct expression *);
diff --git a/jit/args.c b/jit/args.c
index a6cc721..85dd711 100644
--- a/jit/args.c
+++ b/jit/args.c
@@ -69,7 +69,23 @@ insert_arg(struct expression *root,
 {
        struct expression *_expr;
 
+       /* Check if we should put @expr in EXPR_ARG_THIS. */
+       if (!vm_method_is_static(method)) {
+               if (vm_method_is_jni(method)) {
+                       if (index == 1) {
+                               _expr = arg_this_expr(expr);
+                               goto from_expr_this;
+                       }
+               } else
+                       if (index == 0) {
+                               _expr = arg_this_expr(expr);
+                               goto from_expr_this;
+                       }
+       }
+
        _expr = arg_expr(expr);
+
+ from_expr_this:
        _expr->bytecode_offset = expr->bytecode_offset;
        set_expr_arg_reg(_expr, method, index);
 
diff --git a/jit/expression.c b/jit/expression.c
index cc71946..dde3c72 100644
--- a/jit/expression.c
+++ b/jit/expression.c
@@ -51,6 +51,7 @@ int expr_nr_kids(struct expression *expr)
        case EXPR_FINVOKE:
        case EXPR_FINVOKEVIRTUAL:
        case EXPR_ARG:
+       case EXPR_ARG_THIS:
        case EXPR_NEWARRAY:
        case EXPR_ANEWARRAY:
        case EXPR_MULTIANEWARRAY:
@@ -97,6 +98,7 @@ int expr_is_pure(struct expression *expr)
        case EXPR_CONVERSION_TO_FLOAT:
        case EXPR_CONVERSION_FROM_FLOAT:
        case EXPR_ARG:
+       case EXPR_ARG_THIS:
        case EXPR_ARRAYLENGTH:
        case EXPR_INSTANCEOF:
        case EXPR_ARRAY_SIZE_CHECK:
@@ -394,6 +396,14 @@ struct expression *arg_expr(struct expression 
*arg_expression)
        return expr;
 }
 
+struct expression *arg_this_expr(struct expression *arg_expression)
+{
+       struct expression *expr = alloc_expression(EXPR_ARG_THIS, J_REFERENCE);
+       if (expr)
+               expr->arg_expression = &arg_expression->node;
+       return expr;
+}
+
 struct expression *no_args_expr(void)
 {
        return alloc_expression(EXPR_NO_ARGS, J_VOID);
diff --git a/jit/invoke-bc.c b/jit/invoke-bc.c
index 016d1ce..f14dedf 100644
--- a/jit/invoke-bc.c
+++ b/jit/invoke-bc.c
@@ -120,29 +120,34 @@ static struct vm_method 
*resolve_invokeinterface_target(struct parse_context *ct
        return 
vm_class_resolve_interface_method_recursive(ctx->cu->method->class, idx);
 }
 
-/* Replaces first argument with null check expression on that argument */
-static void null_check_first_arg(struct expression *arg)
+static void null_check_arg(struct expression *arg)
 {
        struct expression *expr;
 
-       if (expr_type(arg) == EXPR_ARG) {
-               expr = null_check_expr(to_expr(arg->arg_expression));
-               arg->arg_expression = &expr->node;
-       }
-
-       if (expr_type(arg) == EXPR_ARGS_LIST)
-               null_check_first_arg(to_expr(arg->args_right));
+       expr = null_check_expr(to_expr(arg->arg_expression));
+       arg->arg_expression = &expr->node;
 }
 
-/* Replaces second argument with null check expression on that argument */
-static void null_check_second_arg(struct expression *arg)
+/**
+ * Searches the @arg list for EXPR_ARG_THIS and encapsulates its
+ * arg_expression in null check expression.
+ */
+static void null_check_this_arg(struct expression *arg)
 {
-       assert(expr_type(arg) != EXPR_ARG);
+       if (expr_type(arg) != EXPR_ARGS_LIST) {
+               if (expr_type(arg) == EXPR_ARG_THIS)
+                       null_check_arg(arg);
 
-       if (expr_type(to_expr(arg->args_right)) == EXPR_ARG)
-               null_check_first_arg(to_expr(arg->args_left));
+               return;
+       }
 
-       null_check_second_arg(to_expr(arg->args_right));
+       struct expression *right_arg = to_expr(arg->args_right);
+       if (expr_type(right_arg) == EXPR_ARG_THIS) {
+               null_check_arg(right_arg);
+               return;
+       }
+
+       null_check_this_arg(to_expr(arg->args_left));
 }
 
 int convert_invokeinterface(struct parse_context *ctx)
@@ -240,10 +245,7 @@ int convert_invokespecial(struct parse_context *ctx)
        if (err)
                goto failed;
 
-       if (vm_method_is_jni(invoke_target))
-               null_check_second_arg(to_expr(expr->args_list));
-       else
-               null_check_first_arg(to_expr(expr->args_list));
+       null_check_this_arg(to_expr(expr->args_list));
 
        err = insert_invoke_expr(ctx, expr);
        if (err)
diff --git a/jit/tree-printer.c b/jit/tree-printer.c
index df61e88..1afe60c 100644
--- a/jit/tree-printer.c
+++ b/jit/tree-printer.c
@@ -662,6 +662,22 @@ out:
        return err;
 }
 
+static int
+print_arg_this_expr(int lvl, struct string *str, struct expression *expr)
+{
+       int err;
+
+       err = append_formatted(lvl, str, "ARG_THIS:\n");
+       if (err)
+               goto out;
+
+       err = append_tree_attr(lvl + 1, str, "arg_expression",
+                              expr->arg_expression);
+
+out:
+       return err;
+}
+
 static int print_no_args_expr(int lvl, struct string *str,
                              struct expression *expr)
 {
@@ -902,6 +918,7 @@ static print_expr_fn expr_printers[] = {
        [EXPR_FINVOKEVIRTUAL] = print_invokevirtual_expr,
        [EXPR_ARGS_LIST] = print_args_list_expr,
        [EXPR_ARG] = print_arg_expr,
+       [EXPR_ARG_THIS] = print_arg_this_expr,
        [EXPR_NO_ARGS] = print_no_args_expr,
        [EXPR_NEW] = print_new_expr,
        [EXPR_NEWARRAY] = print_newarray_expr,
diff --git a/test/jit/invoke-bc-test.c b/test/jit/invoke-bc-test.c
index 647d3f4..1d71670 100644
--- a/test/jit/invoke-bc-test.c
+++ b/test/jit/invoke-bc-test.c
@@ -95,7 +95,7 @@ build_invoke_bb(unsigned char invoke_opc,
                struct expression **args)
 {
        const struct cafebabe_method_info target_method_info = {
-               .access_flags = CAFEBABE_METHOD_ACC_STATIC,
+               .access_flags = 0,
        };
 
        struct vm_method target_method = {
-- 
1.6.0.6


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Jatovm-devel mailing list
Jatovm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jatovm-devel

Reply via email to