Bug 52422 - [BUG?/PATCH] LocalVariableGen.getLocalVariable() computes incorrect length
Summary: [BUG?/PATCH] LocalVariableGen.getLocalVariable() computes incorrect length
Alias: None
Product: BCEL - Now in Jira
Classification: Unclassified
Component: Main (show other bugs)
Version: 5.3
Hardware: PC Linux
: P2 normal
Target Milestone: ---
Assignee: issues@commons.apache.org
Depends on:
Reported: 2012-01-04 23:57 UTC by Thiago
Modified: 2012-01-15 02:50 UTC (History)
1 user (show)

Changes the check to add the end instruction length to the local variable length (656 bytes, patch)
2012-01-04 23:57 UTC, Thiago
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thiago 2012-01-04 23:57:56 UTC
Created attachment 28120 [details]
Changes the check to add the end instruction length to the local variable length

It seems that getLocalVariable computes incorrect length for the returning LocalVariable if its range is not the whole method. This is the original code:

public LocalVariable getLocalVariable( ConstantPoolGen cp ) {
        int start_pc = start.getPosition();
        int length = end.getPosition() - start_pc;
        if (length > 0) {
            length += end.getInstruction().getLength();
        int name_index = cp.addUtf8(name);
        int signature_index = cp.addUtf8(type.getSignature());
        return new LocalVariable(start_pc, length, name_index, signature_index, index, cp.getConstantPool());

I think that the check "if (length > 0)" is a "workaround" for local variables whose end targets the last instruction. In this case, we must add the instruction length to recover the actual range for the local variable. However, we should not add the instruction's length if it is not the last instruction of the list because variable ranges are exclusive in the end_pc (note that the JVM spec used to say that the range is inclusive, but this was corrected in JVM5 - see http://java.sun.com/docs/books/jvms/second_edition/jvms-clarify.html or more specifically page 143 of http://java.sun.com/docs/books/jvms/second_edition/ClassFileFormat-Java5.pdf ).

This error can be verified by parsing a method that has local variables whose range is not the whole method (like variables for exceptions in an exception handler), creating a MethodGen and then comparing the local variable tables. Something like this:

ClassParser parser = new ClassParser(...);		
JavaClass clazz = parser.parse();
Method m = clazz.getMethods()[...];
ConstantPoolGen cpg = new ConstantPoolGen(clazz.getConstantPool());
MethodGen mg = new MethodGen(m, clazz.getClassName(), cpg);

would produce some output which includes:

LocalVariable(start_pc = 17, length = 7, index = 2:Exception e1)
LocalVariable(start_pc = 17, length = 6, index = 2:Exception e1)

Note that the length is greater than the original.

If this is really a bug, I believe a fix is to use if (end.getNext() == null) instead (see the patch in attachment). I ran the whole test suite with this fix and it passes, but I am not sure what else it affects.

Comment 1 Dave Brosius 2012-01-15 02:50:23 UTC
Agreed, thanks for the patch, 
Committed revision 1231616.