diff --git a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties index 544b0635c4aa..b098e21ca8d0 100644 --- a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties +++ b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties @@ -30,6 +30,7 @@ change.type.arguments.to.0=Change type arguments to <{0}> convert.0.to.float=Convert ''{0}'' to float dataflow.message.array.index.out.of.bounds=Array index is out of bounds +dataflow.message.negative.array.size=Negative array size dataflow.message.arraystore=Storing element of type {0} to array of {1} elements will produce ArrayStoreException dataflow.message.assigning.null.notannotated=Assigning null value to non-annotated field dataflow.message.assigning.null=null is assigned to a variable that is annotated with @NotNull diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index ceedc0a7e575..d44843547afa 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -1734,14 +1734,20 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(new PushInstruction(length, null, true)); // stack: ... var.length final PsiExpression[] dimensions = expression.getArrayDimensions(); - if (dimensions.length > 0) { - boolean sizeOnStack = false; + int dims = dimensions.length; + if (dims > 0) { for (final PsiExpression dimension : dimensions) { dimension.accept(this); - if (sizeOnStack) { + generateBoxingUnboxingInstructionFor(dimension, PsiType.INT); + } + DfaControlTransferValue transfer = + shouldHandleException() ? + myFactory.controlTransfer(myExceptionCache.get("java.lang.NegativeArraySizeException"), myTrapStack) : null; + for (int i = dims - 1; i >= 0; i--) { + addInstruction(new ArraySizeCheckInstruction(dimensions[i], transfer)); + if (i != 0) { addInstruction(new PopInstruction()); } - sizeOnStack = true; } } else { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index d39a3a66f3e9..6e70179e7f86 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -641,6 +641,9 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec holder.registerProblem(indexExpression, JavaAnalysisBundle.message("dataflow.message.array.index.out.of.bounds")); } }); + visitor.negativeArraySizes().forEach(dimExpression -> { + holder.registerProblem(dimExpression, JavaAnalysisBundle.message("dataflow.message.negative.array.size")); + }); } private static void reportArrayStoreProblems(ProblemsHolder holder, DataFlowInstructionVisitor visitor) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java index 35c55c8810cb..ba2a8751a599 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java @@ -43,6 +43,7 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { private final Map> myArrayStoreProblems = new HashMap<>(); private final Map myMethodReferenceResults = new HashMap<>(); private final Map myOutOfBoundsArrayAccesses = new HashMap<>(); + private final Map myNegativeArraySizes = new HashMap<>(); private final Set myReceiverMutabilityViolation = new HashSet<>(); private final Set myArgumentMutabilityViolation = new HashSet<>(); private final Map mySameValueAssigned = new HashMap<>(); @@ -193,6 +194,10 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { return StreamEx.ofKeys(myOutOfBoundsArrayAccesses, ThreeState.YES::equals); } + Stream negativeArraySizes() { + return StreamEx.ofKeys(myNegativeArraySizes, ThreeState.YES::equals); + } + StreamEx alwaysFailingCalls() { return StreamEx.ofKeys(myFailingCalls, v -> v); } @@ -280,6 +285,11 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { myOutOfBoundsArrayAccesses.merge(expression, ThreeState.fromBoolean(alwaysOutOfBounds), ThreeState::merge); } + @Override + protected void processArrayCreation(PsiExpression expression, boolean alwaysNegative) { + myNegativeArraySizes.merge(expression, ThreeState.fromBoolean(alwaysNegative), ThreeState::merge); + } + @Override protected void processArrayStoreTypeMismatch(PsiAssignmentExpression assignmentExpression, PsiType fromType, PsiType toType) { if (assignmentExpression != null) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java index 88168dedb35b..1f7eafa741f4 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java @@ -304,4 +304,8 @@ public abstract class InstructionVisitor { pushExpressionResult(runner.getFactory().getUnknown(), instruction, memState); return nextInstruction(instruction, runner, memState); } + + public DfaInstructionState[] visitArraySizeCheck(ArraySizeCheckInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { + return nextInstruction(instruction, runner, memState); + } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index e592e4cef5d4..6a6d9eebb47a 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -214,6 +214,10 @@ public class StandardInstructionVisitor extends InstructionVisitor { } + protected void processArrayCreation(PsiExpression expression, boolean alwaysNegative) { + + } + @Override public DfaInstructionState[] visitMethodReference(MethodReferenceInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { PsiMethodReferenceExpression expression = instruction.getExpression(); @@ -285,6 +289,42 @@ public class StandardInstructionVisitor extends InstructionVisitor { return new DfaCallArguments(qualifier, arguments, MutationSignature.fromMethod(method)); } + @Override + public DfaInstructionState[] visitArraySizeCheck(ArraySizeCheckInstruction instruction, + DataFlowRunner runner, DfaMemoryState memState) { + DfaValue arraySize = memState.peek(); + DfaControlTransferValue transfer = instruction.getNegativeSizeExceptionTransfer(); + DfaCondition cond = arraySize.cond(RelationType.GE, runner.getFactory().getInt(0)); + Instruction nextInstruction = runner.getInstruction(instruction.getIndex() + 1); + DfaInstructionState nextState = new DfaInstructionState(nextInstruction, memState); + if (cond.equals(DfaCondition.getTrue())) { + return new DfaInstructionState[]{nextState}; + } + if (transfer == null) { + boolean hasNonNegative = memState.applyCondition(cond); + processArrayCreation(instruction.getExpression(), !hasNonNegative); + if (!hasNonNegative) { + return DfaInstructionState.EMPTY_ARRAY; + } + return new DfaInstructionState[]{nextState}; + } + DfaMemoryState negativeSize = memState.createCopy(); + boolean hasNonNegative = memState.applyCondition(cond); + boolean hasNegative = negativeSize.applyCondition(cond.negate()); + List result = new ArrayList<>(); + if (hasNonNegative) { + result.add(nextState); + } + if (hasNegative) { + List states = transfer.dispatch(negativeSize, runner); + for (DfaInstructionState negState : states) { + negState.getMemoryState().markEphemeral(); + } + result.addAll(states); + } + return result.toArray(DfaInstructionState.EMPTY_ARRAY); + } + @Override public DfaInstructionState[] visitTypeCast(TypeCastInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { PsiType type = instruction.getCastTo(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ArraySizeCheckInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ArraySizeCheckInstruction.java new file mode 100644 index 000000000000..6ed2291d5656 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ArraySizeCheckInstruction.java @@ -0,0 +1,39 @@ +// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.codeInspection.dataFlow.instructions; + +import com.intellij.codeInspection.dataFlow.*; +import com.intellij.psi.PsiExpression; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public class ArraySizeCheckInstruction extends Instruction { + final @NotNull PsiExpression myExpression; + final @Nullable DfaControlTransferValue myTransferValue; + + public ArraySizeCheckInstruction(@NotNull PsiExpression expression, + @Nullable DfaControlTransferValue value) { + myExpression = expression; + myTransferValue = value; + } + + public @NotNull PsiExpression getExpression() { + return myExpression; + } + + @Nullable + public DfaControlTransferValue getNegativeSizeExceptionTransfer() { + return myTransferValue; + } + + @Override + public DfaInstructionState[] accept(DataFlowRunner runner, + DfaMemoryState stateBefore, + InstructionVisitor visitor) { + return visitor.visitArraySizeCheck(this, runner, stateBefore); + } + + @Override + public String toString() { + return "CHECK_ARRAY_SIZE"; + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ArrayNegativeSize.java b/java/java-tests/testData/inspection/dataFlow/fixture/ArrayNegativeSize.java new file mode 100644 index 000000000000..3bcb643afaf3 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ArrayNegativeSize.java @@ -0,0 +1,135 @@ +class ArrayNegativeSize { + private static final int VAL1 = -10; + private static int VAL2 = -10; + private static final int VAL3 = -10 * 2; + private static final short VAL4 = -10 * 2; + + public static void main(String[] args) { + if (Math.random() > 0.5) { + int[] array1 = new int[-10]; + } + if (Math.random() > 0.5) { + int[] array2 = createArray(); + } + if (Math.random() > 0.5) { + int[] array3 = new int[]{}; + } + if (Math.random() > 0.5) { + int[] array4 = new int[10 - 100]; + } + if (Math.random() > 0.5) { + int[] array5 = new int[((int) (Integer.valueOf(Integer.MAX_VALUE).longValue() - 100)) + 100 - 10]; + } + if (Math.random() > 0.5) { + int[] array5 = new int[((int) (Integer.valueOf(Integer.MAX_VALUE).longValue() - 100L)) + 100 + 10]; + } + if (Math.random() > 0.5) { + int[] array6 = new int[((int) (Integer.valueOf(Integer.MAX_VALUE).longValue() - 100)) + 99]; + } + if (Math.random() > 0.5) { + int[] array7 = new int[(int) (((long) ((Integer.MAX_VALUE - 100)) + 100) * 2)]; + } + if (Math.random() > 0.5) { + int[] array8 = new int[num()]; + } + if (Math.random() > 0.5) { + int[] array9 = new int[VAL1]; + } + if (Math.random() > 0.5) { + int[] array10 = new int[VAL2]; + } + if (Math.random() > 0.5) { + int[] array11 = new int[VAL3]; + } + if (Math.random() > 0.5) { + int[] array12 = new int[VAL4]; + } + if (Math.random() > 0.5) { + int[][] array13 = new int[0][-1]; + } + if (Math.random() > 0.5) { + int[][][] array14 = new int[0][0][-1]; + } + if (Math.random() > 0.5) { + int[][][] array15 = new int[-1][-2][-3]; + } + if (Math.random() > 0.5) { + int[][][] array15 = new int[-1][-2][3]; + } + if (Math.random() > 0.5) { + int[][][] array15 = new int[-1][2][3]; + } + if (Math.random() > 0.5) { + int[] array16 = new int[-07]; + } + if (Math.random() > 0.5) { + int[] array17 = new int[100 * -077]; + } + if (Math.random() > 0.5) { + int[] array18 = new int[0x7fffffff]; + } + if (Math.random() > 0.5) { + int[] array19 = new int[0x7fffffff + 1]; + } + if (Math.random() > 0.5) { + int[] array20 = new int[0b1111111111111111111111111111111]; + } + if (Math.random() > 0.5) { + int[] array21 = new int[0b1111111111111111111111111111111 + 1]; + } + if (Math.random() > 0.5) { + action(new int[-1000000000]); + } + if (Math.random() > 0.5) { + action(new int[-0xcafe]); + } + if (Math.random() > 0.5) { + action(new int[(int) -10000000000000L]); + } + if (Math.random() > 0.5) { + action(new int[2147483647 + 1]); + } + if (Math.random() > 0.5) { + action(new int["".length() + 456]); + } + if (Math.random() > 0.5) { + action(new int["".length() - 456]); + } + VAL2++; + } + + private static int[] createArray() { + return new int[0]; + } + + private static void action(Object obj) { + } + + private static int num() { + return -123; + } + + final int[] array1 = new int[-10]; + + void foo(int size) { + int[] data = new int[size]; + if (size < 0) { + System.out.println("Impossible"); + } + } + + void tryCatch(int size) { + try { + int[] arr = new int[size]; + } catch (NegativeArraySizeException e) { + if (size >= 0) { + System.out.println("impossible"); + } + } + } + void testUnboxing(Integer len) { + int[] arr = new int[len]; + long l = len.longValue(); + if (l < 0) {} + } +} \ 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 8f3c01f5d767..4944e1c13122 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -685,4 +685,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testDoubleArrayDiff() { doTest(); } public void testInferenceInPrivateOrLocalClass() { doTest(); } public void testArraysCopyOf() { doTest(); } + public void testArrayNegativeSize() { doTest(); } }