From c79166e1bb87ab8e0dc442d0da97dc90eb2e9276 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 6 Oct 2021 10:30:46 +0700 Subject: [PATCH] [java-dfa] Flush mutable values from stack on call GitOrigin-RevId: 64af8932d62f74966222a24cdb87472091f8a661 --- .../dataFlow/java/inst/MethodCallInstruction.java | 10 +++++++++- .../fixture/GettersAndPureNoFlushing.java | 15 ++++++++++++++- .../dataFlow/fixture/PrimitiveGetters.java | 1 + .../fixture/StringBuilderLengthReturn.java | 11 +++++++++++ .../codeInspection/DataFlowInspectionTest.java | 1 + .../dataFlow/memory/DfaMemoryStateImpl.java | 3 +++ 6 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/StringBuilderLengthReturn.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java index fa2ea8a49cbb..5907194fe0bd 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java @@ -239,8 +239,16 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { DfaValue[] args = callArguments.toArray(); for (DfaMemoryState state : finalStates) { ContractValue.flushContractTempVariables(state); + boolean keepNonFlushed = state.peek() instanceof DfaVariableValue; + DfaValue tos = null; + if (keepNonFlushed) { + tos = state.pop(); + } callArguments.flush(state, factory, realMethod); - pushResult(interpreter, state, state.pop(), args); + if (!keepNonFlushed) { + tos = state.pop(); + } + pushResult(interpreter, state, tos, args); result[i++] = nextState(interpreter, state); } return result; diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/GettersAndPureNoFlushing.java b/java/java-tests/testData/inspection/dataFlow/fixture/GettersAndPureNoFlushing.java index 6dd08ca4ffd7..11ae0904c687 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/GettersAndPureNoFlushing.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/GettersAndPureNoFlushing.java @@ -6,6 +6,10 @@ class Doo { @Nullable Object getMethod() {return null;} + @Nullable + @Contract(pure=true) + Object getMethodPure() {return null;} + boolean isSomething() { return false;} @Contract(pure=true) @@ -24,7 +28,16 @@ class Doo { if (getMethod() == null && !pureSomething()) { return; } else { - System.out.println(getMethod().hashCode()); + // still not sure about nullability as getMethod() is not pure + System.out.println(getMethod().hashCode()); + } + } + + public void main4() { + if (getMethodPure() == null && !pureSomething()) { + return; + } else { + System.out.println(getMethodPure().hashCode()); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/PrimitiveGetters.java b/java/java-tests/testData/inspection/dataFlow/fixture/PrimitiveGetters.java index 652905a9dde9..dfbabdd650e1 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/PrimitiveGetters.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/PrimitiveGetters.java @@ -3,6 +3,7 @@ import java.util.*; public class PrimitiveGetters { interface Xyz { + @Contract(pure = true) boolean isFoo(); } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StringBuilderLengthReturn.java b/java/java-tests/testData/inspection/dataFlow/fixture/StringBuilderLengthReturn.java new file mode 100644 index 000000000000..b09a4be0b9b3 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StringBuilderLengthReturn.java @@ -0,0 +1,11 @@ +public class StringBuilderLengthReturn { + private static StringBuilder update(StringBuilder sb) { + sb.append("xyz"); + return sb; + } + + void test(StringBuilder sb) { + // No 'always zero' warning + int diff = sb.length() - update(sb).length(); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index 65158eb939d5..b5565847c3c5 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -714,4 +714,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testEnumOrdinal() { doTest(); } public void testThisInEnumSubclass() { doTest(); } public void testVarargConstructorNoArgs() { doTest(); } + public void testStringBuilderLengthReturn() { doTest(); } } diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java index 4e43e860daf9..ee628d7ec795 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java +++ b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java @@ -1322,6 +1322,9 @@ public class DfaMemoryStateImpl implements DfaMemoryState { !dv.isStable() && qualifierStatusMap.shouldFlush(val, dv.isCall()))) { return myFactory.fromDfType(type.getBasicType()); } + if (val instanceof DfaVariableValue && qualifierStatusMap.shouldFlush((DfaVariableValue)val)) { + return myFactory.fromDfType(type); + } return val; }); }