From de615594dfb6c406c731911c150fecc22c8e78b3 Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Tue, 24 May 2016 10:06:28 -0700 Subject: [PATCH] Skip whitelisted functions form trace Reviewed By: akotulski Differential Revision: D3335216 fbshipit-source-id: 3bda232 --- infer/src/IR/AttributesTable.re | 14 +++++ infer/src/IR/AttributesTable.rei | 2 + infer/src/backend/config.ml | 10 +++ infer/src/backend/config.mli | 1 + infer/src/backend/tabulation.ml | 3 +- infer/src/clang/cFrontend_config.ml | 8 --- infer/src/clang/cFrontend_config.mli | 2 - infer/src/clang/cFrontend_decl.ml | 9 +-- .../smart_ptr/deref_after_move_example.cpp | 34 ++++++++++ .../endtoend/cpp/DerefAfterMoveTest.java | 63 +++++++++++++++++++ 10 files changed, 128 insertions(+), 18 deletions(-) create mode 100644 infer/tests/codetoanalyze/cpp/errors/smart_ptr/deref_after_move_example.cpp create mode 100644 infer/tests/endtoend/cpp/DerefAfterMoveTest.java diff --git a/infer/src/IR/AttributesTable.re b/infer/src/IR/AttributesTable.re index 08fee41f8..635c7e0ce 100644 --- a/infer/src/IR/AttributesTable.re +++ b/infer/src/IR/AttributesTable.re @@ -106,6 +106,20 @@ let pname_is_cpp_model callee_pname => | None => false }; + +let is_whitelisted_cpp_method method_name => + IList.exists + ( + fun whitelisting_class => + IList.for_all + ( + fun whitelisting_class_substring => + Utils.string_contains whitelisting_class_substring method_name + ) + whitelisting_class + ) + Config.whitelisted_cpp_methods; + let stats () => { let stats = Procname.Hash.stats attr_tbl; let {Hashtbl.num_bindings: num_bindings, num_buckets, max_bucket_length} = stats; diff --git a/infer/src/IR/AttributesTable.rei b/infer/src/IR/AttributesTable.rei index 1d3558c23..0e4a4cc01 100644 --- a/infer/src/IR/AttributesTable.rei +++ b/infer/src/IR/AttributesTable.rei @@ -36,4 +36,6 @@ let get_correct_type_from_objc_class_name: Mangled.t => option Sil.typ; /** Returns true if the method is defined as a C++ model */ let pname_is_cpp_model: Procname.t => bool; +let is_whitelisted_cpp_method : string => bool; + let stats: unit => (string, Yojson.json); diff --git a/infer/src/backend/config.ml b/infer/src/backend/config.ml index 56f21693a..887df2799 100644 --- a/infer/src/backend/config.ml +++ b/infer/src/backend/config.ml @@ -203,6 +203,16 @@ let cpp_models_dir = "models/cpp/include" in cpp_models_dir +let whitelisted_cpp_methods = [ + ["std"; "move"]; + ["google"; "CheckNotNull"]; + ["google"; "GetReferenceableValue"]; + ["google"; "Check_NEImpl"]; + ["google"; "Check_LEImpl"]; + ["google"; "Check_GTImpl"]; + ["google"; "Check_EQImpl"] +] + let os_type = match Sys.os_type with | "Win32" -> Win32 | "Cygwin" -> Cygwin diff --git a/infer/src/backend/config.mli b/infer/src/backend/config.mli index d46f35f66..fbf679ce8 100644 --- a/infer/src/backend/config.mli +++ b/infer/src/backend/config.mli @@ -70,6 +70,7 @@ val max_recursion : int val meet_level : int val models_dir : string val cpp_models_dir : string +val whitelisted_cpp_methods : string list list val nsnotification_center_checker_backend : bool val objc_method_call_semantics : bool val os_type : os_type diff --git a/infer/src/backend/tabulation.ml b/infer/src/backend/tabulation.ml index b79f31ce2..503d44dd4 100644 --- a/infer/src/backend/tabulation.ml +++ b/infer/src/backend/tabulation.ml @@ -665,7 +665,8 @@ let prop_set_exn pname prop se_exn = (** Include a subtrace for a procedure call if the callee is not a model. *) let include_subtrace callee_pname = Specs.get_origin callee_pname <> DB.Models && - not (AttributesTable.pname_is_cpp_model callee_pname) + not (AttributesTable.pname_is_cpp_model callee_pname) && + not (AttributesTable.is_whitelisted_cpp_method (Procname.to_string callee_pname)) (** combine the spec's post with a splitting and actual precondition *) let combine diff --git a/infer/src/clang/cFrontend_config.ml b/infer/src/clang/cFrontend_config.ml index b65883ccc..a883e767f 100644 --- a/infer/src/clang/cFrontend_config.ml +++ b/infer/src/clang/cFrontend_config.ml @@ -115,14 +115,6 @@ let google_LogMessageFatal = "google::LogMessageFatal_LogMessageFatal" let google_MakeCheckOpString = "google::MakeCheckOpString" -let google_whitelisting_functions = [ - "CheckNotNull"; - "GetReferenceableValue"; - "Check_NEImpl"; - "Check_LEImpl"; - "Check_GTImpl"; - "Check_EQImpl"] - let pseudo_object_type = "" let count = "count" diff --git a/infer/src/clang/cFrontend_config.mli b/infer/src/clang/cFrontend_config.mli index 71762be58..535f11153 100644 --- a/infer/src/clang/cFrontend_config.mli +++ b/infer/src/clang/cFrontend_config.mli @@ -117,8 +117,6 @@ val google_LogMessageFatal : string val google_MakeCheckOpString : string -val google_whitelisting_functions : string list - val pseudo_object_type : string val count : string diff --git a/infer/src/clang/cFrontend_decl.ml b/infer/src/clang/cFrontend_decl.ml index 0defe2332..1eeac779e 100644 --- a/infer/src/clang/cFrontend_decl.ml +++ b/infer/src/clang/cFrontend_decl.ml @@ -152,13 +152,8 @@ struct (* named_decl_info.ni_name has name without template parameters.*) (* It makes it possible to capture whole family of function instantiations*) (* to be named the same *) - let fun_name = name_info.Clang_ast_t.ni_name in - let qual_name = name_info.Clang_ast_t.ni_qual_name in - let top_qual = IList.hd (IList.rev qual_name) in - (* Always translate std::move so that it can be analyzed *) - top_qual = "std" && fun_name = "move" || - top_qual = "google" && - IList.mem (=) fun_name CFrontend_config.google_whitelisting_functions + let name = Ast_utils.get_qualified_name name_info in + AttributesTable.is_whitelisted_cpp_method name | _ -> false in let never_translate_decl = match dec with | Clang_ast_t.FunctionDecl (_, name_info, _, _) diff --git a/infer/tests/codetoanalyze/cpp/errors/smart_ptr/deref_after_move_example.cpp b/infer/tests/codetoanalyze/cpp/errors/smart_ptr/deref_after_move_example.cpp new file mode 100644 index 000000000..d2a91bba5 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/errors/smart_ptr/deref_after_move_example.cpp @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2016 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#include + +struct Person { + std::unique_ptr age{new int(35)}; + std::unique_ptr move_age() { return std::move(age); } + int access_age() { return *age; } +}; + +int deref_after_move_crash() { + Person p; + auto x = p.move_age(); + *x; + return p.access_age(); +} + +int deref_ok() { + Person p; + return p.access_age(); +} + +int deref_after_move_ok() { + Person p; + auto x = p.move_age(); + return *x; +} diff --git a/infer/tests/endtoend/cpp/DerefAfterMoveTest.java b/infer/tests/endtoend/cpp/DerefAfterMoveTest.java new file mode 100644 index 000000000..98dd93e4b --- /dev/null +++ b/infer/tests/endtoend/cpp/DerefAfterMoveTest.java @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2016 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +package endtoend.cpp; + +import static org.hamcrest.MatcherAssert.assertThat; +import static utils.matchers.ResultContainsExactly.containsExactly; + +import com.google.common.collect.ImmutableList; + +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; + +import java.io.IOException; + +import utils.DebuggableTemporaryFolder; +import utils.InferException; +import utils.InferResults; +import utils.InferRunner; + +public class DerefAfterMoveTest { + + public static final String FILE = + "infer/tests/codetoanalyze/cpp/errors/smart_ptr/deref_after_move_example.cpp"; + + private static ImmutableList inferCmd; + + public static final String NULL_DEREFERENCE = "NULL_DEREFERENCE"; + + @ClassRule + public static DebuggableTemporaryFolder folder = + new DebuggableTemporaryFolder(); + + @BeforeClass + public static void runInfer() throws InterruptedException, IOException { + inferCmd = InferRunner.createCPPInferCommand(folder, FILE); + } + + @Test + public void whenInferRunsOngetPersonAgeNPEIsFound() + throws InterruptedException, IOException, InferException { + InferResults inferResults = InferRunner.runInferC(inferCmd); + String[] procedures = { + "deref_after_move_crash", + }; + assertThat( + "Results should contain the expected npe", + inferResults, + containsExactly( + NULL_DEREFERENCE, + FILE, + procedures + ) + ); + } +}