diff --git a/infer/src/backend/specs.ml b/infer/src/backend/specs.ml index e5e58be29..1249cc1d9 100644 --- a/infer/src/backend/specs.ml +++ b/infer/src/backend/specs.ml @@ -352,6 +352,7 @@ type payload = ; resources: ResourceLeakDomain.summary option ; siof: SiofDomain.astate option ; racerd: RacerDDomain.summary option + ; should_update: ShouldUpdateDomain.astate option ; buffer_overrun: BufferOverrunDomain.Summary.t option ; uninit: UninitDomain.summary option } @@ -722,6 +723,7 @@ let empty_payload = ; resources= None ; siof= None ; racerd= None + ; should_update= None ; buffer_overrun= None ; uninit= None } diff --git a/infer/src/backend/specs.mli b/infer/src/backend/specs.mli index b065315a0..5a55b6672 100644 --- a/infer/src/backend/specs.mli +++ b/infer/src/backend/specs.mli @@ -137,6 +137,7 @@ type payload = ; resources: ResourceLeakDomain.summary option ; siof: SiofDomain.astate option ; racerd: RacerDDomain.summary option + ; should_update: ShouldUpdateDomain.astate option ; buffer_overrun: BufferOverrunDomain.Summary.t option ; uninit: UninitDomain.summary option } diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index f6a55009b..933935057 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -706,6 +706,7 @@ and ( annotation_reachability , quandary , racerd , resource_leak + , should_update , siof , suggest_nullable , uninit ) = @@ -748,6 +749,9 @@ and ( annotation_reachability mk_checker ~long:"racerd" ~deprecated:["-threadsafety"] ~default:true "the RacerD thread safety analysis" and resource_leak = mk_checker ~long:"resource-leak" "" + and should_update = + mk_checker ~long:"should-update" + "Experimental checker for identifying GraphQL dependencies of a Litho component" and siof = mk_checker ~long:"siof" ~default:true "the Static Initialization Order Fiasco analysis (C++ only)" @@ -805,6 +809,7 @@ and ( annotation_reachability , quandary , racerd , resource_leak + , should_update , siof , suggest_nullable , uninit ) @@ -2618,6 +2623,8 @@ and seconds_per_iteration = !seconds_per_iteration and select = !select +and should_update = !should_update + and show_buckets = !print_buckets and show_progress_bar = !progress_bar diff --git a/infer/src/base/Config.mli b/infer/src/base/Config.mli index dee6575f6..294d7e159 100644 --- a/infer/src/base/Config.mli +++ b/infer/src/base/Config.mli @@ -612,6 +612,8 @@ val seconds_per_iteration : float option val select : int option +val should_update : bool + val show_buckets : bool val show_progress_bar : bool diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index c11977417..a5dc1ebfc 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -227,6 +227,8 @@ let _global_variable_initialized_with_function_or_method_call = from_string ~enabled:false "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL" +let graphql_field_access = from_string "GRAPHQL_FIELD_ACCESS" + let inferbo_alloc_is_big = from_string "INFERBO_ALLOC_IS_BIG" let inferbo_alloc_is_negative = from_string "INFERBO_ALLOC_IS_NEGATIVE" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index 3ef5cb025..3487012df 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -140,6 +140,8 @@ val field_should_be_nullable : t val field_not_null_checked : t +val graphql_field_access : t + val inferbo_alloc_is_big : t val inferbo_alloc_is_negative : t diff --git a/infer/src/checkers/ShouldUpdate.ml b/infer/src/checkers/ShouldUpdate.ml new file mode 100644 index 000000000..ddf2dd2e7 --- /dev/null +++ b/infer/src/checkers/ShouldUpdate.ml @@ -0,0 +1,119 @@ +(* + * Copyright (c) 2017 - 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. + *) + +module F = Format +module L = Logging +module Domain = ShouldUpdateDomain + +module Summary = Summary.Make (struct + type payload = Domain.astate + + let update_payload astate (summary: Specs.summary) = + {summary with payload= {summary.payload with should_update= Some astate}} + + + let read_payload (summary: Specs.summary) = summary.payload.should_update +end) + +module TransferFunctions (CFG : ProcCfg.S) = struct + module CFG = CFG + module Domain = Domain + + type extras = ProcData.no_extras + + let is_getter = function + | Typ.Procname.Java procname -> + PatternMatch.is_getter procname + | _ -> + false + + + let exec_instr astate _ _ (instr: HilInstr.t) : Domain.astate = + match instr with + | Call (Some return_base, Direct procname, (HilExp.AccessPath receiver) :: _, _, _) + when is_getter procname -> + let return_access_path = (return_base, []) in + let return_calls = + (try Domain.find return_access_path astate with Not_found -> Domain.CallSet.empty) + |> Domain.CallSet.add {receiver; procname} + in + Domain.add return_access_path return_calls astate + | Call _ -> + (* TODO: interprocedural analysis + (1) add caller actuals to their callee call set (according to the summary) + (2) bind the caller return value to its callee call set (according to the summary + *) + astate + | Assign (lhs_access_path, HilExp.AccessPath rhs_access_path, _) -> ( + try + (* creating an alias for the rhs binding; assume all reads will now occur through the + alias. this helps us keep track of chains in cases like tmp = getFoo(); x = tmp; + tmp.getBar() *) + let call_set = Domain.find rhs_access_path astate in + Domain.remove rhs_access_path astate |> Domain.add lhs_access_path call_set + with Not_found -> astate ) + | _ -> + astate + +end + +module Analyzer = LowerHil.MakeAbstractInterpreter (ProcCfg.Exceptional) (TransferFunctions) + +let report receiver call_chain summary = + let call_strings = List.map ~f:(Typ.Procname.to_simplified_string ~withclass:false) call_chain in + let call_string = String.concat ~sep:"." call_strings in + let message = F.asprintf "GraphQL getter chain %a.%s" AccessPath.pp receiver call_string in + let issue_id = IssueType.graphql_field_access.unique_id in + let exn = Exceptions.Checkers (issue_id, Localise.verbatim_desc message) in + let loc = Specs.get_loc summary in + Reporting.log_error summary ~loc exn + + +let unroll_call call astate summary = + let max_depth = Domain.cardinal astate in + let rec unroll_call_ ({receiver; procname}: Domain.CallWithReceiver.t) (acc, depth) = + let acc' = procname :: acc in + let depth' = depth + 1 in + let is_cycle access_path = + (* detect direct cycles and cycles due to mutual recursion *) + AccessPath.equal access_path receiver || depth' > max_depth + in + try + let calls' = Domain.find receiver astate in + Domain.CallSet.iter + (fun call -> if not (is_cycle call.receiver) then unroll_call_ call (acc', depth')) + calls' + with Not_found -> report receiver acc' summary + in + unroll_call_ call ([], 0) + + +let should_report proc_desc = + match Procdesc.get_proc_name proc_desc with + | Typ.Procname.Java java_pname -> ( + match Typ.Procname.java_get_method java_pname with "onCreateLayout" -> true | _ -> false ) + | _ -> + false + + +let report_call_chains post summary = + Domain.iter + (fun _ call_set -> Domain.CallSet.iter (fun call -> unroll_call call post summary) call_set) + post + + +let checker {Callbacks.summary; proc_desc; tenv} = + let proc_data = ProcData.make_default proc_desc tenv in + match Analyzer.compute_post proc_data ~initial:Domain.empty with + | Some post -> + if should_report proc_desc then report_call_chains post summary ; + summary + | None -> + summary + diff --git a/infer/src/checkers/ShouldUpdateDomain.ml b/infer/src/checkers/ShouldUpdateDomain.ml new file mode 100644 index 000000000..0cd8f86ed --- /dev/null +++ b/infer/src/checkers/ShouldUpdateDomain.ml @@ -0,0 +1,22 @@ +(* + * Copyright (c) 2017 - 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. + *) + +module F = Format +module L = Logging + +module CallWithReceiver = struct + type t = {receiver: AccessPath.t; procname: Typ.Procname.t} [@@deriving compare] + + let pp fmt {receiver; procname} = + F.fprintf fmt "%a.%a" AccessPath.pp receiver Typ.Procname.pp procname + +end + +module CallSet = AbstractDomain.FiniteSet (CallWithReceiver) +include AbstractDomain.Map (AccessPath) (CallSet) diff --git a/infer/src/checkers/registerCheckers.ml b/infer/src/checkers/registerCheckers.ml index aafcf524f..c2a378d42 100644 --- a/infer/src/checkers/registerCheckers.ml +++ b/infer/src/checkers/registerCheckers.ml @@ -89,6 +89,9 @@ let all_checkers = interprocedural later on *) Procedure ResourceLeaks.checker , Config.Java ) ] } + ; { name= "should update" + ; active= Config.should_update + ; callbacks= [(Procedure ShouldUpdate.checker, Config.Java)] } ; {name= "SIOF"; active= Config.siof; callbacks= [(Procedure Siof.checker, Config.Clang)]} ; { name= "uninitialized variables" ; active= Config.uninit