Hi Tomek,
On Sat, 2009-05-30 at 14:22 +0200, Tomek Grabiec wrote:
> We should backpatch all relative call sites regardless of kind of the
> method. That's because 'invokespecial' instruction can emit relative
> calls to methods which are not static and are not constructors, for
> example when calling a private method.
>
> Signed-off-by: Tomek Grabiec <[email protected]>
> ---
> arch/x86/emit-code_32.c | 22 +++++++++++-----------
> 1 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/emit-code_32.c b/arch/x86/emit-code_32.c
> index 139856b..a79e02d 100644
> --- a/arch/x86/emit-code_32.c
> +++ b/arch/x86/emit-code_32.c
> @@ -1073,7 +1073,7 @@ void emit_body(struct basic_block *bb, struct buffer
> *buf)
> * this, we could suspend all threads before patching, and force them
> * to execute flush_icache() on resume.
> */
> -static void fixup_invoke(struct jit_trampoline *t, unsigned long target)
> +static void fixup_relative_calls(struct jit_trampoline *t, unsigned long
> target)
> {
> struct fixup_site *this, *next;
>
> @@ -1099,9 +1099,8 @@ static void fixup_invoke(struct jit_trampoline *t,
> unsigned long target)
> * This function replaces pointers in vtable so that they point
> * directly to compiled code instead of trampoline code.
> */
> -static void fixup_invokevirtual(struct compilation_unit *cu,
> - struct object *objref,
> - void *target)
> +static void fixup_vtable(struct compilation_unit *cu, struct object *objref,
> + void *target)
> {
> struct classblock *cb = CLASS_CB(objref->class);
>
> @@ -1124,15 +1123,16 @@ void emit_trampoline(struct compilation_unit *cu,
>
> __emit_push_reg(buf, REG_EAX);
>
> - if ((cu->method->access_flags & ACC_STATIC) ||
> - method_is_constructor(cu->method)) {
> - __emit_push_imm(buf, (unsigned long)trampoline);
> - __emit_call(buf, fixup_invoke);
> - __emit_add_imm_reg(buf, 0x4, REG_ESP);
> - } else {
> + /* We should always try to fixup relative calls */
> + __emit_push_imm(buf, (unsigned long)trampoline);
> + __emit_call(buf, fixup_relative_calls);
> + __emit_add_imm_reg(buf, 0x4, REG_ESP);
> +
> + if (!method_is_static(cu->method) &&
> + !method_is_constructor(cu->method)) {
> __emit_push_membase(buf, REG_EBP, 0x08);
> __emit_push_imm(buf, (unsigned long)cu);
> - __emit_call(buf, fixup_invokevirtual);
> + __emit_call(buf, fixup_vtable);
> __emit_add_imm_reg(buf, 0x08, REG_ESP);
> }
I think we need to dig deeper here.
First and foremost, is there a 1:1 mapping between method invocation
type (invokevirtual, invokespecial, etc.) and ->access_flags? We already
know that we need special casing for instance initialization methods
("<init>") but what about the rest?
As far as I can tell, ACC_INTEFACE implies invokeinterface and
ACC_STATIC implies invokestatic. Looking at the VM spec:
The difference between the invokespecial and the invokevirtual
instructions is that invokevirtual invokes a method based on the
class of the object. The invokespecial instruction is used to
invoke instance initialization methods (ยง3.9) as well as private
methods and methods of a superclass of the current class.
So it seems that ACC_PRIVATE implies invokespecial and we already know
that "<init>" does that too. So this only leaves us with "methods of a
superclass of the current class". Looking at ECJ generated bytecode, I
am not sure in which cases we actually do use invokespecial to invoke
superclass methods.
On the other hand, if there is no 1:1 mapping between method invocation
type and ->access_flags and that the same method can be called via
invokevirtual or invokespecial, we probably need to generate per
invocation type trampolines lazily in jit/invoke-bc.c and use those
instead. After all, there we know exactly what kind of method invocation
we're dealing with.
Hmm?
Pekka
------------------------------------------------------------------------------
Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT
is a gathering of tech-side developers & brand creativity professionals. Meet
the minds behind Google Creative Lab, Visual Complexity, Processing, &
iPhoneDevCamp as they present alongside digital heavyweights like Barbarian
Group, R/GA, & Big Spaceship. http://p.sf.net/sfu/creativitycat-com
_______________________________________________
Jatovm-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/jatovm-devel
