From 102173fc11fc6648ed8f2283d3c5ad535e412c73 Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Tue, 30 Sep 2014 11:08:50 -0700 Subject: [PATCH] 445395: invokedynamic in around advice --- tests/bugs183/445395/Code.java | 15 ++++ tests/bugs183/445395/ControlFlow.java | 75 +++++++++++++++++++ tests/bugs183/445395/ControlFlowCommand.java | 7 ++ tests/bugs183/445395/ControlFlowDriver.java | 75 +++++++++++++++++++ tests/bugs183/445395/ForReplacer.java | 44 +++++++++++ .../systemtest/ajc183/Ajc183Tests.java | 8 ++ .../org/aspectj/systemtest/ajc183/ajc183.xml | 17 +++++ .../org/aspectj/weaver/bcel/BcelAdvice.java | 23 +++++- .../aspectj/weaver/bcel/BcelClassWeaver.java | 59 ++++++++++++++- .../org/aspectj/weaver/bcel/LazyClassGen.java | 9 +++ 10 files changed, 328 insertions(+), 4 deletions(-) create mode 100644 tests/bugs183/445395/Code.java create mode 100755 tests/bugs183/445395/ControlFlow.java create mode 100755 tests/bugs183/445395/ControlFlowCommand.java create mode 100755 tests/bugs183/445395/ControlFlowDriver.java create mode 100755 tests/bugs183/445395/ForReplacer.java diff --git a/tests/bugs183/445395/Code.java b/tests/bugs183/445395/Code.java new file mode 100644 index 000000000..647bc0401 --- /dev/null +++ b/tests/bugs183/445395/Code.java @@ -0,0 +1,15 @@ +import java.util.*; + +public class Code { + public void m() { } + public static void main(String []argv) { + new Code().m(); + } +} + +aspect X { + void around(): execution(* m(..)) { + Arrays.asList(4, 5, 6).forEach((i) -> { System.out.println(i);}); + } +} + diff --git a/tests/bugs183/445395/ControlFlow.java b/tests/bugs183/445395/ControlFlow.java new file mode 100755 index 000000000..3be5325f5 --- /dev/null +++ b/tests/bugs183/445395/ControlFlow.java @@ -0,0 +1,75 @@ +//package org.acmsl.pocs.lambdafor; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.function.Function; +import java.util.stream.Collectors; + +public class ControlFlow +{ + public , I, R> Collection forloop( final C items, final Function lambda) + { + return functionalForLoop(items, lambda); + } + + public , I, R> Collection functionalForLoop( final C items, final Function lambda) + { + return items.stream().map(lambda::apply).collect(Collectors.toList()); + } + + + public Collection iterativeForloop( final Collection items, final Function lambda) + { + final List result = new ArrayList<>(); + + for (final Object item: items) + { + result.add(lambda.apply(item)); + } + + return result; + } + + public , I, R> Collection externallyDrivenForloop( + final ControlFlowDriver driver, final C items, final Function lambda) + { + final List result = new ArrayList<>(items.size()); + + final List list = new ArrayList<>(items); + + int position = -1; + + while (true) + { + ControlFlowCommand command = driver.waitForCommand(); + + switch (command) + { + case NEXT: + position++; + break; + case PREVIOUS: + position++; + break; + case RELOAD: + break; + default: + break; + } + + if (position < 0) + { + position = 0; + } + else if (position > list.size() - 1) + { + break; + } + + result.set(position, lambda.apply(list.get(position))); + } + + return result; + } +} diff --git a/tests/bugs183/445395/ControlFlowCommand.java b/tests/bugs183/445395/ControlFlowCommand.java new file mode 100755 index 000000000..55c80d6cd --- /dev/null +++ b/tests/bugs183/445395/ControlFlowCommand.java @@ -0,0 +1,7 @@ +//package org.acmsl.pocs.lambdafor; + +public enum ControlFlowCommand { + NEXT, + PREVIOUS, + RELOAD; +} diff --git a/tests/bugs183/445395/ControlFlowDriver.java b/tests/bugs183/445395/ControlFlowDriver.java new file mode 100755 index 000000000..318362c79 --- /dev/null +++ b/tests/bugs183/445395/ControlFlowDriver.java @@ -0,0 +1,75 @@ +//package org.acmsl.pocs.lambdafor; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.function.Function; + +public class ControlFlowDriver { + + private static boolean m__bUsed = false; + + public ControlFlowDriver() { + } + + protected static void immutableSetUsed(final boolean used) { + m__bUsed = used; + } + + protected static void setUsed(final boolean used) { + immutableSetUsed(used); + } + + public static boolean isUsed() { + return m__bUsed; + } + + public , I, R> Collection forloop(final C items, + final Function lambda) { + setUsed(true); + + final List result = new ArrayList(items.size()); + + final List list = new ArrayList(items); + + int position = -1; + + while (true) { + ControlFlowCommand command = waitForCommand(); + + switch (command) { + case NEXT: + position++; + break; + case PREVIOUS: + position++; + break; + case RELOAD: + break; + default: + break; + } + + if (position < 0) { + position = 0; + } else if (position > list.size() - 1) { + break; + } + + result.set(position, lambda.apply(list.get(position))); + } + + return result; + } + + protected ControlFlowCommand waitForCommand() { + try { + Thread.sleep(1000); + } catch (final InterruptedException interruptedException) { + // whatever + } + + return ControlFlowCommand.NEXT; + } + +} diff --git a/tests/bugs183/445395/ForReplacer.java b/tests/bugs183/445395/ForReplacer.java new file mode 100755 index 000000000..25b7ca22a --- /dev/null +++ b/tests/bugs183/445395/ForReplacer.java @@ -0,0 +1,44 @@ +//package org.acmsl.pocs.lambdafor; + +import java.util.Arrays; + +public aspect ForReplacer +{ + /** + * Intercepting for loop constructs. + */ + pointcut forLoopPointcut(): +// execution(* Sample.sampleCode(..)); // -> error + execution(* forloop(..)); +// && args(Collect, ..); + + /** + * Replacing the loop construct. + */ + Object around() : forLoopPointcut() + { +// return proceed(); +// Collection result = new ArrayList<>(); +// result.addAll(new ControlFlow().externallyDrivenForloop(new ControlFlowDriver(), Arrays.asList(4, 5, 6), (i) -> { System.out.println(i); return i;})); +// return result; + return new ControlFlow().externallyDrivenForloop(new ControlFlowDriver(), Arrays.asList(4, 5, 6), (i) -> { System.out.println(i); return i;}); + } + + /** + * Intercepting for loop constructs. + * + pointcut forLoopPointcut(ControlFlow loop): + call(* ControlFlow.forloop(..)) + && target(loop); +// && args(items, ..); + + /** + * Replacing the loop construct. + * + Collection around(ControlFlow loop) : forLoopPointcut(loop) + { + return loop.externallyDrivenForloop(new ControlFlowDriver(), Arrays.asList(4, 5, 6), (i) -> { System.out.println(i); return i;}); +// return new ControlFlow().externallyDrivenForloop(new ControlFlowDriver(), Arrays.asList(4, 5, 6), (i) -> { System.out.println(i); return i;}); + } + */ +} diff --git a/tests/src/org/aspectj/systemtest/ajc183/Ajc183Tests.java b/tests/src/org/aspectj/systemtest/ajc183/Ajc183Tests.java index 3c0257115..ed7c74aee 100644 --- a/tests/src/org/aspectj/systemtest/ajc183/Ajc183Tests.java +++ b/tests/src/org/aspectj/systemtest/ajc183/Ajc183Tests.java @@ -21,6 +21,14 @@ import org.aspectj.testing.XMLBasedAjcTestCase; */ public class Ajc183Tests extends org.aspectj.testing.XMLBasedAjcTestCase { + public void testConstantPool_445395_0() { + runTest("constant pool 0"); + } + + public void testConstantPool_445395() { + runTest("constant pool"); + } + public void testAbstractAspectNPE_444398() { runTest("abstract aspect npe"); } diff --git a/tests/src/org/aspectj/systemtest/ajc183/ajc183.xml b/tests/src/org/aspectj/systemtest/ajc183/ajc183.xml index 35bdca396..b08be60a9 100644 --- a/tests/src/org/aspectj/systemtest/ajc183/ajc183.xml +++ b/tests/src/org/aspectj/systemtest/ajc183/ajc183.xml @@ -2,6 +2,23 @@ + + + + + + + + + + + + + + + + + diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java index ac30af5f9..668a0b2dc 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java @@ -19,12 +19,14 @@ import java.util.Collection; import java.util.Collections; import java.util.Map; +import org.aspectj.apache.bcel.Constants; import org.aspectj.apache.bcel.classfile.LocalVariable; import org.aspectj.apache.bcel.classfile.LocalVariableTable; import org.aspectj.apache.bcel.generic.InstructionConstants; import org.aspectj.apache.bcel.generic.InstructionFactory; import org.aspectj.apache.bcel.generic.InstructionHandle; import org.aspectj.apache.bcel.generic.InstructionList; +import org.aspectj.apache.bcel.generic.InvokeDynamic; import org.aspectj.apache.bcel.generic.LineNumberTag; import org.aspectj.apache.bcel.generic.LocalVariableTag; import org.aspectj.bridge.ISourceLocation; @@ -69,7 +71,8 @@ class BcelAdvice extends Advice { */ private Test runtimeTest; private ExposedState exposedState; - + private int containsInvokedynamic = 0;// 0 = dontknow, 1=no, 2=yes + public BcelAdvice(AjAttribute.AdviceAttribute attribute, Pointcut pointcut, Member adviceSignature, ResolvedType concreteAspect) { super(attribute, pointcut, simplify(attribute.getKind(), adviceSignature)); this.concreteAspect = concreteAspect; @@ -233,6 +236,24 @@ class BcelAdvice extends Advice { // Could be a symptom that the aspect failed to build last build... return the default answer of false return false; } + // Need isJava8 check + // Does the advice contain invokedynamic... + if (boType.javaClass.getMajor() == Constants.MAJOR_1_8) { + if (containsInvokedynamic == 0) { + containsInvokedynamic = 1; + LazyMethodGen lmg = boType.getLazyClassGen().getLazyMethodGen(this.signature); + InstructionList ilist = lmg.getBody(); + for (InstructionHandle src = ilist.getStart(); src != null; src = src.getNext()) { + if (src.getInstruction().opcode == Constants.INVOKEDYNAMIC) { + containsInvokedynamic = 2; + break; + } + } + } + } + if (containsInvokedynamic == 2) { + return false; + } return boType.getLazyClassGen().isWoven(); } diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java index 2c352a258..a7b35a203 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java @@ -27,6 +27,11 @@ import java.util.Properties; import java.util.Set; import org.aspectj.apache.bcel.Constants; +import org.aspectj.apache.bcel.classfile.Attribute; +import org.aspectj.apache.bcel.classfile.BootstrapMethods; +import org.aspectj.apache.bcel.classfile.ConstantInvokeDynamic; +import org.aspectj.apache.bcel.classfile.ConstantMethodHandle; +import org.aspectj.apache.bcel.classfile.ConstantMethodref; import org.aspectj.apache.bcel.classfile.ConstantPool; import org.aspectj.apache.bcel.classfile.Method; import org.aspectj.apache.bcel.classfile.annotation.AnnotationGen; @@ -42,6 +47,7 @@ import org.aspectj.apache.bcel.generic.InstructionLV; import org.aspectj.apache.bcel.generic.InstructionList; import org.aspectj.apache.bcel.generic.InstructionSelect; import org.aspectj.apache.bcel.generic.InstructionTargeter; +import org.aspectj.apache.bcel.generic.InvokeDynamic; import org.aspectj.apache.bcel.generic.InvokeInstruction; import org.aspectj.apache.bcel.generic.LineNumberTag; import org.aspectj.apache.bcel.generic.LocalVariableTag; @@ -2204,7 +2210,7 @@ class BcelClassWeaver implements IClassWeaver { ConstantPool recipientCpg = recipient.getEnclosingClass().getConstantPool(); boolean isAcrossClass = donorCpg != recipientCpg; - + BootstrapMethods bootstrapMethods = null; // first pass: copy the instructions directly, populate the srcToDest // map, // fix frame instructions @@ -2214,13 +2220,60 @@ class BcelClassWeaver implements IClassWeaver { // OPTIMIZE optimize this stuff? if (fresh.isConstantPoolInstruction()) { - // need to reset index to go to new constant pool. This is - // totally + // need to reset index to go to new constant pool. This is totally // a computation leak... we're testing this LOTS of times. Sigh. if (isAcrossClass) { InstructionCP cpi = (InstructionCP) fresh; cpi.setIndex(recipientCpg.addConstant(donorCpg.getConstant(cpi.getIndex()), donorCpg)); } + // May need to copy bootstrapmethods across too. +// if (fresh instanceof InvokeDynamic) { +// InvokeDynamic id = (InvokeDynamic)fresh; +// ConstantInvokeDynamic cid = (ConstantInvokeDynamic)donorCpg.getConstant(src.getInstruction().getIndex()); +// int bmaIndex = cid.getBootstrapMethodAttrIndex(); +// if (bootstrapMethods == null) { +// Collection attributes = donor.getEnclosingClass().getAttributes(); +// if (attributes != null) { +// for (Attribute attribute: attributes) { +// if (attribute instanceof BootstrapMethods) { +// bootstrapMethods = (BootstrapMethods)attribute; +// } +// } +// } +// BootstrapMethods.BootstrapMethod bootstrapMethod = +// bootstrapMethods.getBootstrapMethods()[bmaIndex]; +// ConstantMethodHandle methodhandle = (ConstantMethodHandle)donorCpg.getConstant(bootstrapMethod.getBootstrapMethodRef()); +// int bootstrapMethodArguments[] = bootstrapMethod.getBootstrapArguments(); +// +// // Finally have all we need to build the new one... +// +// int newMethodHandleIndex = recipientCpg.addConstant(methodhandle, donorCpg); +// int[] newMethodArguments = new int[bootstrapMethodArguments.length]; +// for (int a=0; a newAttributes = recipient.getEnclosingClass().getAttributes(); +// BootstrapMethods newBootstrapMethods = null; +// for (Attribute attr: newAttributes) { +// if (attr instanceof BootstrapMethods) { +// newBootstrapMethods = (BootstrapMethods)newBootstrapMethods; +// } +// } +// if (newBootstrapMethods == null) { +// newBootstrapMethods = +// new BootstrapMethods(recipientCpg.addUtf8("BootstrapMethods"), +// 2+newBootstrapMethod.getLength(), +// new BootstrapMethods.BootstrapMethod[] {newBootstrapMethod}, +// recipientCpg); +// recipient.getEnclosingClass().addAttribute(newBootstrapMethods); +// } +// TODO need to copy over lambda$0 support methods too... +// } +// +// } } if (src.getInstruction() == Range.RANGEINSTRUCTION) { dest = ret.append(Range.RANGEINSTRUCTION); diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java index 62ebac92c..8e2b09867 100644 --- a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java +++ b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.io.PrintStream; import java.lang.reflect.Modifier; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -1582,6 +1583,14 @@ public final class LazyClassGen { public void addAttribute(AjAttribute attribute) { myGen.addAttribute(Utility.bcelAttribute(attribute, getConstantPool())); } + + public void addAttribute(Attribute attribute) { + myGen.addAttribute(attribute); + } + + public Collection getAttributes() { + return myGen.getAttributes(); + } // this test is like asking: // if -- 2.39.5