From d3f1aab8039f2b162c35c62f257daa5532111b7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Wed, 18 Nov 2020 04:59:16 -0800 Subject: [PATCH] [preanalysis] Handle assigning blocks to locals in closure substitution Summary: Existing closure substitution only supported direct block calls to formals. The following didn't work since the domain was only keeping track of loads/calls from formals, but didn't support stores. ``` void foo(dispatch_block_t block1){ dispatch_block_t local_block = block1; local_block(); // we don't substitute the call here } ``` This diff adds support for assigning a block to a local variable so that we can specialize the above example. We now have a pair domain - existing mapping from ids to block vars - a new mapping from mangled to block specializations the latter allows us to update the mapping in local block assignment (via store). Reviewed By: skcho Differential Revision: D25030234 fbshipit-source-id: 3f172341c --- .../backend/ClosureSubstSpecializedMethod.ml | 55 +++++++++++++++---- .../objc/autoreleasepool/arc_block.m | 2 +- .../objc/autoreleasepool/cost-issues.exp | 4 +- .../objc/autoreleasepool/issues.exp | 2 +- 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/infer/src/backend/ClosureSubstSpecializedMethod.ml b/infer/src/backend/ClosureSubstSpecializedMethod.ml index b2287ad73..003219d3d 100644 --- a/infer/src/backend/ClosureSubstSpecializedMethod.ml +++ b/infer/src/backend/ClosureSubstSpecializedMethod.ml @@ -15,19 +15,41 @@ module PPPVar = struct end module VDom = AbstractDomain.Flat (PPPVar) -module Domain = AbstractDomain.SafeInvertedMap (Ident) (VDom) +module BlockIdMap = AbstractDomain.SafeInvertedMap (Ident) (VDom) -let eval_instr astate instr = +module BlockSpec = struct + type t = Procname.t * (Mangled.t * Typ.t) list [@@deriving compare, equal] + + let pp fmt (pname, _) = Procname.pp fmt pname +end + +module SpecDom = AbstractDomain.Flat (BlockSpec) +module BlockPvarSpecMap = AbstractDomain.SafeInvertedMap (Mangled) (SpecDom) +module Domain = AbstractDomain.Pair (BlockIdMap) (BlockPvarSpecMap) + +let eval_instr ((id_to_pvar_map, pvars_to_blocks_map) : Domain.t) instr = let open Sil in match instr with | Load {id; e= Exp.Lvar pvar} -> - Domain.add id (VDom.v pvar) astate + (BlockIdMap.add id (VDom.v pvar) id_to_pvar_map, pvars_to_blocks_map) + | Store {e1= Exp.Lvar pvar; e2= Exp.Var id} -> + let pvars_to_blocks_map = + match Option.bind ~f:VDom.get (BlockIdMap.find_opt id id_to_pvar_map) with + | Some block_var -> + Option.value_map + (BlockPvarSpecMap.find_opt (Pvar.get_name block_var) pvars_to_blocks_map) + ~default:pvars_to_blocks_map + ~f:(fun res -> BlockPvarSpecMap.add (Pvar.get_name pvar) res pvars_to_blocks_map) + | None -> + pvars_to_blocks_map + in + (id_to_pvar_map, pvars_to_blocks_map) | Load {id} -> - Domain.add id VDom.top astate + (BlockIdMap.add id VDom.top id_to_pvar_map, pvars_to_blocks_map) | Call ((id, _), _, _, _, _) -> - Domain.add id VDom.top astate + (BlockIdMap.add id VDom.top id_to_pvar_map, pvars_to_blocks_map) | _ -> - astate + (id_to_pvar_map, pvars_to_blocks_map) module TransferFunctions = struct @@ -44,7 +66,7 @@ end module Analyzer = AbstractInterpreter.MakeRPO (TransferFunctions) -let exec_instr proc_name formals_to_blocks_map domain instr = +let exec_instr proc_name (id_to_pvar_map, pvars_to_blocks_map) instr = let open Sil in let res = let exec_exp exp = @@ -58,11 +80,14 @@ let exec_instr proc_name formals_to_blocks_map domain instr = [Store {e1= exec_exp e1; root_typ; typ; e2= exec_exp e2; loc}] | Call (ret_id_typ, Var id, origin_args, loc, call_flags) -> ( let converted_args = List.map ~f:(fun (exp, typ) -> (exec_exp exp, typ)) origin_args in - match Option.bind ~f:VDom.get (Domain.find_opt id domain) with + match Option.bind ~f:VDom.get (BlockIdMap.find_opt id id_to_pvar_map) with | None -> [instr] | Some pvar -> ( - match Mangled.Map.find_opt (Pvar.get_name pvar) formals_to_blocks_map with + match + BlockPvarSpecMap.find_opt (Pvar.get_name pvar) pvars_to_blocks_map + |> Option.bind ~f:SpecDom.get + with | Some (procname, extra_formals) -> let extra_args, load_instrs = List.map @@ -97,7 +122,7 @@ let analyze_at_node (map : Analyzer.invariant_map) node : Domain.t = | Some abstate -> abstate.pre | None -> - Domain.top + (BlockIdMap.top, BlockPvarSpecMap.top) let process summary = @@ -110,13 +135,19 @@ let process summary = | Some orig_proc_desc -> let formals_to_blocks_map = spec_with_blocks_info.formals_to_procs_and_new_formals in Procdesc.shallow_copy_code_from_pdesc ~orig_pdesc:orig_proc_desc ~dest_pdesc:pdesc ; + let pvars_to_blocks_map = + Mangled.Map.map SpecDom.v formals_to_blocks_map + |> Mangled.Map.to_seq |> BlockPvarSpecMap.of_seq + in let node_cfg = CFG.from_pdesc pdesc in - let invariant_map = Analyzer.exec_cfg node_cfg () ~initial:Domain.empty in + let invariant_map = + Analyzer.exec_cfg node_cfg () ~initial:(BlockIdMap.empty, pvars_to_blocks_map) + in let update_context = eval_instr in CFG.fold_nodes node_cfg ~init:() ~f:(fun _ node -> let used_ids = Instrs.instrs_get_normal_vars (CFG.instrs node) in Ident.update_name_generator used_ids ) ; - let replace_instr _node = exec_instr proc_name formals_to_blocks_map in + let replace_instr _node = exec_instr proc_name in let context_at_node node = analyze_at_node invariant_map node in let _has_changed : bool = Procdesc.replace_instrs_by_using_context pdesc ~f:replace_instr ~update_context diff --git a/infer/tests/codetoanalyze/objc/autoreleasepool/arc_block.m b/infer/tests/codetoanalyze/objc/autoreleasepool/arc_block.m index d623daf43..f61efb4b9 100644 --- a/infer/tests/codetoanalyze/objc/autoreleasepool/arc_block.m +++ b/infer/tests/codetoanalyze/objc/autoreleasepool/arc_block.m @@ -49,7 +49,7 @@ BOOL x; - (void)callBlock:(dispatch_block_t)block { dispatch_block_t local_block = block; - local_block(); // pre-analysis can't specialize here and we drop the trace + local_block(); } - (void)call_CallBlock_linear:(int)k { diff --git a/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp b/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp index 328f79a8b..039620e21 100644 --- a/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp +++ b/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp @@ -1,10 +1,10 @@ ${XCODE_ISYSROOT}/System/Library/Frameworks/Foundation.framework/Headers/NSArray.h, NSArray.indexOfObjectPassingTest:[objc_blockArcBlock.callIndexOfObjectPassingTest_linear:_1], 0, OnUIThread:false, [] ${XCODE_ISYSROOT}/System/Library/Frameworks/Foundation.framework/Headers/NSArray.h, NSArray.indexOfObjectPassingTest:[objc_blockArcBlock.callIndexOfObjectPassingTest_param_linear:_2], 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.callBlock:, |block|, OnUIThread:false, [] -codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.callBlock:[objc_blockArcBlock.call_CallBlock_linear:_4], |block|, OnUIThread:false, [] +codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.callBlock:[objc_blockArcBlock.call_CallBlock_linear:_4], 1, OnUIThread:false, [autorelease,Call to objc_blockArcBlock.call_CallBlock_linear:_4,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.callIndexOfObjectPassingTest_linear:, x->elements.length.ub, OnUIThread:false, [{x->elements.length.ub},Modeled call to indexOfObjectPassingTest:,autorelease,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.callIndexOfObjectPassingTest_param_linear:, x->elements.length.ub, OnUIThread:false, [{x->elements.length.ub},Modeled call to indexOfObjectPassingTest:,autorelease,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] -codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.call_CallBlock_linear:, k, OnUIThread:false, [{k},Loop] +codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.call_CallBlock_linear:, k, OnUIThread:false, [{k},Loop,autorelease,Call to ArcBlock.callBlock:[objc_blockArcBlock.call_CallBlock_linear:_4],Call to objc_blockArcBlock.call_CallBlock_linear:_4,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.call_ConditionalRunBlock_linear:, k, OnUIThread:false, [{k},Loop,autorelease,Call to ArcBlock.conditionalRunBlock:[objc_blockArcBlock.call_ConditionalRunBlock_linear:_3],Call to objc_blockArcBlock.call_ConditionalRunBlock_linear:_3,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.conditionalRunBlock:, |block|, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.conditionalRunBlock:[objc_blockArcBlock.call_ConditionalRunBlock_linear:_3], 1, OnUIThread:false, [autorelease,Call to objc_blockArcBlock.call_ConditionalRunBlock_linear:_3,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] diff --git a/infer/tests/codetoanalyze/objc/autoreleasepool/issues.exp b/infer/tests/codetoanalyze/objc/autoreleasepool/issues.exp index 64a28eaa6..f0d704c2e 100644 --- a/infer/tests/codetoanalyze/objc/autoreleasepool/issues.exp +++ b/infer/tests/codetoanalyze/objc/autoreleasepool/issues.exp @@ -1,6 +1,6 @@ codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.callIndexOfObjectPassingTest_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{x->elements.length.ub},Modeled call to indexOfObjectPassingTest:,autorelease,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.callIndexOfObjectPassingTest_param_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{x->elements.length.ub},Modeled call to indexOfObjectPassingTest:,autorelease,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] -codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.call_CallBlock_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{k},Loop] +codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.call_CallBlock_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{k},Loop,autorelease,Call to ArcBlock.callBlock:[objc_blockArcBlock.call_CallBlock_linear:_4],Call to objc_blockArcBlock.call_CallBlock_linear:_4,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.call_ConditionalRunBlock_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{k},Loop,autorelease,Call to ArcBlock.conditionalRunBlock:[objc_blockArcBlock.call_ConditionalRunBlock_linear:_3],Call to objc_blockArcBlock.call_ConditionalRunBlock_linear:_3,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_caller.m, ArcCaller.callGiveMeObject_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{n},Loop,autorelease,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.callMyEnumerator_linear_FP:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,{x->elements.length.ub + 1},Loop]