From aa68099bb252dd1a1c275459f8babe537868bcaf Mon Sep 17 00:00:00 2001 From: jeff Date: Sun, 14 Sep 2014 19:06:55 -0400 Subject: fixed bug with method references pointing to wrong class --- src/cuchaz/enigma/analysis/JarIndex.java | 120 ++++++++++++++------- .../cuchaz/enigma/TestJarIndexInheritanceTree.java | 3 +- .../enigma/inputs/inheritanceTree/BaseClass.java | 5 + .../enigma/inputs/inheritanceTree/SubclassA.java | 3 + .../enigma/inputs/inheritanceTree/SubclassB.java | 11 +- .../inputs/inheritanceTree/SubsubclassAA.java | 6 ++ 6 files changed, 107 insertions(+), 41 deletions(-) diff --git a/src/cuchaz/enigma/analysis/JarIndex.java b/src/cuchaz/enigma/analysis/JarIndex.java index b4096e9d..f4843164 100644 --- a/src/cuchaz/enigma/analysis/JarIndex.java +++ b/src/cuchaz/enigma/analysis/JarIndex.java @@ -127,7 +127,7 @@ public class JarIndex } for( CtField field : c.getDeclaredFields() ) { - indexField( field ); + m_translationIndex.addField( className, field.getName() ); } for( CtBehavior behavior : c.getDeclaredBehaviors() ) { @@ -135,9 +135,19 @@ public class JarIndex } } + // step 4: index field, method, constructor references + for( CtClass c : JarClassIterator.classes( jar ) ) + { + ClassRenamer.moveAllClassesOutOfDefaultPackage( c, Constants.NonePackage ); + for( CtBehavior behavior : c.getDeclaredBehaviors() ) + { + indexBehaviorReferences( behavior ); + } + } + if( buildInnerClasses ) { - // step 4: index inner classes and anonymous classes + // step 5: index inner classes and anonymous classes for( CtClass c : JarClassIterator.classes( jar ) ) { ClassRenamer.moveAllClassesOutOfDefaultPackage( c, Constants.NonePackage ); @@ -163,7 +173,7 @@ public class JarIndex } } - // step 5: update other indices with inner class info + // step 6: update other indices with inner class info Map renames = Maps.newHashMap(); for( Map.Entry entry : m_outerClasses.entrySet() ) { @@ -183,25 +193,14 @@ public class JarIndex EntryRenamer.renameMethodsInMultimap( m_bridgeMethods, m_fieldReferences ); } - private void indexField( CtField field ) - { - String className = Descriptor.toJvmName( field.getDeclaringClass().getName() ); - m_translationIndex.addField( className, field.getName() ); - } - private void indexBehavior( CtBehavior behavior ) { - // get the method entry + // get the behavior entry String className = Descriptor.toJvmName( behavior.getDeclaringClass().getName() ); - final BehaviorEntry thisEntry; - if( behavior instanceof CtMethod ) + final BehaviorEntry behaviorEntry = getBehaviorEntry( behavior ); + if( behaviorEntry instanceof MethodEntry ) { - MethodEntry methodEntry = new MethodEntry( - new ClassEntry( className ), - behavior.getName(), - behavior.getSignature() - ); - thisEntry = methodEntry; + MethodEntry methodEntry = (MethodEntry)behaviorEntry; // index implementation m_methodImplementations.put( className, methodEntry ); @@ -218,24 +217,13 @@ public class JarIndex m_bridgeMethods.put( bridgedMethodEntry, methodEntry ); } } - else if( behavior instanceof CtConstructor ) - { - boolean isStatic = behavior.getName().equals( "" ); - if( isStatic ) - { - thisEntry = new ConstructorEntry( new ClassEntry( className ) ); - } - else - { - thisEntry = new ConstructorEntry( new ClassEntry( className ), behavior.getSignature() ); - } - } - else - { - throw new IllegalArgumentException( "behavior must be a method or a constructor!" ); - } - + // looks like we don't care about constructors here + } + + private void indexBehaviorReferences( CtBehavior behavior ) + { // index method calls + final BehaviorEntry behaviorEntry = getBehaviorEntry( behavior ); try { behavior.instrument( new ExprEditor( ) @@ -249,9 +237,10 @@ public class JarIndex call.getMethodName(), call.getSignature() ); + calledMethodEntry = resolveMethodClass( calledMethodEntry ); EntryReference reference = new EntryReference( calledMethodEntry, - thisEntry + behaviorEntry ); m_behaviorReferences.put( calledMethodEntry, reference ); } @@ -266,7 +255,7 @@ public class JarIndex ); EntryReference reference = new EntryReference( calledFieldEntry, - thisEntry + behaviorEntry ); m_fieldReferences.put( calledFieldEntry, reference ); } @@ -284,7 +273,7 @@ public class JarIndex ); EntryReference reference = new EntryReference( calledConstructorEntry, - thisEntry + behaviorEntry ); m_behaviorReferences.put( calledConstructorEntry, reference ); } @@ -299,7 +288,7 @@ public class JarIndex ); EntryReference reference = new EntryReference( calledConstructorEntry, - thisEntry + behaviorEntry ); m_behaviorReferences.put( calledConstructorEntry, reference ); } @@ -311,6 +300,59 @@ public class JarIndex } } + private BehaviorEntry getBehaviorEntry( CtBehavior behavior ) + { + String className = Descriptor.toJvmName( behavior.getDeclaringClass().getName() ); + if( behavior instanceof CtMethod ) + { + return new MethodEntry( + new ClassEntry( className ), + behavior.getName(), + behavior.getSignature() + ); + } + else if( behavior instanceof CtConstructor ) + { + boolean isStatic = behavior.getName().equals( "" ); + if( isStatic ) + { + return new ConstructorEntry( new ClassEntry( className ) ); + } + else + { + return new ConstructorEntry( new ClassEntry( className ), behavior.getSignature() ); + } + } + else + { + throw new IllegalArgumentException( "behavior must be a method or a constructor!" ); + } + } + + private MethodEntry resolveMethodClass( MethodEntry methodEntry ) + { + // this entry could refer to a method on a class where the method is not actually implemented + // travel up the inheritance tree to find the closest implementation + while( !isMethodImplemented( methodEntry ) ) + { + // is there a parent class? + String superclassName = m_translationIndex.getSuperclassName( methodEntry.getClassName() ); + if( superclassName == null ) + { + // this is probably a method from a class in a library + // we can't trace the implementation up any higher unless we index the library + return methodEntry; + } + + // move up to the parent class + methodEntry = new MethodEntry( + new ClassEntry( superclassName ), + methodEntry.getName(), + methodEntry.getSignature() + ); + } + return methodEntry; + } private CtMethod getBridgedMethod( CtMethod method ) { diff --git a/test/cuchaz/enigma/TestJarIndexInheritanceTree.java b/test/cuchaz/enigma/TestJarIndexInheritanceTree.java index 5ded5df0..3d488ee4 100644 --- a/test/cuchaz/enigma/TestJarIndexInheritanceTree.java +++ b/test/cuchaz/enigma/TestJarIndexInheritanceTree.java @@ -220,7 +220,8 @@ public class TestJarIndexInheritanceTree source = new MethodEntry( m_baseClass, "a", "()Ljava/lang/String;" ); references = m_index.getBehaviorReferences( source ); assertThat( references, containsInAnyOrder( - newBehaviorReferenceByMethod( source, m_subClassAA.getName(), "a", "()Ljava/lang/String;" ) + newBehaviorReferenceByMethod( source, m_subClassAA.getName(), "a", "()Ljava/lang/String;" ), + newBehaviorReferenceByMethod( source, m_subClassB.getName(), "a", "()V" ) ) ); // subclassAA.getName() diff --git a/test/cuchaz/enigma/inputs/inheritanceTree/BaseClass.java b/test/cuchaz/enigma/inputs/inheritanceTree/BaseClass.java index a6b38454..8402dde3 100644 --- a/test/cuchaz/enigma/inputs/inheritanceTree/BaseClass.java +++ b/test/cuchaz/enigma/inputs/inheritanceTree/BaseClass.java @@ -1,18 +1,23 @@ package cuchaz.enigma.inputs.inheritanceTree; +// none/a public abstract class BaseClass { + // a private String m_name; + // (Ljava/lang/String;)V protected BaseClass( String name ) { m_name = name; } + // a()Ljava/lang/String; public String getName( ) { return m_name; } + // a()V public abstract void doBaseThings( ); } diff --git a/test/cuchaz/enigma/inputs/inheritanceTree/SubclassA.java b/test/cuchaz/enigma/inputs/inheritanceTree/SubclassA.java index f4780a26..ed507093 100644 --- a/test/cuchaz/enigma/inputs/inheritanceTree/SubclassA.java +++ b/test/cuchaz/enigma/inputs/inheritanceTree/SubclassA.java @@ -1,9 +1,12 @@ package cuchaz.enigma.inputs.inheritanceTree; +// none/b extends none/a public abstract class SubclassA extends BaseClass { + // (Ljava/lang/String;)V protected SubclassA( String name ) { + // call to none/a.(Ljava/lang/String)V super( name ); } } diff --git a/test/cuchaz/enigma/inputs/inheritanceTree/SubclassB.java b/test/cuchaz/enigma/inputs/inheritanceTree/SubclassB.java index 4001e7a3..fc4c8eed 100644 --- a/test/cuchaz/enigma/inputs/inheritanceTree/SubclassB.java +++ b/test/cuchaz/enigma/inputs/inheritanceTree/SubclassB.java @@ -1,24 +1,33 @@ package cuchaz.enigma.inputs.inheritanceTree; +// none/c extends none/a public class SubclassB extends BaseClass { + // a private int m_numThings; + // ()V protected SubclassB( ) { + // none/a.(Ljava/lang/String;)V super( "B" ); + // access to a m_numThings = 4; } @Override + // a()V public void doBaseThings( ) { - System.out.println( "Base things by B!" ); + // call to none/a.a()Ljava/lang/String; + System.out.println( "Base things by B! " + getName() ); } + // b()V public void doBThings( ) { + // access to a System.out.println( "" + m_numThings + " B things!" ); } } diff --git a/test/cuchaz/enigma/inputs/inheritanceTree/SubsubclassAA.java b/test/cuchaz/enigma/inputs/inheritanceTree/SubsubclassAA.java index 11196d16..b3b83429 100644 --- a/test/cuchaz/enigma/inputs/inheritanceTree/SubsubclassAA.java +++ b/test/cuchaz/enigma/inputs/inheritanceTree/SubsubclassAA.java @@ -1,21 +1,27 @@ package cuchaz.enigma.inputs.inheritanceTree; +// none/d extends none/b public class SubsubclassAA extends SubclassA { protected SubsubclassAA( ) { + // call to none/b.(Ljava/lang/String;)V super( "AA" ); } @Override + // a()Ljava/lang/String; public String getName( ) { + // call to none/b.a()Ljava/lang/String; return "subsub" + super.getName(); } @Override + // a()V public void doBaseThings( ) { + // call to none/d.a()Ljava/lang/String; System.out.println( "Base things by " + getName() ); } } -- cgit v1.2.3