[selfInBlock] Remove false positive from mix self weakSelf

Summary: The translation of closures includes a load instruction for the captured variable, then we add that corresponding id to the closure. This doesn't correspond to an actual "use" of the captured variable in the source program though, and causes false positives. Here we remove the ids from the domain when that id is being added to a closure.

Reviewed By: ngorogiannis

Differential Revision: D19557352

fbshipit-source-id: 52b426011
master
Dulma Churchill 5 years ago committed by Facebook Github Bot
parent 237aac4cd0
commit 9dfd1943e6

@ -246,6 +246,26 @@ module TransferFunctions = struct
List.iter ~f:report_unchecked_strongself_issues_on_exp exps
(* The translation of closures includes a load instruction for the captured variable,
then we add that corresponding id to the closure. This doesn't correspond to an
actual "use" of the captured variable in the source program though, and causes false
positives. Here we remove the ids from the domain when that id is being added to a closure. *)
let remove_ids_in_closures_from_domain (domain : Domain.t) (instr : Sil.instr) =
let remove_id_in_closures_from_domain vars ((exp : Exp.t), _, _) =
match exp with Var id -> Vars.remove id vars | _ -> vars
in
let do_exp vars (exp : Exp.t) =
match exp with
| Closure {captured_vars} ->
List.fold ~init:vars ~f:remove_id_in_closures_from_domain captured_vars
| _ ->
vars
in
let exps = Sil.exps_of_instr instr in
let vars = List.fold ~init:domain.vars ~f:do_exp exps in
{domain with vars}
let is_objc_instance proc_desc_opt =
match proc_desc_opt with
| Some proc_desc -> (
@ -299,6 +319,7 @@ module TransferFunctions = struct
let exec_instr (astate : Domain.t) {ProcData.summary} _cfg_node (instr : Sil.instr) =
let attributes = Summary.get_attributes summary in
report_unchecked_strongself_issues_on_exps astate summary instr ;
let astate = remove_ids_in_closures_from_domain astate instr in
match instr with
| Load {id; e= Lvar pvar; loc; typ} ->
let vars =

@ -170,7 +170,7 @@ void m2(_Nullable SelfInBlockTest* obj) {}
};
}
- (void)wekSelfMultiple_bad {
- (void)weakSelfMultiple_bad {
__weak __typeof(self) weakSelf = self;
int (^my_block)(BOOL) = ^(BOOL isTapped) {
[weakSelf foo];
@ -194,4 +194,15 @@ void m2(_Nullable SelfInBlockTest* obj) {}
};
}
- (void)mixSelfWeakSelf_good:(NSArray*)resources {
__weak __typeof(self) weakSelf = self;
int (^my_block)() = ^() {
[self foo]; // no bug here
int (^my_block)() = ^() {
[weakSelf foo];
return 0;
};
return 0;
};
}
@end

@ -4,5 +4,5 @@ codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strong
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 8, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 10, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfNoCheckNotWeakSelf_good_2, 1, CAPTURED_STRONG_SELF, no_bucket, ERROR, [Using captured &weakSelf]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::wekSelfMultiple_bad_11, 1, MULTIPLE_WEAKSELF, no_bucket, ERROR, [Using &weakSelf,Using &weakSelf]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::weakSelfMultiple_bad_11, 1, MULTIPLE_WEAKSELF, no_bucket, ERROR, [Using &weakSelf,Using &weakSelf]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockobjc_blockSelfInBlockTest::capturedStrongSelf_bad_12_13, 1, CAPTURED_STRONG_SELF, no_bucket, ERROR, [Using captured &strongSelf]

Loading…
Cancel
Save