From 6f3719f5f2f428c5ce53a1190d99bffdb6decad8 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Thu, 5 Jul 2018 05:00:21 -0700 Subject: [PATCH] [siof] do not warn on simply taking reference of global Summary: The addresses of global variables do not need initialisation to exist and be valid as they are part of the code or data segment of the program. This means that taking the address of a global is not in itself a danger for SIOF. However, dereferencing such an address would be. In order to avoid false positives but avoid being too unsound, only ignore them when the address is taken only to set another global. The general case would require a more complicated abstract domain. Fixes #866 Reviewed By: ngorogiannis Differential Revision: D8055627 fbshipit-source-id: 92307b2 --- infer/src/checkers/Siof.ml | 16 +++++++++++++++- infer/tests/codetoanalyze/cpp/siof/Makefile | 1 + infer/tests/codetoanalyze/cpp/siof/issues.exp | 1 + .../tests/codetoanalyze/cpp/siof/siof/by_ref.cpp | 14 ++++++++++++++ 4 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 infer/tests/codetoanalyze/cpp/siof/siof/by_ref.cpp diff --git a/infer/src/checkers/Siof.ml b/infer/src/checkers/Siof.ml index dd5419a07..a2bfe3aa2 100644 --- a/infer/src/checkers/Siof.ml +++ b/infer/src/checkers/Siof.ml @@ -135,7 +135,21 @@ module TransferFunctions (CFG : ProcCfg.S) = struct let exec_instr astate {ProcData.pdesc} _ (instr: Sil.instr) = match instr with - | Load (_, exp, _, loc) | Store (_, _, exp, loc) | Prune (exp, loc, _, _) -> + | Store (Lvar global, Typ.({desc= Tptr _}), Lvar _, loc) + when (Option.equal Typ.Procname.equal) + (Pvar.get_initializer_pname global) + (Some (Procdesc.get_proc_name pdesc)) -> + (* if we are just taking the reference of another global then we are not really accessing + it. This is a dumb heuristic as something also might take that result and then + dereference it, thus requiring the target object to be initialized. Solving this would + involve a more complicated domain and analysis. + + The heuristic is limited to the case where the access sets the global being initialized + in the current variable initializer function. *) + add_globals astate loc (GlobalVarSet.singleton global) + | Load (_, exp, _, loc) (* dereference -> add all the dangerous variables *) + | Store (_, _, exp, loc) (* except in the case above, consider all reads as dangerous *) + | Prune (exp, loc, _, _) -> get_globals pdesc exp |> add_globals astate loc | Call (_, Const (Cfun callee_pname), _, _, _) when is_whitelisted callee_pname -> at_least_nonbottom astate diff --git a/infer/tests/codetoanalyze/cpp/siof/Makefile b/infer/tests/codetoanalyze/cpp/siof/Makefile index 9d5826196..505e86fc8 100644 --- a/infer/tests/codetoanalyze/cpp/siof/Makefile +++ b/infer/tests/codetoanalyze/cpp/siof/Makefile @@ -12,6 +12,7 @@ INFER_OPTIONS = --siof-only --cxx-infer-headers --debug-exceptions --project-roo INFERPRINT_OPTIONS = --issues-tests SOURCES = \ + siof/by_ref.cpp \ siof/const.cpp \ siof/const_use.cpp \ siof/duplicate_reports.cpp \ diff --git a/infer/tests/codetoanalyze/cpp/siof/issues.exp b/infer/tests/codetoanalyze/cpp/siof/issues.exp index faed588ef..d78775918 100644 --- a/infer/tests/codetoanalyze/cpp/siof/issues.exp +++ b/infer/tests/codetoanalyze/cpp/siof/issues.exp @@ -1,3 +1,4 @@ +codetoanalyze/cpp/siof/siof/by_ref.cpp, __infer_globals_initializer_init_pointer_by_val_to_dangerous_global_bad, 0, STATIC_INITIALIZATION_ORDER_FIASCO, no_bucket, ERROR, [initialization of init_pointer_by_val_to_dangerous_global_bad,access to dangerous_object|] codetoanalyze/cpp/siof/siof/duplicate_reports.cpp, __infer_globals_initializer_many_paths_to_siof_bad, 0, STATIC_INITIALIZATION_ORDER_FIASCO, no_bucket, ERROR, [initialization of many_paths_to_siof_bad,call to X_X,call to access_rick,access to rick|] codetoanalyze/cpp/siof/siof/duplicate_reports.cpp, __infer_globals_initializer_many_paths_to_siof_bad, 0, STATIC_INITIALIZATION_ORDER_FIASCO, no_bucket, ERROR, [initialization of many_paths_to_siof_bad,call to X_X,call to nested_access,access to dangerous|] codetoanalyze/cpp/siof/siof/siof.cpp, __infer_globals_initializer_X::static_pod_accesses_non_pod_bad, 0, STATIC_INITIALIZATION_ORDER_FIASCO, no_bucket, ERROR, [initialization of X::static_pod_accesses_non_pod_bad,call to access_to_non_pod,access to global_object2|codetoanalyze/cpp/siof/siof/siof_different_tu.cpp] diff --git a/infer/tests/codetoanalyze/cpp/siof/siof/by_ref.cpp b/infer/tests/codetoanalyze/cpp/siof/siof/by_ref.cpp new file mode 100644 index 000000000..70416eb7d --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/siof/siof/by_ref.cpp @@ -0,0 +1,14 @@ +/* + * Copyright (c) 2018-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +#include + +extern const std::string dangerous_object; + +const std::string* init_pointer_by_ref_to_dangerous_global_good = { + &dangerous_object}; +const std::string init_pointer_by_val_to_dangerous_global_bad = + dangerous_object;