From 3d0fc895443ddd85d6ba91e37b41df664012bda3 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 9 Mar 2021 12:16:30 +0700 Subject: [PATCH] [java-dfa] Apply relations like a>b on a=b+c if c>0 & no overflow GitOrigin-RevId: 604d68f6741b9fde615067e3295ac7dce7cea286 --- .../dataFlow/DfaMemoryStateImpl.java | 138 ++++++++++-------- .../dataFlow/rangeSet/LongRangeSet.java | 14 +- .../dataFlow/fixture/RelationsOnAddition.java | 19 +++ .../DataFlowRangeAnalysisTest.java | 1 + 4 files changed, 111 insertions(+), 61 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/RelationsOnAddition.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index 7e2b06bb02b6..3cfe4b9698ad 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -231,6 +231,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { checkEphemeral(var, value); } recordVariableType(var, dfType); + applyBinOpRelations(value, RelationType.EQ, var); applyRelation(var, value, false); Couple specialFields = getSpecialEquivalencePair(var, value); if (specialFields != null && specialFields.getFirst() instanceof DfaVariableValue) { @@ -772,74 +773,93 @@ public class DfaMemoryStateImpl implements DfaMemoryState { private boolean applyBinOpRelations(DfaValue left, RelationType type, DfaValue right) { if (type != RelationType.LT && type != RelationType.GT && type != RelationType.NE && type != RelationType.EQ) return true; - if (left instanceof DfaBinOpValue) { - DfaBinOpValue sum = (DfaBinOpValue)left; - LongRangeBinOp op = sum.getOperation(); - if (op != LongRangeBinOp.PLUS && op != LongRangeBinOp.MINUS) return true; - LongRangeSet leftRange = DfLongType.extractRange(getDfType(sum.getLeft())); - LongRangeSet rightRange = DfLongType.extractRange(getDfType(sum.getRight())); - boolean isLong = PsiType.LONG.equals(sum.getType()); - LongRangeSet rightNegated = rightRange.negate(isLong); - LongRangeSet rightCorrected = op == LongRangeBinOp.MINUS ? rightNegated : rightRange; + if (!(left instanceof DfaBinOpValue)) { + if (right instanceof DfaBinOpValue) { + return applyBinOpRelations(right, type.getFlipped(), left); + } + return true; + } + DfaBinOpValue binOp = (DfaBinOpValue)left; + LongRangeBinOp op = binOp.getOperation(); + if (op != LongRangeBinOp.PLUS && op != LongRangeBinOp.MINUS) return true; + DfaVariableValue leftLeft = binOp.getLeft(); + DfaValue leftRight = binOp.getRight(); + LongRangeSet leftRange = DfLongType.extractRange(getDfType(leftLeft)); + LongRangeSet rightRange = DfLongType.extractRange(getDfType(leftRight)); + boolean isLong = PsiType.LONG.equals(binOp.getType()); + LongRangeSet rightNegated = rightRange.negate(isLong); + LongRangeSet rightCorrected = op == LongRangeBinOp.MINUS ? rightNegated : rightRange; - LongRangeSet resultRange = DfLongType.extractRange(getDfType(right)); - RelationType correctedRelation = correctRelation(type, leftRange, rightCorrected, resultRange, isLong); - if (op == LongRangeBinOp.MINUS) { - long min = resultRange.min(); - long max = resultRange.max(); - if (min == 0 && max == 0) { - // a-b (rel) 0 => a (rel) b - if (!applyCondition(sum.getLeft().cond(correctedRelation, sum.getRight()))) return false; - } - else if (min == 0 && type == RelationType.GT || min >= 1 && RelationType.GE.isSubRelation(type)) { - RelationType correctedGt = correctRelation(RelationType.GT, leftRange, rightCorrected, resultRange, isLong); - if (!applyCondition(sum.getLeft().cond(correctedGt, sum.getRight()))) return false; - } - else if (max == 0 && type == RelationType.LT || max <= -1 && RelationType.LE.isSubRelation(type)) { - RelationType correctedLt = correctRelation(RelationType.LT, leftRange, rightCorrected, resultRange, isLong); - if (!applyCondition(sum.getLeft().cond(correctedLt, sum.getRight()))) return false; - } - if (RelationType.EQ.equals(type) && !resultRange.contains(0)) { - // a-b == non-zero => a != b - if (!applyRelation(sum.getLeft(), sum.getRight(), true)) return false; - } + LongRangeSet resultRange = DfLongType.extractRange(getDfType(right)); + RelationType correctedRelation = correctRelation(type, leftRange, rightCorrected, resultRange, isLong); + if (op == LongRangeBinOp.MINUS) { + long min = resultRange.min(); + long max = resultRange.max(); + if (min == 0 && max == 0) { + // a-b (rel) 0 => a (rel) b + if (!applyCondition(leftLeft.cond(correctedRelation, leftRight))) return false; } - if (op == LongRangeBinOp.PLUS && RelationType.EQ == type && - !resultRange.intersects(LongRangeSet.all().mul(LongRangeSet.point(2), true))) { - // a+b == odd => a != b - if (!applyRelation(sum.getLeft(), sum.getRight(), true)) return false; + else if (min == 0 && type == RelationType.GT || min >= 1 && RelationType.GE.isSubRelation(type)) { + RelationType correctedGt = correctRelation(RelationType.GT, leftRange, rightCorrected, resultRange, isLong); + if (!applyCondition(leftLeft.cond(correctedGt, leftRight))) return false; + } + else if (max == 0 && type == RelationType.LT || max <= -1 && RelationType.LE.isSubRelation(type)) { + RelationType correctedLt = correctRelation(RelationType.LT, leftRange, rightCorrected, resultRange, isLong); + if (!applyCondition(leftLeft.cond(correctedLt, leftRight))) return false; + } + if (RelationType.EQ.equals(type) && !resultRange.contains(0)) { + // a-b == non-zero => a != b + if (!applyRelation(leftLeft, leftRight, true)) return false; + } + } + if (op == LongRangeBinOp.PLUS && RelationType.EQ == type && + !resultRange.intersects(LongRangeSet.all().mul(LongRangeSet.point(2), true))) { + // a+b == odd => a != b + if (!applyRelation(leftLeft, leftRight, true)) return false; + } + if (right instanceof DfaVariableValue) { + // a+b (rel) c && a == c => b (rel) 0 + if (areEqual(leftLeft, right)) { + RelationType finalRelation = op == LongRangeBinOp.MINUS ? + Objects.requireNonNull(correctedRelation.getFlipped()) : correctedRelation; + if (!applyCondition(leftRight.cond(finalRelation, myFactory.getInt(0)))) return false; + } + // a+b (rel) c && b == c => a (rel) 0 + if (op == LongRangeBinOp.PLUS && areEqual(leftRight, right)) { + if (!applyCondition(leftLeft.cond(correctedRelation, myFactory.getInt(0)))) return false; } - if (right instanceof DfaVariableValue) { - // a+b (rel) c && a == c => b (rel) 0 - if (areEqual(sum.getLeft(), right)) { - RelationType finalRelation = op == LongRangeBinOp.MINUS ? - Objects.requireNonNull(correctedRelation.getFlipped()) : correctedRelation; - if (!applyCondition(sum.getRight().cond(finalRelation, myFactory.getInt(0)))) return false; - } - // a+b (rel) c && b == c => a (rel) 0 - if (op == LongRangeBinOp.PLUS && areEqual(sum.getRight(), right)) { - if (!applyCondition(sum.getLeft().cond(correctedRelation, myFactory.getInt(0)))) return false; - } - if (!leftRange.subtractionMayOverflow(op == LongRangeBinOp.MINUS ? rightRange : rightNegated, isLong)) { - // a-positiveNumber >= b => a > b - if (rightCorrected.max() < 0 && RelationType.GE.isSubRelation(type)) { - if (!applyLessThanRelation(right, sum.getLeft())) return false; - } - // a+positiveNumber >= b => a > b - if (rightCorrected.min() > 0 && RelationType.LE.isSubRelation(type)) { - if (!applyLessThanRelation(sum.getLeft(), right)) return false; - } - } - if (RelationType.EQ == type && !rightRange.contains(0)) { - // a+nonZero == b => a != b - if (!applyRelation(sum.getLeft(), right, true)) return false; - } + if (!applyRelationOnAddition(type, leftLeft, leftRange, rightCorrected, right, isLong)) return false; + if (op == LongRangeBinOp.PLUS && leftRight instanceof DfaVariableValue) { + if (!applyRelationOnAddition(type, (DfaVariableValue)leftRight, rightRange, leftRange, right, isLong)) return false; } } return true; } + private boolean applyRelationOnAddition(@NotNull RelationType type, + @NotNull DfaVariableValue left, + @NotNull LongRangeSet leftRange, + @NotNull LongRangeSet rightRange, + @NotNull DfaValue sum, + boolean isLong) { + if (!leftRange.additionMayOverflow(rightRange, isLong)) { + // a-positiveNumber >= b => a > b + if (rightRange.max() < 0 && RelationType.GE.isSubRelation(type)) { + if (!applyLessThanRelation(sum, left)) return false; + } + // a+positiveNumber >= b => a > b + if (rightRange.min() > 0 && RelationType.LE.isSubRelation(type)) { + if (!applyLessThanRelation(left, sum)) return false; + } + } + if (RelationType.EQ == type && !rightRange.contains(0)) { + // a+nonZero == b => a != b + if (!applyRelation(left, sum, true)) return false; + } + return true; + } + private static RelationType correctRelation(RelationType relation, LongRangeSet summand1, LongRangeSet summand2, LongRangeSet resultRange, boolean isLong) { if (relation != RelationType.LT && relation != RelationType.GT) return relation; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/rangeSet/LongRangeSet.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/rangeSet/LongRangeSet.java index 9f5f8703d634..ed93c2c40867 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/rangeSet/LongRangeSet.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/rangeSet/LongRangeSet.java @@ -379,13 +379,23 @@ public abstract class LongRangeSet { return result; } + /** + * Checks whether addition of this and other range may overflow + * @param other range to add to this range + * @param isLong whether addition should be performed on long values (otherwise int is assumed) + * @return true if result may overflow, false if it never overflows + */ + public boolean additionMayOverflow(@NotNull LongRangeSet other, boolean isLong) { + return subtractionMayOverflow(other.negate(isLong), isLong); + } + /** * Checks whether subtraction of this and other range may overflow * @param other range to subtract from this range * @param isLong whether subtraction should be performed on long values (otherwise int is assumed) * @return true if result may overflow, false if it never overflows */ - public boolean subtractionMayOverflow(LongRangeSet other, boolean isLong) { + public boolean subtractionMayOverflow(@NotNull LongRangeSet other, boolean isLong) { long leftMin = min(); long leftMax = max(); long rightMin = other.min(); @@ -1863,7 +1873,7 @@ public abstract class LongRangeSet { long bits = rotateRemainders(myBits, myMod, myMod - bit); for (int i = 0; i < ranges.length; i += 2) { LongRangeSet plus; - if (Integer.bitCount(myMod) == 1 || !subtractionMayOverflow(other.negate(isLong), isLong)) { + if (Integer.bitCount(myMod) == 1 || !additionMayOverflow(other, isLong)) { plus = modRange(ranges[i], ranges[i + 1], myMod, bits); } else { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/RelationsOnAddition.java b/java/java-tests/testData/inspection/dataFlow/fixture/RelationsOnAddition.java new file mode 100644 index 000000000000..f89d6c4ef9f1 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/RelationsOnAddition.java @@ -0,0 +1,19 @@ +import java.util.*; + +public class RelationsOnAddition { + void test(int a, int b) { + if (a <= 0 || a > 1000) return; + if (b <= 0 || b > 1000) return; + int c = a + b; + if (c > a) {} + if (c > b) {} + } + + void test1(String s1, String s2) { + if (s1.isEmpty()) return; + int sum = s1.length() + s2.length(); + // may overflow + if (sum < s2.length()) {} + if (sum == s2.length()) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java index 90d3e2b38dad..9987daca393c 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java @@ -78,4 +78,5 @@ public class DataFlowRangeAnalysisTest extends DataFlowInspectionTestCase { public void testWidenMismatch() { doTest(); } public void testDontWidenPlusInLoop() { doTest(); } public void testCollectionAddRemove() { doTest(); } + public void testRelationsOnAddition() { doTest(); } }