pghs975uc
/
infer_clone
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

[SelfInBlock] Refactor the reporting to run only once over the domain

Browse Source 
Reviewed By: skcho

Differential Revision: D19744946

fbshipit-source-id: d7983000f
master
Dulma Churchill 5 years ago committed by Facebook Github Bot
parent a6da208e9d
commit 7631d34f43
2 changed files with 84 additions and 89 deletions
Show Stats Download Patch File Download Diff File

151
infer/src/checkers/SelfInBlock.ml
Unescape View File

@ -16,14 +16,8 @@ module DomainData = struct
| WEAK_SELF | WEAK_SELF
[@@deriving compare] [@@deriving compare]
let is_self kind = match kind with SELF -> true | _ -> false
let is_weak_self kind = match kind with WEAK_SELF -> true | _ -> false
let is_unchecked_strong_self kind = match kind with UNCHECKED_STRONG_SELF -> true | _ -> false let is_unchecked_strong_self kind = match kind with UNCHECKED_STRONG_SELF -> true | _ -> false
let is_captured_strong_self kind = match kind with CAPTURED_STRONG_SELF -> true | _ -> false
let pp_self_pointer_kind fmt kind = let pp_self_pointer_kind fmt kind =
let s = let s =
match kind with match kind with
@ -386,102 +380,106 @@ let make_trace_use_self_weakself domain =
Location.compare loc1 loc2 ) Location.compare loc1 loc2 )
let report_mix_self_weakself_issues summary domain = let make_trace_captured_strong_self domain =
let weakSelf_opt = let trace_elems =
Vars.filter (fun _ {kind} -> DomainData.is_weak_self kind) domain |> Vars.choose_opt Vars.fold
(fun _ {pvar; loc; kind} trace_elems ->
match kind with
| CAPTURED_STRONG_SELF ->
let trace_elem_desc = F.asprintf "Using captured %a" (Pvar.pp Pp.text) pvar in
let trace_elem = Errlog.make_trace_element 0 loc trace_elem_desc [] in
trace_elem :: trace_elems
| _ ->
trace_elems )
domain []
in in
let self_opt = Vars.filter (fun _ {kind} -> DomainData.is_self kind) domain |> Vars.choose_opt in List.sort trace_elems ~compare:(fun {Errlog.lt_loc= loc1} {Errlog.lt_loc= loc2} ->
match (weakSelf_opt, self_opt) with Location.compare loc1 loc2 )
| Some (_, {pvar= weakSelf; loc= weakLoc}), Some (_, {pvar= self; loc= selfLoc}) ->
let report_mix_self_weakself_issues summary domain (weakSelf : DomainData.t) (self : DomainData.t) =
let message = let message =
F.asprintf F.asprintf
"This block uses both `%a` (%a) and `%a` (%a). This could lead to retain cycles or \ "This block uses both `%a` (%a) and `%a` (%a). This could lead to retain cycles or \
unexpected behavior." unexpected behavior."
(Pvar.pp Pp.text) weakSelf Location.pp weakLoc (Pvar.pp Pp.text) self Location.pp selfLoc (Pvar.pp Pp.text) weakSelf.pvar Location.pp weakSelf.loc (Pvar.pp Pp.text) self.pvar
Location.pp self.loc
in in
let ltr = make_trace_use_self_weakself domain in let ltr = make_trace_use_self_weakself domain in
Reporting.log_error summary ~ltr ~loc:selfLoc IssueType.mixed_self_weakself message Reporting.log_error summary ~ltr ~loc:self.loc IssueType.mixed_self_weakself message
| _ ->
()
let report_weakself_in_no_escape_block_issues summary domain attributes = let report_weakself_in_no_escape_block_issues summary domain (weakSelf : DomainData.t) =
if attributes.ProcAttributes.is_no_escape_block then
let weakSelf_opt =
Vars.filter (fun _ {kind} -> DomainData.is_weak_self kind) domain |> Vars.choose_opt
in
match weakSelf_opt with
| Some (_, {pvar= weakSelf; loc= weakLoc}) ->
let message = let message =
F.asprintf F.asprintf
"This block uses `%a` at %a. This is probably not needed since the block is passed to \ "This block uses `%a` at %a. This is probably not needed since the block is passed to a \
a method in a position annotated with NS_NOESCAPE. Use `self` instead." method in a position annotated with NS_NOESCAPE. Use `self` instead."
(Pvar.pp Pp.text) weakSelf Location.pp weakLoc (Pvar.pp Pp.text) weakSelf.pvar Location.pp weakSelf.loc
in in
let ltr = make_trace_use_self_weakself domain in let ltr = make_trace_use_self_weakself domain in
Reporting.log_error summary ~ltr ~loc:weakLoc IssueType.weak_self_in_noescape_block message Reporting.log_error summary ~ltr ~loc:weakSelf.loc IssueType.weak_self_in_noescape_block message
| _ ->
()
let report_weakself_multiple_issues summary domain = let report_weakself_multiple_issue summary domain (weakSelf1 : DomainData.t)
let weakSelfs = (weakSelf2 : DomainData.t) =
Vars.filter (fun _ {kind} -> DomainData.is_weak_self kind) domain
|> Vars.bindings |> List.map ~f:snd
in
let sorted_WeakSelfs =
List.sort weakSelfs ~compare:(fun {DomainData.loc= loc1} {DomainData.loc= loc2} ->
Location.compare loc1 loc2 )
in
match sorted_WeakSelfs with
| {pvar= weakSelf; loc= weakLoc1} :: {loc= weakLoc2} :: _ ->
let message = let message =
F.asprintf F.asprintf
"This block uses the weak pointer `%a` more than once (%a) and (%a). This could lead to \ "This block uses the weak pointer `%a` more than once (%a) and (%a). This could lead to \
unexpected behavior. Even if `%a` is not nil in the first use, it could be nil in the \ unexpected behavior. Even if `%a` is not nil in the first use, it could be nil in the \
following uses since the object that `%a` points to could be freed anytime; assign it \ following uses since the object that `%a` points to could be freed anytime; assign it to a \
to a strong variable first." strong variable first."
(Pvar.pp Pp.text) weakSelf Location.pp weakLoc1 Location.pp weakLoc2 (Pvar.pp Pp.text) (Pvar.pp Pp.text) weakSelf1.pvar Location.pp weakSelf1.loc Location.pp weakSelf2.loc
weakSelf (Pvar.pp Pp.text) weakSelf (Pvar.pp Pp.text) weakSelf1.pvar (Pvar.pp Pp.text) weakSelf1.pvar
in in
let ltr = make_trace_use_self_weakself domain in let ltr = make_trace_use_self_weakself domain in
Reporting.log_error summary ~ltr ~loc:weakLoc1 IssueType.multiple_weakself message Reporting.log_error summary ~ltr ~loc:weakSelf1.loc IssueType.multiple_weakself message
| _ ->
()
let make_trace_captured_strong_self domain = let report_captured_strongself_issue domain summary (capturedStrongSelf : DomainData.t) =
let trace_elems = let message =
Vars.fold F.asprintf
(fun _ {pvar; loc; kind} trace_elems -> "The variable `%a`, used at `%a`, is a strong pointer to `self` captured in this block. This \
match kind with could lead to retain cycles or unexpected behavior since to avoid retain cycles one usually \
| CAPTURED_STRONG_SELF -> uses a local strong pointer or a captured weak pointer instead."
let trace_elem_desc = F.asprintf "Using captured %a" (Pvar.pp Pp.text) pvar in (Pvar.pp Pp.text) capturedStrongSelf.pvar Location.pp capturedStrongSelf.loc
let trace_elem = Errlog.make_trace_element 0 loc trace_elem_desc [] in
trace_elem :: trace_elems
| _ ->
trace_elems )
domain []
in in
List.sort trace_elems ~compare:(fun {Errlog.lt_loc= loc1} {Errlog.lt_loc= loc2} -> let ltr = make_trace_captured_strong_self domain in
Location.compare loc1 loc2 ) Reporting.log_error summary ~ltr ~loc:capturedStrongSelf.loc IssueType.captured_strong_self
message
let report_captured_strongself_issues summary domain = let report_issues summary domain attributes =
let capturedStrongSelf_opt = let process_domain_item _ (domain_data : DomainData.t) (weakSelfList, selfList) =
Vars.filter (fun _ {kind} -> DomainData.is_captured_strong_self kind) domain |> Vars.choose_opt match domain_data.kind with
| DomainData.CAPTURED_STRONG_SELF ->
report_captured_strongself_issue domain summary domain_data ;
(weakSelfList, selfList)
| DomainData.WEAK_SELF ->
if attributes.ProcAttributes.is_no_escape_block then
report_weakself_in_no_escape_block_issues summary domain domain_data ;
(domain_data :: weakSelfList, selfList)
| DomainData.SELF ->
(weakSelfList, domain_data :: selfList)
| _ ->
(weakSelfList, selfList)
in in
match capturedStrongSelf_opt with let weakSelfList, selfList = Vars.fold process_domain_item domain ([], []) in
| Some (_, {pvar; loc}) -> let sorted_WeakSelfList =
let message = List.sort weakSelfList ~compare:(fun {DomainData.loc= loc1} {DomainData.loc= loc2} ->
F.asprintf Location.compare loc1 loc2 )
"The variable `%a`, used at `%a`, is a strong pointer to `self` captured in this block. \
This could lead to retain cycles or unexpected behavior since to avoid retain cycles \
one usually uses a local strong pointer or a captured weak pointer instead."
(Pvar.pp Pp.text) pvar Location.pp loc
in in
let ltr = make_trace_captured_strong_self domain in let sorted_selfList =
Reporting.log_error summary ~ltr ~loc IssueType.captured_strong_self message List.sort selfList ~compare:(fun {DomainData.loc= loc1} {DomainData.loc= loc2} ->
Location.compare loc1 loc2 )
in
( match (sorted_WeakSelfList, sorted_selfList) with
| weakSelf :: _, self :: _ ->
report_mix_self_weakself_issues summary domain weakSelf self
| _ ->
() ) ;
match sorted_WeakSelfList with
| weakSelf1 :: weakSelf2 :: _ ->
report_weakself_multiple_issue summary domain weakSelf1 weakSelf2
| _ -> | _ ->
() ()
@ -497,10 +495,7 @@ let checker {Callbacks.exe_env; summary} =
( if Procname.is_objc_block procname then ( if Procname.is_objc_block procname then
match Analyzer.compute_post proc_data ~initial with match Analyzer.compute_post proc_data ~initial with
| Some domain -> | Some domain ->
report_mix_self_weakself_issues summary domain.vars ; report_issues summary domain.vars attributes
report_weakself_multiple_issues summary domain.vars ;
report_captured_strongself_issues summary domain.vars ;
report_weakself_in_no_escape_block_issues summary domain.vars attributes
| None -> | None ->
() ) ; () ) ;
summary summary

2
infer/tests/codetoanalyze/objc/self-in-block/issues.exp
Unescape View File

@ -1,5 +1,5 @@
codetoanalyze/objc/self-in-block/NoescapeBlock.m, objc_blockA::weak_in_noescape_block_bad:_1, 1, WEAK_SELF_IN_NO_ESCAPE_BLOCK, no_bucket, ERROR, [Using &weakSelf] codetoanalyze/objc/self-in-block/NoescapeBlock.m, objc_blockA::weak_in_noescape_block_bad:_1, 1, WEAK_SELF_IN_NO_ESCAPE_BLOCK, no_bucket, ERROR, [Using &weakSelf]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::mixSelfWeakSelf_bad_1, 5, MIXED_SELF_WEAKSELF, no_bucket, ERROR, [Using &weakSelf,Using &self,Using &self] codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::mixSelfWeakSelf_bad_1, 4, MIXED_SELF_WEAKSELF, no_bucket, ERROR, [Using &weakSelf,Using &self,Using &self]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheck3_bad_7, 2, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null] codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheck3_bad_7, 2, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 6, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null] codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 6, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null]
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, 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]

Write Preview
Loading…
Cancel
Save