From bf581e0b72309167890ad188f23b73f801869184 Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Thu, 14 Nov 2019 07:57:35 -0800 Subject: [PATCH] [self in block] Add a check for strongSelf not checked for null Summary: The variable strongSelf, because it is equal to a weak captured pointer, needs to be checked for null before being used, otherwise it could be null by the time the block is executed. Added this check to the SelfInBlock checker, and removed it from biabduction. We want to migrate all the objc checks from biabduction, so it will be easier to change and faster and more reliable. Moreover, this check is more general, it will flag any use of unchecked strongSelf, not just a dereference. Reviewed By: skcho Differential Revision: D18403849 fbshipit-source-id: a9cf5d80b --- infer/man/man1/infer-full.txt | 1 + infer/man/man1/infer-report.txt | 1 + infer/man/man1/infer.txt | 1 + infer/src/IR/Localise.ml | 8 - infer/src/IR/Localise.mli | 3 - infer/src/base/IssueType.ml | 6 +- infer/src/base/IssueType.mli | 4 +- infer/src/biabduction/Rearrange.ml | 33 +-- infer/src/checkers/SelfInBlock.ml | 215 +++++++++++++++--- .../codetoanalyze/objc/errors/issues.exp | 1 - .../objc/self-in-block/StrongSelf.m | 83 +++++++ .../objc/self-in-block/issues.exp | 6 +- 12 files changed, 290 insertions(+), 72 deletions(-) diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt index b0bc18d17..93961a8c1 100644 --- a/infer/man/man1/infer-full.txt +++ b/infer/man/man1/infer-full.txt @@ -475,6 +475,7 @@ OPTIONS STARVATION (enabled by default), STATIC_INITIALIZATION_ORDER_FIASCO (enabled by default), STRICT_MODE_VIOLATION (enabled by default), + STRONG_SELF_NOT_CHECKED (enabled by default), Symexec_memory_error (enabled by default), TAINTED_BUFFER_ACCESS (enabled by default), TAINTED_MEMORY_ALLOCATION (enabled by default), diff --git a/infer/man/man1/infer-report.txt b/infer/man/man1/infer-report.txt index 93a946621..ae20cc1cb 100644 --- a/infer/man/man1/infer-report.txt +++ b/infer/man/man1/infer-report.txt @@ -217,6 +217,7 @@ OPTIONS STARVATION (enabled by default), STATIC_INITIALIZATION_ORDER_FIASCO (enabled by default), STRICT_MODE_VIOLATION (enabled by default), + STRONG_SELF_NOT_CHECKED (enabled by default), Symexec_memory_error (enabled by default), TAINTED_BUFFER_ACCESS (enabled by default), TAINTED_MEMORY_ALLOCATION (enabled by default), diff --git a/infer/man/man1/infer.txt b/infer/man/man1/infer.txt index 67a9fce81..3dc0c52aa 100644 --- a/infer/man/man1/infer.txt +++ b/infer/man/man1/infer.txt @@ -475,6 +475,7 @@ OPTIONS STARVATION (enabled by default), STATIC_INITIALIZATION_ORDER_FIASCO (enabled by default), STRICT_MODE_VIOLATION (enabled by default), + STRONG_SELF_NOT_CHECKED (enabled by default), Symexec_memory_error (enabled by default), TAINTED_BUFFER_ACCESS (enabled by default), TAINTED_MEMORY_ALLOCATION (enabled by default), diff --git a/infer/src/IR/Localise.ml b/infer/src/IR/Localise.ml index b5ef77cf9..c816212f8 100644 --- a/infer/src/IR/Localise.ml +++ b/infer/src/IR/Localise.ml @@ -200,14 +200,6 @@ let deref_str_nullable proc_name_opt nullable_obj_str = deref_str_null proc_name_opt -(** dereference strings for null dereference due to weak captured variable in block *) -let deref_str_weak_variable_in_block proc_name_opt nullable_obj_str = - let tags = Tags.create () in - Tags.update tags Tags.weak_captured_var_src nullable_obj_str ; - let problem_str = "" in - deref_str_null_ proc_name_opt problem_str - - (** dereference strings for nonterminal nil arguments in c/objc variadic methods *) let deref_str_nil_argument_in_variadic_method pn total_args arg_number = let function_method, nil_null = diff --git a/infer/src/IR/Localise.mli b/infer/src/IR/Localise.mli index 707d4d72d..d2fc1f084 100644 --- a/infer/src/IR/Localise.mli +++ b/infer/src/IR/Localise.mli @@ -69,9 +69,6 @@ val deref_str_null : Typ.Procname.t option -> deref_str val deref_str_nullable : Typ.Procname.t option -> string -> deref_str (** dereference strings for null dereference due to Nullable annotation *) -val deref_str_weak_variable_in_block : Typ.Procname.t option -> string -> deref_str -(** dereference strings for null dereference due to weak captured variable in block *) - val deref_str_undef : Typ.Procname.t * Location.t -> deref_str (** dereference strings for an undefined value coming from the given procedure *) diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 9e013f614..aa55ed446 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -345,6 +345,8 @@ let missing_fld = register_from_string "Missing_fld" ~hum:"Missing Field" let missing_required_prop = register_from_string "MISSING_REQUIRED_PROP" +let mixed_self_weakself = register_from_string "MIXED_SELF_WEAKSELF" ~hum:"Mixed Self WeakSelf" + let mutable_local_variable_in_component_file = register_from_string "MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE" @@ -413,7 +415,9 @@ let strict_mode_violation = register_from_string "STRICT_MODE_VIOLATION" ~hum:"Strict Mode Violation" -let mixed_self_weakself = register_from_string "MIXED_SELF_WEAKSELF" ~hum:"Mixed Self WeakSelf" +let strong_self_not_checked = + register_from_string "STRONG_SELF_NOT_CHECKED" ~hum:"Strong Self Not Checked" + let symexec_memory_error = register_from_string "Symexec_memory_error" ~hum:"Symbolic Execution Memory Error" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index 6f84e4565..07ca4c44a 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -230,6 +230,8 @@ val missing_fld : t val missing_required_prop : t +val mixed_self_weakself : t + val mutable_local_variable_in_component_file : t val null_dereference : t @@ -286,7 +288,7 @@ val static_initialization_order_fiasco : t val strict_mode_violation : t -val mixed_self_weakself : t +val strong_self_not_checked : t val symexec_memory_error : t diff --git a/infer/src/biabduction/Rearrange.ml b/infer/src/biabduction/Rearrange.ml index 27ea2baf0..4c642ba82 100644 --- a/infer/src/biabduction/Rearrange.ml +++ b/infer/src/biabduction/Rearrange.ml @@ -1463,29 +1463,11 @@ let rec iter_rearrange pname tenv lexp typ_from_instr prop iter inst : res -let is_weak_captured_var pdesc var_name = - let pname = Procdesc.get_proc_name pdesc in - match pname with - | Block _ -> - let is_weak_captured (var, typ) = - match typ.Typ.desc with - | Typ.Tptr (_, Pk_objc_weak) -> - String.equal var_name (Mangled.to_string var) - | _ -> - false - in - List.exists ~f:is_weak_captured (Procdesc.get_captured pdesc) - | _ -> - false - - -let var_has_annotation ?(check_weak_captured_var = false) pdesc is_annotation pvar = - let is_weak_captured_var = is_weak_captured_var pdesc (Pvar.to_string pvar) in +let var_has_annotation pdesc is_annotation pvar = let ann_sig = Models.get_modelled_annotated_signature_for_biabduction (Procdesc.get_attributes pdesc) in AnnotatedSignature.param_has_annot is_annotation pvar ann_sig - || (check_weak_captured_var && is_weak_captured_var) let attr_has_annot is_annotation tenv prop exp = @@ -1521,16 +1503,13 @@ let is_strexp_pt_fld_with_annot tenv obj_str is_annotation typ deref_exp (fld, s (* This returns true if the exp is pointed to only by fields or parameters with a given annotation. In that case it also returns a string representation of the annotation recipient. *) -let is_only_pt_by_fld_or_param_with_annot ?(check_weak_captured_var = false) pdesc tenv prop - deref_exp is_annotation = +let is_only_pt_by_fld_or_param_with_annot pdesc tenv prop deref_exp is_annotation = let obj_str = ref None in let is_pt_by_fld_or_param_with_annot hpred = match hpred with | Sil.Hpointsto (Exp.Lvar pvar, Sil.Eexp ((Exp.Var _ as exp), _), _) when Exp.equal exp deref_exp -> - let var_has_annotation = - Pvar.is_seed pvar && var_has_annotation ~check_weak_captured_var pdesc is_annotation pvar - in + let var_has_annotation = Pvar.is_seed pvar && var_has_annotation pdesc is_annotation pvar in if var_has_annotation then obj_str := Some (Pvar.to_string pvar) ; let procname_str_opt = attr_has_annot is_annotation tenv prop exp in if Option.is_some procname_str_opt then obj_str := procname_str_opt ; @@ -1547,8 +1526,7 @@ let is_only_pt_by_fld_or_param_with_annot ?(check_weak_captured_var = false) pde let is_only_pt_by_fld_or_param_nullable pdesc tenv prop deref_exp = - is_only_pt_by_fld_or_param_with_annot ~check_weak_captured_var:true pdesc tenv prop deref_exp - Annotations.ia_is_nullable + is_only_pt_by_fld_or_param_with_annot pdesc tenv prop deref_exp Annotations.ia_is_nullable let is_only_pt_by_fld_or_param_nonnull pdesc tenv prop deref_exp = @@ -1599,8 +1577,7 @@ let check_dereference_error tenv pdesc (prop : Prop.normal Prop.t) lexp loc = let deref_str = match nullable_var_opt with | Some str -> - if is_weak_captured_var pdesc str then Localise.deref_str_weak_variable_in_block None str - else Localise.deref_str_nullable None str + Localise.deref_str_nullable None str | None -> Localise.deref_str_null None in diff --git a/infer/src/checkers/SelfInBlock.ml b/infer/src/checkers/SelfInBlock.ml index 528c675f7..e9d246dc6 100644 --- a/infer/src/checkers/SelfInBlock.ml +++ b/infer/src/checkers/SelfInBlock.ml @@ -8,27 +8,123 @@ open! IStd module F = Format module DomainData = struct - type self_pointer_kind = SELF | WEAK_SELF [@@deriving compare] + type self_pointer_kind = SELF | UNCHECKED_STRONG_SELF | CHECKED_STRONG_SELF | WEAK_SELF + [@@deriving compare] + + let is_self kind = match kind with SELF -> true | _ -> false - let is_self kind = match kind with SELF -> true | WEAK_SELF -> false + let is_weak_self kind = match kind with WEAK_SELF -> true | _ -> false - let is_weak_self kind = match kind with SELF -> false | WEAK_SELF -> true + let is_unchecked_strong_self kind = match kind with UNCHECKED_STRONG_SELF -> true | _ -> false let pp_self_pointer_kind fmt kind = - let s = match kind with SELF -> "SELF" | WEAK_SELF -> "WEAK_SELF" in + let s = + match kind with + | CHECKED_STRONG_SELF -> + "CHECKED_STRONG_SELF" + | SELF -> + "SELF" + | UNCHECKED_STRONG_SELF -> + "UNCHECKED_STRONG_SELF" + | WEAK_SELF -> + "WEAK_SELF" + in F.fprintf fmt "%s" s + let kind_join kind1 kind2 = + if phys_equal kind1 kind2 then kind1 + else + match (kind1, kind2) with + | CHECKED_STRONG_SELF, _ | _, CHECKED_STRONG_SELF -> + CHECKED_STRONG_SELF + | _ -> + Logging.die InternalError + "The only unequal kinds that can be joined in this domain are CHECKED_STRONG_SELF and \ + UNCHECKED_STRONG_SELF, but found %a, %a" + pp_self_pointer_kind kind1 pp_self_pointer_kind kind2 + + + let kind_leq kind1 kind2 = + if phys_equal kind1 kind2 then true + else + match (kind1, kind2) with + | CHECKED_STRONG_SELF, UNCHECKED_STRONG_SELF -> + false + | UNCHECKED_STRONG_SELF, CHECKED_STRONG_SELF -> + true + | _ -> + Logging.die InternalError + "The only unequal kinds that can be compared in this domain are CHECKED_STRONG_SELF \ + and UNCHECKED_STRONG_SELF, but found %a, %a" + pp_self_pointer_kind kind1 pp_self_pointer_kind kind2 + + type t = {pvar: Pvar.t; typ: Typ.t [@compare.ignore]; loc: Location.t; kind: self_pointer_kind} [@@deriving compare] let pp fmt {pvar; typ; loc; kind} = F.fprintf fmt "%a:%a, at %a (%a)" (Pvar.pp Pp.text) pvar (Typ.pp Pp.text) typ Location.pp loc pp_self_pointer_kind kind + + + let join elem1 elem2 = + assert (Pvar.equal elem1.pvar elem2.pvar) ; + {elem1 with kind= kind_join elem1.kind elem2.kind} + + + let widen ~prev ~next ~num_iters:_ = join prev next + + let leq ~lhs ~rhs = + assert (Pvar.equal lhs.pvar rhs.pvar) ; + kind_leq lhs.kind rhs.kind +end + +module PPPVar = struct + type t = Pvar.t [@@deriving compare] + + let pp = Pvar.pp Pp.text +end + +module CheckedForNull = struct + type t = {checked: bool} + + let pp fmt {checked} = + let s = match checked with true -> "Checked" | false -> "NotChecked" in + F.fprintf fmt "%s" s + + + let join {checked= elem1} {checked= elem2} = {checked= AbstractDomain.BooleanAnd.join elem1 elem2} + + let widen ~prev ~next ~num_iters:_ = join prev next + + let leq ~lhs:{checked= lhs} ~rhs:{checked= rhs} = AbstractDomain.BooleanAnd.leq ~lhs ~rhs +end + +module StrongEqualToWeakCapturedVars = AbstractDomain.Map (PPPVar) (CheckedForNull) +module Vars = AbstractDomain.Map (Ident) (DomainData) + +module Domain = struct + type t = {vars: Vars.t; strongVars: StrongEqualToWeakCapturedVars.t} + + let pp fmt {vars; strongVars} = + F.fprintf fmt "%a@.%a" Vars.pp vars StrongEqualToWeakCapturedVars.pp strongVars + + + let join lhs rhs = + { vars= Vars.join lhs.vars rhs.vars + ; strongVars= StrongEqualToWeakCapturedVars.join lhs.strongVars rhs.strongVars } + + + let widen ~prev ~next ~num_iters:_ = join prev next + + let leq ~lhs ~rhs = + Vars.leq ~lhs:lhs.vars ~rhs:rhs.vars + && StrongEqualToWeakCapturedVars.leq ~lhs:lhs.strongVars ~rhs:rhs.strongVars end module TransferFunctions = struct - module Domain = AbstractDomain.FiniteSet (DomainData) + module Domain = Domain module CFG = ProcCfg.Normal type extras = unit @@ -52,66 +148,127 @@ module TransferFunctions = struct attributes.ProcAttributes.captured + let exec_null_check_id (astate : Domain.t) id = + try + let elem = Vars.find id astate.vars in + if StrongEqualToWeakCapturedVars.mem elem.pvar astate.strongVars then + let strongVars = + StrongEqualToWeakCapturedVars.add elem.pvar {checked= true} astate.strongVars + in + let vars = + (* We may have added UNCHECKED_STRONG_SELF in the previous Load instr, + but this occurrence is not a bug, since we are checking right here! *) + Vars.add id {elem with kind= CHECKED_STRONG_SELF} astate.vars + in + {Domain.vars; strongVars} + else astate + with Not_found_s _ | Caml.Not_found -> astate + + let exec_instr (astate : Domain.t) {ProcData.summary} _cfg_node (instr : Sil.instr) = let attributes = Summary.get_attributes summary in match instr with - | Load {e= Lvar pvar; loc; typ} -> - if is_captured_strong_self attributes pvar then - Domain.add {pvar; typ; loc; kind= SELF} astate - else if is_captured_weak_self attributes pvar then - Domain.add {pvar; typ; loc; kind= WEAK_SELF} astate - else astate + | Load {id; e= Lvar pvar; loc; typ} -> + let vars = + if is_captured_strong_self attributes pvar then + Vars.add id {pvar; typ; loc; kind= SELF} astate.vars + else if is_captured_weak_self attributes pvar then + Vars.add id {pvar; typ; loc; kind= WEAK_SELF} astate.vars + else + try + let isChecked = StrongEqualToWeakCapturedVars.find pvar astate.strongVars in + if not isChecked.checked then + Vars.add id {pvar; typ; loc; kind= UNCHECKED_STRONG_SELF} astate.vars + else astate.vars + with Not_found_s _ | Caml.Not_found -> astate.vars + in + {astate with vars} + | Store {e1= Lvar pvar; e2= Var id; typ= pvar_typ} -> + let strongVars = + try + let {DomainData.pvar= binding_for_id} = Vars.find id astate.vars in + if is_captured_weak_self attributes binding_for_id && Typ.is_strong_pointer pvar_typ + then StrongEqualToWeakCapturedVars.add pvar {checked= false} astate.strongVars + else astate.strongVars + with Not_found_s _ | Caml.Not_found -> astate.strongVars + in + {astate with strongVars} + | Prune (Var id, _, _, Sil.Ik_if) -> + exec_null_check_id astate id + (* If (strongSelf != nil) or equivalent else branch *) + | Prune (BinOp (Binop.Ne, Var id, e), _, _, Sil.Ik_if) + (* If (!(strongSelf == nil)) or equivalent else branch *) + | Prune (UnOp (LNot, BinOp (Binop.Eq, Var id, e), _), _, _, Sil.Ik_if) -> + if Exp.is_null_literal e then exec_null_check_id astate id else astate | _ -> astate end -let make_trace_elements domain = +let make_trace_mixed_self_weakself domain = let trace_elems = - TransferFunctions.Domain.fold - (fun {pvar; loc} trace_elems -> - let trace_elem_desc = F.asprintf "Using %a" (Pvar.pp Pp.text) pvar in - let trace_elem = Errlog.make_trace_element 0 loc trace_elem_desc [] in - trace_elem :: trace_elems ) + Vars.fold + (fun _ {pvar; loc; kind} trace_elems -> + match kind with + | SELF | WEAK_SELF -> + let trace_elem_desc = F.asprintf "Using %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 List.sort trace_elems ~compare:(fun {Errlog.lt_loc= loc1} {Errlog.lt_loc= loc2} -> Location.compare loc1 loc2 ) -let report_issues summary domain = +let report_mix_self_weakself_issues summary domain = let weakSelf_opt = - TransferFunctions.Domain.filter (fun {kind} -> DomainData.is_weak_self kind) domain - |> TransferFunctions.Domain.choose_opt - in - let self_opt = - TransferFunctions.Domain.filter (fun {kind} -> DomainData.is_self kind) domain - |> TransferFunctions.Domain.choose_opt + Vars.filter (fun _ {kind} -> DomainData.is_weak_self kind) domain |> Vars.choose_opt in + let self_opt = Vars.filter (fun _ {kind} -> DomainData.is_self kind) domain |> Vars.choose_opt in match (weakSelf_opt, self_opt) with - | Some {pvar= weakSelf; loc= weakLoc}, Some {pvar= self; loc= selfLoc} -> + | Some (_, {pvar= weakSelf; loc= weakLoc}), Some (_, {pvar= self; loc= selfLoc}) -> let message = 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." (Pvar.pp Pp.text) weakSelf Location.pp weakLoc (Pvar.pp Pp.text) self Location.pp selfLoc in - let ltr = make_trace_elements domain in + let ltr = make_trace_mixed_self_weakself domain in Reporting.log_error summary ~ltr ~loc:selfLoc IssueType.mixed_self_weakself message | _ -> () +let report_unchecked_strongself_issues summary domain = + let unchecked_strongSelf_opt = + Vars.filter (fun _ {kind} -> DomainData.is_unchecked_strong_self kind) domain |> Vars.choose_opt + in + match unchecked_strongSelf_opt with + | Some (_, {pvar; loc}) -> + let message = + F.asprintf + "The variable `%a`, equal to a weak pointer to `self`, is used without a check for null \ + at %a. This could cause a crash." + (Pvar.pp Pp.text) pvar Location.pp loc + in + Reporting.log_error summary ~loc IssueType.strong_self_not_checked message + | _ -> + () + + module Analyzer = AbstractInterpreter.MakeWTO (TransferFunctions) let checker {Callbacks.exe_env; summary} = - let initial = TransferFunctions.Domain.empty in + let initial = {Domain.vars= Vars.empty; strongVars= StrongEqualToWeakCapturedVars.empty} in let procname = Summary.get_proc_name summary in let tenv = Exe_env.get_tenv exe_env procname in let proc_data = ProcData.make summary tenv () in ( if Typ.Procname.is_objc_block procname then match Analyzer.compute_post proc_data ~initial with | Some domain -> - report_issues summary domain + report_mix_self_weakself_issues summary domain.vars ; + report_unchecked_strongself_issues summary domain.vars | None -> () ) ; summary diff --git a/infer/tests/codetoanalyze/objc/errors/issues.exp b/infer/tests/codetoanalyze/objc/errors/issues.exp index 843f58ea3..44f657008 100644 --- a/infer/tests/codetoanalyze/objc/errors/issues.exp +++ b/infer/tests/codetoanalyze/objc/errors/issues.exp @@ -45,7 +45,6 @@ codetoanalyze/objc/errors/npe/UpdateDict.m, nullable_NSDictionary_objectForKeyed codetoanalyze/objc/errors/npe/UpdateDict.m, nullable_NSMapTable_objectForKey, 4, NULL_DEREFERENCE, B5, ERROR, [start of procedure nullable_NSMapTable_objectForKey(),Taking true branch,Condition is true,Taking true branch] codetoanalyze/objc/errors/npe/UpdateDict.m, update_array_with_null, 5, NULL_DEREFERENCE, B1, ERROR, [start of procedure update_array_with_null()] codetoanalyze/objc/errors/npe/UpdateDict.m, update_dict_with_key_null, 10, NULL_DEREFERENCE, B1, ERROR, [start of procedure update_dict_with_key_null(),Skipping dictionaryWithObjectsAndKeys:: method has no implementation] -codetoanalyze/objc/errors/npe/WeakCapturedVarsNPE.m, objc_blockWeakCapturedA::strongSelfNoCheck_2, 2, NULL_DEREFERENCE, B1, ERROR, [start of procedure block] codetoanalyze/objc/errors/npe/nil_in_array_literal.m, Arr::insertNilBad, 3, NULL_DEREFERENCE, B1, ERROR, [start of procedure insertNilBad,start of procedure initA,Taking true branch,return from a call to A::initA] codetoanalyze/objc/errors/npe/nil_in_array_literal.m, Arr::nilInArrayLiteral0, 4, NULL_DEREFERENCE, B1, ERROR, [start of procedure nilInArrayLiteral0] codetoanalyze/objc/errors/npe/nil_in_array_literal.m, Arr::nilInArrayLiteral1, 4, NULL_DEREFERENCE, B1, ERROR, [start of procedure nilInArrayLiteral1] diff --git a/infer/tests/codetoanalyze/objc/self-in-block/StrongSelf.m b/infer/tests/codetoanalyze/objc/self-in-block/StrongSelf.m index 86779d855..d14c37c0e 100644 --- a/infer/tests/codetoanalyze/objc/self-in-block/StrongSelf.m +++ b/infer/tests/codetoanalyze/objc/self-in-block/StrongSelf.m @@ -33,4 +33,87 @@ }; } +- (void)strongSelfNoCheckNotWeakSelf_good { + __typeof(self) weakSelf = self; + int (^my_block)(BOOL) = ^(BOOL isTapped) { + __strong __typeof(weakSelf) strongSelf = weakSelf; + return strongSelf->x; + }; +} + +- (void)strongSelfNoCheck_bad { + __weak __typeof(self) weakSelf = self; + int (^my_block)(BOOL) = ^(BOOL isTapped) { + __strong __typeof(weakSelf) strongSelf = weakSelf; + [strongSelf foo]; + return 0; + }; +} + +// very unlikely pattern, but still complies with invariant: +// any use of strongSelf is bad unless checked for null beforehand +- (void)strongSelfNoCheck2_bad { + __weak __typeof(self) weakSelf = self; + int (^my_block)() = ^() { + __strong __typeof(weakSelf) strongSelf = weakSelf; + __strong __typeof(weakSelf) strongSelf2 = strongSelf; + return 0; + }; +} + +- (void)strongSelfCheckOnce_bad { + __weak __typeof(self) weakSelf = self; + int (^my_block)(BOOL) = ^(BOOL isTapped) { + __strong __typeof(weakSelf) strongSelf = weakSelf; + if (strongSelf) { + [strongSelf foo]; + int x = strongSelf->x; + } else { + [strongSelf foo]; + } + [strongSelf foo]; + if (strongSelf != nil) { + [strongSelf foo]; + int x = strongSelf->x; + } + return 0; + }; +} + +- (void)strongSelfCheck2_good { + __weak __typeof(self) weakSelf = self; + int (^my_block)(BOOL) = ^(BOOL isTapped) { + __strong __typeof(weakSelf) strongSelf = weakSelf; + if (!strongSelf) { + return 0; + } else { + [strongSelf foo]; + } + return 0; + }; +} + +- (void)strongSelfCheck3_bad { + __weak __typeof(self) weakSelf = self; + int (^my_block)(BOOL) = ^(BOOL isTapped) { + __strong __typeof(weakSelf) strongSelf = weakSelf; + int y = strongSelf->x; + if (strongSelf == nil) { + return 0; + } + return strongSelf->x; + }; +} + +- (void)strongSelfCheck4_good { + __weak __typeof(self) weakSelf = self; + int (^my_block)() = ^() { + __strong __typeof(weakSelf) strongSelf = weakSelf; + if (!strongSelf) { + } else { + } + return 0; + }; +} + @end diff --git a/infer/tests/codetoanalyze/objc/self-in-block/issues.exp b/infer/tests/codetoanalyze/objc/self-in-block/issues.exp index e27ae9926..461bd5e53 100644 --- a/infer/tests/codetoanalyze/objc/self-in-block/issues.exp +++ b/infer/tests/codetoanalyze/objc/self-in-block/issues.exp @@ -1 +1,5 @@ -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::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::strongSelfCheck3_bad_7, 2, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [] +codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 8, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [] +codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfNoCheck2_bad_4, 2, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [] +codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfNoCheck_bad_3, 2, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, []