From a326c0617a8342ad70181affc9efbdf13c8e1e19 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 1 Jun 2017 13:38:06 +0700 Subject: [PATCH] Bytecode analysis: basic return value tracking --- .../BytecodeAnalysisConverter.java | 6 +- .../BytecodeAnalysisIndex.java | 207 +++++++------ .../bytecodeAnalysis/ClassDataIndexer.java | 6 +- .../codeInspection/bytecodeAnalysis/Data.java | 16 +- .../bytecodeAnalysis/HardCodedPurity.java | 13 +- .../ProjectBytecodeAnalysis.java | 36 ++- .../bytecodeAnalysis/PurityAnalysis.java | 274 ++++++++++++------ .../bytecodeAnalysis/asm/ASMUtils.java | 11 + .../bytecodeAnalysis/data/Test01.java | 13 + 9 files changed, 385 insertions(+), 197 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisConverter.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisConverter.java index 3ec9de9212ac..7e23ad360a36 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisConverter.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisConverter.java @@ -332,12 +332,12 @@ public class BytecodeAnalysisConverter { return contractClauses; } - public static void addEffectAnnotations(Map> puritySolutions, + public static void addEffectAnnotations(Map puritySolutions, MethodAnnotations result, EKey methodKey, boolean constructor) { - for (Map.Entry> entry : puritySolutions.entrySet()) { - Set effects = entry.getValue(); + for (Map.Entry entry : puritySolutions.entrySet()) { + Set effects = entry.getValue().effects; EKey key = entry.getKey().mkStable(); EKey baseKey = key.mkBase(); if (!methodKey.equals(baseKey)) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisIndex.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisIndex.java index 07daf26356ea..c1dc89840393 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisIndex.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisIndex.java @@ -53,7 +53,7 @@ public class BytecodeAnalysisIndex extends ScalarIndexExtension { private static final ID NAME = ID.create("bytecodeAnalysis"); private static final HKeyDescriptor KEY_DESCRIPTOR = new HKeyDescriptor(); - private static final int VERSION = 4; // change when inference algorithm changes + private static final int VERSION = 5; // change when inference algorithm changes private static final int VERSION_MODIFIER = HardCodedPurity.AGGRESSIVE_HARDCODED_PURITY ? 1 : 0; private static final int FINAL_VERSION = VERSION * 2 + VERSION_MODIFIER; @@ -202,10 +202,7 @@ public class BytecodeAnalysisIndex extends ScalarIndexExtension { EKey[] ids = component.ids; DataInputOutputUtil.writeINT(out, ids.length); for (EKey hKey : ids) { - out.write(hKey.method.hashed(md).myBytes); - int rawDirKey = hKey.negated ? -hKey.dirKey : hKey.dirKey; - DataInputOutputUtil.writeINT(out, rawDirKey); - out.writeBoolean(hKey.stable); + writeKey(out, hKey, md); } } } @@ -213,45 +210,9 @@ public class BytecodeAnalysisIndex extends ScalarIndexExtension { Effects effects = (Effects)rhs; DataInputOutputUtil.writeINT(out, effects.effects.size()); for (EffectQuantum effect : effects.effects) { - if (effect == EffectQuantum.TopEffectQuantum) { - DataInputOutputUtil.writeINT(out, -1); - } - else if (effect == EffectQuantum.ThisChangeQuantum) { - DataInputOutputUtil.writeINT(out, -2); - } - else if (effect instanceof EffectQuantum.CallQuantum) { - DataInputOutputUtil.writeINT(out, -3); - EffectQuantum.CallQuantum callQuantum = (EffectQuantum.CallQuantum)effect; - out.write(callQuantum.key.method.hashed(md).myBytes); - DataInputOutputUtil.writeINT(out, callQuantum.key.dirKey); - out.writeBoolean(callQuantum.key.stable); - out.writeBoolean(callQuantum.isStatic); - DataInputOutputUtil.writeINT(out, callQuantum.data.length); - for (DataValue dataValue : callQuantum.data) { - if (dataValue == DataValue.ThisDataValue) { - DataInputOutputUtil.writeINT(out, -1); - } - else if (dataValue == DataValue.LocalDataValue) { - DataInputOutputUtil.writeINT(out, -2); - } - else if (dataValue == DataValue.OwnedDataValue) { - DataInputOutputUtil.writeINT(out, -3); - } - else if (dataValue == DataValue.UnknownDataValue1) { - DataInputOutputUtil.writeINT(out, -4); - } - else if (dataValue == DataValue.UnknownDataValue2) { - DataInputOutputUtil.writeINT(out, -5); - } - else if (dataValue instanceof DataValue.ParameterDataValue) { - DataInputOutputUtil.writeINT(out, ((DataValue.ParameterDataValue)dataValue).n); - } - } - } - else if (effect instanceof EffectQuantum.ParamChangeQuantum) { - DataInputOutputUtil.writeINT(out, ((EffectQuantum.ParamChangeQuantum)effect).n); - } + writeEffect(out, effect, md); } + writeDataValue(out, effects.returnValue, md); } } } @@ -267,50 +228,10 @@ public class BytecodeAnalysisIndex extends ScalarIndexExtension { Set effects = new HashSet<>(); int effectsSize = DataInputOutputUtil.readINT(in); for (int i = 0; i < effectsSize; i++) { - int effectMask = DataInputOutputUtil.readINT(in); - if (effectMask == -1) { - effects.add(EffectQuantum.TopEffectQuantum); - } - else if (effectMask == -2) { - effects.add(EffectQuantum.ThisChangeQuantum); - } - else if (effectMask == -3){ - byte[] bytes = new byte[HMethod.HASH_SIZE]; - in.readFully(bytes); - int rawDirKey = DataInputOutputUtil.readINT(in); - boolean isStable = in.readBoolean(); - EKey key = new EKey(new HMethod(bytes), Math.abs(rawDirKey), isStable, false); - boolean isStatic = in.readBoolean(); - int dataLength = DataInputOutputUtil.readINT(in); - DataValue[] data = new DataValue[dataLength]; - for (int di = 0; di < dataLength; di++) { - int dataI = DataInputOutputUtil.readINT(in); - if (dataI == -1) { - data[di] = DataValue.ThisDataValue; - } - else if (dataI == -2) { - data[di] = DataValue.LocalDataValue; - } - else if (dataI == -3) { - data[di] = DataValue.OwnedDataValue; - } - else if (dataI == -4) { - data[di] = DataValue.UnknownDataValue1; - } - else if (dataI == -5) { - data[di] = DataValue.UnknownDataValue2; - } - else { - data[di] = new DataValue.ParameterDataValue(dataI); - } - } - effects.add(new EffectQuantum.CallQuantum(key, data, isStatic)); - } - else { - effects.add(new EffectQuantum.ParamChangeQuantum(effectMask)); - } + effects.add(readEffect(in)); } - results.add(new DirectionResultPair(directionKey, new Effects(effects))); + DataValue returnValue = readDataValue(in); + results.add(new DirectionResultPair(directionKey, new Effects(returnValue, effects))); } else { boolean isFinal = in.readBoolean(); // flag @@ -329,10 +250,7 @@ public class BytecodeAnalysisIndex extends ScalarIndexExtension { int componentSize = DataInputOutputUtil.readINT(in); EKey[] ids = new EKey[componentSize]; for (int j = 0; j < componentSize; j++) { - byte[] bytes = new byte[HMethod.HASH_SIZE]; - in.readFully(bytes); - int rawDirKey = DataInputOutputUtil.readINT(in); - ids[j] = new EKey(new HMethod(bytes), Direction.fromInt(Math.abs(rawDirKey)), in.readBoolean(), rawDirKey < 0); + ids[j] = readKey(in); } components[i] = new Component(value, ids); } @@ -342,5 +260,114 @@ public class BytecodeAnalysisIndex extends ScalarIndexExtension { } return new Equations(results, stable); } + + @NotNull + private static EKey readKey(@NotNull DataInput in) throws IOException { + byte[] bytes = new byte[HMethod.HASH_SIZE]; + in.readFully(bytes); + int rawDirKey = DataInputOutputUtil.readINT(in); + return new EKey(new HMethod(bytes), Direction.fromInt(Math.abs(rawDirKey)), in.readBoolean(), rawDirKey < 0); + } + + private static void writeKey(@NotNull DataOutput out, EKey key, MessageDigest md) throws IOException { + out.write(key.method.hashed(md).myBytes); + int rawDirKey = key.negated ? -key.dirKey : key.dirKey; + DataInputOutputUtil.writeINT(out, rawDirKey); + out.writeBoolean(key.stable); + } + + private static void writeEffect(@NotNull DataOutput out, EffectQuantum effect, MessageDigest md) throws IOException { + if (effect == EffectQuantum.TopEffectQuantum) { + DataInputOutputUtil.writeINT(out, -1); + } + else if (effect == EffectQuantum.ThisChangeQuantum) { + DataInputOutputUtil.writeINT(out, -2); + } + else if (effect instanceof EffectQuantum.CallQuantum) { + DataInputOutputUtil.writeINT(out, -3); + EffectQuantum.CallQuantum callQuantum = (EffectQuantum.CallQuantum)effect; + writeKey(out, callQuantum.key, md); + out.writeBoolean(callQuantum.isStatic); + DataInputOutputUtil.writeINT(out, callQuantum.data.length); + for (DataValue dataValue : callQuantum.data) { + writeDataValue(out, dataValue, md); + } + } + else if (effect instanceof EffectQuantum.ReturnChangeQuantum) { + DataInputOutputUtil.writeINT(out, -4); + writeKey(out, ((EffectQuantum.ReturnChangeQuantum)effect).key, md); + } + else if (effect instanceof EffectQuantum.ParamChangeQuantum) { + DataInputOutputUtil.writeINT(out, ((EffectQuantum.ParamChangeQuantum)effect).n); + } + } + + private static EffectQuantum readEffect(@NotNull DataInput in) throws IOException { + int effectMask = DataInputOutputUtil.readINT(in); + switch (effectMask) { + case -1: + return EffectQuantum.TopEffectQuantum; + case -2: + return EffectQuantum.ThisChangeQuantum; + case -3: + EKey key = readKey(in); + boolean isStatic = in.readBoolean(); + int dataLength = DataInputOutputUtil.readINT(in); + DataValue[] data = new DataValue[dataLength]; + for (int di = 0; di < dataLength; di++) { + data[di] = readDataValue(in); + } + return new EffectQuantum.CallQuantum(key, data, isStatic); + case -4: + return new EffectQuantum.ReturnChangeQuantum(readKey(in)); + default: + return new EffectQuantum.ParamChangeQuantum(effectMask); + } + } + + private static void writeDataValue(@NotNull DataOutput out, DataValue dataValue, MessageDigest md) throws IOException { + if (dataValue == DataValue.ThisDataValue) { + DataInputOutputUtil.writeINT(out, -1); + } + else if (dataValue == DataValue.LocalDataValue) { + DataInputOutputUtil.writeINT(out, -2); + } + else if (dataValue == DataValue.OwnedDataValue) { + DataInputOutputUtil.writeINT(out, -3); + } + else if (dataValue == DataValue.UnknownDataValue1) { + DataInputOutputUtil.writeINT(out, -4); + } + else if (dataValue == DataValue.UnknownDataValue2) { + DataInputOutputUtil.writeINT(out, -5); + } + else if (dataValue instanceof DataValue.ReturnDataValue) { + DataInputOutputUtil.writeINT(out, -6); + writeKey(out, ((DataValue.ReturnDataValue)dataValue).key, md); + } + else if (dataValue instanceof DataValue.ParameterDataValue) { + DataInputOutputUtil.writeINT(out, ((DataValue.ParameterDataValue)dataValue).n); + } + } + + private static DataValue readDataValue(@NotNull DataInput in) throws IOException { + int dataI = DataInputOutputUtil.readINT(in); + switch (dataI) { + case -1: + return DataValue.ThisDataValue; + case -2: + return DataValue.LocalDataValue; + case -3: + return DataValue.OwnedDataValue; + case -4: + return DataValue.UnknownDataValue1; + case -5: + return DataValue.UnknownDataValue2; + case -6: + return new DataValue.ReturnDataValue(readKey(in)); + default: + return new DataValue.ParameterDataValue(dataI); + } + } } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ClassDataIndexer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ClassDataIndexer.java index 576ef4b232ab..2b6589177318 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ClassDataIndexer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ClassDataIndexer.java @@ -95,7 +95,8 @@ public class ClassDataIndexer implements VirtualFileGist.GistCalculator hash(effect, md)).toSet()); + Effects effects = (Effects)result; + return new Effects(effects.returnValue, StreamEx.of(effects.effects).map(effect -> hash(effect, md)).toSet()); } else if(result instanceof Pending) { return new Pending(ContainerUtil.map(((Pending)result).delta, component -> hash(component, md))); } @@ -513,6 +514,9 @@ public class ClassDataIndexer implements VirtualFileGist.GistCalculator effects; - Effects(@NotNull Set effects) { + Effects(@NotNull DataValue returnValue, @NotNull Set effects) { + this.returnValue = returnValue; this.effects = effects; } + Stream dependencies() { + return Stream.concat(returnValue.dependencies(), effects.stream().flatMap(EffectQuantum::dependencies)); + } + @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - return this.effects.equals(((Effects)o).effects); + Effects that = (Effects)o; + return this.returnValue.equals(that.returnValue) && this.effects.equals(that.effects); } @Override public int hashCode() { - return effects.hashCode(); + return effects.hashCode() * 31 + returnValue.hashCode(); } @Override public String toString() { - return "Effects["+effects.size()+"]"; + return "Effects[" + effects.size() + "|" + returnValue + "]"; } } \ No newline at end of file diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/HardCodedPurity.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/HardCodedPurity.java index 502aeb824f92..f7bee55764f0 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/HardCodedPurity.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/HardCodedPurity.java @@ -66,8 +66,17 @@ class HardCodedPurity { return AGGRESSIVE_HARDCODED_PURITY ? new AggressiveHardCodedPurity() : new HardCodedPurity(); } - Set getHardCodedSolution(Method method) { - return isThisChangingMethod(method) ? thisChange : isPureMethod(method) ? Collections.emptySet() : solutions.get(method); + Effects getHardCodedSolution(Method method) { + if (isThisChangingMethod(method)) { + return new Effects(isBuilderChainCall(method) ? DataValue.ThisDataValue : DataValue.UnknownDataValue1, thisChange); + } + else if (isPureMethod(method)) { + return new Effects(DataValue.LocalDataValue, Collections.emptySet()); + } + else { + Set effects = solutions.get(method); + return effects == null ? null : new Effects(DataValue.UnknownDataValue1, effects); + } } boolean isThisChangingMethod(Method method) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java index 272037730649..b7616d4c91b9 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java @@ -27,11 +27,10 @@ import com.intellij.openapi.util.registry.Registry; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.*; +import com.intellij.psi.impl.compiled.ClsClassImpl; +import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.search.ProjectScope; -import com.intellij.psi.util.CachedValueProvider; -import com.intellij.psi.util.CachedValuesManager; -import com.intellij.psi.util.PsiFormatUtil; -import com.intellij.psi.util.PsiModificationTracker; +import com.intellij.psi.util.*; import com.intellij.testFramework.LightVirtualFile; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.ConcurrentFactoryMap; @@ -303,7 +302,7 @@ public class ProjectBytecodeAnalysis { final PuritySolver puritySolver = new PuritySolver(); collectPurityEquations(key.withDirection(Pure), puritySolver); - Map> puritySolutions = puritySolver.solve(); + Map puritySolutions = puritySolver.solve(); int arity = owner.getParameterList().getParameters().length; BytecodeAnalysisConverter.addEffectAnnotations(puritySolutions, result, key, owner.isConstructor()); @@ -358,17 +357,14 @@ public class ProjectBytecodeAnalysis { for (DirectionResultPair pair : equations.results) { int dirKey = pair.directionKey; if (dirKey == hKey.dirKey) { - Set effects = ((Effects)pair.result).effects; + Effects effects = (Effects)pair.result; puritySolver.addEquation(new EKey(hKey.method, dirKey, stable, false), effects); - for (EffectQuantum effect : effects) { - if (effect instanceof EffectQuantum.CallQuantum) { - EKey depKey = ((EffectQuantum.CallQuantum)effect).key; - if (!queued.contains(depKey)) { - queue.push(depKey); - queued.add(depKey); - } + effects.dependencies().forEach(depKey -> { + if (!queued.contains(depKey)) { + queue.push(depKey); + queued.add(depKey); } - } + }); } } } @@ -480,7 +476,17 @@ public class ProjectBytecodeAnalysis { String packageName = StringUtil.getPackageName(internalClassName, '/').replace('/', '.'); String className = StringUtil.getShortName(internalClassName, '/'); PsiPackage aPackage = JavaPsiFacade.getInstance(myProject).findPackage(packageName); - if (aPackage == null) return null; + if (aPackage == null) { + PsiClass psiClass = JavaPsiFacade.getInstance(myProject).findClass(StringUtil.getQualifiedName(packageName, className), GlobalSearchScope + .allScope(myProject)); + if(psiClass != null) { + PsiModifierListOwner compiledClass = PsiUtil.preferCompiledElement(psiClass); + if(compiledClass instanceof ClsClassImpl) { + return compiledClass.getContainingFile().getVirtualFile(); + } + } + return null; + } String classFileName = className + ".class"; for (PsiDirectory directory : aPackage.getDirectories()) { VirtualFile file = directory.getVirtualFile().findChild(classFileName); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/PurityAnalysis.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/PurityAnalysis.java index 74545a6e658d..7575ede44661 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/PurityAnalysis.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/PurityAnalysis.java @@ -26,6 +26,8 @@ import org.jetbrains.org.objectweb.asm.tree.analysis.AnalyzerException; import org.jetbrains.org.objectweb.asm.tree.analysis.Interpreter; import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Produces equations for inference of @Contract(pure=true) annotations. @@ -44,9 +46,9 @@ public class PurityAnalysis { @Nullable public static Equation analyze(Method method, MethodNode methodNode, boolean stable) { EKey key = new EKey(method, Direction.Pure, stable); - Set hardCodedSolution = HardCodedPurity.getInstance().getHardCodedSolution(method); + Effects hardCodedSolution = HardCodedPurity.getInstance().getHardCodedSolution(method); if (hardCodedSolution != null) { - return new Equation(key, new Effects(hardCodedSolution)); + return new Equation(key, hardCodedSolution); } if ((methodNode.access & UN_ANALYZABLE_FLAG) != 0) return null; @@ -59,14 +61,17 @@ public class PurityAnalysis { return null; } EffectQuantum[] quanta = dataInterpreter.effects; + DataValue returnValue = dataInterpreter.returnValue == null ? DataValue.UnknownDataValue1 : dataInterpreter.returnValue; Set effects = new HashSet<>(); for (EffectQuantum effectQuantum : quanta) { if (effectQuantum != null) { - if (effectQuantum == EffectQuantum.TopEffectQuantum) return null; + if (effectQuantum == EffectQuantum.TopEffectQuantum) { + return returnValue == DataValue.UnknownDataValue1 ? null : new Equation(key, new Effects(returnValue, PuritySolver.TOP_EFFECTS)); + } effects.add(effectQuantum); } } - return new Equation(key, new Effects(effects)); + return new Equation(key, new Effects(returnValue, effects)); } } @@ -83,6 +88,10 @@ abstract class DataValue implements org.jetbrains.org.objectweb.asm.tree.analysi return myHash; } + Stream dependencies() { + return Stream.empty(); + } + static final DataValue ThisDataValue = new DataValue(-1) { @Override public int getSize() { @@ -123,8 +132,7 @@ abstract class DataValue implements org.jetbrains.org.objectweb.asm.tree.analysi if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; ParameterDataValue that = (ParameterDataValue)o; - if (n != that.n) return false; - return true; + return n == that.n; } @Override @@ -132,6 +140,37 @@ abstract class DataValue implements org.jetbrains.org.objectweb.asm.tree.analysi return "DataValue: arg#" + n; } } + static class ReturnDataValue extends DataValue { + final EKey key; + + ReturnDataValue(EKey key) { + super(key.hashCode()); + this.key = key; + } + + @Override + public int getSize() { + return 1; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ReturnDataValue that = (ReturnDataValue)o; + return key.equals(that.key); + } + + @Override + Stream dependencies() { + return Stream.of(key); + } + + @Override + public String toString() { + return "Return of: " + key; + } + } static final DataValue OwnedDataValue = new DataValue(-3) { @Override public int getSize() { @@ -174,6 +213,10 @@ abstract class EffectQuantum { myHash = hash; } + Stream dependencies() { + return Stream.empty(); + } + @Override public final int hashCode() { return myHash; @@ -191,7 +234,32 @@ abstract class EffectQuantum { return "Changes this"; } }; - static class ParamChangeQuantum extends EffectQuantum { + + static final class ReturnChangeQuantum extends EffectQuantum { + final EKey key; + public ReturnChangeQuantum(EKey key) { + super(key.hashCode()); + this.key = key; + } + + @Override + Stream dependencies() { + return Stream.of(key); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + return o != null && getClass() == o.getClass() && key == ((ReturnChangeQuantum)o).key; + } + + @Override + public String toString() { + return "Changes return value of " + key; + } + } + + static final class ParamChangeQuantum extends EffectQuantum { final int n; public ParamChangeQuantum(int n) { super(n); @@ -201,13 +269,7 @@ abstract class EffectQuantum { @Override public boolean equals(Object o) { if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - ParamChangeQuantum that = (ParamChangeQuantum)o; - - if (n != that.n) return false; - - return true; + return o != null && getClass() == o.getClass() && n == ((ParamChangeQuantum)o).n; } @Override @@ -215,7 +277,8 @@ abstract class EffectQuantum { return "Changes param#" + n; } } - static class CallQuantum extends EffectQuantum { + + static final class CallQuantum extends EffectQuantum { final EKey key; final DataValue[] data; final boolean isStatic; @@ -235,12 +298,15 @@ abstract class EffectQuantum { if (isStatic != that.isStatic) return false; if (!key.equals(that.key)) return false; - // Probably incorrect - comparing Object[] arrays with Arrays.equals if (!Arrays.equals(data, that.data)) return false; - return true; } + @Override + Stream dependencies() { + return Stream.concat(Stream.of(key), Stream.of(data).flatMap(DataValue::dependencies)); + } + @Override public String toString() { return "Calls " + key; @@ -256,6 +322,7 @@ class DataInterpreter extends Interpreter { final int rangeEnd; final int arity; final EffectQuantum[] effects; + DataValue returnValue = null; protected DataInterpreter(MethodNode methodNode) { super(Opcodes.API_VERSION); @@ -336,16 +403,7 @@ class DataInterpreter extends Interpreter { case Opcodes.LXOR: return DataValue.UnknownDataValue2; case Opcodes.PUTFIELD: - final EffectQuantum effectQuantum; - if (value1 == DataValue.ThisDataValue || value1 == DataValue.OwnedDataValue) { - effectQuantum = EffectQuantum.ThisChangeQuantum; - } else if (value1 == DataValue.LocalDataValue) { - effectQuantum = null; - } else if (value1 instanceof DataValue.ParameterDataValue) { - effectQuantum = new EffectQuantum.ParamChangeQuantum(((DataValue.ParameterDataValue)value1).n); - } else { - effectQuantum = EffectQuantum.TopEffectQuantum; - } + final EffectQuantum effectQuantum = getChangeQuantum(value1); int insnIndex = methodNode.instructions.indexOf(insn); effects[insnIndex] = effectQuantum; return DataValue.UnknownDataValue1; @@ -354,6 +412,23 @@ class DataInterpreter extends Interpreter { } } + @Nullable + private static EffectQuantum getChangeQuantum(DataValue value) { + if (value == DataValue.ThisDataValue || value == DataValue.OwnedDataValue) { + return EffectQuantum.ThisChangeQuantum; + } + if (value == DataValue.LocalDataValue) { + return null; + } + if (value instanceof DataValue.ParameterDataValue) { + return new EffectQuantum.ParamChangeQuantum(((DataValue.ParameterDataValue)value).n); + } + if (value instanceof DataValue.ReturnDataValue) { + return new EffectQuantum.ReturnChangeQuantum(((DataValue.ReturnDataValue)value).key); + } + return EffectQuantum.TopEffectQuantum; + } + @Override public DataValue copyOperation(AbstractInsnNode insn, DataValue value) throws AnalyzerException { return value; @@ -382,7 +457,17 @@ class DataInterpreter extends Interpreter { Method method = new Method(mNode.owner, mNode.name, mNode.desc); EKey key = new EKey(method, Direction.Pure, stable); EffectQuantum quantum = new EffectQuantum.CallQuantum(key, data, opCode == Opcodes.INVOKESTATIC); - DataValue result = (ASMUtils.getReturnSizeFast(mNode.desc) == 1) ? DataValue.UnknownDataValue1 : DataValue.UnknownDataValue2; + DataValue result; + if (ASMUtils.getReturnSizeFast(mNode.desc) == 1) { + if (ASMUtils.isReferenceReturnType(mNode.desc)) { + result = new DataValue.ReturnDataValue(key); + } else { + result = DataValue.UnknownDataValue1; + } + } + else { + result = DataValue.UnknownDataValue2; + } if (HardCodedPurity.getInstance().isPureMethod(method)) { quantum = null; result = DataValue.LocalDataValue; @@ -446,21 +531,20 @@ class DataInterpreter extends Interpreter { @Override public DataValue ternaryOperation(AbstractInsnNode insn, DataValue value1, DataValue value2, DataValue value3) throws AnalyzerException { int insnIndex = methodNode.instructions.indexOf(insn); - if (value1 == DataValue.ThisDataValue || value1 == DataValue.OwnedDataValue) { - effects[insnIndex] = EffectQuantum.ThisChangeQuantum; - } else if (value1 instanceof DataValue.ParameterDataValue) { - effects[insnIndex] = new EffectQuantum.ParamChangeQuantum(((DataValue.ParameterDataValue)value1).n); - } else if (value1 == DataValue.LocalDataValue) { - effects[insnIndex] = null; - } else { - effects[insnIndex] = EffectQuantum.TopEffectQuantum; - } + effects[insnIndex] = getChangeQuantum(value1); return DataValue.UnknownDataValue1; } @Override public void returnOperation(AbstractInsnNode insn, DataValue value, DataValue expected) throws AnalyzerException { - + if (insn.getOpcode() == Opcodes.ARETURN) { + if (returnValue == null) { + returnValue = value; + } + else if (!returnValue.equals(value)) { + returnValue = DataValue.UnknownDataValue1; + } + } } @Override @@ -475,95 +559,99 @@ class DataInterpreter extends Interpreter { } final class PuritySolver { - private static final Set TOP_EFFECTS = Collections.singleton(EffectQuantum.TopEffectQuantum); - private HashMap> solved = new HashMap<>(); + static final Set TOP_EFFECTS = Collections.singleton(EffectQuantum.TopEffectQuantum); + private HashMap solved = new HashMap<>(); private HashMap> dependencies = new HashMap<>(); private final Stack moving = new Stack<>(); - private HashMap> pending = new HashMap<>(); + private HashMap pending = new HashMap<>(); - void addEquation(EKey key, Set effects) { - Set callKeys = new HashSet<>(); - for (EffectQuantum effect : effects) { - if (effect instanceof EffectQuantum.CallQuantum) { - callKeys.add(((EffectQuantum.CallQuantum)effect).key); - } - } + void addEquation(EKey key, Effects effects) { + Set depKeys = effects.dependencies().collect(Collectors.toSet()); - if (callKeys.isEmpty()) { + if (depKeys.isEmpty()) { solved.put(key, effects); moving.add(key); } else { pending.put(key, effects); - for (EKey callKey : callKeys) { - Set deps = dependencies.get(callKey); - if (deps == null) { - deps = new HashSet<>(); - dependencies.put(callKey, deps); - } - deps.add(key); + for (EKey depKey : depKeys) { + dependencies.computeIfAbsent(depKey, k -> new HashSet<>()).add(key); } } } - public Map> solve() { + public Map solve() { while (!moving.isEmpty()) { EKey key = moving.pop(); - Set effects = solved.get(key); + Effects effects = solved.get(key); EKey[] propagateKeys; - Set[] propagateEffects; + Effects[] propagateEffects; if (key.stable) { propagateKeys = new EKey[]{key, key.mkUnstable()}; - propagateEffects = new Set[]{effects, effects}; + propagateEffects = new Effects[]{effects, effects}; } else { propagateKeys = new EKey[]{key.mkStable(), key}; - propagateEffects = new Set[]{effects, TOP_EFFECTS}; + propagateEffects = new Effects[]{effects, new Effects(DataValue.UnknownDataValue1, TOP_EFFECTS)}; } for (int i = 0; i < propagateKeys.length; i++) { EKey pKey = propagateKeys[i]; - @SuppressWarnings("unchecked") - Set pEffects = propagateEffects[i]; + Effects pEffects = propagateEffects[i]; Set dKeys = dependencies.remove(pKey); if (dKeys != null) { for (EKey dKey : dKeys) { - Set dEffects = pending.remove(dKey); + Effects dEffects = pending.remove(dKey); if (dEffects == null) { // already solved, for example, solution is top continue; } - Set callKeys = new HashSet<>(); Set newEffects = new HashSet<>(); Set delta = null; + DataValue returnValue = substitute(dEffects.returnValue, pKey, pEffects); - for (EffectQuantum dEffect : dEffects) { + for (EffectQuantum dEffect : dEffects.effects) { if (dEffect instanceof EffectQuantum.CallQuantum) { - EffectQuantum.CallQuantum call = ((EffectQuantum.CallQuantum)dEffect); + EffectQuantum.CallQuantum call = substitute((EffectQuantum.CallQuantum)dEffect, pKey, pEffects); if (call.key.equals(pKey)) { delta = substitute(pEffects, call.data, call.isStatic); + if(delta.equals(TOP_EFFECTS)) { + newEffects = delta; + break; + } newEffects.addAll(delta); } else { - callKeys.add(call.key); newEffects.add(call); } + continue; } - else { - newEffects.add(dEffect); + if (dEffect instanceof EffectQuantum.ReturnChangeQuantum) { + EffectQuantum.ReturnChangeQuantum retChange = (EffectQuantum.ReturnChangeQuantum)dEffect; + if (retChange.key.equals(pKey)) { + if(pEffects.returnValue != DataValue.LocalDataValue) { + newEffects = delta = TOP_EFFECTS; + break; + } + continue; + } } + newEffects.add(dEffect); } - if (TOP_EFFECTS.equals(delta)) { - solved.put(dKey, TOP_EFFECTS); - moving.push(dKey); - } - else if (callKeys.isEmpty()) { - solved.put(dKey, newEffects); + if (TOP_EFFECTS.equals(delta) && returnValue.equals(DataValue.UnknownDataValue1)) { + solved.put(dKey, new Effects(returnValue, TOP_EFFECTS)); moving.push(dKey); } else { - pending.put(dKey, newEffects); + Effects result = new Effects(returnValue, newEffects); + if (result.dependencies().findFirst().isPresent()) { + pending.put(dKey, result); + } + else { + solved.put(dKey, result); + moving.push(dKey); + } } } @@ -575,13 +663,31 @@ final class PuritySolver { return solved; } - private static Set substitute(Set effects, DataValue[] data, boolean isStatic) { - if (effects.isEmpty() || TOP_EFFECTS.equals(effects)) { - return effects; + private static EffectQuantum.CallQuantum substitute(EffectQuantum.CallQuantum call, EKey pKey, Effects pEffects) { + List list = new ArrayList<>(); + boolean same = true; + for (DataValue value : call.data) { + DataValue newValue = substitute(value, pKey, pEffects); + same &= newValue.equals(value); + list.add(newValue); } - Set newEffects = new HashSet<>(effects.size()); + return same ? call : new EffectQuantum.CallQuantum(call.key, list.toArray(new DataValue[0]), call.isStatic); + } + + private static DataValue substitute(DataValue value, EKey key, Effects effects) { + if(value instanceof DataValue.ReturnDataValue && ((DataValue.ReturnDataValue)value).key.equals(key)) { + return effects.returnValue == DataValue.LocalDataValue ? DataValue.LocalDataValue : DataValue.UnknownDataValue1; + } + return value; + } + + private static Set substitute(Effects effects, DataValue[] data, boolean isStatic) { + if (effects.effects.isEmpty() || TOP_EFFECTS.equals(effects.effects)) { + return effects.effects; + } + Set newEffects = new HashSet<>(effects.effects.size()); int shift = isStatic ? 0 : 1; - for (EffectQuantum effect : effects) { + for (EffectQuantum effect : effects.effects) { DataValue arg = null; if (effect == EffectQuantum.ThisChangeQuantum) { arg = data[0]; @@ -600,6 +706,10 @@ final class PuritySolver { newEffects.add(new EffectQuantum.ParamChangeQuantum(((DataValue.ParameterDataValue)arg).n)); continue; } + if (arg instanceof DataValue.ReturnDataValue) { + newEffects.add(new EffectQuantum.ReturnChangeQuantum(((DataValue.ReturnDataValue)arg).key)); + continue; + } return TOP_EFFECTS; } return newEffects; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/asm/ASMUtils.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/asm/ASMUtils.java index d4b6d8171a6f..19c0240fc004 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/asm/ASMUtils.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/asm/ASMUtils.java @@ -59,4 +59,15 @@ public class ASMUtils { } } + @Contract(pure = true) + public static boolean isReferenceReturnType(String methodDesc) { + switch (methodDesc.charAt(methodDesc.indexOf(')') + 1)) { + case 'L': + case '[': + return true; + default: + return false; + } + } + } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/data/Test01.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/data/Test01.java index 52d907ee46d5..9dd8a1db8c81 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/data/Test01.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/data/Test01.java @@ -163,6 +163,19 @@ public class Test01 { return copy; } + @ExpectNotNull + @ExpectContract(pure = true) + public static long[] copyAndModify(@ExpectNotNull long[] input) { + long[] copy = copyOfRange(input, 0, input.length); + copy[0] = 1; + set(copy); + return copy; + } + + private static void set(@ExpectNotNull long[] copy) { + copy[1] = 2; + } + @ExpectContract(pure = true) public static O[] copyOfRangeObject(@ExpectNotNull I[] arr, int from, int to, @ExpectNotNull Class newType) { int diff = to - from;