230817: remap NPE - switch ordering in match to do exception handlers first and insert an extra NOP if required.

2 files changed, 108 insertions, 69 deletions

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
index 5ac6b975d..87c8b390a 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
@@ -62,6 +62,7 @@ import org.aspectj.apache.bcel.generic.PUTSTATIC;
 import org.aspectj.apache.bcel.generic.RET;
 import org.aspectj.apache.bcel.generic.ReturnInstruction;
 import org.aspectj.apache.bcel.generic.Select;
+import org.aspectj.apache.bcel.generic.StoreInstruction;
 import org.aspectj.apache.bcel.generic.Tag;
 import org.aspectj.apache.bcel.generic.Type;
 import org.aspectj.apache.bcel.generic.annotation.AnnotationGen;
@@ -2454,7 +2455,53 @@ class BcelClassWeaver implements IClassWeaver {
 		BcelShadow enclosingShadow,
 		List shadowAccumulator) 
 	{
-		Instruction i = ih.getInstruction();
+	    Instruction i = ih.getInstruction();
+        
+        // Exception handlers (pr230817)
+        if (canMatch(Shadow.ExceptionHandler) && !Range.isRangeHandle(ih)) {
+            InstructionTargeter[] targeters = ih.getTargeters();
+            if (targeters != null) {
+                for (int j = 0; j < targeters.length; j++) {
+                    InstructionTargeter t = targeters[j];
+                    if (t instanceof ExceptionRange) {
+                        // assert t.getHandler() == ih
+                        ExceptionRange er = (ExceptionRange) t;
+                        if (er.getCatchType() == null) continue;
+                        if (isInitFailureHandler(ih)) return;
+                        
+                        if (!(ih.getInstruction() instanceof StoreInstruction) && ih.getInstruction().getOpcode()!=Constants.NOP) {
+                            // If using cobertura, the catch block stats with INVOKESTATIC rather than ASTORE, in order that the ranges 
+                            // for the methodcall and exceptionhandler shadows that occur at this same 
+                            // line, we need to modify the instruction list to split them - adding a 
+                            // NOP before the invokestatic that gets all the targeters
+                            // that were aimed at the INVOKESTATIC
+                            mg.getBody().insert(ih,InstructionConstants.NOP);
+                            InstructionHandle newNOP = ih.getPrev();
+                            // what about a try..catch that starts at the start of the exception handler? need to only include certain targeters really.
+                            er.updateTarget(ih, newNOP,mg.getBody());
+                            for (int ii=0;ii<targeters.length;ii++) {
+                                newNOP.addTargeter(targeters[ii]);
+                            }
+                            ih.removeAllTargeters();
+                            match(
+                                BcelShadow.makeExceptionHandler(
+                                    world, 
+                                    er,
+                                    mg, newNOP, enclosingShadow),
+                                shadowAccumulator);
+                        } else {                    
+                            match(
+                                BcelShadow.makeExceptionHandler(
+                                    world, 
+                                    er,
+                                    mg, ih, enclosingShadow),
+                                shadowAccumulator);
+                        }
+                    }
+                }
+            }
+        }
+        
 		if ((i instanceof FieldInstruction) && 
 			(canMatch(Shadow.FieldGet) || canMatch(Shadow.FieldSet))
 		) {
@@ -2550,30 +2597,7 @@ class BcelClassWeaver implements IClassWeaver {
 			  }
 			// }
 		}
-		// performance optimization... we only actually care about ASTORE instructions, 
-		// since that's what every javac type thing ever uses to start a handler, but for
-		// now we'll do this for everybody.
-		if (!canMatch(Shadow.ExceptionHandler)) return;
-		if (Range.isRangeHandle(ih)) return;
-		InstructionTargeter[] targeters = ih.getTargeters();
-		if (targeters != null) {
-			for (int j = 0; j < targeters.length; j++) {
-				InstructionTargeter t = targeters[j];
-				if (t instanceof ExceptionRange) {
-					// assert t.getHandler() == ih
-					ExceptionRange er = (ExceptionRange) t;
-					if (er.getCatchType() == null) continue;
-					if (isInitFailureHandler(ih)) return;
-					
-					match(
-						BcelShadow.makeExceptionHandler(
-							world, 
-							er,
-							mg, ih, enclosingShadow),
-						shadowAccumulator);
-				}
-			}
-		}
+		
 	}
 
 	private boolean isInitFailureHandler(InstructionHandle ih) {
@@ -2769,10 +2793,11 @@ class BcelClassWeaver implements IClassWeaver {
 							effectiveSig.getShadowKind(), rm), shadowAccumulator);
 			}
 		} else {
-			if (canMatch(Shadow.MethodCall))
+			if (canMatch(Shadow.MethodCall)) {
 				match(
 						BcelShadow.makeMethodCall(world, mg, ih, enclosingShadow),
 						shadowAccumulator);
+			}
 		}
 	}
 	
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
index 872a5c0e4..6bdc54c9c 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
@@ -1137,13 +1137,13 @@ public final class LazyMethodGen implements Traceable {
         for (Iterator iter = exnList.iterator(); iter.hasNext();) {
             ExceptionRange r = (ExceptionRange) iter.next();
             if (r.isEmpty()) continue;
-            gen.addExceptionHandler(
-                remap(r.getRealStart(), map), 
-                remap(r.getRealEnd(), map),
-                remap(r.getHandler(), map),
-                (r.getCatchType() == null)
-                ? null 
-                : (ObjectType) BcelWorld.makeBcelType(r.getCatchType()));
+            InstructionHandle rMappedStart = remap(r.getRealStart(), map);
+            InstructionHandle rMappedEnd = remap(r.getRealEnd(), map);
+            InstructionHandle rMappedHandler = remap(r.getHandler(), map);
+            gen.addExceptionHandler(rMappedStart,rMappedEnd,rMappedHandler,
+            (r.getCatchType() == null)
+            ? null 
+            : (ObjectType) BcelWorld.makeBcelType(r.getCatchType()));
         }
 	}
 
@@ -1337,37 +1337,45 @@ public final class LazyMethodGen implements Traceable {
 	}
 	
 	public static void assertGoodBody(InstructionList il, String from) {
-		if (true) return;  // only to be enabled for debugging
-		if (il == null) return;
-		Set body = new HashSet();
-		Stack ranges = new Stack();
-		for (InstructionHandle ih = il.getStart(); ih != null; ih = ih.getNext()) {
-			body.add(ih);
-			if (ih.getInstruction() instanceof BranchInstruction) {
-				body.add(ih.getInstruction());
-			}
-		}
-		
-		for (InstructionHandle ih = il.getStart(); ih != null; ih = ih.getNext()) {
-			assertGoodHandle(ih, body, ranges, from);
-			InstructionTargeter[] ts = ih.getTargeters();
-			if (ts != null) {
-				for (int i = ts.length - 1; i >= 0; i--) {
-					assertGoodTargeter(ts[i], ih, body, from);
-				}
-			}
-		}
+        if (true) return;  // only to be enabled for debugging // should be switchonable via an option
+        StringBuffer assertionLog = new StringBuffer();
+        assertionLog.append("Checking state of an instruction body, from="+from+"\n");
+	    try {
+    		if (il == null) return;
+    		Set body = new HashSet();
+    		Stack ranges = new Stack();
+    		for (InstructionHandle ih = il.getStart(); ih != null; ih = ih.getNext()) {
+    			body.add(ih);
+    			if (ih.getInstruction() instanceof BranchInstruction) {
+    				body.add(ih.getInstruction());
+    			}
+    		}
+    		
+    		for (InstructionHandle ih = il.getStart(); ih != null; ih = ih.getNext()) {
+    			assertGoodHandle(ih, body, ranges, from, assertionLog);
+    			InstructionTargeter[] ts = ih.getTargeters();
+    			if (ts != null) {
+    				for (int i = ts.length - 1; i >= 0; i--) {
+    					assertGoodTargeter(ts[i], ih, body, from, assertionLog);
+    				}
+    			}
+    		}
+	    } catch (RuntimeException re) {
+	        System.err.println(assertionLog.toString());
+	        throw re;
+	    }
 	}
 
-	private static void assertGoodHandle(InstructionHandle ih, Set body, Stack ranges, String from) {
+	private static void assertGoodHandle(InstructionHandle ih, Set body, Stack ranges, String from, StringBuffer log) {
 		Instruction inst = ih.getInstruction();
 		if ((inst instanceof BranchInstruction) ^ (ih instanceof BranchHandle)) {
 			throw new BCException("bad instruction/handle pair in " + from);
 		}
 		if (Range.isRangeHandle(ih)) {
-			assertGoodRangeHandle(ih, body, ranges, from);
+		    log.append("Checking range handle '"+ih+"'\n");
+			assertGoodRangeHandle(ih, body, ranges, from,log);
 		} else if (inst instanceof BranchInstruction) {
-			assertGoodBranchInstruction((BranchHandle) ih, (BranchInstruction) inst, body, ranges, from);
+			assertGoodBranchInstruction((BranchHandle) ih, (BranchInstruction) inst, body, ranges, from, log);
 		}
 	}
 
@@ -1376,32 +1384,33 @@ public final class LazyMethodGen implements Traceable {
 		BranchInstruction inst,
 		Set body,
 		Stack ranges,
-		String from)
+		String from, StringBuffer log)
 	{
 		if (ih.getTarget() != inst.getTarget()) {
 			throw new BCException("bad branch instruction/handle pair in " + from);
 		}
 		InstructionHandle target = ih.getTarget();
-		assertInBody(target, body, from);
+		assertInBody(target, body, from, log);
 		assertTargetedBy(target, inst, from);
 		if (inst instanceof Select) {
 			Select sel = (Select) inst;
             InstructionHandle[] itargets = sel.getTargets();
             for (int k = itargets.length - 1; k >= 0; k--) { 
-				assertInBody(itargets[k], body, from);
+				assertInBody(itargets[k], body, from, log);
+				
 				assertTargetedBy(itargets[k], inst, from);
             }
         }
 	}
 
 	/** ih is an InstructionHandle or a BranchInstruction */
-	private static void assertInBody(Object ih, Set body, String from) {
+	private static void assertInBody(Object ih, Set body, String from,StringBuffer log) {
 		if (! body.contains(ih)) throw new BCException("thing not in body in " + from);
 	}
 
-    private static void assertGoodRangeHandle(InstructionHandle ih, Set body, Stack ranges, String from) {
+    private static void assertGoodRangeHandle(InstructionHandle ih, Set body, Stack ranges, String from, StringBuffer log) {
 		Range r = getRangeAndAssertExactlyOne(ih, from);
-		assertGoodRange(r, body, from);
+		assertGoodRange(r, body, from, log);
 		if (r.getStart() == ih) {
 			ranges.push(r);
 		} else if (r.getEnd() == ih) {
@@ -1410,18 +1419,23 @@ public final class LazyMethodGen implements Traceable {
 		}
     }
     
-    private static void assertGoodRange(Range r, Set body, String from) {
-		assertInBody(r.getStart(), body, from);
+    private static void assertGoodRange(Range r, Set body, String from, StringBuffer log) {
+		assertInBody(r.getStart(), body, from, log);
 		assertRangeHandle(r.getStart(), from);
 		assertTargetedBy(r.getStart(), r, from);
 
-		assertInBody(r.getEnd(), body, from);
+		assertInBody(r.getEnd(), body, from, log);
 		assertRangeHandle(r.getEnd(), from);
 		assertTargetedBy(r.getEnd(), r, from);
 		
 		if (r instanceof ExceptionRange) {
 			ExceptionRange er = (ExceptionRange) r;
-			assertInBody(er.getHandler(), body, from);
+			log.append("Checking exception range \n");
+			if (!body.contains(er.getHandler())) {
+			    log.append("Exception handler not within body\n");
+			    throw new BCException("exception handler not in body");
+			}
+//			assertInBody(er.getHandler(), body, from, log);
 			assertTargetedBy(er.getHandler(), r, from);
 		}    	
     }
@@ -1485,13 +1499,13 @@ public final class LazyMethodGen implements Traceable {
         InstructionTargeter t,
         InstructionHandle ih,
         Set body,
-        String from)
+        String from, StringBuffer log)
     {
     	assertTargets(t, ih, from);
 		if (t instanceof Range) {
-			assertGoodRange((Range) t, body, from);
+			assertGoodRange((Range) t, body, from, log);
 		} else if (t instanceof BranchInstruction) {
-			assertInBody(t, body, from);
+			assertInBody(t, body, from, log);
 		}
     }