From 7e5381b7a94afb275d19ede4ae782a56ff802acf Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Mon, 14 May 2018 02:24:38 -0700 Subject: [PATCH] [starvation] model legacy synchronized containers Reviewed By: ddino Differential Revision: D7968334 fbshipit-source-id: 3e79e3d --- infer/src/concurrency/RacerDConfig.ml | 13 +++++++++ infer/src/concurrency/RacerDConfig.mli | 4 +++ infer/src/concurrency/starvation.ml | 13 +++++++-- .../java/starvation/LegacySync.java | 29 +++++++++++++++++++ .../codetoanalyze/java/starvation/issues.exp | 1 + 5 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 infer/tests/codetoanalyze/java/starvation/LegacySync.java diff --git a/infer/src/concurrency/RacerDConfig.ml b/infer/src/concurrency/RacerDConfig.ml index 628cca6ff..37c98878d 100644 --- a/infer/src/concurrency/RacerDConfig.ml +++ b/infer/src/concurrency/RacerDConfig.ml @@ -685,4 +685,17 @@ module Models = struct ; is_asyncTask_get ] in fun tenv pn actuals -> List.exists matchers ~f:(fun matcher -> matcher tenv pn actuals) + + + let is_synchronized_library_call = + let targets = ["java.lang.StringBuffer"; "java.util.Hashtable"; "java.util.Vector"] in + fun tenv pn -> + not (Typ.Procname.is_constructor pn) + && + match pn with + | Typ.Procname.Java java_pname -> + let classname = Typ.Procname.Java.get_class_type_name java_pname in + List.exists targets ~f:(PatternMatch.is_subtype_of_str tenv classname) + | _ -> + false end diff --git a/infer/src/concurrency/RacerDConfig.mli b/infer/src/concurrency/RacerDConfig.mli index 2ecdc41d9..f262d38d2 100644 --- a/infer/src/concurrency/RacerDConfig.mli +++ b/infer/src/concurrency/RacerDConfig.mli @@ -82,4 +82,8 @@ module Models : sig val may_block : Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool (** is the method call potentially blocking, given the actuals passed? *) + + val is_synchronized_library_call : Tenv.t -> Typ.Procname.t -> bool + (** does the method call lock-then-unlock the underlying object? + legacy Java containers like Vector do this, and can interact with explicit locking *) end diff --git a/infer/src/concurrency/starvation.ml b/infer/src/concurrency/starvation.ml index ea47377cb..f8722e267 100644 --- a/infer/src/concurrency/starvation.ml +++ b/infer/src/concurrency/starvation.ml @@ -61,15 +61,24 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | _ -> None in + let do_lock actuals loc astate = + get_path actuals |> Option.value_map ~default:astate ~f:(Domain.acquire astate loc) + in + let do_unlock actuals astate = + get_path actuals |> Option.value_map ~default:astate ~f:(Domain.release astate) + in match instr with | Call (_, Direct callee, actuals, _, loc) -> ( match Models.get_lock callee actuals with | Lock -> - get_path actuals |> Option.value_map ~default:astate ~f:(Domain.acquire astate loc) + do_lock actuals loc astate | Unlock -> - get_path actuals |> Option.value_map ~default:astate ~f:(Domain.release astate) + do_unlock actuals astate | LockedIfTrue -> astate + | NoEffect when Models.is_synchronized_library_call tenv callee -> + (* model a synchronized call without visible internal behaviour *) + do_lock actuals loc astate |> do_unlock actuals | NoEffect when Models.may_block tenv callee actuals -> let caller = Procdesc.get_proc_name pdesc in Domain.blocking_call ~caller ~callee loc astate diff --git a/infer/tests/codetoanalyze/java/starvation/LegacySync.java b/infer/tests/codetoanalyze/java/starvation/LegacySync.java new file mode 100644 index 000000000..015b34be2 --- /dev/null +++ b/infer/tests/codetoanalyze/java/starvation/LegacySync.java @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2018 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +import java.util.concurrent.Future; +import java.util.concurrent.ExecutionException; +import android.support.annotation.UiThread; +import java.util.Hashtable; + +class LegacySync { + Hashtable table; + Future future; + + void notOnUiThreadSyncedBad() throws InterruptedException, ExecutionException { + synchronized(table) { + future.get(); + } + } + + @UiThread + Object onUiThreadOpBad() { + return table.get("blabla"); + } +} diff --git a/infer/tests/codetoanalyze/java/starvation/issues.exp b/infer/tests/codetoanalyze/java/starvation/issues.exp index 2b1517209..96b785d4e 100644 --- a/infer/tests/codetoanalyze/java/starvation/issues.exp +++ b/infer/tests/codetoanalyze/java/starvation/issues.exp @@ -21,6 +21,7 @@ codetoanalyze/java/starvation/Interproc.java, void Interproc.interproc1Bad(Inter codetoanalyze/java/starvation/Intraproc.java, void Intraproc.intraBad(IntraprocA), 11, DEADLOCK, ERROR, [[Trace 1] void Intraproc.intraBad(IntraprocA),locks `this` in class `Intraproc*`,locks `o` in class `IntraprocA*`,[Trace 2] void IntraprocA.intraBad(Intraproc),locks `this` in class `IntraprocA*`,locks `o` in class `Intraproc*`] codetoanalyze/java/starvation/JavaIO.java, void JavaIO.fileReadBad(), 32, STARVATION, ERROR, [ void JavaIO.fileReadBad(),Method call: int JavaIO.doFileRead(),calls int InputStreamReader.read() from int JavaIO.doFileRead()] codetoanalyze/java/starvation/JavaIO.java, void JavaIO.streamReadBad(), 37, STARVATION, ERROR, [ void JavaIO.streamReadBad(),Method call: String JavaIO.doStreamRead(),calls String DataInputStream.readUTF() from String JavaIO.doStreamRead()] +codetoanalyze/java/starvation/LegacySync.java, Object LegacySync.onUiThreadOpBad(), 26, STARVATION, ERROR, [[Trace 1] Object LegacySync.onUiThreadOpBad(),locks `this.LegacySync.table` in class `LegacySync*`,[Trace 2] void LegacySync.notOnUiThreadSyncedBad(),locks `this.LegacySync.table` in class `LegacySync*`,calls Object Future.get() from void LegacySync.notOnUiThreadSyncedBad()] codetoanalyze/java/starvation/ServiceOnUIThread.java, IBinder ServiceOnUIThread.onBind(Intent), 20, STARVATION, ERROR, [ IBinder ServiceOnUIThread.onBind(Intent),Method call: void ServiceOnUIThread.transactBad(),calls boolean IBinder.transact(int,Parcel,Parcel,int) from void ServiceOnUIThread.transactBad()] codetoanalyze/java/starvation/ServiceOnUIThread.java, void ServiceOnUIThread.transactBad(), 25, STARVATION, ERROR, [ void ServiceOnUIThread.transactBad(),calls boolean IBinder.transact(int,Parcel,Parcel,int) from void ServiceOnUIThread.transactBad()] codetoanalyze/java/starvation/StaticLock.java, void StaticLock.lockOtherClassOneWayBad(), 24, DEADLOCK, ERROR, [[Trace 1] void StaticLock.lockOtherClassOneWayBad(),locks `StaticLock$0` in class `java.lang.Class*`,locks `this` in class `StaticLock*`,[Trace 2] void StaticLock.lockOtherClassAnotherWayNad(),locks `this` in class `StaticLock*`,Method call: void StaticLock.staticSynced(),locks `StaticLock$0` in class `java.lang.Class*`]