mur47x111 / DiSL-graal

Other
1 stars 1 forks source link

ArrayIndexOutOfBoundsException when using @ThreadLocal variable #1

Open lschuetze opened 7 years ago

lschuetze commented 7 years ago

Hi,

I wanted to try out DiSL, but I get an java.lang.ArrayIndexOutOfBoundsException when exeuting the code.

I cloned the repo and built the source code with ant. I build my instrumentation code with ant, too. I use Mac OS with Java 1.8.0_111 and GraalVM 0.19 to start with disl.py. Using a @SyntheticLocalvariable makes the code run without problems, but as the semantics of SyntheticLocal is defined it is not a persisted value that I can increase from different methods :-)

How I call disl.py:

export DISL_HOME=/Users/lschuetze/Development/repos/DiSL-graal/output
export GRAAL_HOME=/Users/lschuetze/Development/repos/graal/graal-core
JVM_ARGS="-d64 -Xms1024m -Xmx4048m -server"
ROP_HOME=/Users/lschuetze/Development/repos/benchmarks/benchmarks/rop

$DISL_HOME/../bin/disl.py \
  -s_debug \
  -cs -i=$ROP_HOME/instr/build/disl-instr.jar \
  -c_opts="$JVM_ARGS" \
  -c_app="-jar $ROP_HOME/app/build/benchmark.jar Benchmark 1 100"

The stack trace:

DiSL-agent error: instrumentation server error:
error instrumenting java/lang/Thread: java.lang.ArrayIndexOutOfBoundsException: -1
    at java.util.ArrayList.elementData(ArrayList.java:418)
    at java.util.ArrayList.remove(ArrayList.java:495)
    at org.objectweb.asm.commons.AdviceAdapter.popValue(AdviceAdapter.java:581)
    at org.objectweb.asm.commons.AdviceAdapter.visitFieldInsn(AdviceAdapter.java:365)
    at ch.usi.dag.disl.TLVInserter$TLVInitializer.onMethodEnter(TLVInserter.java:129)
    at org.objectweb.asm.commons.AdviceAdapter.doVisitMethodInsn(AdviceAdapter.java:459)
    at org.objectweb.asm.commons.AdviceAdapter.visitMethodInsn(AdviceAdapter.java:424)
    at org.objectweb.asm.MethodVisitor.visitMethodInsn(MethodVisitor.java:481)
    at org.objectweb.asm.commons.AdviceAdapter.visitMethodInsn(AdviceAdapter.java:432)
    at org.objectweb.asm.tree.MethodInsnNode.accept(MethodInsnNode.java:133)
    at org.objectweb.asm.tree.InsnList.accept(InsnList.java:162)
    at org.objectweb.asm.tree.MethodNode.accept(MethodNode.java:817)
    at org.objectweb.asm.tree.MethodNode.accept(MethodNode.java:727)
    at org.objectweb.asm.tree.ClassNode.accept(ClassNode.java:412)
    at ch.usi.dag.disl.DiSL.instrumentClass(DiSL.java:440)
    at ch.usi.dag.disl.DiSL.instrument(DiSL.java:484)
    at ch.usi.dag.dislserver.RequestProcessor.process(RequestProcessor.java:71)
    at ch.usi.dag.dislserver.DiSLServer$ConnectionHandler.run(DiSLServer.java:99)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)

The instrumentation code:

public class MethodInvocationProfiler {

    @ThreadLocal
    static long methodInvocationCounter;

    @Before(marker = BodyMarker.class,  scope = "bank.Account.*(..)")
    public static void afterInvocation() {
        methodInvocationCounter++;
    }
}

Do you know how to fix this issue? I already tried ASM 5.2 (from Dec '16) but this did not solve the error.

Just ping back if you need any further information!

lschuetze commented 7 years ago

Turning the ThreadLocal from long to int results in no error. Now it has to be checked if the error is with this library or ASM. After running in debug mode and debugging DiSL as well as ASM the problem manifests in TLVInserter$TLVInitializer#onMethodEnter().

// -- put value to the field --
visitLabel(putValue);
visitFieldInsn(PUTFIELD, THREAD_CLASS_NAME, tlv.getName(), tlv.getTypeAsDesc());

The crash happens then in visitFieldInsns when there is a 3rd popValue() triggered. The stack is empty but it tries to remove an element.

case PUTFIELD:
  popValue();
  popValue();
  if (longOrDouble) {
    popValue();
  }
  break;
lschuetze commented 7 years ago

The bug was in DiSL. The problem was that for long or double values the value has to be put twice on the stack. When stored to the variable it will be popped twice.

However, as initialisation value visitLdcInsn (0); was called which will be boxed into an Integer. visitLdcInsn will check if the type is Long or Double and push the value twice. This will never be triggered. The fix is to change the line to the following

final char c = tlv.getTypeAsDesc ().charAt (0);
final boolean longOrDouble = c == 'J' || c == 'D';
// insert 0 as default
if (longOrDouble) {
  visitLdcInsn (0L);
} else {
  visitLdcInsn (0);
}