From 0a32ff440025431936640fa4f6bd2df264c037fa Mon Sep 17 00:00:00 2001 From: Andrzej Kotulski Date: Thu, 17 Mar 2016 06:51:50 -0700 Subject: [PATCH] Translate std::move from system headers to improve analysis quality Summary:public Implementation of std::move is straightforward and infer understands it without any problems. To use it, we translate it even though it's coming from system headers. Reviewed By: jvillard Differential Revision: D3064019 fb-gh-sync-id: 823ae75 shipit-source-id: 823ae75 --- infer/src/clang/cFrontend_decl.ml | 24 +++++-- .../codetoanalyze/cpp/errors/models/move.cpp | 43 ++++++++++++ .../cpp/errors/smart_ptr/shared_ptr_deref.cpp | 6 ++ infer/tests/endtoend/cpp/MoveModelTest.java | 65 +++++++++++++++++++ .../endtoend/cpp/SharedPtrDerefTest.java | 1 + 5 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 infer/tests/codetoanalyze/cpp/errors/models/move.cpp create mode 100644 infer/tests/endtoend/cpp/MoveModelTest.java diff --git a/infer/src/clang/cFrontend_decl.ml b/infer/src/clang/cFrontend_decl.ml index 95d385b30..aef2905ab 100644 --- a/infer/src/clang/cFrontend_decl.ml +++ b/infer/src/clang/cFrontend_decl.ml @@ -108,6 +108,23 @@ struct let process_methods tenv cg cfg curr_class decl_list = IList.iter (process_one_method_decl tenv cg cfg curr_class) decl_list + let should_translate_decl dec = + let info = Clang_ast_proj.get_decl_tuple dec in + CLocation.update_curr_file info; + let source_range = info.Clang_ast_t.di_source_range in + let translate_location = CLocation.should_translate_lib source_range in + let always_translate_decl = match dec with + | Clang_ast_t.FunctionDecl (_, name_info, _, _) -> + (* 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 top_qual = IList.hd (IList.rev name_info.Clang_ast_t.ni_qual_name) in + (* Always translate std::move so that it can be analyzed *) + top_qual="std" && fun_name = "move" + | _ -> false in + translate_location || always_translate_decl + (* Translate one global declaration *) let rec translate_one_declaration tenv cg cfg parent_dec dec = let open Clang_ast_t in @@ -115,11 +132,8 @@ struct CFrontend_errors.run_frontend_checkers_on_decl tenv cg cfg dec; (* each procedure has different scope: start names from id 0 *) Ident.NameGenerator.reset (); - let info = Clang_ast_proj.get_decl_tuple dec in - CLocation.update_curr_file info; - let source_range = info.Clang_ast_t.di_source_range in - let should_translate_decl = CLocation.should_translate_lib source_range in - (if should_translate_decl then + + (if should_translate_decl dec then match dec with | FunctionDecl(_, _, _, _) -> function_decl tenv cfg cg dec None diff --git a/infer/tests/codetoanalyze/cpp/errors/models/move.cpp b/infer/tests/codetoanalyze/cpp/errors/models/move.cpp new file mode 100644 index 000000000..4f7900d6f --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/errors/models/move.cpp @@ -0,0 +1,43 @@ +/* + * 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 X { + X() : f(1) {} + X(X&& x) { + f = x.f; + x.f = 0; // assign zero to field of moved_from object + } + int f; +}; + +int div0_moved_from() { + X x1; + X x2 = std::move(x1); + return 1 / x1.f; +} + +int div1_moved_from() { + X x1; + X x2 = std::move(x1); + return 1 / (x1.f + 1); +} + +int div1_moved_to() { + X x1; + X x2 = std::move(x1); + return 1 / x2.f; +} + +int div0_moved_to() { + X x1; + X x2 = std::move(x1); + return 1 / (x2.f - 1); +} diff --git a/infer/tests/codetoanalyze/cpp/errors/smart_ptr/shared_ptr_deref.cpp b/infer/tests/codetoanalyze/cpp/errors/smart_ptr/shared_ptr_deref.cpp index f832a783f..d90122b7f 100644 --- a/infer/tests/codetoanalyze/cpp/errors/smart_ptr/shared_ptr_deref.cpp +++ b/infer/tests/codetoanalyze/cpp/errors/smart_ptr/shared_ptr_deref.cpp @@ -101,3 +101,9 @@ int shared_ptr_assign_ok_deref() { p1.reset(); return *p2; } + +int shared_ptr_move_null_deref() { + std::shared_ptr p1(new int); + std::shared_ptr p2 = std::move(p1); + return *p1; +} diff --git a/infer/tests/endtoend/cpp/MoveModelTest.java b/infer/tests/endtoend/cpp/MoveModelTest.java new file mode 100644 index 000000000..dfb819900 --- /dev/null +++ b/infer/tests/endtoend/cpp/MoveModelTest.java @@ -0,0 +1,65 @@ +/* + * 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 MoveModelTest { + + public static final String FILE = + "infer/tests/codetoanalyze/cpp/errors/models/move.cpp"; + + private static ImmutableList inferCmd; + + public static final String DIVIDE_BY_ZERO = "DIVIDE_BY_ZERO"; + + @ClassRule + public static DebuggableTemporaryFolder folder = + new DebuggableTemporaryFolder(); + + @BeforeClass + public static void runInfer() throws InterruptedException, IOException { + inferCmd = InferRunner.createCPPInferCommand(folder, FILE); + } + + @Test + public void whenInferRunsOnDiv0FunctionsErrorIsFound() + throws InterruptedException, IOException, InferException { + String[] procedures = { + "div0_moved_from", + "div0_moved_to", + }; + InferResults inferResults = InferRunner.runInferCPP(inferCmd); + assertThat( + "Results should contain divide by 0 error", + inferResults, + containsExactly( + DIVIDE_BY_ZERO, + FILE, + procedures + ) + ); + } +} diff --git a/infer/tests/endtoend/cpp/SharedPtrDerefTest.java b/infer/tests/endtoend/cpp/SharedPtrDerefTest.java index 859c8793c..30dc96e6a 100644 --- a/infer/tests/endtoend/cpp/SharedPtrDerefTest.java +++ b/infer/tests/endtoend/cpp/SharedPtrDerefTest.java @@ -57,6 +57,7 @@ public class SharedPtrDerefTest { "reset_ptr_null_deref2", "shared_ptr_copy_null_deref", "shared_ptr_assign_null_deref", + "shared_ptr_move_null_deref", }; InferResults inferResults = InferRunner.runInferCPP(inferCmd); assertThat(