MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

BytecodeAnalysis: support NPE transitions specially

Browse Source 
Restored contracts lost in previous fix
This commit is contained in:
Tagir Valeev
2017-10-04 16:15:08 +07:00
parent 38936bdfd4
commit 2a0574a7e2
8 changed files with 126 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Analysis.java
+15 -5
View File
@@ -15,6 +15,7 @@
*/
package com.intellij.codeInspection.bytecodeAnalysis;
import com.intellij.codeInspection.bytecodeAnalysis.asm.ASMUtils;
import com.intellij.codeInspection.bytecodeAnalysis.asm.ControlFlowGraph;
import com.intellij.codeInspection.bytecodeAnalysis.asm.DFSTree;
import com.intellij.codeInspection.bytecodeAnalysis.asm.RichControlFlow;
@@ -199,17 +200,18 @@ final class State {
final boolean hasCompanions;
/**
* Whether this state was reached via an exceptional path (jump to catch block)
* Whether we are unsure that this state can be reached at all (e.g.
* it goes via exceptional path and we don't known whether this exception may actually happen).
*/
final boolean exceptional;
final boolean unsure;
State(int index, Conf conf, List<Conf> history, boolean taken, boolean hasCompanions, boolean exceptional) {
State(int index, Conf conf, List<Conf> history, boolean taken, boolean hasCompanions, boolean unsure) {
this.index = index;
this.conf = conf;
this.history = history;
this.taken = taken;
this.hasCompanions = hasCompanions;
this.exceptional = exceptional;
this.unsure = unsure;
}
}
@@ -249,7 +251,7 @@ abstract class Analysis<Res> {
if (curr.taken != prev.taken) {
return false;
}
if (curr.exceptional != prev.exceptional) {
if (curr.unsure != prev.unsure) {
return false;
}
if (curr.conf.fastHashCode != prev.conf.fastHashCode) {
@@ -304,6 +306,14 @@ abstract class Analysis<Res> {
return frame;
}
@NotNull
static Frame<BasicValue> createCatchFrame(Frame<BasicValue> frame) {
Frame<BasicValue> catchFrame = new Frame<>(frame);
catchFrame.clearStack();
catchFrame.push(ASMUtils.THROWABLE_VALUE);
return catchFrame;
}
static BasicValue popValue(Frame<BasicValue> frame) {
return frame.getStack(frame.getStackSize() - 1);
}
java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Contracts.java
+33 -23
View File
@@ -16,7 +16,6 @@
package com.intellij.codeInspection.bytecodeAnalysis;
import com.intellij.codeInspection.bytecodeAnalysis.Direction.ParamValueBasedDirection;
import com.intellij.codeInspection.bytecodeAnalysis.asm.ASMUtils;
import com.intellij.codeInspection.bytecodeAnalysis.asm.ControlFlowGraph.Edge;
import com.intellij.codeInspection.bytecodeAnalysis.asm.RichControlFlow;
import com.intellij.openapi.progress.ProgressManager;
@@ -48,7 +47,7 @@ abstract class ContractAnalysis extends Analysis<Result> {
final Value inValue;
private final int generalizeShift;
Result internalResult;
boolean exceptionalOnly = true;
boolean unsureOnly = true;
private int id;
private int pendingTop;
@@ -121,7 +120,7 @@ abstract class ContractAnalysis extends Analysis<Result> {
}
if (earlyResult != null) {
return mkEquation(earlyResult);
} else if (exceptionalOnly) {
} else if (unsureOnly) {
// We are not sure whether exceptional paths were actually taken or not
// probably they handle exceptions which can never be thrown before dereference occurs
return mkEquation(ClassDataIndexer.FINAL_BOT);
@@ -145,46 +144,59 @@ abstract class ContractAnalysis extends Analysis<Result> {
addComputed(insnIndex, state);
int opcode = insnNode.getOpcode();
if (handleReturn(frame, opcode, state.exceptional)) return;
if (interpreter.deReferenced && controlFlow.npeTransitions.containsKey(insnIndex)) {
interpreter.deReferenced = false;
int npeTarget = controlFlow.npeTransitions.get(insnIndex);
for (int nextInsnIndex : controlFlow.transitions[insnIndex]) {
if (!controlFlow.errorTransitions.contains(new Edge(insnIndex, nextInsnIndex))) continue;
Frame<BasicValue> nextFrame1 = createCatchFrame(frame);
boolean unsure = state.unsure || nextInsnIndex != npeTarget;
pendingPush(new State(++id, new Conf(nextInsnIndex, nextFrame1), nextHistory, taken, false, unsure));
}
return;
}
if (handleReturn(frame, opcode, state.unsure)) return;
if (opcode == IFNONNULL && popValue(frame) instanceof ParamValue) {
int nextInsnIndex = inValue == Value.Null ? insnIndex + 1 : methodNode.instructions.indexOf(((JumpInsnNode)insnNode).label);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false, state.exceptional);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false, state.unsure);
pendingPush(nextState);
return;
}
if (opcode == IFNULL && popValue(frame) instanceof ParamValue) {
int nextInsnIndex = inValue == Value.NotNull ? insnIndex + 1 : methodNode.instructions.indexOf(((JumpInsnNode)insnNode).label);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false, state.exceptional);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false, state.unsure);
pendingPush(nextState);
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, state.exceptional);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false, state.unsure);
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, state.exceptional);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false, state.unsure);
pendingPush(nextState);
return;
}
if (opcode == IFEQ && popValue(frame) == InstanceOfCheckValue && inValue == Value.Null) {
int nextInsnIndex = methodNode.instructions.indexOf(((JumpInsnNode)insnNode).label);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false, state.exceptional);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false, state.unsure);
pendingPush(nextState);
return;
}
if (opcode == IFNE && popValue(frame) == InstanceOfCheckValue && inValue == Value.Null) {
int nextInsnIndex = insnIndex + 1;
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false, state.exceptional);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, false, state.unsure);
pendingPush(nextState);
return;
}
@@ -192,18 +204,16 @@ abstract class ContractAnalysis extends Analysis<Result> {
// general case
for (int nextInsnIndex : controlFlow.transitions[insnIndex]) {
Frame<BasicValue> nextFrame1 = nextFrame;
boolean exceptional = state.exceptional;
boolean unsure = state.unsure;
if (controlFlow.errors[nextInsnIndex] && controlFlow.errorTransitions.contains(new Edge(insnIndex, nextInsnIndex))) {
nextFrame1 = new Frame<>(frame);
nextFrame1.clearStack();
nextFrame1.push(ASMUtils.THROWABLE_VALUE);
exceptional = true;
nextFrame1 = createCatchFrame(frame);
unsure = true;
}
pendingPush(new State(++id, new Conf(nextInsnIndex, nextFrame1), nextHistory, taken, false, exceptional));
pendingPush(new State(++id, new Conf(nextInsnIndex, nextFrame1), nextHistory, taken, false, unsure));
}
}
abstract boolean handleReturn(Frame<BasicValue> frame, int opcode, boolean exceptional) throws AnalyzerException;
abstract boolean handleReturn(Frame<BasicValue> frame, int opcode, boolean unsure) throws AnalyzerException;
private void pendingPush(State st) {
TooComplexException.check(method, pendingTop);
@@ -263,7 +273,7 @@ class InOutAnalysis extends ContractAnalysis {
super(richControlFlow, direction, resultOrigins, stable, pending);
}
boolean handleReturn(Frame<BasicValue> frame, int opcode, boolean exceptional) throws AnalyzerException {
boolean handleReturn(Frame<BasicValue> frame, int opcode, boolean unsure) throws AnalyzerException {
if (interpreter.deReferenced) {
return true;
}
@@ -300,8 +310,8 @@ class InOutAnalysis extends ContractAnalysis {
return true;
}
internalResult = checkLimit(resultUtil.join(internalResult, subResult));
exceptionalOnly &= exceptional;
if (!exceptional && internalResult instanceof Final && ((Final)internalResult).value == Value.Top) {
unsureOnly &= unsure;
if (!unsure && internalResult instanceof Final && ((Final)internalResult).value == Value.Top) {
earlyResult = internalResult;
}
return true;
@@ -325,7 +335,7 @@ class InThrowAnalysis extends ContractAnalysis {
super(richControlFlow, direction, resultOrigins, stable, pending);
}
boolean handleReturn(Frame<BasicValue> frame, int opcode, boolean exceptional) {
boolean handleReturn(Frame<BasicValue> frame, int opcode, boolean unsure) {
Result subResult;
if (interpreter.deReferenced) {
subResult = new Final(Value.Top);
@@ -356,8 +366,8 @@ class InThrowAnalysis extends ContractAnalysis {
}
}
internalResult = resultUtil.join(internalResult, subResult);
exceptionalOnly &= exceptional;
if (!exceptional && internalResult instanceof Final && ((Final)internalResult).value == Value.Top && myHasNonTrivialReturn) {
unsureOnly &= unsure;
if (!unsure && internalResult instanceof Final && ((Final)internalResult).value == Value.Top && myHasNonTrivialReturn) {
earlyResult = internalResult;
}
return true;
java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Parameters.java
+7 -12
View File
@@ -15,7 +15,6 @@
*/
package com.intellij.codeInspection.bytecodeAnalysis;
import com.intellij.codeInspection.bytecodeAnalysis.asm.ASMUtils;
import com.intellij.codeInspection.bytecodeAnalysis.asm.ControlFlowGraph.Edge;
import com.intellij.codeInspection.bytecodeAnalysis.asm.RichControlFlow;
import org.jetbrains.annotations.NotNull;
@@ -323,7 +322,7 @@ class NonNullInAnalysis extends Analysis<PResult> {
if (opcode == IFNONNULL && popValue(frame) instanceof ParamValue) {
int nextInsnIndex = insnIndex + 1;
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, hasCompanions || notEmptySubResult, state.exceptional);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, hasCompanions || notEmptySubResult, state.unsure);
pendingPush(new MakeResult(state, subResult, new int[]{nextState.index}));
pendingPush(new ProceedState(nextState));
return;
@@ -331,7 +330,7 @@ class NonNullInAnalysis extends Analysis<PResult> {
if (opcode == IFNULL && popValue(frame) instanceof ParamValue) {
int nextInsnIndex = methodNode.instructions.indexOf(((JumpInsnNode)insnNode).label);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, hasCompanions || notEmptySubResult, state.exceptional);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, hasCompanions || notEmptySubResult, state.unsure);
pendingPush(new MakeResult(state, subResult, new int[]{nextState.index}));
pendingPush(new ProceedState(nextState));
return;
@@ -339,7 +338,7 @@ class NonNullInAnalysis extends Analysis<PResult> {
if (opcode == IFEQ && popValue(frame) == InstanceOfCheckValue) {
int nextInsnIndex = methodNode.instructions.indexOf(((JumpInsnNode)insnNode).label);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, hasCompanions || notEmptySubResult, state.exceptional);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, hasCompanions || notEmptySubResult, state.unsure);
pendingPush(new MakeResult(state, subResult, new int[]{nextState.index}));
pendingPush(new ProceedState(nextState));
return;
@@ -347,7 +346,7 @@ class NonNullInAnalysis extends Analysis<PResult> {
if (opcode == IFNE && popValue(frame) == InstanceOfCheckValue) {
int nextInsnIndex = insnIndex + 1;
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, hasCompanions || notEmptySubResult, state.exceptional);
State nextState = new State(++id, new Conf(nextInsnIndex, nextFrame), nextHistory, true, hasCompanions || notEmptySubResult, state.unsure);
pendingPush(new MakeResult(state, subResult, new int[]{nextState.index}));
pendingPush(new ProceedState(nextState));
return;
@@ -363,11 +362,9 @@ class NonNullInAnalysis extends Analysis<PResult> {
for (int i = 0; i < nextInsnIndices.length; i++) {
int nextInsnIndex = nextInsnIndices[i];
Frame<BasicValue> nextFrame1 = nextFrame;
boolean exceptional = state.exceptional;
boolean exceptional = state.unsure;
if (controlFlow.errors[nextInsnIndex] && controlFlow.errorTransitions.contains(new Edge(insnIndex, nextInsnIndex))) {
nextFrame1 = new Frame<>(frame);
nextFrame1.clearStack();
nextFrame1.push(ASMUtils.THROWABLE_VALUE);
nextFrame1 = createCatchFrame(frame);
exceptional = true;
}
pendingPush(new ProceedState(new State(subIndices[i], new Conf(nextInsnIndex, nextFrame1), nextHistory, taken, hasCompanions || notEmptySubResult,
@@ -550,9 +547,7 @@ class NullableInAnalysis extends Analysis<PResult> {
for (int nextInsnIndex : controlFlow.transitions[insnIndex]) {
Frame<BasicValue> nextFrame1 = nextFrame;
if (controlFlow.errors[nextInsnIndex] && controlFlow.errorTransitions.contains(new Edge(insnIndex, nextInsnIndex))) {
nextFrame1 = new Frame<>(frame);
nextFrame1.clearStack();
nextFrame1.push(ASMUtils.THROWABLE_VALUE);
nextFrame1 = createCatchFrame(frame);
}
pendingPush(new State(++id, new Conf(nextInsnIndex, nextFrame1), nextHistory, taken, false, false));
}
java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/asm/ControlFlowGraph.java
+22 -4
View File
@@ -17,6 +17,7 @@ package com.intellij.codeInspection.bytecodeAnalysis.asm;
import com.intellij.codeInspection.bytecodeAnalysis.asm.ControlFlowGraph.Edge;
import gnu.trove.TIntArrayList;
import gnu.trove.TIntIntHashMap;
import org.jetbrains.org.objectweb.asm.tree.MethodNode;
import org.jetbrains.org.objectweb.asm.tree.analysis.AnalyzerException;
@@ -60,14 +61,25 @@ public final class ControlFlowGraph {
public final int edgeCount;
public final boolean[] errors;
public final Set<Edge> errorTransitions;
/**
* Where execution goes if NPE occurs at given instruction
*/
public final TIntIntHashMap npeTransitions;
ControlFlowGraph(String className, MethodNode methodNode, int[][] transitions, int edgeCount, boolean[] errors, Set<Edge> errorTransitions) {
ControlFlowGraph(String className,
MethodNode methodNode,
int[][] transitions,
int edgeCount,
boolean[] errors,
Set<Edge> errorTransitions,
TIntIntHashMap npeTransitions) {
this.className = className;
this.methodNode = methodNode;
this.transitions = transitions;
this.edgeCount = edgeCount;
this.errors = errors;
this.errorTransitions = errorTransitions;
this.npeTransitions = npeTransitions;
}
public static ControlFlowGraph build(String className, MethodNode methodNode, boolean jsr) throws AnalyzerException {
@@ -80,6 +92,7 @@ final class ControlFlowBuilder implements FramelessAnalyzer.EdgeCreator {
final MethodNode methodNode;
final TIntArrayList[] transitions;
final Set<ControlFlowGraph.Edge> errorTransitions;
final TIntIntHashMap npeTransitions;
final FramelessAnalyzer myAnalyzer;
private final boolean[] errors;
private int edgeCount;
@@ -94,6 +107,7 @@ final class ControlFlowBuilder implements FramelessAnalyzer.EdgeCreator {
transitions[i] = new TIntArrayList();
}
errorTransitions = new HashSet<>();
npeTransitions = new TIntIntHashMap();
}
final ControlFlowGraph buildCFG() throws AnalyzerException {
@@ -104,7 +118,7 @@ final class ControlFlowBuilder implements FramelessAnalyzer.EdgeCreator {
for (int i = 0; i < resultTransitions.length; i++) {
resultTransitions[i] = transitions[i].toNativeArray();
}
return new ControlFlowGraph(className, methodNode, resultTransitions, edgeCount, errors, errorTransitions);
return new ControlFlowGraph(className, methodNode, resultTransitions, edgeCount, errors, errorTransitions, npeTransitions);
}
@Override
@@ -116,11 +130,15 @@ final class ControlFlowBuilder implements FramelessAnalyzer.EdgeCreator {
}
@Override
public final boolean newControlFlowExceptionEdge(int insn, int successor) {
public final boolean newControlFlowExceptionEdge(int insn, int successor, boolean npe) {
if (!transitions[insn].contains(successor)) {
transitions[insn].add(successor);
edgeCount++;
errorTransitions.add(new Edge(insn, successor));
Edge edge = new Edge(insn, successor);
errorTransitions.add(edge);
if(npe && !npeTransitions.containsKey(insn)) {
npeTransitions.put(insn, successor);
}
errors[successor] = true;
}
return true;
java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/asm/FramelessAnalyzer.java
+8 -8
View File
@@ -15,14 +15,12 @@
*/
package com.intellij.codeInspection.bytecodeAnalysis.asm;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.org.objectweb.asm.tree.*;
import org.jetbrains.org.objectweb.asm.tree.analysis.AnalyzerException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.*;
/**
* Specialized version of {@link org.jetbrains.org.objectweb.asm.tree.analysis.Analyzer}.
@@ -30,6 +28,9 @@ import java.util.Map;
* So, the main point here is handling of subroutines (jsr) and try-catch-finally blocks.
*/
public class FramelessAnalyzer extends SubroutineFinder {
private static final Set<String> NPE_HANDLERS = ContainerUtil.set("java/lang/Throwable", "java/lang/Exception",
"java/lang/RuntimeException", "java/lang/NullPointerException");
protected boolean[] wasQueued;
protected boolean[] queued;
protected int[] queue;
@@ -52,8 +53,7 @@ public class FramelessAnalyzer extends SubroutineFinder {
top = 0;
// computes exception handlers for each instruction
for (int i = 0; i < m.tryCatchBlocks.size(); ++i) {
TryCatchBlockNode tcb = m.tryCatchBlocks.get(i);
for (TryCatchBlockNode tcb : m.tryCatchBlocks) {
int begin = insns.indexOf(tcb.start);
int end = insns.indexOf(tcb.end);
for (int j = begin; j < end; ++j) {
@@ -193,7 +193,7 @@ public class FramelessAnalyzer extends SubroutineFinder {
}
protected boolean newControlFlowExceptionEdge(final int insn, final TryCatchBlockNode tcb) {
return myEdgeCreator.newControlFlowExceptionEdge(insn, insns.indexOf(tcb.handler));
return myEdgeCreator.newControlFlowExceptionEdge(insn, insns.indexOf(tcb.handler), NPE_HANDLERS.contains(tcb.type));
}
// -------------------------------------------------------------------------
@@ -244,6 +244,6 @@ public class FramelessAnalyzer extends SubroutineFinder {
interface EdgeCreator {
void newControlFlowEdge(final int insn, final int successor);
boolean newControlFlowExceptionEdge(final int insn, final int successor);
boolean newControlFlowExceptionEdge(final int insn, final int successor, boolean npe);
}
}
java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/javax/swing/annotations.xml
+3
View File
@@ -1008,6 +1008,9 @@
<annotation name='org.jetbrains.annotations.NotNull'/>
</item>
<item name='javax.swing.JTable.GenericEditor java.awt.Component getTableCellEditorComponent(javax.swing.JTable, java.lang.Object, boolean, int, int)'>
<annotation name='org.jetbrains.annotations.Contract'>
<val val="&quot;null,_,_,_,_-&gt;null&quot;"/>
</annotation>
<annotation name='org.jetbrains.annotations.Nullable'/>
</item>
<item name='javax.swing.JTable.GenericEditor java.lang.Object getCellEditorValue()'>
java/java-tests/testData/codeInspection/bytecodeAnalysis/annotations/org/apache/velocity/util/annotations.xml
+3
View File
@@ -164,6 +164,9 @@
</annotation>
</item>
<item name='org.apache.velocity.util.StringUtils java.lang.String stackTrace(java.lang.Throwable)'>
<annotation name='org.jetbrains.annotations.Contract'>
<val val="&quot;null-&gt;null&quot;"/>
</annotation>
<annotation name='org.jetbrains.annotations.Nullable'/>
</item>
<item name='org.apache.velocity.util.StringUtils java.lang.String sub(java.lang.String, java.lang.String, java.lang.String)'>
java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/data/Test01.java
+35
View File
@@ -18,9 +18,11 @@ package com.intellij.java.codeInspection.bytecodeAnalysis.data;
import com.intellij.java.codeInspection.bytecodeAnalysis.ExpectContract;
import com.intellij.java.codeInspection.bytecodeAnalysis.ExpectNotNull;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.lang.reflect.Array;
import java.nio.file.Files;
/**
* @author lambdamix
@@ -242,6 +244,16 @@ public class Test01 {
}
}
@ExpectContract("null->null")
String getStringTryNPECatched(String s) {
try {
return String.valueOf(new FileReader(s.trim()).read());
}
catch (Exception ex) {
return null;
}
}
@ExpectContract(pure = true)
void testThrow(@ExpectNotNull String s) {
if(s.isEmpty()) {
@@ -259,4 +271,27 @@ public class Test01 {
}
return s;
}
boolean testCatchBool(File file) {
try {
Files.createDirectories(file.toPath());
return true;
}
catch (IOException ignored) {
}
return false;
}
@ExpectContract("null->false")
boolean testCatchBool2(File file) {
try {
Files.createDirectories(file.toPath());
return true;
}
catch (Throwable ignored) {
}
return false;
}
}