Project: AspectJ
Bug 230817 LazyMethodGen.remap() NullPointerException
Reported: 2008-05-06 22:43:15 | Status: resolved fixed

This has been reported a few times by different users, but has always proved tough to diagnose. The typical stack trace is something like: java.lang.NullPointerException org.aspectj.weaver.bcel.LazyMethodGen.remap(LazyMethodGen.java:1237) org.aspectj.weaver.bcel.LazyMethodGen.addExceptionHandlers(LazyMethodGen.java:1132) org.aspectj.weaver.bcel.LazyMethodGen.packBody(LazyMethodGen.java:1078) org.aspectj.weaver.bcel.LazyMethodGen.pack(LazyMethodGen.java:977) org.aspectj.weaver.bcel.LazyMethodGen.getMethod(LazyMethodGen.java:484) org.aspectj.weaver.bcel.LazyClassGen.writeBack(LazyClassGen.java:512) org.aspectj.weaver.bcel.LazyClassGen.getJavaClassBytesIncludingReweavable(LazyClassGen.java:652) org.aspectj.weaver.bcel.BcelWeaver.getClassFilesFor(BcelWeaver.java:1420) org.aspectj.weaver.bcel.BcelWeaver.weaveAndNotify(BcelWeaver.java:1390) And that is an exception on this line in remap() ih = ih.getNext(); called from the addExceptionHandlers() line: gen.addExceptionHandler( remap(r.getRealStart(), map), remap(r.getRealEnd(), map), remap(r.getHandler(), map), (r.getCatchType() == null) ? null : (ObjectType) BcelWorld.makeBcelType(r.getCatchType())); During weaving, an instruction list is augmented with extra entries representing where shadows start and end (method-call, method-execution, handler, etc). When weaving is complete we want to remove these temporary elements and use the remaining instructions to produce the method bytecode. Now sometimes these temporary entries are targeted by other elements (line number tags, local variable tags and exception handlers usually). During packing we use remap() to reposition the locations so they move off of temporary placeholders and onto real instructions that will make it out into the bytecode. What the above exception tells us is that we started walking over temporary placeholder entries, but before we came to a real instruction, we ran out of instructions! This cannot happen, and indicates something went seriously wrong, we should always encounter an instruction when remapping off a temporary element. After some digging it is actually the remap() call for the handler (not the start or end) that leads to the problem. The handler represents where to jump to in the code when an exception of the specified type occurs between the instructions pointed at by start and end. I sent a debug build to a user encountering this problem (I could not recreate it) and in that I was looking at where in fact the handler was pointing before we called remap(). I learned that when this problem occurs, the handler is not pointing to anywhere in the method currently being processed (not good). In a second debug build I tried to ascertain who was setting these handlers to point to nowhere. This debug never triggered, no-one was setting them to point to nowhere... I happened to notice whilst going through the instructions in the debug output that cobertura was being used, a coverage toolkit that works by doing bytecode manipulation to insert calls out to a library. AspectJ was being called after cobertura and I asked the user to try the steps the other way round - it worked fine. Indicating cobertura was doing something to the bytecode that gave us problems. After much messing about, I recreated it by applying around advice to within(*) to all the classes in rt.jar (I just used that as a very large standalone jar file I could weave into). I learned that Cobertura creates catch blocks that look a little different to what javac and other compilers create. The typical bytecode sequence a compiler produces for a catch block starts with a STORE instruction, to store the exception being caught (whether the body of the catch block uses it or not). But the cobertura catch blocks started with an INVOKESTATIC instruction, a call out to another method. What does this mean? It means the same instruction has two shadows, a 'handler' shadow and a 'method-call' shadow - and it turns out this is what causes our problem. If around advice is applied to the call join point and it cannot be inlined then the body of the call shadow (the call itself) is pulled out into a new method. Because the handler was the same instruction, this meant the handler *was also being pulled out* into the new method, leaving behind an exception handler that jumped to an invalid location (in fact it 'jumped' to an instruction in a different method!). So the reason I never saw the handler location being set incorrectly is that it was set correctly up front, but then dragged out with the method-call shadow into the wrong place. In bytecode terms it looks like this: method-execution() | ICONST_0 | ISTORE_2 | SIPUSH -1 | ISTORE_3 | catch java.lang.Exception (1806389629) -> E0 | | method-call(ProjectData ProjectData.getGlobalProjectData()) | | | INVOKESTATIC ProjectData.getGlobalProjectData () | | method-call(ProjectData getGlobalProjectData()) | | LDC "SomeString" | | method-call(ClassData getOrCreateClassData(java.lang.String)) | | | INVOKEVIRTUAL ProjectData.getOrCreateClassData (LString;) | | method-call(ClassData ProjectData.getOrCreateClassData(String)) | | SIPUSH 106 | | method-call(void ClassData.touch(int)) | | | INVOKEVIRTUAL ClassData.touch (I)V | | method-call(void ClassData.touch(int)) | | ALOAD_1 | | method-call(Object Expression.getValue()) | | | INVOKEVIRTUAL Expression.getValue () | | method-call(Object Expression.getValue()) | catch java.lang.Exception (1806389629) -> E0 | ARETURN | method-call(nProjectData ProjectData.getGlobalProjectData()) | | E0: INVOKESTATIC ProjectData.getGlobalProjectData () | method-call(ProjectData ProjectData.getGlobalProjectData()) | LDC "Object" We can see the problem in that final method-call. The target for the exception handler seen earlier (E0) is within the method-call shadow. What to do?

Files changed in fix:
weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java