diff --git a/Makefile b/Makefile index 386823dee0..3ff8858377 100644 --- a/Makefile +++ b/Makefile @@ -140,6 +140,7 @@ DIRECT_TESTS += \ objc_pulse \ objc_pulse-data-lineage \ objc_self-in-block \ + objc_dispatch-once-static-init \ objcpp_biabduction \ objcpp_frontend \ objcpp_liveness \ diff --git a/infer/man/man1/infer-analyze.txt b/infer/man/man1/infer-analyze.txt index 27941a1859..2039939d20 100644 --- a/infer/man/man1/infer-analyze.txt +++ b/infer/man/man1/infer-analyze.txt @@ -114,10 +114,11 @@ OPTIONS and/or reporting. (Conversely: --deduplicate) --no-default-checkers - Deactivates: Default checkers: --fragment-retains-view, - --inefficient-keyset-iterator, --liveness, - --parameter-not-null-checked, --pulse, --racerd, --siof, - --self-in-block, --starvation (Conversely: --default-checkers) + Deactivates: Default checkers: --dispatch-once-static-init, + --fragment-retains-view, --inefficient-keyset-iterator, + --liveness, --parameter-not-null-checked, --pulse, --racerd, + --siof, --self-in-block, --starvation (Conversely: + --default-checkers) --detach-analysis-dependency Activates: Detach analysis dependencies of checkers during the @@ -130,6 +131,15 @@ OPTIONS --dict-missing-key-var-block-list +string Skip analyzing the variables in the dict-missing-key checker. + --no-dispatch-once-static-init + Deactivates: dispatch-once-static-init checker: Detect if + dispatch_once is called from a static constructor. (Conversely: + --dispatch-once-static-init) + + --dispatch-once-static-init-only + Activates: Enable dispatch-once-static-init and disable all other + checkers (Conversely: --no-dispatch-once-static-init-only) + --files-to-analyze-index file File containing a list of source files where analysis should start from. When used, the set of files given to this argument must be a diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt index ce6ce9a810..d9a4677a39 100644 --- a/infer/man/man1/infer-full.txt +++ b/infer/man/man1/infer-full.txt @@ -539,10 +539,11 @@ OPTIONS infer-reportdiff(1). --no-default-checkers - Deactivates: Default checkers: --fragment-retains-view, - --inefficient-keyset-iterator, --liveness, - --parameter-not-null-checked, --pulse, --racerd, --siof, - --self-in-block, --starvation (Conversely: --default-checkers) + Deactivates: Default checkers: --dispatch-once-static-init, + --fragment-retains-view, --inefficient-keyset-iterator, + --liveness, --parameter-not-null-checked, --pulse, --racerd, + --siof, --self-in-block, --starvation (Conversely: + --default-checkers) See also infer-analyze(1). --dependencies @@ -633,6 +634,7 @@ OPTIONS DATA_FLOW_TO_SINK (disabled by default), DEADLOCK (enabled by default), DEAD_STORE (enabled by default), + DISPATCH_ONCE_IN_STATIC_INIT (disabled by default), DIVIDE_BY_ZERO (disabled by default), DO_NOT_REPORT (enabled by default), EMPTY_VECTOR_ACCESS (enabled by default), @@ -767,6 +769,17 @@ OPTIONS See also infer-report(1). + --no-dispatch-once-static-init + Deactivates: dispatch-once-static-init checker: Detect if + dispatch_once is called from a static constructor. (Conversely: + --dispatch-once-static-init) + See also infer-analyze(1). + + --dispatch-once-static-init-only + Activates: Enable dispatch-once-static-init and disable all other + checkers (Conversely: --no-dispatch-once-static-init-only) + See also infer-analyze(1). + --dump-duplicate-symbols Activates: Dump all symbols with the same name that are defined in more than one file. (Conversely: --no-dump-duplicate-symbols) diff --git a/infer/man/man1/infer-report.txt b/infer/man/man1/infer-report.txt index 25b60e082a..9a650561f1 100644 --- a/infer/man/man1/infer-report.txt +++ b/infer/man/man1/infer-report.txt @@ -149,6 +149,7 @@ OPTIONS DATA_FLOW_TO_SINK (disabled by default), DEADLOCK (enabled by default), DEAD_STORE (enabled by default), + DISPATCH_ONCE_IN_STATIC_INIT (disabled by default), DIVIDE_BY_ZERO (disabled by default), DO_NOT_REPORT (enabled by default), EMPTY_VECTOR_ACCESS (enabled by default), diff --git a/infer/man/man1/infer.txt b/infer/man/man1/infer.txt index 3e4a18ff74..30b6d8bb5b 100644 --- a/infer/man/man1/infer.txt +++ b/infer/man/man1/infer.txt @@ -539,10 +539,11 @@ OPTIONS infer-reportdiff(1). --no-default-checkers - Deactivates: Default checkers: --fragment-retains-view, - --inefficient-keyset-iterator, --liveness, - --parameter-not-null-checked, --pulse, --racerd, --siof, - --self-in-block, --starvation (Conversely: --default-checkers) + Deactivates: Default checkers: --dispatch-once-static-init, + --fragment-retains-view, --inefficient-keyset-iterator, + --liveness, --parameter-not-null-checked, --pulse, --racerd, + --siof, --self-in-block, --starvation (Conversely: + --default-checkers) See also infer-analyze(1). --dependencies @@ -633,6 +634,7 @@ OPTIONS DATA_FLOW_TO_SINK (disabled by default), DEADLOCK (enabled by default), DEAD_STORE (enabled by default), + DISPATCH_ONCE_IN_STATIC_INIT (disabled by default), DIVIDE_BY_ZERO (disabled by default), DO_NOT_REPORT (enabled by default), EMPTY_VECTOR_ACCESS (enabled by default), @@ -767,6 +769,17 @@ OPTIONS See also infer-report(1). + --no-dispatch-once-static-init + Deactivates: dispatch-once-static-init checker: Detect if + dispatch_once is called from a static constructor. (Conversely: + --dispatch-once-static-init) + See also infer-analyze(1). + + --dispatch-once-static-init-only + Activates: Enable dispatch-once-static-init and disable all other + checkers (Conversely: --no-dispatch-once-static-init-only) + See also infer-analyze(1). + --dump-duplicate-symbols Activates: Dump all symbols with the same name that are defined in more than one file. (Conversely: --no-dump-duplicate-symbols) diff --git a/infer/src/backend/registerCheckers.ml b/infer/src/backend/registerCheckers.ml index 3209b0c25a..f5c9158904 100644 --- a/infer/src/backend/registerCheckers.ml +++ b/infer/src/backend/registerCheckers.ml @@ -91,6 +91,8 @@ let all_checkers = [ {checker= SelfInBlock; callbacks= [(intraprocedural SelfInBlock.checker, Clang)]} ; { checker= ParameterNotNullChecked ; callbacks= [(intraprocedural ParameterNotNullChecked.checker, Clang)] } + ; { checker= DispatchOnceStaticInit + ; callbacks= [(intraprocedural DispatchOnceStaticInit.checker, Clang)] } ; { checker= BufferOverrunAnalysis ; callbacks= (let bo_analysis = diff --git a/infer/src/base/Checker.ml b/infer/src/base/Checker.ml index 5a90f1025d..d8b4feea24 100644 --- a/infer/src/base/Checker.ml +++ b/infer/src/base/Checker.ml @@ -17,6 +17,7 @@ type t = | ConfigImpactAnalysis | Cost | DisjunctiveDemo + | DispatchOnceStaticInit | FragmentRetainsView | Impurity | InefficientKeysetIterator @@ -174,6 +175,14 @@ let config_unsafe checker = ; cli_flags= Some {deprecated= []; show_in_help= false} ; enabled_by_default= false ; activates= [] } + | DispatchOnceStaticInit -> + { id= "dispatch-once-static-init" + ; kind= UserFacing {title= "dispatch-once in static init"; markdown_body= ""} + ; support= mk_support_func ~clang:Support () + ; short_documentation= "Detect if dispatch_once is called from a static constructor." + ; cli_flags= Some {deprecated= []; show_in_help= true} + ; enabled_by_default= true + ; activates= [] } | FragmentRetainsView -> { id= "fragment-retains-view" ; kind= UserFacing {title= "Fragment Retains View"; markdown_body= ""} diff --git a/infer/src/base/Checker.mli b/infer/src/base/Checker.mli index 6c9dc860f4..e577236131 100644 --- a/infer/src/base/Checker.mli +++ b/infer/src/base/Checker.mli @@ -16,6 +16,7 @@ type t = | ConfigImpactAnalysis | Cost | DisjunctiveDemo + | DispatchOnceStaticInit | FragmentRetainsView | Impurity | InefficientKeysetIterator diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index dcb461c1cb..3d569049dc 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -566,6 +566,14 @@ let deadlock = ~user_documentation:[%blob "./documentation/issues/DEADLOCK.md"] +let dispatch_once_in_static_init = + register ~category:Concurrency ~enabled:false ~id:"DISPATCH_ONCE_IN_STATIC_INIT" + ~hum:"dispatch_once in static init" Error DispatchOnceStaticInit + ~user_documentation: + "Calling dispatch_once during the static initialization of objects is risky, for example it \ + could cause deadlocks, because other objects might not have been initialized yet." + + let divide_by_zero = register ~category:NoCategory ~enabled:false ~id:"DIVIDE_BY_ZERO" Error Biabduction (* TODO *) ~user_documentation:"" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index 428fd0f935..366e7ba519 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -183,6 +183,8 @@ val dead_store : t val deadlock : t +val dispatch_once_in_static_init : t + val divide_by_zero : t val do_not_report : t diff --git a/infer/src/checkers/DispatchOnceStaticInit.ml b/infer/src/checkers/DispatchOnceStaticInit.ml new file mode 100644 index 0000000000..f67db7504d --- /dev/null +++ b/infer/src/checkers/DispatchOnceStaticInit.ml @@ -0,0 +1,70 @@ +(* + * 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 Mem = struct + type t = {loc: Location.t} [@@deriving compare] + + let pp fmt {loc} = F.fprintf fmt "calls_dispatch_once at %a" Location.pp loc +end + +module Domain = struct + include AbstractDomain.FiniteSet (Mem) + + let add_calls_dispatch_once loc astate = add {Mem.loc} astate +end + +module TransferFunctions = struct + module Domain = Domain + module CFG = ProcCfg.Normal + + type analysis_data = IntraproceduralAnalysis.t + + let pp_session_name _node fmt = F.pp_print_string fmt "DispatchOnceStaticInit" + + let exec_instr (astate : Domain.t) _ _cfg_node _ (instr : Sil.instr) = + match instr with + | Call (_, Exp.Const (Const.Cfun procname), _, loc, _) -> + let calls_dispatch_once = String.equal "_dispatch_once" (Procname.get_method procname) in + if calls_dispatch_once then Domain.add_calls_dispatch_once loc astate else astate + | _ -> + astate +end + +module Analyzer = AbstractInterpreter.MakeWTO (TransferFunctions) + +let make_trace loc = + let trace_elem = Errlog.make_trace_element 0 loc "Call to `dispatch_once` here" [] in + [trace_elem] + + +let report_issue proc_desc err_log {Mem.loc} = + let ltr = make_trace loc in + let message = + F.asprintf + "There is a call to `disptach_once` at line %a from a static constructor. This could cause a \ + deadlock." + Location.pp loc + in + Reporting.log_issue proc_desc err_log ~ltr ~loc DispatchOnceStaticInit + IssueType.dispatch_once_in_static_init message + + +let checker ({IntraproceduralAnalysis.proc_desc; err_log} as analysis_data) = + let attributes = Procdesc.get_attributes proc_desc in + if attributes.ProcAttributes.is_static_ctor then + let initial = Domain.empty in + match Analyzer.compute_post analysis_data ~initial proc_desc with + | Some domain -> ( + match Domain.choose_opt domain with + | Some mem -> + report_issue proc_desc err_log mem + | None -> + () ) + | None -> + () diff --git a/infer/tests/codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m b/infer/tests/codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m index 5de4edc974..e23844b674 100644 --- a/infer/tests/codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m +++ b/infer/tests/codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m @@ -28,8 +28,7 @@ + (instancetype)getInstance { [Manager getInstance]; } -__attribute__((constructor)) static void initializer_test_intraproc_bad_FN( - void) { +__attribute__((constructor)) static void initializer_test_intraproc_bad(void) { static Manager* manager; static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ diff --git a/infer/tests/codetoanalyze/objc/dispatch-once-static-init/Makefile b/infer/tests/codetoanalyze/objc/dispatch-once-static-init/Makefile new file mode 100644 index 0000000000..5a5f532097 --- /dev/null +++ b/infer/tests/codetoanalyze/objc/dispatch-once-static-init/Makefile @@ -0,0 +1,19 @@ +# 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 = --dispatch-once-static-init-only --debug-exceptions --project-root $(TESTS_DIR) +INFERPRINT_OPTIONS = \ + --issues-tests-fields file,procedure,line_offset,bug_type,bucket,severity,bug_trace,taint_extra,transitive_callees_extra,autofix \ + --issues-tests + +SOURCES = $(wildcard *.m) + +include $(TESTS_DIR)/clang.make +include $(TESTS_DIR)/objc.make + +infer-out/report.json: $(MAKEFILE_LIST) diff --git a/infer/tests/codetoanalyze/objc/dispatch-once-static-init/issues.exp b/infer/tests/codetoanalyze/objc/dispatch-once-static-init/issues.exp new file mode 100644 index 0000000000..5c5296aa8f --- /dev/null +++ b/infer/tests/codetoanalyze/objc/dispatch-once-static-init/issues.exp @@ -0,0 +1 @@ +codetoanalyze/objc/dispatch-once-static-init/DispatchOnceInStaticInit.m, initializer_test_intraproc_bad, 3, DISPATCH_ONCE_IN_STATIC_INIT, no_bucket, ERROR, [macro expanded here,Call to `dispatch_once` here]