From 862bbdb5fc93090e2ebfdfefb66f954285c8c09b Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Wed, 21 Mar 2018 06:25:33 -0700 Subject: [PATCH] [retain cycles] Take weak pointers into account to avoid false positives Reviewed By: mbouaziz Differential Revision: D7323649 fbshipit-source-id: e05a066 --- infer/src/backend/RetainCycles.ml | 35 +++++++++++++++++-- .../tests/codetoanalyze/objc/errors/Makefile | 2 +- .../codetoanalyze/objc/errors/issues.exp | 2 +- .../CADisplayLinkRetainCycle.m | 14 ++++---- 4 files changed, 43 insertions(+), 10 deletions(-) diff --git a/infer/src/backend/RetainCycles.ml b/infer/src/backend/RetainCycles.ml index c92abf01b..859b31b5e 100644 --- a/infer/src/backend/RetainCycles.ml +++ b/infer/src/backend/RetainCycles.ml @@ -84,7 +84,14 @@ let edge_is_strong tenv obj_edge = || String.equal Config.assign att ) params in - let weak_edge = + let weak_edge_by_type = + match obj_edge.rc_from.rc_node_typ.Typ.desc with + | Typ.Tptr (_, Typ.Pk_objc_weak) | Typ.Tptr (_, Typ.Pk_objc_unsafe_unretained) -> + true + | _ -> + false + in + let weak_edge_by_field = match get_item_annotation obj_edge.rc_from.rc_node_typ obj_edge.rc_field.rc_field_name with | Some ia -> List.exists @@ -97,7 +104,7 @@ let edge_is_strong tenv obj_edge = true (* Assume the edge is weak if the type cannot be found in the tenv, to avoid FPs *) in - not weak_edge + not (weak_edge_by_type || weak_edge_by_field) exception Max_retain_cycles of RetainCyclesType.Set.t @@ -128,6 +135,23 @@ let get_cycle_blocks root_node exp = None +let get_weak_alias_type prop e = + let sigma = prop.Prop.sigma in + let check_weak_alias hpred = + match hpred with + | Sil.Hpointsto (_, Sil.Eexp (e', _), Sizeof {typ}) -> ( + match typ.Typ.desc with + | (Typ.Tptr (_, Typ.Pk_objc_weak) | Typ.Tptr (_, Typ.Pk_objc_unsafe_unretained)) + when Exp.equal e' e -> + Some typ + | _ -> + None ) + | _ -> + None + in + List.find_map ~f:check_weak_alias sigma + + let get_cycles found_cycles root tenv prop = let open RetainCyclesType in let sigma = prop.Prop.sigma in @@ -138,6 +162,13 @@ let get_cycles found_cycles root tenv prop = in (* Perform a dfs of a graph stopping when e_root is reached. Returns the set of cycles reached. *) let rec dfs ~found_cycles ~root_node ~from_node ~rev_path ~fields ~visited = + let from_node = + match get_weak_alias_type prop from_node.rc_node_exp with + | Some typ -> + {from_node with rc_node_typ= typ} + | None -> + from_node + in match fields with | [] -> found_cycles diff --git a/infer/tests/codetoanalyze/objc/errors/Makefile b/infer/tests/codetoanalyze/objc/errors/Makefile index 244bf6cb4..e21f6312f 100644 --- a/infer/tests/codetoanalyze/objc/errors/Makefile +++ b/infer/tests/codetoanalyze/objc/errors/Makefile @@ -65,7 +65,6 @@ SOURCES_BUCKET_ALL = \ field_superclass/field.c \ global_const/global_const.m \ initialization/compound_literal.c \ - memory_leaks_benchmark/CADisplayLinkRetainCycle.m \ memory_leaks_benchmark/RetainCycleStaticVar.m \ npe/blockenum.m \ npe/nil_param.m \ @@ -89,6 +88,7 @@ SOURCES_ARC = \ field_superclass/SubtypingExample.m \ blocks_in_heap/BlockInHeap.m \ initialization/struct_initlistexpr.c \ + memory_leaks_benchmark/CADisplayLinkRetainCycle.m \ memory_leaks_benchmark/RetainCycleExampleWeak.m \ memory_leaks_benchmark/RetainReleaseExampleBucketingArc.m \ memory_leaks_benchmark/retain_cycle.m \ diff --git a/infer/tests/codetoanalyze/objc/errors/issues.exp b/infer/tests/codetoanalyze/objc/errors/issues.exp index 0f1ea6993..f9f6201b1 100644 --- a/infer/tests/codetoanalyze/objc/errors/issues.exp +++ b/infer/tests/codetoanalyze/objc/errors/issues.exp @@ -2,7 +2,6 @@ codetoanalyze/objc/errors/field_superclass/field.c, field_superclass_main, 3, RE codetoanalyze/objc/errors/global_const/global_const.m, SimpleRoot_doSomethingBadWithDict:andString:, 3, NULL_DEREFERENCE, ERROR, [start of procedure doSomethingBadWithDict:andString:,Message stringByAppendingString: with receiver nil returns nil.] codetoanalyze/objc/errors/global_const/global_const.m, SimpleRoot_doSomethingOkWithDict:andString:, 3, NULL_DEREFERENCE, ERROR, [start of procedure doSomethingOkWithDict:andString:,Message stringByAppendingString: with receiver nil returns nil.] codetoanalyze/objc/errors/initialization/compound_literal.c, init_with_compound_literal, 2, DIVIDE_BY_ZERO, ERROR, [start of procedure init_with_compound_literal()] -codetoanalyze/objc/errors/memory_leaks_benchmark/CADisplayLinkRetainCycle.m, CADisplay_init, 1, RETAIN_CYCLE, ERROR, [start of procedure init] codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleStaticVar.m, RetainCSV_foo, 13, RETAIN_CYCLE, ERROR, [start of procedure foo] codetoanalyze/objc/errors/npe/null_returned_by_method.m, NullReturnedByMethodA_test1, 1, NULL_DEREFERENCE, ERROR, [start of procedure test1,start of procedure test,return from a call to NullReturnedByMethodA_test] codetoanalyze/objc/errors/procdescs/main.c, ProcdescMain, 3, MEMORY_LEAK, ERROR, [start of procedure ProcdescMain(),Skipping plusX:andY:: function or method not found] @@ -29,6 +28,7 @@ codetoanalyze/objc/errors/field_superclass/SubtypingExample.m, subtyping_test, 0 codetoanalyze/objc/errors/initialization/struct_initlistexpr.c, field_set_correctly, 2, DIVIDE_BY_ZERO, ERROR, [start of procedure field_set_correctly()] codetoanalyze/objc/errors/initialization/struct_initlistexpr.c, implicit_expr_set_correctly, 3, DIVIDE_BY_ZERO, ERROR, [start of procedure implicit_expr_set_correctly()] codetoanalyze/objc/errors/initialization/struct_initlistexpr.c, point_coords_set_correctly, 2, DIVIDE_BY_ZERO, ERROR, [start of procedure point_coords_set_correctly()] +codetoanalyze/objc/errors/memory_leaks_benchmark/CADisplayLinkRetainCycle.m, CADisplay_init, 1, RETAIN_CYCLE, ERROR, [start of procedure init] codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleBlockCapturedVar.m, LinkResolver_test_bad, 3, RETAIN_CYCLE, ERROR, [start of procedure test_bad] codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleBlocks.m, RCBlock_retain_self_in_block, 1, RETAIN_CYCLE, ERROR, [start of procedure retain_self_in_block] codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleBlocks.m, objc_blockretain_a_in_block_cycle_3, 1, RETAIN_CYCLE, ERROR, [start of procedure block] diff --git a/infer/tests/codetoanalyze/objc/errors/memory_leaks_benchmark/CADisplayLinkRetainCycle.m b/infer/tests/codetoanalyze/objc/errors/memory_leaks_benchmark/CADisplayLinkRetainCycle.m index 36ab7a67f..430b8c4ee 100644 --- a/infer/tests/codetoanalyze/objc/errors/memory_leaks_benchmark/CADisplayLinkRetainCycle.m +++ b/infer/tests/codetoanalyze/objc/errors/memory_leaks_benchmark/CADisplayLinkRetainCycle.m @@ -19,13 +19,20 @@ @implementation CADisplay -- init { +- (instancetype)init { _displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(bla)]; return self; } +- (void)cycle_weak_good { + __weak __typeof__(self) weakSelf = self; + _displayLink = + [CADisplayLink displayLinkWithTarget:weakSelf + selector:@selector(handleDisplayLink:)]; +} + - (void)bla{}; - (void)invalidate { @@ -33,10 +40,6 @@ [_displayLink invalidate]; }; -- (void)dealloc { - [super dealloc]; -} - @end void testCycle() { @@ -49,5 +52,4 @@ void testNoCycle() { CADisplay* a = [[CADisplay alloc] init]; [a invalidate]; // break the cycle - [a dealloc]; }