Bug in AdviceAdapter.java with superclass constructors
First, we are huge fans and loyal users!
Unfortunately, I believe there is a bug in AdviceAdapter#visitLabel() in all 6.x versions. I am happy to be convinced otherwise.
Here's the code from 5.2:
public void visitLabel(final Label label) {
mv.visitLabel(label);
if (constructor && branches != null) {
List<Object> frame = branches.get(label);
if (frame != null) {
stackFrame = frame;
branches.remove(label);
}
}
}
If this code visits a label in a constructor that was found before the INVOKESPECIAL instruction, it removes it from the branches
set. I believe this is a fine idea because I think you want to avoid code that could jump to before the superclass constructor after it executed. I have no problems with this.
However, that same if
check became more aggressive in 6.x:
public void visitLabel(final Label label) {
super.visitLabel(label);
if (isConstructor && forwardJumpStackFrames != null) {
List<Object> labelStackFrame = forwardJumpStackFrames.get(label);
if (labelStackFrame != null) {
stackFrame = labelStackFrame;
superClassConstructorCalled = false; // the problem is here in this new line
forwardJumpStackFrames.remove(label);
}
}
}
Starting with version 6.x, it incorrectly infers that the super class constructor has not been called yet. This is a stronger step, and I think it may be overkill. I think it's wrong because I am not creating any labels until onMethodEnter()
occurs, which necessarily takes place after INVOKESPECIAL is seen. I do see the visitLabel()
is called within readCode()
when reading the exception_table
, so maybe this is the source of the troublesome labels.
Anyway, this causes a problem when you call methods on AdviceAdapter like visitJumpInsn()
that try to update the model of the stack because it now believes it's in the area before the superclass constructor is called:
public void visitJumpInsn(final int opcode, final Label label) {
super.visitJumpInsn(opcode, label);
if (isConstructor && !superClassConstructorCalled) {
switch (opcode) {
case IFEQ:
case IFNE:
case IFLT:
case IFGE:
case IFGT:
case IFLE:
case IFNULL:
case IFNONNULL:
popValue();
The popValue()
is called with an empty stack, and then you get an exception like:
at java.util.ArrayList.elementData(ArrayList.java:422)
at java.util.ArrayList.remove(ArrayList.java:499)
at org.objectweb.asm.commons.AdviceAdapter.popValue(AdviceAdapter.java:589)
at org.objectweb.asm.commons.AdviceAdapter.visitJumpInsn(AdviceAdapter.java:520)
Please see the attached minimal test case.
I am not sure of all the conditions that could cause this issue to occur, but it seems like InputStreamReader.<init>(InputStream)
is one candidate. Maybe I am missing something, because it seems like these conditions don't seem hard to satisfy. Maybe the compiler is producing a funny exception_table for InputStreamReader, but I'm not sure I'm qualified to speculate any further.
My workaround, which is working just fine and instrumenting a number of diverse JREs and systems, is to create my own version of AdviceAdapter with the superClassConstructorCalled = false
line taken out.
There weren't a lot of comments and no change logs around this change in behavior so I wanted to know:
- Do you agree that setting
superClassConstructorCalled = false
is jarring and probably incorrect? - Is there any piece of the story am I missing? Was there a change in Java 9/10/11 that precipitated this more aggressive behavior?
- Is whoever's compiling the Oracle InputStreamReader class creating a funny bytecode?
I have other examples of classes that cause similar issues, but this was the simplest test case to give to you.