From 582239346e5aa8e318def3a10c5486f66921d735 Mon Sep 17 00:00:00 2001 From: Ilya Klyuchnikov Date: Wed, 1 Oct 2014 17:45:26 +0400 Subject: [PATCH] bytecode analysis: no inference of "true|false->..." contracts --- .../BytecodeAnalysisConverter.java | 9 +- .../bytecodeAnalysis/ClassDataIndexer.java | 82 +++++++------------ .../bytecodeAnalysis/Contracts.java | 14 ---- .../bytecodeAnalysis/asm/ASMUtils.java | 5 ++ .../annotations/java/security/annotations.xml | 5 -- .../apache/commons/lang/math/annotations.xml | 2 +- .../apache/velocity/runtime/annotations.xml | 2 +- 7 files changed, 39 insertions(+), 80 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 91f1b685faa8..9817c68f677b 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 @@ -321,14 +321,7 @@ public class BytecodeAnalysisConverter { PsiParameter[] parameters = psiMethod.getParameterList().getParameters(); ArrayList keys = new ArrayList(parameters.length * 2 + 1); for (int i = 0; i < parameters.length; i++) { - PsiParameter parameter = parameters[i]; - PsiType parameterType = parameter.getType(); - if (parameterType instanceof PsiPrimitiveType) { - if (PsiType.BOOLEAN.equals(parameterType)) { - keys.add(primaryKey.updateDirection(mkDirectionKey(new InOut(i, Value.False)))); - keys.add(primaryKey.updateDirection(mkDirectionKey(new InOut(i, Value.True)))); - } - } else { + if (!(parameters[i].getType() instanceof PsiPrimitiveType)) { keys.add(primaryKey.updateDirection(mkDirectionKey(new InOut(i, Value.NotNull)))); keys.add(primaryKey.updateDirection(mkDirectionKey(new InOut(i, Value.Null)))); } 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 d4f52370a929..ad856455de79 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 @@ -179,10 +179,11 @@ public class ClassDataIndexer implements DataIndexer ..., !null -> ... List> result = new ArrayList>(argumentTypes.length * 4 + 2); boolean maybeLeakingParameter = isInterestingResult; for (Type argType : argumentTypes) { - if (ASMUtils.isReferenceType(argType) || (isReferenceResult && ASMUtils.isBooleanType(argType))) { + if (ASMUtils.isReferenceType(argType)) { maybeLeakingParameter = true; break; } @@ -212,10 +213,9 @@ public class ClassDataIndexer implements DataIndexer(new Key(method, new In(i, In.NOT_NULL), stable), FINAL_TOP)); } + if (leakingNullableParameters[i]) { if (notNullParam || possibleNPE) { result.add(new Equation(new Key(method, new In(i, In.NULLABLE), stable), FINAL_TOP)); @@ -239,34 +240,24 @@ public class ClassDataIndexer implements DataIndexer(new Key(method, new In(i, In.NULLABLE), stable), FINAL_NULL)); } - } - if (isReferenceArg && isInterestingResult) { - if (leakingParameters[i]) { - if (!notNullParam) { - // may be null on some branch, running "null->..." analysis - result.add(new InOutAnalysis(richControlFlow, new InOut(i, Value.Null), origins, stable).analyze()); + + if (isInterestingResult) { + if (leakingParameters[i]) { + if (notNullParam) { + // @NotNull, so "null->fail" + result.add(new Equation(new Key(method, new InOut(i, Value.Null), stable), FINAL_BOT)); + } + else { + // may be null on some branch, running "null->..." analysis + result.add(new InOutAnalysis(richControlFlow, new InOut(i, Value.Null), origins, stable).analyze()); + } + result.add(new InOutAnalysis(richControlFlow, new InOut(i, Value.NotNull), origins, stable).analyze()); } else { - // @NotNull, so "null->fail" - result.add(new Equation(new Key(method, new InOut(i, Value.Null), stable), FINAL_BOT)); + // parameter is not leaking, so a contract is the same as for the whole method + result.add(new Equation(new Key(method, new InOut(i, Value.Null), stable), outEquation.rhs)); + result.add(new Equation(new Key(method, new InOut(i, Value.NotNull), stable), outEquation.rhs)); } - result.add(new InOutAnalysis(richControlFlow, new InOut(i, Value.NotNull), origins, stable).analyze()); - } - else { - // parameter is not leaking, so a contract is the same as for the whole method - result.add(new Equation(new Key(method, new InOut(i, Value.Null), stable), outEquation.rhs)); - result.add(new Equation(new Key(method, new InOut(i, Value.NotNull), stable), outEquation.rhs)); - } - } - if (ASMUtils.isBooleanType(argumentTypes[i]) && isInterestingResult) { - if (leakingParameters[i]) { - result.add(new InOutAnalysis(richControlFlow, new InOut(i, Value.False), origins, stable).analyze()); - result.add(new InOutAnalysis(richControlFlow, new InOut(i, Value.True), origins, stable).analyze()); - } - else { - // parameter is not leaking, so a contract is the same as for the whole method - result.add(new Equation(new Key(method, new InOut(i, Value.False), stable), outEquation.rhs)); - result.add(new Equation(new Key(method, new InOut(i, Value.True), stable), outEquation.rhs)); } } } @@ -279,6 +270,7 @@ public class ClassDataIndexer implements DataIndexer ..., !null -> ... List> result = new ArrayList>(argumentTypes.length * 4 + 2); CombinedAnalysis analyzer = new CombinedAnalysis(method, graph); analyzer.analyze(); @@ -288,18 +280,13 @@ public class ClassDataIndexer implements DataIndexer ..., !null -> ... List> result = new ArrayList>(argumentTypes.length * 4 + 2); if (isReferenceResult) { result.add(new Equation(new Key(method, Out, stable), FINAL_TOP)); result.add(new Equation(new Key(method, NullableOut, stable), FINAL_BOT)); } for (int i = 0; i < argumentTypes.length; i++) { - Type argType = argumentTypes[i]; - boolean isReferenceArg = ASMUtils.isReferenceType(argType); - boolean isBooleanArg = ASMUtils.isBooleanType(argType); - - if (isReferenceArg) { + if (ASMUtils.isReferenceType(argumentTypes[i])) { result.add(new Equation(new Key(method, new In(i, In.NOT_NULL), stable), FINAL_TOP)); result.add(new Equation(new Key(method, new In(i, In.NULLABLE), stable), FINAL_TOP)); - } - if (isReferenceArg && isInterestingResult) { - result.add(new Equation(new Key(method, new InOut(i, Value.Null), stable), FINAL_TOP)); - result.add(new Equation(new Key(method, new InOut(i, Value.NotNull), stable), FINAL_TOP)); - } - if (isBooleanArg && isInterestingResult) { - result.add(new Equation(new Key(method, new InOut(i, Value.False), stable), FINAL_TOP)); - result.add(new Equation(new Key(method, new InOut(i, Value.True), stable), FINAL_TOP)); + if (isInterestingResult) { + result.add(new Equation(new Key(method, new InOut(i, Value.Null), stable), FINAL_TOP)); + result.add(new Equation(new Key(method, new InOut(i, Value.NotNull), stable), FINAL_TOP)); + } } } return result; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Contracts.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Contracts.java index 73972871ee7b..46ec090db16c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Contracts.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Contracts.java @@ -200,20 +200,6 @@ class InOutAnalysis extends Analysis> { return; } - if (opcode == IFEQ && popValue(frame) instanceof ParamValue) { - int nextInsnIndex = inValue == Value.True ? insnIndex + 1 : methodNode.instructions.indexOf(((JumpInsnNode)insnNode).label); - State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false); - pendingPush(nextState); - return; - } - - if (opcode == IFNE && popValue(frame) instanceof ParamValue) { - int nextInsnIndex = inValue == Value.False ? insnIndex + 1 : methodNode.instructions.indexOf(((JumpInsnNode)insnNode).label); - State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false); - pendingPush(nextState); - return; - } - // general case for (int nextInsnIndex : controlFlow.transitions[insnIndex]) { Frame nextFrame1 = nextFrame; 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 a6bfdc38732e..d4b6d8171a6f 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 @@ -15,6 +15,7 @@ */ package com.intellij.codeInspection.bytecodeAnalysis.asm; +import org.jetbrains.annotations.Contract; import org.jetbrains.org.objectweb.asm.Type; import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue; @@ -25,15 +26,18 @@ public class ASMUtils { public static final Type THROWABLE_TYPE = Type.getType("java/lang/Throwable"); public static final BasicValue THROWABLE_VALUE = new BasicValue(THROWABLE_TYPE); + @Contract(pure = true) public static boolean isReferenceType(Type tp) { int sort = tp.getSort(); return sort == Type.OBJECT || sort == Type.ARRAY; } + @Contract(pure = true) public static boolean isBooleanType(Type tp) { return Type.BOOLEAN_TYPE.equals(tp); } + @Contract(pure = true) public static int getSizeFast(String desc) { switch (desc.charAt(0)) { case 'J': @@ -44,6 +48,7 @@ public class ASMUtils { } } + @Contract(pure = true) public static int getReturnSizeFast(String methodDesc) { switch (methodDesc.charAt(methodDesc.indexOf(')') + 1)) { case 'J': diff --git a/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/java/security/annotations.xml b/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/java/security/annotations.xml index b99175ab871b..39926fee1fd0 100644 --- a/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/java/security/annotations.xml +++ b/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/java/security/annotations.xml @@ -14,11 +14,6 @@ - - - - - diff --git a/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/org/apache/commons/lang/math/annotations.xml b/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/org/apache/commons/lang/math/annotations.xml index d0657899989a..0b694cbd086f 100644 --- a/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/org/apache/commons/lang/math/annotations.xml +++ b/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/org/apache/commons/lang/math/annotations.xml @@ -90,7 +90,7 @@ - + diff --git a/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/org/apache/velocity/runtime/annotations.xml b/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/org/apache/velocity/runtime/annotations.xml index 20fa8692e567..0b38f9094c69 100644 --- a/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/org/apache/velocity/runtime/annotations.xml +++ b/java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/org/apache/velocity/runtime/annotations.xml @@ -102,7 +102,7 @@ - +