From f8cae1b509cdf8bd417a08ea39cf7fdb9c250f59 Mon Sep 17 00:00:00 2001 From: Stephan Gocht Date: Mon, 19 Oct 2015 23:20:36 +0200 Subject: [PATCH] Added Tests and Testdata for array bounds analysis. --- .../src/arraybounds/Detectable.java | 177 ++++++++++++++++++ .../src/arraybounds/NotDetectable.java | 141 ++++++++++++++ .../src/arraybounds/NotInBound.java | 44 +++++ .../arraybounds/ArrayboundsAnalysisTest.java | 124 ++++++++++++ com.ibm.wala.core/META-INF/MANIFEST.MF | 1 + 5 files changed, 487 insertions(+) create mode 100644 com.ibm.wala.core.testdata/src/arraybounds/Detectable.java create mode 100644 com.ibm.wala.core.testdata/src/arraybounds/NotDetectable.java create mode 100644 com.ibm.wala.core.testdata/src/arraybounds/NotInBound.java create mode 100644 com.ibm.wala.core.tests/src/com/ibm/wala/core/tests/arraybounds/ArrayboundsAnalysisTest.java diff --git a/com.ibm.wala.core.testdata/src/arraybounds/Detectable.java b/com.ibm.wala.core.testdata/src/arraybounds/Detectable.java new file mode 100644 index 000000000..0a6f066f2 --- /dev/null +++ b/com.ibm.wala.core.testdata/src/arraybounds/Detectable.java @@ -0,0 +1,177 @@ +package arraybounds; + +/** + * + * All array accesses in the following class are unnecessary and they will be + * detected correctly by the array bounds analysis. + * + * @author Stephan Gocht + * + */ +public class Detectable { + private int[] memberArr = new int[5]; + + /** + * Note: This is correct, even if memberArr is not final! + * + * @param i + * @return memberArr[i] + */ + public int memberLocalGet(int i) { + int[] arr = memberArr; + if (i >= 0 && i < arr.length) { + return arr[i]; + } else { + throw new IllegalArgumentException(); + } + } + + public int[] constantCreation() { + return new int[] { 3, 4, 5 }; + } + + public int get(int i, int[] arr) { + if (i >= 0 && i < arr.length) { + return arr[i]; + } else { + throw new IllegalArgumentException(); + } + } + + public void loop(int[] arr) { + for (int i = 0; i < arr.length; i++) { + arr[i] = 0; + } + } + + public boolean equals(int[] a, int[] b) { + int lenA = a.length; + int lenB = b.length; + boolean result; + + if (lenA == lenB) { + result = true; + for (int i = 0; i < lenA; i++) { + if (a[i] != b[i]) { + result = false; + } + } + } else { + result = false; + } + + return result; + } + + public void copy(int[] src, int[] dst) { + int lenSrc = src.length; + int lenDst = dst.length; + + if (lenSrc < lenDst) { + for (int i = 0; i < lenSrc; i++) { + dst[i] = src[i]; + } + } else { + throw new IllegalArgumentException(); + } + } + + /** + * swaps elements of a and b for all i: 0 <= i < min(a.length, b.length) + * + * @param a + * @param b + */ + public void swapWithMin(int[] a, int[] b) { + final int l1 = a.length; + final int l2 = b.length; + int length; + if (l1 < l2) { + length = l1; + } else { + length = l2; + } + + for (int i = 0; i < length; i++) { + int tmp = a[i]; + a[i] = b[i]; + b[i] = tmp; + } + } + + /** + * Invert the order of all elements of arr with index i: fromIndex <= i < + * toIndex. + * + * @param arr + * @param fromIndex + * @param toIndex + */ + public void partialInvert(int[] arr, int fromIndex, int toIndex) { + if (fromIndex >= 0 && toIndex <= arr.length && fromIndex < toIndex) { + for (int next = fromIndex; next < toIndex; ++next) { + int tmp = arr[toIndex - 1]; + int i; + for (i = toIndex - 1; i > next; --i) { + arr[i] = arr[i - 1]; + } + arr[i] = tmp; // i == next + } + } + } + + /** + * The constant 3 is stored in a variable. The pi construction for the + * variable allows to detect, that the array access is in bound. + * + * Compare to {@link NotDetectable#dueToConstantPropagation(int[])} + * + * @param arr + * @return arr[3] + */ + public int nonFinalConstant(int[] arr) { + int i = 3; + if (i < arr.length) { + return arr[i]; + } else { + throw new IllegalArgumentException(); + } + } + + /** + * Workaround for {@link NotDetectable#constants(int[])} + * + * Note: It is important, that the variable five, is compared directly to the + * length, and that further computations are performed with this variable. + * + * @param arr + * @return arr[3] + */ + public int constantsWorkaround(int[] arr) { + int five = 5; + if (arr.length > five) { + return arr[five - 2]; + } else { + throw new IllegalArgumentException(); + } + } + + /** + * Actually aliasing is only working, because it is removed during + * construction by wala. + * + * @param i + * @param arr + * @return arr[i] + */ + public int aliasing(int i, int[] arr) { + int[] a = arr; + int[] b = arr; + + if (0 <= i && i < a.length) { + return b[i]; + } else { + throw new IllegalArgumentException(); + } + } +} diff --git a/com.ibm.wala.core.testdata/src/arraybounds/NotDetectable.java b/com.ibm.wala.core.testdata/src/arraybounds/NotDetectable.java new file mode 100644 index 000000000..002e376a2 --- /dev/null +++ b/com.ibm.wala.core.testdata/src/arraybounds/NotDetectable.java @@ -0,0 +1,141 @@ +package arraybounds; + +/** + * + * All array accesses in the following class are unnecessary but they will not + * be detected correctly by the array bounds analysis. + * + * @author Stephan Gocht + * + */ +public class NotDetectable { + private final int[] memberArr = new int[5]; + + /** + * Member calls are not supported. See {@link Detectable#memberLocalGet(int)} + * for workaround. + * + * @param i + * @return memberArr[i] + */ + public int memberGet(int i) { + if (i >= 0 && i < memberArr.length) { + return memberArr[i]; + } else { + throw new IllegalArgumentException(); + } + } + + private int getLength() { + return memberArr.length; + } + + /** + * Interprocedural analysis is not supported. + * + * @param i + * @return memberArr[i] + */ + public int interproceduralGet(int i) { + if (i >= 0 && i < getLength()) { + return memberArr[i]; + } else { + throw new IllegalArgumentException(); + } + } + + /** + * This example does not work: We know 5 > 3 and sometimes length > 5 > 3. In + * case of variables this conditional relation is resolved by introducing pi + * nodes. For constants pi nodes can be generated, but the pi variables will + * not be used (maybe due to constant propagation?). Additionally 5 != 3, so + * even if we would use pi-variables for 5, there would be no relation to 3: 0 + * -(5)-> 5, 5 -(-5)-> 0, {5,length} -(0)-> 5', 0 -(3)-> 3, 3 -(-3)-> 0 Given + * the inequality graph above, we know that 5,5',3 are larger than 0 and 5 + * larger 3 and length is larger than 5', but not 5' larger than 3. Which is + * not always the case in general anyway. + * + * This may be implemented by replacing each use of a constant dominated by a + * definition of a pi of a constant, with a fresh variable, that is connected + * to the inequality graph accordingly. + * + * For a workaround see {@link Detectable#nonFinalConstant(int[])} + * + * + * + * @param arr + * @return arr[3] + */ + public int constants(int[] arr) { + if (arr.length > 5) { + return arr[3]; + } else { + throw new IllegalArgumentException(); + } + } + + /** + * As the variable i is final, constant propagation will prevent the detection + * (See also {@link NotDetectable#constants(int[])}), for a working example + * see {@link Detectable#nonFinalConstant(int[])}. + * + * @param arr + * @return arr[3] + */ + public int dueToConstantPropagation(int[] arr) { + final int i = 3; + if (i < arr.length) { + return arr[i]; + } else { + throw new IllegalArgumentException(); + } + } + + public int indirectComparison(int[] arr) { + int i = 3; + int e = i + 1; + if (e < arr.length) { + return arr[i]; + } else { + throw new IllegalArgumentException(); + } + } + + /** + * Neither modulo, multiplication or division with constants will work. + * + * Note: Any operation can be performed BEFORE comparing a variable to the + * array length. + * + * @param arr + * @param i + * @return arr[i] + */ + public int modulo(int[] arr, int i) { + if (0 <= i && i < arr.length) { + return arr[i % 2]; + } else { + throw new IllegalArgumentException(); + } + } + + /** + * Neither subtraction of variables nor inverting variables will work. + * + * Note: Any operation can be performed BEFORE comparing a variable to the + * array length. + * + * @param arr + * @param i + * @return arr[i] + */ + public int variableSubtraction(int[] arr, int i) { + if (0 <= i && i < arr.length) { + int i2 = 0 - i; + int i3 = -i2; + return arr[i3]; + } else { + throw new IllegalArgumentException(); + } + } +} diff --git a/com.ibm.wala.core.testdata/src/arraybounds/NotInBound.java b/com.ibm.wala.core.testdata/src/arraybounds/NotInBound.java new file mode 100644 index 000000000..e76e05769 --- /dev/null +++ b/com.ibm.wala.core.testdata/src/arraybounds/NotInBound.java @@ -0,0 +1,44 @@ +package arraybounds; + +/** + * + * All array accesses in the following class are necessary and they will be + * detected correctly by the array bounds analysis. + * + * @author Stephan Gocht + * + */ +public class NotInBound { + public int phiOverwrite(int[] arr, boolean condition) { + if (arr.length > 0) { + int l = arr.length; + if (condition) { + l = 5; + } + + return arr[l - 1]; + } else { + throw new IllegalArgumentException(); + } + } + + public void offByOne(int[] arr) { + for (int i = 0; i <= arr.length; i++) { + arr[i] = 0; + } + } + + public int unknownLength(int[] arr) { + return arr[4]; + } + + public int ambiguity(int[] arr1, int[] arr2, int i) { + int[] arr = arr2; + if (0 <= i && i < arr.length) { + arr = arr1; + return arr[i]; + } else { + throw new IllegalArgumentException(); + } + } +} diff --git a/com.ibm.wala.core.tests/src/com/ibm/wala/core/tests/arraybounds/ArrayboundsAnalysisTest.java b/com.ibm.wala.core.tests/src/com/ibm/wala/core/tests/arraybounds/ArrayboundsAnalysisTest.java new file mode 100644 index 000000000..58c5d6696 --- /dev/null +++ b/com.ibm.wala.core.tests/src/com/ibm/wala/core/tests/arraybounds/ArrayboundsAnalysisTest.java @@ -0,0 +1,124 @@ +package com.ibm.wala.core.tests.arraybounds; + +import java.io.IOException; + +import org.hamcrest.Matcher; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ErrorCollector; + +import static org.hamcrest.CoreMatchers.*; + +import com.ibm.wala.analysis.arraybounds.ArrayOutOfBoundsAnalysis; +import com.ibm.wala.analysis.arraybounds.ArrayOutOfBoundsAnalysis.UnnecessaryCheck; +import com.ibm.wala.classLoader.IClass; +import com.ibm.wala.classLoader.IMethod; +import com.ibm.wala.core.tests.util.TestConstants; +import com.ibm.wala.ipa.callgraph.AnalysisOptions; +import com.ibm.wala.ipa.callgraph.AnalysisScope; +import com.ibm.wala.ipa.callgraph.impl.Everywhere; +import com.ibm.wala.ipa.cha.ClassHierarchy; +import com.ibm.wala.ipa.cha.ClassHierarchyException; +import com.ibm.wala.ssa.AllDueToBranchePiPolicy; +import com.ibm.wala.ssa.DefaultIRFactory; +import com.ibm.wala.ssa.IR; +import com.ibm.wala.ssa.IRFactory; +import com.ibm.wala.types.TypeReference; +import com.ibm.wala.util.config.AnalysisScopeReader; + +public class ArrayboundsAnalysisTest { + private static ClassLoader CLASS_LOADER = ArrayboundsAnalysisTest.class.getClassLoader(); + + private static final String DETECTABLE_TESTDATA = "Larraybounds/Detectable"; + private static final int DETECTABLE_NUMBER_OF_ARRAY_ACCESS = 21; + + private static final String NOT_DETECTABLE_TESTDATA = "Larraybounds/NotDetectable"; + private static final int NOT_DETECTABLE_NUMBER_OF_ARRAY_ACCESS = 7; + + private static final String NOT_IN_BOUND_TESTDATA = "Larraybounds/NotInBound"; + private static final int NOT_IN_BOUND_TESTDATA_NUMBER_OF_ARRAY_ACCESS = 4; + + private static IRFactory irFactory; + private static AnalysisOptions options; + private static AnalysisScope scope; + private static ClassHierarchy cha; + + @Rule + public ErrorCollector collector = new ErrorCollector(); + + @BeforeClass + public static void init() throws IOException, ClassHierarchyException { + scope = AnalysisScopeReader.readJavaScope(TestConstants.WALA_TESTDATA, null, CLASS_LOADER); + cha = ClassHierarchy.make(scope); + + irFactory = new DefaultIRFactory(); + options = new AnalysisOptions(); + options.getSSAOptions().setPiNodePolicy(new AllDueToBranchePiPolicy()); + } + + public static IR getIr(IMethod method) { + return irFactory.makeIR(method, Everywhere.EVERYWHERE, options.getSSAOptions()); + } + + public static IClass getIClass(String name) { + final TypeReference typeRef = TypeReference.findOrCreate(scope.getApplicationLoader(), name); + return cha.lookupClass(typeRef); + } + + @Test + public void detectable() { + IClass iClass = getIClass(DETECTABLE_TESTDATA); + assertAllSameNecessity(iClass, DETECTABLE_NUMBER_OF_ARRAY_ACCESS, equalTo(UnnecessaryCheck.BOTH)); + } + + @Test + public void notDetectable() { + IClass iClass = getIClass(NOT_DETECTABLE_TESTDATA); + assertAllSameNecessity(iClass, NOT_DETECTABLE_NUMBER_OF_ARRAY_ACCESS, not(equalTo(UnnecessaryCheck.BOTH))); + } + + @Test + public void notInBound() { + IClass iClass = getIClass(NOT_IN_BOUND_TESTDATA); + assertAllSameNecessity(iClass, NOT_IN_BOUND_TESTDATA_NUMBER_OF_ARRAY_ACCESS, not(equalTo(UnnecessaryCheck.BOTH))); + } + + public void assertAllSameNecessity(IClass iClass, int expectedNumberOfArrayAccesses, Matcher matcher) { + int numberOfArrayAccesses = 0; + for (IMethod method : iClass.getAllMethods()) { + if (method.getDeclaringClass().equals(iClass)) { + String identifyer = method.getDeclaringClass().getName().toString() + "#" + method.getName().toString(); + + ArrayOutOfBoundsAnalysis analysis = new ArrayOutOfBoundsAnalysis(getIr(method)); + for (UnnecessaryCheck unnecessary : analysis.getBoundsCheckNecessary().values()) { + numberOfArrayAccesses++; + collector.checkThat("Unexpected necessity for bounds check in " + identifyer, unnecessary, matcher); + } + } + } + + /* + * Possible reasons for this to fail are: + * + * + * *_NUMBER_OF_ARRAY_ACCESS is not set to the correct value (maybe the test + * data has changed). + * + * Not all methods of the class analyzed. + * + * There is a bug, so not all array accesses are found. + */ + collector.checkThat("Number of found array accesses is not as expected for " + iClass.getName().toString(), + numberOfArrayAccesses, equalTo(expectedNumberOfArrayAccesses)); + } + + @AfterClass + public static void free() throws IOException, ClassHierarchyException { + scope = null; + cha = null; + irFactory = null; + options = null; + } +} diff --git a/com.ibm.wala.core/META-INF/MANIFEST.MF b/com.ibm.wala.core/META-INF/MANIFEST.MF index 54ee2c7fd..7df9cb01b 100644 --- a/com.ibm.wala.core/META-INF/MANIFEST.MF +++ b/com.ibm.wala.core/META-INF/MANIFEST.MF @@ -10,6 +10,7 @@ Require-Bundle: com.ibm.wala.shrike, com.ibm.wala.util;bundle-version="1.0.0";visibility:=reexport Bundle-ActivationPolicy: lazy Export-Package: ., + com.ibm.wala.analysis.arraybounds, com.ibm.wala.analysis.pointers, com.ibm.wala.analysis.reflection, com.ibm.wala.analysis.reflection.java7,