Comment 1 Kamal Govindraj 2004-05-18 09:06:17 EDT

Created attachment 10760 [details]
Contains the class file & the required jars to reproduce the problem

I have attached a zip file with all the necessary files to reproduce the issue.
The readme.txt explains the purpose of each file and the sequence of commands
to reproduce the issue.

Comment 2 Kamal Govindraj 2004-05-18 09:08:55 EDT

I have tested this with aspectj 1.2 as well. The problem reproduces in 1.2 as 
well.

Comment 3 Andrew Clement 2004-05-18 11:30:48 EDT

Looks like a silent compilation failure leaves a broken class file on disk - 
the class file is truncated at a method boundary.

Comment 4 Andrew Clement 2004-05-19 08:10:12 EDT

Please can you tell me which Java compiler (IBM/SUN/JDT/etc???) and which 
version of that compiler you used to build the stub into a class file? thanks.

I currently suspect this is a BCEL bug.  But I'm having trouble making 
progress.  I have tried decompiling the stub class and then doing the 
compilation using the source rather than a binary weave, and that doesn't fail.

I find that I can make the aspect much more simple and still get the problem, 
I can just use before or after advice, I don't need both.  I find I can target 
single methods in the stub and sometimes the weave works, sometimes it fails - 
haven't found the pattern yet ...

Comment 5 Kamal Govindraj 2004-05-19 09:08:51 EDT

The stub class file is generated using the weblogic appc command. I tried 
various options but couldn't get hold of the source file.

It appears that the stub class file is generated on the fly based on another 
class file without going through the step of first generating a source file and 
then compiling it.

Comment 6 Andrew Clement 2004-05-20 08:57:47 EDT

Got it !

Definetly worth a nice write up.

The class file is not truncated but it does contain a bit of garbage that 
chokes the JVM or various decompilers (e.g. jad).  The problem only occurs if 
you weave a method that contains a certain bytecode pattern.  The appc tool 
that is constructing the class file in question puts in this particular 
bytecode sequence:

   32:  ldc_w   #10; //String tavant.POCSession
   35:  invokestatic    #165; //Method class$:(Ljava/lang/String;)
Ljava/lang/Class;

Which translates to:   19,0,10,184,165

ldc_w takes the following two bytes to construct an index into the constant 
pool (byte1<<8+byte2).

It is only when we weave a method that we let BCEL process the method, because 
we are going to munge the bytecodes in some way.  BCEL sees the code sequence 
above and tries to be 'clever'.  It sees an ldc_w, it then pulls in the 0 and 
10.  The actual code that does it is (from LDC_W.java):

protected void initFromFile(ByteSequence bytes, boolean wide) throws 
IOException {
    setIndex(bytes.readUnsignedShort());
    // Override just in case it has been changed
    opcode = org.apache.bcel.Constants.LDC_W;
}

setIndex() is in the superclass of LDC_W, called LDC.  setIndex() calls setSize
() in LDC which contains this piece of code:

  protected final void setSize() {
    if(index <= org.apache.bcel.Constants.MAX_BYTE) { // Fits in one byte?
      opcode = org.apache.bcel.Constants.LDC;
      length = 2;
    } else {
      opcode = org.apache.bcel.Constants.LDC_W;
      length = 3;
    }
  }

Here there is an optimization attempted.  If it is an LDC_W instruction but 
the constant pool index is less than one byte, transform it into an LDC 
instruction which only needs a single byte index after it.  It remembers that 
it made this optimization by setting the length to 2.  Of course, we then come 
out of setSize(), back up to initFromFile() which then does this:

    // Override just in case it has been changed
    opcode = org.apache.bcel.Constants.LDC_W;

which resets the opcode back again !  So now we have stored an LDC_W 
instruction that takes a single byte.  When we write this sequence out onto 
disk using this method:

  public void dump(DataOutputStream out) throws IOException {
    out.writeByte(opcode);

    if(length == 2)
      out.writeByte(index);
    else // Applies for LDC_W
      out.writeShort(index);
  }

We get:   19,10,184,165

and of course, you give this to a JVM or a decompiler and it sees:

LDC_W 2744
IF_ACMPEQ

(2744 is 10<<8+184)

Of course, there aren't 2744 entries in the constant pool and we blow up.

The fix is to comment out the line that resets the opcode back to LDC_W if we 
have made the optimization.

So, you see this bug if:
- You used a compiler or tool to generate the class file that didn't already 
make the optimization.
- You weave a method containing the unoptimized bytecode sequence.

Comment 7 Andrew Clement 2004-05-24 12:23:26 EDT

Fix checked in.  Fix will be in AspectJ 1.2 final.