Bug 104212

Summary: static method call from subclass signature is wrong
Product: [Tools] AspectJ Reporter: Alexandre Vasseur <avasseur>
Component: CompilerAssignee: Alexandre Vasseur <avasseur>
Status: RESOLVED FIXED QA Contact:
Severity: blocker    
Priority: P1    
Version: DEVELOPMENT   
Target Milestone: 1.5.0 M3   
Hardware: All   
OS: All   
Whiteboard:

Description Alexandre Vasseur CLA 2005-07-18 09:25:01 EDT
a very bad bug... or ?

in the snip below, getMethod() says null and the factory is actually thinking
that test() is a static method of AspectJBugMain instead of Assert...

wondering why we don't catch that in the test suite or what could happen
recently around that. Or is it something I am confused about ? (i doubt a
jp.getSignature().getMethod is supposed to return null in some cases though..)

@Aspect
public class Sam {

    @Pointcut("call(* *.*(..))")
            public void methodCalls() {
    }

    @Around("methodCalls() && !within(alex.sam.Sam) && within(alex..*)")
            public Object aroundMethodCalls(ProceedingJoinPoint jp) throws
Throwable {
        String typeName = jp.getSignature().getDeclaringTypeName();
        System.out.println("declType " + typeName);
        System.out.println("method " +
((MethodSignature)jp.getSignature()).getMethod());

        return jp.proceed();
    }

}

class Assert {
    public static void test() {
        System.out.println("RUN Assert.test");
    }
}

class AspectJBugMain extends Assert {
    public static void main(String[] args) {
        test();
    }
//    public static void test() {
//        System.out.println("RUN AspectJBugMain.test");
//    }
}
Comment 1 Alexandre Vasseur CLA 2005-07-18 09:30:49 EDT
don't know why but javap says:

invoke static #2

const #2 = Method       #3.#19; //  alex/sam/AspectJBugMain.test:()V

why is that AspectJBugMain there ?
(1.5_3, both Sun  and  Jrockit )
Comment 2 Adrian Colyer CLA 2005-07-18 10:31:03 EDT
javap shows the same thing under jdk 1.4 too.

There was a change from 1.3 to 1.4 in the bytecodes generated for invoke calls.
In the 1.3 spec, the spec. said you generate a call to the root defining type of
the method. In 1.4 compilers were required to preserve the target type of the
call. It looks very odd to see this for static methods too, but that seems to be
what is happening. 

The getMethod() method was added to MethodSignature only as part of the
annotation work done in M2 (so comparatively recent for the aspectj runtime
library). Without looking, my guess is that it is not walking up the hierarchy
correctly to find the method in this case. I don't think
MethodSignature.getMethod() should ever return null so that's definitely a bug!
Comment 3 Alexandre Vasseur CLA 2005-07-18 10:38:46 EDT
note that also means toShort/LongString provides a wrong information as well
(this even in old version)
Comment 4 Alexandre Vasseur CLA 2005-07-18 11:42:26 EDT
given what you describe, should the toShort/LongString give the name of the base
class or not (currently: not, it has the name of the class that is in the
bytecode from the compiler)?

I think we need to correct class name instead, ie that the issue is not in
runtime/ but in the shadow signature creation down in the weaver/

thoughts ?
Comment 5 Alexandre Vasseur CLA 2005-07-22 10:01:50 EDT
i have changed the bcelshadow when it gets created

odd that we were having a match - there might be duplicate logic now somewhere
Comment 6 Alexandre Vasseur CLA 2005-07-22 10:58:49 EDT
fixed
close when ship
Comment 7 Alexandre Vasseur CLA 2005-09-27 09:27:29 EDT
was M3 remind
Comment 8 Alexandre Vasseur CLA 2005-09-27 09:28:40 EDT
was M3 remind