[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

@@ -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

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 \

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]

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 <string>
+
+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;