Comment 1 Andrew Clement 2005-09-16 09:11:18 EDT

In the 1.5 case we go wrong when looking at this line of code:

 table = (int[]) get_PLUS().clone();

in the OperatorExpression class.

get_PLUS() is defined as "public static final int[] get_PLUS()"

so we are trying to create a call shadow for a call to clone on "[I".

If we look in the bytecode, in the 1.3 case we see this when clone is called:

   9:	invokevirtual	#278; //Method j/l/Object.clone:()Lj/l/Object;

and in the 1.5 case (as you might be expecting...):

   9:	invokevirtual	#277; //Method "[I".clone:()Ljava/lang/Object;

I have fixed it by improving the code in BcelWorld.nameToSignature() - although
it already looks like a hacky mess in that method....

I need to try it on the whole suite now.

Comment 2 Andrew Clement 2005-09-19 03:29:20 EDT

If built with 1.4, we see the same as for 1.3:

   9:	invokevirtual	#278; //Method j/l/Object.clone:()Lj/l/Object;

(clone() called on Object and not '[I')

Comment 3 Andrew Clement 2005-09-19 11:19:16 EDT

Fixes for 1.5 case:

Ok, I've found the relevant bit in the JVM spec.  What i found confusing for a
while is that in the constant pool a CONSTANT_Class_info structure can refer to
either a class name or a descriptor.  So this mean't if clone (or any method
actually) was being called on a real object type , the CONSTANT_Class_info would
point to a name like:  java/io/PrintStream  (note, this is not a signature
(descriptor), it is a classname).  If the method is being called on an array
type then the entry referred to by the CONSTANT_Class_info object is a signature
(descriptor): [Ljava/io/PrintStream;

This difference means when you retrieve the name for the target of an invoke
instruction, you have to think twice about what to do with it...  you may need
to call UnresolvedType.forName() or you may need UnresolvedType.forSignature().

Bcel tries to be helpful but doesn't allow for the array case - BCEL attempts to
replace all '/' with '.' in the names it retrieves from constant pool through
CONSTANT_Class_info objects.  This means it would corrupt the signature:
[Ljava/io/PrintStream

So, first fix is Bcel so it doesnt damage the signature in the array case.
Second fix is in our code - we currently have a nasty hack in nameToSignature()
which attempts to recognize you passing in a signature and does nothing with it
if thats what you try.  That hack suggests the caller doesn't know what they are
doing ... I've changed the hack to throw an exception if you call
nameToSignature() with a signature.  And the one place so far where I've seen us
exploiting the hack I've also changed, we now call the correct factory method in
UnresolvedType (forSignature or forName) for the string we are processing.

Oh, the relevant lines about things being different when arrays are involved is
in section "4.4.1 CONSTANT_Class" (at least in my copy of the spec)

I've checked in these bits of the fix - now to find out why the woven thing
won't work!

Comment 4 Andrew Clement 2005-09-21 03:07:32 EDT

as we know, the compiler at 1.4/1.5 level now generates more accurate
instructions for something like:

class A { int i; }
class B extends A {}

B b = new B();
b.i;

under 1.3 you would see:
9:   getfield        #24; //Field A.i:I
under 1.4/1.5 you would see:
9:   getfield        #22; //Field B.i:I


Applying around advice to a field reference causes us to create an aroundBody
helper method and extract the instructions 'b.i' into this helper method.  
Unfortunately we were basing the parameter types for the aroundBody method on
the declaring type of the field rather than that used in the instruction, so we
would end up with something like:

aroundBody18(A x) {
  getfield        #4; //Field B.i:I
}

The getfield is against B.i and there is an 'A' on the stack - this causes a
verify error.  what we should have done is use the type used in the instruction
from the byte code as the type for the parameter:

aroundBody18(B x) {
  getfield        #4; //Field B.i:I
}

this works OK. fix checked in and built already available.  I've only fixed it
for field-get - it may manifest at other JPs...

Comment 5 Timothy Halloran 2007-04-28 02:22:24 EDT

This bug was not entirely fixed...it is the cause of bug 172107.  I posted a patch to that bug which "clones" the fix made to get field for set field.