[self in block] Add a new checker to detect correct uses of when ObjC blocks capture self.

Reviewed By: skcho

Differential Revision: D18245267

fbshipit-source-id: 6e3f1a7f7
master
Dulma Churchill 5 years ago committed by Facebook Github Bot
parent 4d708befd1
commit 43823266ec

@ -111,6 +111,7 @@ DIRECT_TESTS += \
objc_performance \
objc_pulse \
objc_quandary \
objc_self-in-block \
objc_uninit \
objcpp_errors \
objcpp_frontend \

@ -103,8 +103,8 @@ OPTIONS
--no-default-checkers
Deactivates: Default checkers: --biabduction,
--fragment-retains-view, --inefficient-keyset-iterator, --linters,
--liveness, --racerd, --siof, --starvation, --uninit (Conversely:
--default-checkers)
--liveness, --racerd, --siof, --starvation, --self_in_block,
--uninit (Conversely: --default-checkers)
--eradicate
Activates: the eradicate @Nullable checker for Java annotations
@ -308,6 +308,14 @@ OPTIONS
--results-dir,-o dir
Write results and internal files in the specified directory
--no-self_in_block
Deactivates: checker to flag incorrect uses of when Objective-C
blocks capture self (Conversely: --self_in_block)
--self_in_block-only
Activates: Enable --self_in_block and disable all other checkers
(Conversely: --no-self_in_block-only)
--no-siof
Deactivates: the Static Initialization Order Fiasco analysis (C++
only) (Conversely: --siof)

@ -310,8 +310,8 @@ OPTIONS
--no-default-checkers
Deactivates: Default checkers: --biabduction,
--fragment-retains-view, --inefficient-keyset-iterator, --linters,
--liveness, --racerd, --siof, --starvation, --uninit (Conversely:
--default-checkers) See also infer-analyze(1).
--liveness, --racerd, --siof, --starvation, --self_in_block,
--uninit (Conversely: --default-checkers) See also infer-analyze(1).
--no-default-linters
Deactivates: Use the default linters for the analysis.
@ -440,6 +440,7 @@ OPTIONS
Leak_in_footprint (enabled by default),
MEMORY_LEAK (enabled by default),
MISSING_REQUIRED_PROP (enabled by default),
MIXED_SELF_WEAKSELF (enabled by default),
MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE (enabled by default),
Missing_fld (enabled by default),
NULLSAFE_FIELD_NOT_NULLABLE (enabled by default),
@ -990,6 +991,14 @@ OPTIONS
--select N
Select bug number N. If omitted, prompt for input. See also infer-explore(1).
--no-self_in_block
Deactivates: checker to flag incorrect uses of when Objective-C
blocks capture self (Conversely: --self_in_block) See also infer-analyze(1).
--self_in_block-only
Activates: Enable --self_in_block and disable all other checkers
(Conversely: --no-self_in_block-only) See also infer-analyze(1).
--no-siof
Deactivates: the Static Initialization Order Fiasco analysis (C++
only) (Conversely: --siof) See also infer-analyze(1).

@ -182,6 +182,7 @@ OPTIONS
Leak_in_footprint (enabled by default),
MEMORY_LEAK (enabled by default),
MISSING_REQUIRED_PROP (enabled by default),
MIXED_SELF_WEAKSELF (enabled by default),
MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE (enabled by default),
Missing_fld (enabled by default),
NULLSAFE_FIELD_NOT_NULLABLE (enabled by default),

@ -310,8 +310,8 @@ OPTIONS
--no-default-checkers
Deactivates: Default checkers: --biabduction,
--fragment-retains-view, --inefficient-keyset-iterator, --linters,
--liveness, --racerd, --siof, --starvation, --uninit (Conversely:
--default-checkers) See also infer-analyze(1).
--liveness, --racerd, --siof, --starvation, --self_in_block,
--uninit (Conversely: --default-checkers) See also infer-analyze(1).
--no-default-linters
Deactivates: Use the default linters for the analysis.
@ -440,6 +440,7 @@ OPTIONS
Leak_in_footprint (enabled by default),
MEMORY_LEAK (enabled by default),
MISSING_REQUIRED_PROP (enabled by default),
MIXED_SELF_WEAKSELF (enabled by default),
MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE (enabled by default),
Missing_fld (enabled by default),
NULLSAFE_FIELD_NOT_NULLABLE (enabled by default),
@ -990,6 +991,14 @@ OPTIONS
--select N
Select bug number N. If omitted, prompt for input. See also infer-explore(1).
--no-self_in_block
Deactivates: checker to flag incorrect uses of when Objective-C
blocks capture self (Conversely: --self_in_block) See also infer-analyze(1).
--self_in_block-only
Activates: Enable --self_in_block and disable all other checkers
(Conversely: --no-self_in_block-only) See also infer-analyze(1).
--no-siof
Deactivates: the Static Initialization Order Fiasco analysis (C++
only) (Conversely: --siof) See also infer-analyze(1).

@ -246,6 +246,10 @@ let is_restrict {is_restrict} = is_restrict
let is_volatile {is_volatile} = is_volatile
let is_weak_pointer t = match t.desc with Tptr (_, Pk_objc_weak) -> true | _ -> false
let is_strong_pointer t = match t.desc with Tptr (_, Pk_pointer) -> true | _ -> false
let mk ?default ?quals desc : t =
let default_ = {desc; quals= mk_type_quals ()} in
let mk_aux ?(default = default_) ?(quals = default.quals) desc = {desc; quals} in

@ -150,6 +150,10 @@ val get_ikind_opt : t -> ikind option
val size_t : ikind
(** ikind of size_t *)
val is_weak_pointer : t -> bool
val is_strong_pointer : t -> bool
module Name : sig
(** Named types. *)
type t = name [@@deriving compare]

@ -35,6 +35,7 @@ type checkers =
; liveness: bool ref
; loop_hoisting: bool ref
; nullsafe: bool ref
; self_in_block: bool ref
; printf_args: bool ref
; pulse: bool ref
; purity: bool ref
@ -649,6 +650,7 @@ and { annotation_reachability
; liveness
; loop_hoisting
; nullsafe
; self_in_block
; printf_args
; pulse
; purity
@ -726,6 +728,9 @@ and { annotation_reachability
mk_checker ~long:"siof" ~default:true
"the Static Initialization Order Fiasco analysis (C++ only)"
and starvation = mk_checker ~long:"starvation" ~default:true "starvation analysis"
and self_in_block =
mk_checker ~long:"self_in_block" ~default:true
"checker to flag incorrect uses of when Objective-C blocks capture self"
and uninit = mk_checker ~long:"uninit" "checker for use of uninitialized values" ~default:true in
let mk_only (var, long, doc, _) =
let (_ : bool ref) =
@ -785,6 +790,7 @@ and { annotation_reachability
; quandaryBO
; racerd
; resource_leak
; self_in_block
; siof
; starvation
; uninit }
@ -3005,6 +3011,8 @@ and only_footprint = !only_footprint
and only_show = !only_show
and self_in_block = !self_in_block
and passthroughs = !passthroughs
and patterns_modeled_expensive = match patterns_modeled_expensive with k, r -> (k, !r)

@ -506,6 +506,8 @@ val only_footprint : bool
val only_show : bool
val self_in_block : bool
val perf_profiler_data_file : string option
val pmd_xml : bool

@ -411,6 +411,8 @@ 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 symexec_memory_error =
register_from_string "Symexec_memory_error" ~hum:"Symbolic Execution Memory Error"

@ -286,6 +286,8 @@ val static_initialization_order_fiasco : t
val strict_mode_violation : t
val mixed_self_weakself : t
val symexec_memory_error : t
val tainted_buffer_access : t

@ -0,0 +1,101 @@
(*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*)
open! IStd
module F = Format
module DomainData = struct
type self_pointer_kind = SELF | WEAK_SELF [@@deriving compare]
let is_self kind = match kind with SELF -> true | WEAK_SELF -> false
let is_weak_self kind = match kind with SELF -> false | WEAK_SELF -> true
let pp_self_pointer_kind fmt kind =
let s = match kind with SELF -> "SELF" | WEAK_SELF -> "WEAK_SELF" in
F.fprintf fmt "%s" s
type t = {pvar: Pvar.t; typ: Typ.t; 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
end
module TransferFunctions = struct
module Domain = AbstractDomain.FiniteSet (DomainData)
module CFG = ProcCfg.Normal
type extras = unit
let pp_session_name _node fmt = F.pp_print_string fmt "SelfCapturedInBlock"
let is_captured_strong_self attributes pvar =
List.exists
~f:(fun (captured, typ) ->
Mangled.equal captured (Pvar.get_name pvar)
&& Pvar.is_self pvar && Typ.is_strong_pointer typ )
attributes.ProcAttributes.captured
let is_captured_weak_self attributes pvar =
List.exists
~f:(fun (captured, typ) ->
Mangled.equal captured (Pvar.get_name pvar)
&& String.is_substring ~substring:"self" (String.lowercase (Mangled.to_string captured))
&& Typ.is_weak_pointer typ )
attributes.ProcAttributes.captured
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
| _ ->
astate
end
let report_issues summary domain =
let weakSelf_opt =
TransferFunctions.Domain.find_first_opt (fun {kind} -> DomainData.is_weak_self kind) domain
in
let self_opt =
TransferFunctions.Domain.find_first_opt (fun {kind} -> DomainData.is_self kind) domain
in
match (weakSelf_opt, self_opt) with
| 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 \
unexpected behavior."
(Pvar.pp Pp.text) weakSelf Location.pp weakLoc (Pvar.pp Pp.text) self Location.pp selfLoc
in
Reporting.log_error summary ~loc:selfLoc IssueType.mixed_self_weakself message
| _ ->
()
module Analyzer = AbstractInterpreter.MakeWTO (TransferFunctions)
let checker {Callbacks.exe_env; summary} =
let initial = TransferFunctions.Domain.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
| None ->
() ) ;
summary

@ -0,0 +1,10 @@
(*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*)
open! IStd
val checker : Callbacks.proc_callback_t

@ -143,7 +143,10 @@ let all_checkers =
[(Procedure Purity.checker, Language.Java); (Procedure Purity.checker, Language.Clang)] }
; { name= "Class loading analysis"
; active= Config.class_loads
; callbacks= [(Procedure ClassLoads.analyze_procedure, Language.Java)] } ]
; callbacks= [(Procedure ClassLoads.analyze_procedure, Language.Java)] }
; { name= "Self captured in block checker"
; active= Config.self_in_block
; callbacks= [(Procedure SelfInBlock.checker, Language.Clang)] } ]
let get_active_checkers () =

@ -0,0 +1,17 @@
# Copyright (c) Facebook, Inc. and its affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.
TESTS_DIR = ../../..
CLANG_OPTIONS = -c $(OBJC_CLANG_OPTIONS) -fobjc-arc
INFER_OPTIONS = --self_in_block-only --debug-exceptions --project-root $(TESTS_DIR)
INFERPRINT_OPTIONS = --issues-tests
SOURCES = $(wildcard *.m)
include $(TESTS_DIR)/clang.make
include $(TESTS_DIR)/objc.make
infer-out/report.json: $(MAKEFILE_LIST)

@ -0,0 +1,35 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
#include <Foundation/NSObject.h>
@interface SelfInBlockTest : NSObject
- (void)foo;
@end
@implementation SelfInBlockTest {
int x;
}
- (void)foo {
}
- (void)mixSelfWeakSelf_bad {
__weak __typeof(self) weakSelf = self;
int (^my_block)() = ^() {
__strong __typeof(weakSelf) strongSelf = weakSelf;
if (strongSelf) {
[strongSelf foo];
int x = self->x; // bug here
}
return 0;
};
}
@end

@ -0,0 +1 @@
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::mixSelfWeakSelf_bad_1, 4, MIXED_SELF_WEAKSELF, no_bucket, ERROR, []
Loading…
Cancel
Save