Check protected method/field target in visibility index (#157)

6 files changed, 178 insertions, 55 deletions

diff --git a/src/main/java/cuchaz/enigma/analysis/index/EntryIndex.java b/src/main/java/cuchaz/enigma/analysis/index/EntryIndex.java
index 31c6f54..9a2726e 100644
--- a/src/main/java/cuchaz/enigma/analysis/index/EntryIndex.java
+++ b/src/main/java/cuchaz/enigma/analysis/index/EntryIndex.java
@@ -12,9 +12,11 @@ public class EntryIndex implements JarIndexer {
         private Map<ClassEntry, AccessFlags> classes = new HashMap<>();
         private Map<FieldEntry, AccessFlags> fields = new HashMap<>();
         private Map<MethodEntry, AccessFlags> methods = new HashMap<>();
+        private Map<ClassEntry, ClassDefEntry> definitions = new HashMap<>();
 
         @Override
         public void indexClass(ClassDefEntry classEntry) {
+                definitions.put(classEntry, classEntry);
                 classes.put(classEntry, classEntry.getAccess());
         }
 
@@ -82,6 +84,10 @@ public class EntryIndex implements JarIndexer {
                 return null;
         }
 
+        public ClassDefEntry getDefinition(ClassEntry entry) {
+                return definitions.get(entry);
+        }
+
         public Collection<ClassEntry> getClasses() {
                 return classes.keySet();
         }
diff --git a/src/main/java/cuchaz/enigma/analysis/index/IndexReferenceVisitor.java b/src/main/java/cuchaz/enigma/analysis/index/IndexReferenceVisitor.java
index b730a8a..f3d419e 100644
--- a/src/main/java/cuchaz/enigma/analysis/index/IndexReferenceVisitor.java
+++ b/src/main/java/cuchaz/enigma/analysis/index/IndexReferenceVisitor.java
@@ -1,52 +1,162 @@
 package cuchaz.enigma.analysis.index;
 
+import cuchaz.enigma.analysis.IndexSimpleVerifier;
+import cuchaz.enigma.analysis.InterpreterPair;
+import cuchaz.enigma.analysis.MethodNodeWithAction;
+import cuchaz.enigma.analysis.ReferenceTargetType;
 import cuchaz.enigma.translation.representation.AccessFlags;
 import cuchaz.enigma.translation.representation.Lambda;
 import cuchaz.enigma.translation.representation.MethodDescriptor;
 import cuchaz.enigma.translation.representation.Signature;
 import cuchaz.enigma.translation.representation.entry.*;
 import org.objectweb.asm.*;
+import org.objectweb.asm.tree.AbstractInsnNode;
+import org.objectweb.asm.tree.FieldInsnNode;
+import org.objectweb.asm.tree.InvokeDynamicInsnNode;
+import org.objectweb.asm.tree.MethodInsnNode;
+import org.objectweb.asm.tree.analysis.*;
+
+import java.util.List;
+import java.util.stream.Collectors;
 
 public class IndexReferenceVisitor extends ClassVisitor {
         private final JarIndexer indexer;
+        private final EntryIndex entryIndex;
+        private final InheritanceIndex inheritanceIndex;
         private ClassEntry classEntry;
+        private String className;
 
-        public IndexReferenceVisitor(JarIndexer indexer, int api) {
+        public IndexReferenceVisitor(JarIndexer indexer, EntryIndex entryIndex, InheritanceIndex inheritanceIndex, int api) {
                 super(api);
                 this.indexer = indexer;
+                this.entryIndex = entryIndex;
+                this.inheritanceIndex = inheritanceIndex;
         }
 
         @Override
         public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) {
-                this.classEntry = new ClassEntry(name);
+                classEntry = new ClassEntry(name);
+                className = name;
         }
 
         @Override
         public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
                 MethodDefEntry entry = new MethodDefEntry(classEntry, name, new MethodDescriptor(desc), Signature.createSignature(signature), new AccessFlags(access));
-                return new Method(this.indexer, entry, this.api);
+                return new MethodNodeWithAction(api, access, name, desc, signature, exceptions, methodNode -> {
+                        try {
+                                new Analyzer<>(new MethodInterpreter(entry, indexer, entryIndex, inheritanceIndex)).analyze(className, methodNode);
+                        } catch (AnalyzerException e) {
+                                throw new RuntimeException(e);
+                        }
+                });
         }
 
-        private static class Method extends MethodVisitor {
-                private final JarIndexer indexer;
+        private static class MethodInterpreter extends InterpreterPair<BasicValue, SourceValue> {
                 private final MethodDefEntry callerEntry;
+                private JarIndexer indexer;
 
-                public Method(JarIndexer indexer, MethodDefEntry callerEntry, int api) {
-                        super(api);
-                        this.indexer = indexer;
+                public MethodInterpreter(MethodDefEntry callerEntry, JarIndexer indexer, EntryIndex entryIndex, InheritanceIndex inheritanceIndex) {
+                        super(new IndexSimpleVerifier(entryIndex, inheritanceIndex), new SourceInterpreter());
                         this.callerEntry = callerEntry;
+                        this.indexer = indexer;
                 }
 
                 @Override
-                public void visitFieldInsn(int opcode, String owner, String name, String desc) {
-                        FieldEntry fieldEntry = FieldEntry.parse(owner, name, desc);
-                        this.indexer.indexFieldReference(callerEntry, fieldEntry);
+                public PairValue<BasicValue, SourceValue> newOperation(AbstractInsnNode insn) throws AnalyzerException {
+                        if (insn.getOpcode() == Opcodes.GETSTATIC) {
+                                FieldInsnNode field = (FieldInsnNode) insn;
+                                indexer.indexFieldReference(callerEntry, FieldEntry.parse(field.owner, field.name, field.desc), ReferenceTargetType.none());
+                        }
+
+                        return super.newOperation(insn);
                 }
 
                 @Override
-                public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
-                        MethodEntry methodEntry = MethodEntry.parse(owner, name, desc);
-                        this.indexer.indexMethodReference(callerEntry, methodEntry);
+                public PairValue<BasicValue, SourceValue> unaryOperation(AbstractInsnNode insn, PairValue<BasicValue, SourceValue> value) throws AnalyzerException {
+                        if (insn.getOpcode() == Opcodes.PUTSTATIC) {
+                                FieldInsnNode field = (FieldInsnNode) insn;
+                                indexer.indexFieldReference(callerEntry, FieldEntry.parse(field.owner, field.name, field.desc), ReferenceTargetType.none());
+                        }
+
+                        if (insn.getOpcode() == Opcodes.GETFIELD) {
+                                FieldInsnNode field = (FieldInsnNode) insn;
+                                indexer.indexFieldReference(callerEntry, FieldEntry.parse(field.owner, field.name, field.desc), getReferenceTargetType(value, insn));
+                        }
+
+                        return super.unaryOperation(insn, value);
+                }
+
+
+                @Override
+                public PairValue<BasicValue, SourceValue> binaryOperation(AbstractInsnNode insn, PairValue<BasicValue, SourceValue> value1, PairValue<BasicValue, SourceValue> value2) throws AnalyzerException {
+                        if (insn.getOpcode() == Opcodes.PUTFIELD) {
+                                FieldInsnNode field = (FieldInsnNode) insn;
+                                FieldEntry fieldEntry = FieldEntry.parse(field.owner, field.name, field.desc);
+                                indexer.indexFieldReference(callerEntry, fieldEntry, ReferenceTargetType.none());
+                        }
+
+                        return super.binaryOperation(insn, value1, value2);
+                }
+
+                @Override
+                public PairValue<BasicValue, SourceValue> naryOperation(AbstractInsnNode insn, List<? extends PairValue<BasicValue, SourceValue>> values) throws AnalyzerException {
+                        if (insn.getOpcode() == Opcodes.INVOKEINTERFACE || insn.getOpcode() == Opcodes.INVOKESPECIAL || insn.getOpcode() == Opcodes.INVOKEVIRTUAL) {
+                                MethodInsnNode methodInsn = (MethodInsnNode) insn;
+                                indexer.indexMethodReference(callerEntry, MethodEntry.parse(methodInsn.owner, methodInsn.name, methodInsn.desc), getReferenceTargetType(values.get(0), insn));
+                        }
+
+                        if (insn.getOpcode() == Opcodes.INVOKESTATIC) {
+                                MethodInsnNode methodInsn = (MethodInsnNode) insn;
+                                indexer.indexMethodReference(callerEntry, MethodEntry.parse(methodInsn.owner, methodInsn.name, methodInsn.desc), ReferenceTargetType.none());
+                        }
+
+                        if (insn.getOpcode() == Opcodes.INVOKEDYNAMIC) {
+                                InvokeDynamicInsnNode invokeDynamicInsn = (InvokeDynamicInsnNode) insn;
+                                List<AbstractInsnNode> args = values.stream().map(v -> v.right.insns.stream().findFirst().orElseThrow(AssertionError::new)).collect(Collectors.toList());
+
+                                if ("java/lang/invoke/LambdaMetafactory".equals(invokeDynamicInsn.bsm.getOwner()) && "metafactory".equals(invokeDynamicInsn.bsm.getName())) {
+                                        Type samMethodType = (Type) invokeDynamicInsn.bsmArgs[0];
+                                        Handle implMethod = (Handle) invokeDynamicInsn.bsmArgs[1];
+                                        Type instantiatedMethodType = (Type) invokeDynamicInsn.bsmArgs[2];
+
+                                        ReferenceTargetType targetType;
+                                        if (implMethod.getTag() != Opcodes.H_GETSTATIC && implMethod.getTag() != Opcodes.H_PUTFIELD && implMethod.getTag() != Opcodes.H_INVOKESTATIC) {
+                                                if (instantiatedMethodType.getArgumentTypes().length < Type.getArgumentTypes(implMethod.getDesc()).length) {
+                                                        targetType = getReferenceTargetType(values.get(0), insn);
+                                                } else {
+                                                        targetType = ReferenceTargetType.none(); // no "this" argument
+                                                }
+                                        } else {
+                                                targetType = ReferenceTargetType.none();
+                                        }
+
+                                        indexer.indexLambda(callerEntry, new Lambda(
+                                                        invokeDynamicInsn.name,
+                                                        new MethodDescriptor(invokeDynamicInsn.desc),
+                                                        new MethodDescriptor(samMethodType.getDescriptor()),
+                                                        getHandleEntry(implMethod),
+                                                        new MethodDescriptor(instantiatedMethodType.getDescriptor())
+                                        ), targetType);
+                                }
+                        }
+
+                        return super.naryOperation(insn, values);
+                }
+
+                private ReferenceTargetType getReferenceTargetType(PairValue<BasicValue, SourceValue> target, AbstractInsnNode insn) throws AnalyzerException {
+                        if (target.left == BasicValue.UNINITIALIZED_VALUE) {
+                                return ReferenceTargetType.uninitialized();
+                        }
+
+                        if (target.left.getType().getSort() == Type.OBJECT) {
+                                return ReferenceTargetType.classType(new ClassEntry(target.left.getType().getInternalName()));
+                        }
+
+                        if (target.left.getType().getSort() == Type.ARRAY) {
+                                return ReferenceTargetType.classType(new ClassEntry("java/lang/Object"));
+                        }
+
+                        throw new AnalyzerException(insn, "called method on or accessed field of non-object type");
                 }
 
                 private static ParentedEntry<?> getHandleEntry(Handle handle) {
@@ -63,24 +173,8 @@ public class IndexReferenceVisitor extends ClassVisitor {
                                 case Opcodes.H_NEWINVOKESPECIAL:
                                         return MethodEntry.parse(handle.getOwner(), handle.getName(), handle.getDesc());
                         }
-                        throw new RuntimeException("Invalid handle tag " + handle.getTag());
-                }
 
-                @Override
-                public void visitInvokeDynamicInsn(String name, String desc, Handle bsm, Object... bsmArgs) {
-                        if ("java/lang/invoke/LambdaMetafactory".equals(bsm.getOwner()) && "metafactory".equals(bsm.getName())) {
-                                Type samMethodType = (Type) bsmArgs[0];
-                                Handle implMethod = (Handle) bsmArgs[1];
-                                Type instantiatedMethodType = (Type) bsmArgs[2];
-
-                                this.indexer.indexLambda(callerEntry, new Lambda(
-                                        name,
-                                        new MethodDescriptor(desc),
-                                        new MethodDescriptor(samMethodType.getDescriptor()),
-                                        getHandleEntry(implMethod),
-                                        new MethodDescriptor(instantiatedMethodType.getDescriptor())
-                                ));
-                        }
+                        throw new RuntimeException("Invalid handle tag " + handle.getTag());
                 }
         }
 }
diff --git a/src/main/java/cuchaz/enigma/analysis/index/JarIndex.java b/src/main/java/cuchaz/enigma/analysis/index/JarIndex.java
index 763282b..8e92dd8 100644
--- a/src/main/java/cuchaz/enigma/analysis/index/JarIndex.java
+++ b/src/main/java/cuchaz/enigma/analysis/index/JarIndex.java
@@ -15,6 +15,7 @@ import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Multimap;
 import cuchaz.enigma.ProgressListener;
 import cuchaz.enigma.analysis.ClassCache;
+import cuchaz.enigma.analysis.ReferenceTargetType;
 import cuchaz.enigma.translation.mapping.EntryResolver;
 import cuchaz.enigma.translation.mapping.IndexEntryResolver;
 import cuchaz.enigma.translation.representation.Lambda;
@@ -63,7 +64,7 @@ public class JarIndex implements JarIndexer {
                 classCache.visit(() -> new IndexClassVisitor(this, Opcodes.ASM5), ClassReader.SKIP_CODE);
 
                 progress.step(2, "Entry references...");
-                classCache.visit(() -> new IndexReferenceVisitor(this, Opcodes.ASM5), ClassReader.SKIP_FRAMES);
+                classCache.visit(() -> new IndexReferenceVisitor(this, entryIndex, inheritanceIndex, Opcodes.ASM5), 0);
 
                 progress.step(3, "Bridge methods...");
                 bridgeMethodIndex.findBridgeMethods();
@@ -115,30 +116,30 @@ public class JarIndex implements JarIndexer {
         }
 
         @Override
-        public void indexMethodReference(MethodDefEntry callerEntry, MethodEntry referencedEntry) {
+        public void indexMethodReference(MethodDefEntry callerEntry, MethodEntry referencedEntry, ReferenceTargetType targetType) {
                 if (callerEntry.getParent().isJre()) {
                         return;
                 }
 
-                indexers.forEach(indexer -> indexer.indexMethodReference(callerEntry, referencedEntry));
+                indexers.forEach(indexer -> indexer.indexMethodReference(callerEntry, referencedEntry, targetType));
         }
 
         @Override
-        public void indexFieldReference(MethodDefEntry callerEntry, FieldEntry referencedEntry) {
+        public void indexFieldReference(MethodDefEntry callerEntry, FieldEntry referencedEntry, ReferenceTargetType targetType) {
                 if (callerEntry.getParent().isJre()) {
                         return;
                 }
 
-                indexers.forEach(indexer -> indexer.indexFieldReference(callerEntry, referencedEntry));
+                indexers.forEach(indexer -> indexer.indexFieldReference(callerEntry, referencedEntry, targetType));
         }
 
         @Override
-        public void indexLambda(MethodDefEntry callerEntry, Lambda lambda) {
+        public void indexLambda(MethodDefEntry callerEntry, Lambda lambda, ReferenceTargetType targetType) {
                 if (callerEntry.getParent().isJre()) {
                         return;
                 }
 
-                indexers.forEach(indexer -> indexer.indexLambda(callerEntry, lambda));
+                indexers.forEach(indexer -> indexer.indexLambda(callerEntry, lambda, targetType));
         }
 
         public EntryIndex getEntryIndex() {
diff --git a/src/main/java/cuchaz/enigma/analysis/index/JarIndexer.java b/src/main/java/cuchaz/enigma/analysis/index/JarIndexer.java
index 4f885b3..f17e7c9 100644
--- a/src/main/java/cuchaz/enigma/analysis/index/JarIndexer.java
+++ b/src/main/java/cuchaz/enigma/analysis/index/JarIndexer.java
@@ -1,5 +1,6 @@
 package cuchaz.enigma.analysis.index;
 
+import cuchaz.enigma.analysis.ReferenceTargetType;
 import cuchaz.enigma.translation.representation.Lambda;
 import cuchaz.enigma.translation.representation.entry.*;
 
@@ -13,13 +14,13 @@ public interface JarIndexer {
         default void indexMethod(MethodDefEntry methodEntry) {
         }
 
-        default void indexMethodReference(MethodDefEntry callerEntry, MethodEntry referencedEntry) {
+        default void indexMethodReference(MethodDefEntry callerEntry, MethodEntry referencedEntry, ReferenceTargetType targetType) {
         }
 
-        default void indexFieldReference(MethodDefEntry callerEntry, FieldEntry referencedEntry) {
+        default void indexFieldReference(MethodDefEntry callerEntry, FieldEntry referencedEntry, ReferenceTargetType targetType) {
         }
 
-        default void indexLambda(MethodDefEntry callerEntry, Lambda lambda) {
+        default void indexLambda(MethodDefEntry callerEntry, Lambda lambda, ReferenceTargetType targetType) {
         }
 
         default void processIndex(JarIndex index) {
diff --git a/src/main/java/cuchaz/enigma/analysis/index/PackageVisibilityIndex.java b/src/main/java/cuchaz/enigma/analysis/index/PackageVisibilityIndex.java
index da28ac4..63eb730 100644
--- a/src/main/java/cuchaz/enigma/analysis/index/PackageVisibilityIndex.java
+++ b/src/main/java/cuchaz/enigma/analysis/index/PackageVisibilityIndex.java
@@ -5,6 +5,7 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;

 import com.google.common.collect.Sets;

 import cuchaz.enigma.analysis.EntryReference;

+import cuchaz.enigma.analysis.ReferenceTargetType;

 import cuchaz.enigma.translation.representation.AccessFlags;

 import cuchaz.enigma.translation.representation.entry.*;

 

@@ -12,12 +13,29 @@ import java.util.*;
 

 public class PackageVisibilityIndex implements JarIndexer {

         private static boolean requiresSamePackage(AccessFlags entryAcc, EntryReference ref, InheritanceIndex inheritanceIndex) {

-                if (entryAcc.isPublic()) return false;

+                if (entryAcc.isPublic()) {

+                        return false;

+                }

+

                 if (entryAcc.isProtected()) {

-                        Set<ClassEntry> callerAncestors = inheritanceIndex.getAncestors(ref.context.getContainingClass());

-                        return !callerAncestors.contains(ref.entry.getContainingClass());

+                        ClassEntry contextClass = ref.context.getContainingClass();

+                        ClassEntry referencedClass = ref.entry.getContainingClass();

+

+                        if (!inheritanceIndex.getAncestors(contextClass).contains(referencedClass)) {

+                                return true; // access to protected member not in superclass

+                        }

+

+                        if (ref.targetType.getKind() == ReferenceTargetType.Kind.NONE) {

+                                return false; // access to superclass or static superclass member

+                        }

+

+                        // access to instance member only valid if target's class assignable to context class

+                        return !(ref.targetType.getKind() == ReferenceTargetType.Kind.UNINITIALIZED ||

+                                        ((ReferenceTargetType.ClassType) ref.targetType).getEntry().equals(contextClass) ||

+                                        inheritanceIndex.getAncestors(((ReferenceTargetType.ClassType) ref.targetType).getEntry()).contains(contextClass));

                 }

-                return !entryAcc.isPrivate(); // if isPrivate is false, it must be package-private

+

+                return true;

         }

 

         private final HashMultimap<ClassEntry, ClassEntry> connections = HashMultimap.create();

@@ -25,8 +43,10 @@ public class PackageVisibilityIndex implements JarIndexer {
         private final Map<ClassEntry, Set<ClassEntry>> classPartitions = Maps.newHashMap();

 

         private void addConnection(ClassEntry classA, ClassEntry classB) {

-                connections.put(classA, classB);

-                connections.put(classB, classA);

+                if (classA != classB) {

+                        connections.put(classA, classB);

+                        connections.put(classB, classA);

+                }

         }

 

         private void buildPartition(Set<ClassEntry> unassignedClasses, Set<ClassEntry> partition, ClassEntry member) {

diff --git a/src/main/java/cuchaz/enigma/analysis/index/ReferenceIndex.java b/src/main/java/cuchaz/enigma/analysis/index/ReferenceIndex.java
index f54c90d..b26b08b 100644
--- a/src/main/java/cuchaz/enigma/analysis/index/ReferenceIndex.java
+++ b/src/main/java/cuchaz/enigma/analysis/index/ReferenceIndex.java
@@ -3,6 +3,7 @@ package cuchaz.enigma.analysis.index;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Multimap;
 import cuchaz.enigma.analysis.EntryReference;
+import cuchaz.enigma.analysis.ReferenceTargetType;
 import cuchaz.enigma.translation.mapping.ResolutionStrategy;
 import cuchaz.enigma.translation.representation.Lambda;
 import cuchaz.enigma.translation.representation.MethodDescriptor;
@@ -57,27 +58,27 @@ public class ReferenceIndex implements JarIndexer {
         }
 
         @Override
-        public void indexMethodReference(MethodDefEntry callerEntry, MethodEntry referencedEntry) {
-                referencesToMethods.put(referencedEntry, new EntryReference<>(referencedEntry, referencedEntry.getName(), callerEntry));
+        public void indexMethodReference(MethodDefEntry callerEntry, MethodEntry referencedEntry, ReferenceTargetType targetType) {
+                referencesToMethods.put(referencedEntry, new EntryReference<>(referencedEntry, referencedEntry.getName(), callerEntry, targetType));
                 methodReferences.put(callerEntry, referencedEntry);
 
                 if (referencedEntry.isConstructor()) {
                         ClassEntry referencedClass = referencedEntry.getParent();
-                        referencesToClasses.put(referencedClass, new EntryReference<>(referencedClass, referencedEntry.getName(), callerEntry));
+                        referencesToClasses.put(referencedClass, new EntryReference<>(referencedClass, referencedEntry.getName(), callerEntry, targetType));
                 }
         }
 
         @Override
-        public void indexFieldReference(MethodDefEntry callerEntry, FieldEntry referencedEntry) {
-                referencesToFields.put(referencedEntry, new EntryReference<>(referencedEntry, referencedEntry.getName(), callerEntry));
+        public void indexFieldReference(MethodDefEntry callerEntry, FieldEntry referencedEntry, ReferenceTargetType targetType) {
+                referencesToFields.put(referencedEntry, new EntryReference<>(referencedEntry, referencedEntry.getName(), callerEntry, targetType));
         }
 
         @Override
-        public void indexLambda(MethodDefEntry callerEntry, Lambda lambda) {
+        public void indexLambda(MethodDefEntry callerEntry, Lambda lambda, ReferenceTargetType targetType) {
                 if (lambda.getImplMethod() instanceof MethodEntry) {
-                        indexMethodReference(callerEntry, (MethodEntry) lambda.getImplMethod());
+                        indexMethodReference(callerEntry, (MethodEntry) lambda.getImplMethod(), targetType);
                 } else {
-                        indexFieldReference(callerEntry, (FieldEntry) lambda.getImplMethod());
+                        indexFieldReference(callerEntry, (FieldEntry) lambda.getImplMethod(), targetType);
                 }
 
                 indexMethodDescriptor(callerEntry, lambda.getInvokedType());