From 34aa9c294933bfcd181ae988cbb59d100a9947ff Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Thu, 25 Jun 2015 15:55:54 -0100 Subject: [PATCH] [infer][java] handle int boxing in HashMap model Summary: @public Adds a special case for comparing Integer instances. Makes the tests more relevant too. Test Plan: - Added a new test --- infer/models/java/src/java/util/HashMap.java | 10 ++- infer/src/backend/sil.ml | 4 +- .../java/infer/HashMapExample.java | 86 +++++++++++++++++++ .../java/infer/HashMapModelTest.java | 82 ------------------ .../endtoend/java/infer/HashMapModelTest.java | 24 ++++-- 5 files changed, 114 insertions(+), 92 deletions(-) create mode 100644 infer/tests/codetoanalyze/java/infer/HashMapExample.java delete mode 100644 infer/tests/codetoanalyze/java/infer/HashMapModelTest.java diff --git a/infer/models/java/src/java/util/HashMap.java b/infer/models/java/src/java/util/HashMap.java index 3fcdc160d..4999f96a3 100644 --- a/infer/models/java/src/java/util/HashMap.java +++ b/infer/models/java/src/java/util/HashMap.java @@ -49,7 +49,7 @@ public abstract class HashMap extends AbstractMap return null; } - public V put(Object key, Object value) { + public V put(K key, V value) { pushKey(key); if (InferUndefined.boolean_undefined()) { @@ -66,7 +66,13 @@ public abstract class HashMap extends AbstractMap } private boolean _containsKey(Object key) { - return lastKey1 == key || lastKey2 == key; + return areEqual(key, lastKey1) || areEqual(key, lastKey2); + } + + /* does explicit dynamic dispatch to help Infer out */ + private static boolean areEqual(Object x, Object y) { + return x == y + || (x instanceof Integer && ((Integer) x).equals(y)); } } diff --git a/infer/src/backend/sil.ml b/infer/src/backend/sil.ml index 997638971..2c9582b03 100644 --- a/infer/src/backend/sil.ml +++ b/infer/src/backend/sil.ml @@ -441,9 +441,9 @@ module Subtype = struct if (should_add) then c:: rest' else rest' let get_subtypes (c1, (st1, flag1)) (c2, (st2, flag2)) f is_interface = - (* (L.d_strln_color Orange ((Mangled.to_string c1)^(subtypes_to_string (st1, flag1))^" <: "^ - (Mangled.to_string c2)^(subtypes_to_string (st2, flag2))^" ?"^(string_of_bool is_sub)));*) let is_sub = f c1 c2 in + (* L.d_strln_color Orange ((Mangled.to_string c1)^(subtypes_to_string (st1, flag1))^" <: "^ *) + (* (Mangled.to_string c2)^(subtypes_to_string (st2, flag2))^" ?"^(string_of_bool is_sub)); *) let pos_st, neg_st = match st1, st2 with | Exact, Exact -> if (is_sub) then (Some st1, None) diff --git a/infer/tests/codetoanalyze/java/infer/HashMapExample.java b/infer/tests/codetoanalyze/java/infer/HashMapExample.java new file mode 100644 index 000000000..11345af20 --- /dev/null +++ b/infer/tests/codetoanalyze/java/infer/HashMapExample.java @@ -0,0 +1,86 @@ +/* + * Copyright (c) 2013- Facebook. + * All rights reserved. + */ + +package codetoanalyze.java.infer; + +import java.util.HashMap; + +public class HashMapExample { + + public static void putIntegerTwiceThenGetTwice( + HashMap hashMap + ) { + Integer i32 = new Integer(32); + Integer i52 = new Integer(52); + + hashMap.put(i32, i32); + hashMap.put(i52, i52); + + Integer a = hashMap.get(i32); + Integer b = hashMap.get(i52); + + a.intValue(); + b.intValue(); + } + + public static void containsIntegerTwiceThenGetTwice( + HashMap hashMap + ) { + Integer i32 = new Integer(32); + Integer i52 = new Integer(52); + + if (hashMap.containsKey(i32) && hashMap.containsKey(i52)) { + Integer a = hashMap.get(i32); + Integer b = hashMap.get(i52); + a.intValue(); + b.intValue(); + } + } + + public static int getOneIntegerWithoutCheck() { + HashMap hashMap = new HashMap<>(); + Integer i32 = new Integer(32); + + Integer a = hashMap.get(i32); + + return a.intValue(); + } + + public static void getTwoIntegersWithOneCheck( + Integer i, + Integer j + ) { + HashMap hashMap = new HashMap<>(); + + if (hashMap.containsKey(i) && !i.equals(j)) { + Integer a = hashMap.get(i); + Integer b = hashMap.get(j); + + a.intValue(); + b.intValue(); + } + } + + public static Integer getOrCreateInteger( + final HashMap map, + final int id) { + Integer x = null; + if (map.containsKey(id)) { + x = map.get(id); + } else { + x = new Integer(0); + map.put(id, x); + } + return x; + } + + public static void getOrCreateIntegerThenDeref( + final HashMap map) { + Integer x = getOrCreateInteger(map, 42); + // dereference x + x.toString(); + } + +} diff --git a/infer/tests/codetoanalyze/java/infer/HashMapModelTest.java b/infer/tests/codetoanalyze/java/infer/HashMapModelTest.java deleted file mode 100644 index aec73f7ee..000000000 --- a/infer/tests/codetoanalyze/java/infer/HashMapModelTest.java +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright (c) 2013- Facebook. - * All rights reserved. - */ - -package codetoanalyze.java.infer; - -import java.util.HashMap; - -public class HashMapModelTest { - - public static int putIntegerTwiceThenGetTwice() { - HashMap hashMap = new HashMap<>(); - Integer i32 = new Integer(32); - Integer i52 = new Integer(52); - - hashMap.put(i32, i32); - hashMap.put(i52, i52); - - Integer a = hashMap.get(i32); - Integer b = hashMap.get(i52); - - return a.intValue() * b.intValue(); - } - - public static int containsIntegerTwiceThenGetTwice( - HashMap hashMap - ) { - Integer i32 = new Integer(32); - Integer i52 = new Integer(52); - - if (hashMap.containsKey(i32) && hashMap.containsKey(i52)) { - Integer a = hashMap.get(i32); - Integer b = hashMap.get(i52); - return a.intValue() * b.intValue(); - } - - return 0; - } - - public static int getOneIntegerWithoutCheck( - HashMap hashMap - ) { - Integer i32 = new Integer(32); - - Integer a = hashMap.get(i32); - - return a.intValue(); - } - - public static int getTwoIntegersWithOneCheck( - HashMap hashMap - ) { - Integer i32 = new Integer(32); - Integer i52 = new Integer(52); - - if (hashMap.containsKey(i32)) { - Integer a = hashMap.get(i32); - Integer b = hashMap.get(i52); - - return a.intValue() * b.intValue(); - } - - return 0; - } - - public static int getTwoParameterIntegersWithOneCheck( - HashMap hashMap, - Integer i32, - Integer i52 - ) { - if (hashMap.containsKey(i32)) { - Integer a = hashMap.get(i32); - Integer b = hashMap.get(i52); - - return a.intValue() * b.intValue(); - } - - return 0; - } - -} diff --git a/infer/tests/endtoend/java/infer/HashMapModelTest.java b/infer/tests/endtoend/java/infer/HashMapModelTest.java index e93c292f5..72d42bba7 100644 --- a/infer/tests/endtoend/java/infer/HashMapModelTest.java +++ b/infer/tests/endtoend/java/infer/HashMapModelTest.java @@ -8,7 +8,6 @@ package endtoend.java.infer; import static org.hamcrest.MatcherAssert.assertThat; import static utils.matchers.ResultContainsErrorInMethod.contains; import static utils.matchers.ResultContainsNoErrorInMethod.doesNotContain; -import static utils.matchers.ResultContainsOnlyTheseErrors.containsOnly; import org.junit.BeforeClass; import org.junit.Test; @@ -21,7 +20,7 @@ import utils.InferResults; public class HashMapModelTest { public static final String HashMapModelTest = - "infer/tests/codetoanalyze/java/infer/HashMapModelTest.java"; + "infer/tests/codetoanalyze/java/infer/HashMapExample.java"; public static final String NULL_DEREFERENCE = "NULL_DEREFERENCE"; @@ -87,15 +86,28 @@ public class HashMapModelTest { } @Test - public void whenInferRunsOnGetTwoParameterIntegersWithOneCheckThenNPEIsFound() + public void whenInferRunsOnGetOrCreateIntegerThenNPEIsNotFound() throws InterruptedException, IOException, InferException { assertThat( - "Results should contain null pointer exception error", + "Results should not contain null pointer exception error", inferResults, - contains( + doesNotContain( + NULL_DEREFERENCE, + HashMapModelTest, + "getOrCreateInteger") + ); + } + + @Test + public void whenInferRunsOnGetOrCreateIntegerThenDerefThenNPEIsNotFound() + throws InterruptedException, IOException, InferException { + assertThat( + "Results should not contain null pointer exception error", + inferResults, + doesNotContain( NULL_DEREFERENCE, HashMapModelTest, - "getTwoParameterIntegersWithOneCheck") + "getOrCreateIntegerThenDeref") ); }