From 851da68a07bcbfac4414fadc1b9f3bc02fa810a5 Mon Sep 17 00:00:00 2001 From: aclement Date: Mon, 9 Aug 2004 10:26:53 +0000 Subject: [PATCH] Fix for Bug 71377: Cannot advise private method call in around advice --- .../compiler/ast/AspectDeclaration.java | 46 +++++++++++--- .../FieldGetJoinPointsInAroundAdvice.java | 62 +++++++++++++++++++ .../FieldJoinPointsInAroundAdvice.java | 62 +++++++++++++++++++ .../JoinPointInAroundAdvice.java | 44 +++++++++++++ .../systemtest/ajc121/Ajc121Tests.java | 17 ++++- .../systemtest/ajc121/ajc121-tests.xml | 19 +++++- .../org/aspectj/weaver/bcel/LazyClassGen.java | 17 ++++- 7 files changed, 257 insertions(+), 10 deletions(-) create mode 100644 tests/bugs/AroundAdviceJPs/FieldGetJoinPointsInAroundAdvice.java create mode 100644 tests/bugs/AroundAdviceJPs/FieldJoinPointsInAroundAdvice.java create mode 100644 tests/bugs/AroundAdviceJPs/JoinPointInAroundAdvice.java diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java index 432e4f76c..df0c5aeeb 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java @@ -288,10 +288,38 @@ public class AspectDeclaration extends TypeDeclaration { } private void generateMethod(ClassFile classFile, MethodBinding methodBinding, BodyGenerator gen) { + generateMethod(classFile,methodBinding,null,gen); + } + + protected List makeEffectiveSignatureAttribute(ResolvedMember sig,Shadow.Kind kind,boolean weaveBody) { + List l = new ArrayList(1); + l.add(new EclipseAttributeAdapter( + new AjAttribute.EffectiveSignatureAttribute(sig, kind, weaveBody))); + return l; + } + + /* + * additionalAttributes allows us to pass some optional attributes we want to attach to the method we generate. + * Currently this is used for inline accessor methods that have been generated to allow private field references or + * private method calls to be inlined (PR71377). In these cases the optional attribute is an effective signature + * attribute which means calls to these methods are able to masquerade as any join point (a field set, field get or + * method call). The effective signature attribute is 'unwrapped' in BcelClassWeaver.matchInvokeInstruction() + */ + private void generateMethod(ClassFile classFile, MethodBinding methodBinding, List additionalAttributes/*ResolvedMember realMember*/, BodyGenerator gen) { // EclipseFactory world = EclipseFactory.fromScopeLookupEnvironment(this.scope); classFile.generateMethodInfoHeader(methodBinding); int methodAttributeOffset = classFile.contentsOffset; - int attributeNumber = classFile.generateMethodInfoAttribute(methodBinding, AstUtil.getAjSyntheticAttribute()); + + int attributeNumber; + if (additionalAttributes!=null) { // mini optimization + List attrs = new ArrayList(); + attrs.addAll(AstUtil.getAjSyntheticAttribute()); + attrs.addAll(additionalAttributes); + attributeNumber = classFile.generateMethodInfoAttribute(methodBinding, attrs); + } else { + attributeNumber = classFile.generateMethodInfoAttribute(methodBinding, AstUtil.getAjSyntheticAttribute()); + } + int codeAttributeOffset = classFile.contentsOffset; classFile.generateCodeAttributeHeader(); CodeStream codeStream = classFile.codeStream; @@ -644,9 +672,11 @@ public class AspectDeclaration extends TypeDeclaration { } private void generateInlineAccessors(ClassFile classFile, final InlineAccessFieldBinding accessField, final ResolvedMember field) { - final FieldBinding fieldBinding = factory.makeFieldBinding(field); - generateMethod(classFile, accessField.reader, - new BodyGenerator() { + final FieldBinding fieldBinding = factory.makeFieldBinding(field); + + generateMethod(classFile, accessField.reader, + makeEffectiveSignatureAttribute(field,Shadow.FieldGet,false), + new BodyGenerator() { public void generate(CodeStream codeStream) { // body starts here if (field.isStatic()) { @@ -661,7 +691,8 @@ public class AspectDeclaration extends TypeDeclaration { }}); generateMethod(classFile, accessField.writer, - new BodyGenerator() { + makeEffectiveSignatureAttribute(field,Shadow.FieldSet,false), + new BodyGenerator() { public void generate(CodeStream codeStream) { // body starts here if (field.isStatic()) { @@ -681,8 +712,9 @@ public class AspectDeclaration extends TypeDeclaration { private void generateInlineAccessMethod(ClassFile classFile, final MethodBinding accessMethod, final ResolvedMember method) { - generateMethod(classFile, accessMethod, - new BodyGenerator() { + generateMethod(classFile, accessMethod, + makeEffectiveSignatureAttribute(method, Shadow.MethodCall, false), + new BodyGenerator() { public void generate(CodeStream codeStream) { // body starts here diff --git a/tests/bugs/AroundAdviceJPs/FieldGetJoinPointsInAroundAdvice.java b/tests/bugs/AroundAdviceJPs/FieldGetJoinPointsInAroundAdvice.java new file mode 100644 index 000000000..950cc1604 --- /dev/null +++ b/tests/bugs/AroundAdviceJPs/FieldGetJoinPointsInAroundAdvice.java @@ -0,0 +1,62 @@ +import java.util.*; + +public aspect FieldGetJoinPointsInAroundAdvice { + + private static int secretField1; + private int secretField2; + public static int nonsecretField3; + public int nonsecretField4; + + + static int privateNonstaticFieldGets = 0; + static int privateStaticFieldGets = 0; + static int publicNonstaticFieldGets = 0; + static int publicStaticFieldGets = 0; + + before () : cflow(adviceexecution()) && get(private !static * *secret*) { privateNonstaticFieldGets++; tjps.add(thisJoinPoint.getSourceLocation());} + before () : cflow(adviceexecution()) && get(private static * *secret*) { privateStaticFieldGets++;} + before () : cflow(adviceexecution()) && get(public !static * *secret*) { publicNonstaticFieldGets++;} + before () : cflow(adviceexecution()) && get(public static * *secret*) { publicStaticFieldGets++;} + + pointcut execTest () : execution(* FieldGetJoinPointsInAroundAdvice.test()); + + before () : execTest() { + int i = secretField1; + i=secretField2; + i=nonsecretField3; + i=nonsecretField4; + } + + void around () : execTest() { + int i=secretField1; + i=secretField2; + i=nonsecretField3; + i=nonsecretField4; + proceed(); + } + + after () : execTest () { + int i=secretField1; + i=secretField2; + i=nonsecretField3; + i=nonsecretField4; + } + + private static List tjps = new ArrayList(); + + public static void test () { + System.out.println("? test()"); + } + + public static void main (String[] args) { + test(); + if (privateNonstaticFieldGets!=privateStaticFieldGets || + privateStaticFieldGets!=publicStaticFieldGets || + publicStaticFieldGets!=publicNonstaticFieldGets) throw new RuntimeException( + "\n privateNonstaticFieldGets="+privateNonstaticFieldGets+ + "\n publicNonstaticFieldGets="+publicNonstaticFieldGets+ + "\n privateStaticFieldGets="+privateStaticFieldGets+ + "\n publicStaticFieldGets="+publicStaticFieldGets); + //System.err.println(tjps); + } +} \ No newline at end of file diff --git a/tests/bugs/AroundAdviceJPs/FieldJoinPointsInAroundAdvice.java b/tests/bugs/AroundAdviceJPs/FieldJoinPointsInAroundAdvice.java new file mode 100644 index 000000000..7d92d1322 --- /dev/null +++ b/tests/bugs/AroundAdviceJPs/FieldJoinPointsInAroundAdvice.java @@ -0,0 +1,62 @@ +import java.util.*; + +public aspect FieldJoinPointsInAroundAdvice { + + private static int secretField1; + private int secretField2; + public static int nonsecretField3; + public int nonsecretField4; + + + static int privateNonstaticFieldSets = 0; + static int privateStaticFieldSets = 0; + static int publicNonstaticFieldSets = 0; + static int publicStaticFieldSets = 0; + + before () : cflow(adviceexecution()) && set(private !static * *secret*) { privateNonstaticFieldSets++; tjps.add(thisJoinPoint.getSourceLocation());} + before () : cflow(adviceexecution()) && set(private static * *secret*) { privateStaticFieldSets++; tjps.add(thisJoinPoint.getSourceLocation());} + before () : cflow(adviceexecution()) && set(public !static * *secret*) { publicNonstaticFieldSets++;} + before () : cflow(adviceexecution()) && set(public static * *secret*) { publicStaticFieldSets++;} + + pointcut execTest () : execution(* FieldJoinPointsInAroundAdvice.test()); + + before () : execTest() { + secretField1++; + secretField2++; + nonsecretField3++; + nonsecretField4++; + } + + void around () : execTest() { + secretField1++; + secretField2++; + nonsecretField3++; + nonsecretField4++; + proceed(); + } + + after () : execTest () { + secretField1++; + secretField2++; + nonsecretField3++; + nonsecretField4++; + } + + private static List tjps = new ArrayList(); + + public static void test () { + System.out.println("? test()"); + } + + public static void main (String[] args) { + test(); + if (privateNonstaticFieldSets!=privateStaticFieldSets || + privateStaticFieldSets!=publicStaticFieldSets || + publicStaticFieldSets!=publicNonstaticFieldSets) throw new RuntimeException( + "\n privateNonstaticFieldSets="+privateNonstaticFieldSets+ + "\n publicNonstaticFieldSets="+publicNonstaticFieldSets+ + "\n privateStaticFieldSets="+privateStaticFieldSets+ + "\n publicStaticFieldSets="+publicStaticFieldSets); + //System.err.println(tjps); + } +} \ No newline at end of file diff --git a/tests/bugs/AroundAdviceJPs/JoinPointInAroundAdvice.java b/tests/bugs/AroundAdviceJPs/JoinPointInAroundAdvice.java new file mode 100644 index 000000000..db8ba6b24 --- /dev/null +++ b/tests/bugs/AroundAdviceJPs/JoinPointInAroundAdvice.java @@ -0,0 +1,44 @@ +import java.util.*; + +public aspect JoinPointInAroundAdvice { + + static int i = 0; + static int j = 0; + + before () : call(* JoinPointInAroundAdvice.privateMethod(..)) { i++; tjps.add(thisJoinPoint.getSourceLocation());} + before () : call(* JoinPointInAroundAdvice.publicMethod(..)) { j++;} + + pointcut execTest () : execution(* JoinPointInAroundAdvice.test()); + + before () : execTest() { + privateMethod("before"); + publicMethod("before"); + } + + void around () : execTest() { + privateMethod("around"); + publicMethod("around"); + proceed(); + } + + after () : execTest () { + privateMethod("after"); + publicMethod("after"); + } + + private static List tjps = new ArrayList(); + + private static void privateMethod(String from) { }//System.out.println("? privateMethod() " + from); } + public static void publicMethod(String from) { }//System.out.println("? publicMethod() " + from); } + + public static void test () { + System.out.println("? test()"); + } + + public static void main (String[] args) { + test(); + if (i!=j || i!=3) throw new RuntimeException("Missing join point: private="+i+" public="+j); + //System.err.println(tjps); + } +} + diff --git a/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java b/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java index abaf4d5d5..32eb250b9 100644 --- a/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java +++ b/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java @@ -25,7 +25,7 @@ public class Ajc121Tests extends org.aspectj.testing.XMLBasedAjcTestCase { public void test001(){ - runTest("false ambigous binding error (introduced in 1.2rc2)"); + runTest("false ambiguous binding error (introduced in 1.2rc2)"); } public void test002(){ @@ -99,5 +99,20 @@ public class Ajc121Tests extends org.aspectj.testing.XMLBasedAjcTestCase { assertTrue("Expected to find [s32767] in this output but didn't:"+output,output.indexOf("[s32767]")!=-1); assertTrue("Expected to find [b0] in this output but didn't:"+output,output.indexOf("[b0]")!=-1); } + + public void test017_PrivateMethodCallsInAroundAdvice() { + runTest("Cannot advise private method call in around advice"); + System.err.println(getLastRunResult().getStdErr()); + } + + public void test018_PrivateFieldSetsInAroundAdvice() { + runTest("Cannot advise private field sets in around advice"); + System.err.println(getLastRunResult().getStdErr()); + } + + public void test019_PrivateFieldGetsInAroundAdvice() { + runTest("Cannot advise private field gets in around advice"); + System.err.println(getLastRunResult().getStdErr()); + } } diff --git a/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml b/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml index 02d1acb97..8d2f21453 100644 --- a/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml +++ b/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml @@ -2,7 +2,7 @@ + pr="62073" title="false ambiguous binding error (introduced in 1.2rc2)"> @@ -138,3 +138,20 @@ + + + + + + + + + + + + + + \ No newline at end of file diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java index b25d10aa6..3185300fa 100644 --- a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java +++ b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java @@ -645,7 +645,22 @@ public final class LazyClassGen { if (ret != null) return ret; int modifiers = Modifier.STATIC | Modifier.FINAL; - if (getType().isInterface()) { + + // XXX - Do we ever inline before or after advice? If we do, then we + // better include them in the check below. (or just change it to + // shadow.getEnclosingMethod().getCanInline()) + + // If the enclosing method is around advice, we could inline the join point + // that has led to this shadow. If we do that then the TJP we are creating + // here must be PUBLIC so it is visible to the type in which the + // advice is inlined. (PR71377) + LazyMethodGen encMethod = shadow.getEnclosingMethod(); + boolean shadowIsInAroundAdvice = false; + if (encMethod!=null && encMethod.getName().startsWith(NameMangler.PREFIX+"around")) { + shadowIsInAroundAdvice = true; + } + + if (getType().isInterface() || shadowIsInAroundAdvice) { modifiers |= Modifier.PUBLIC; } else { -- 2.39.5