From 4ec54406924ac2192d2a8a07c272343689a3d015 Mon Sep 17 00:00:00 2001 From: Jeremy Dubreil Date: Fri, 15 Sep 2017 13:04:32 -0700 Subject: [PATCH] [infer][clang] port the nullable suggestion on fields on C++ Summary: Suggesting to add `_Nullable` on the fields checked for, or assigned to, `nullptr` will allow the biabduction analysis to report null dereferences that are related to the lifetime of objects. Depends on D5832147 Reviewed By: sblackshear Differential Revision: D5836538 fbshipit-source-id: c1b8e48 --- Makefile | 2 +- infer/src/checkers/NullabilitySuggest.ml | 5 ++- infer/src/checkers/registerCheckers.ml | 3 +- .../tests/codetoanalyze/cpp/nullable/Makefile | 21 ++++++++++++ .../codetoanalyze/cpp/nullable/example.cpp | 34 +++++++++++++++++++ .../codetoanalyze/cpp/nullable/issues.exp | 4 +++ 6 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 infer/tests/codetoanalyze/cpp/nullable/Makefile create mode 100644 infer/tests/codetoanalyze/cpp/nullable/example.cpp create mode 100644 infer/tests/codetoanalyze/cpp/nullable/issues.exp diff --git a/Makefile b/Makefile index 64743bce7..9dd0b3b2e 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,7 @@ BUILD_SYSTEMS_TESTS += \ DIRECT_TESTS += \ c_biabduction c_bufferoverrun c_errors c_frontend \ - cpp_bufferoverrun cpp_errors cpp_frontend cpp_liveness cpp_quandary cpp_siof cpp_threadsafety \ + cpp_bufferoverrun cpp_errors cpp_frontend cpp_liveness cpp_quandary cpp_siof cpp_threadsafety cpp_nullable \ ifneq ($(BUCK),no) BUILD_SYSTEMS_TESTS += buck-clang-db buck_flavors buck_flavors_run buck_flavors_deterministic diff --git a/infer/src/checkers/NullabilitySuggest.ml b/infer/src/checkers/NullabilitySuggest.ml index a0d96d5af..78bad6140 100644 --- a/infer/src/checkers/NullabilitySuggest.ml +++ b/infer/src/checkers/NullabilitySuggest.ml @@ -145,6 +145,9 @@ let pretty_field_name proc_data field_name = Typ.Fieldname.to_string field_name let checker {Callbacks.summary; proc_desc; tenv} = + let nullable_annotation = + if Typ.Procname.is_java (Procdesc.get_proc_name proc_desc) then "@Nullable" else "_Nullable" + in let report astate (proc_data: extras ProcData.t) = let report_access_path ap udchain = let issue_kind = IssueType.field_should_be_nullable.unique_id in @@ -156,7 +159,7 @@ let checker {Callbacks.summary; proc_desc; tenv} = -> ( let message = F.asprintf "Field %a should be annotated with %a" MF.pp_monospaced - (pretty_field_name proc_data field_name) MF.pp_monospaced "@Nullable" + (pretty_field_name proc_data field_name) MF.pp_monospaced nullable_annotation in let exn = Exceptions.Checkers (issue_kind, Localise.verbatim_desc message) in match make_error_trace astate ap udchain with diff --git a/infer/src/checkers/registerCheckers.ml b/infer/src/checkers/registerCheckers.ml index b9dcbef82..7f10de4b9 100644 --- a/infer/src/checkers/registerCheckers.ml +++ b/infer/src/checkers/registerCheckers.ml @@ -43,7 +43,8 @@ let checkers = ; ("printf args", Config.printf_args, [(Procedure PrintfArgs.callback_printf_args, Config.Java)]) ; ( "nullable suggestion" , Config.suggest_nullable - , [(Procedure NullabilitySuggest.checker, Config.Java)] ) + , [ (Procedure NullabilitySuggest.checker, Config.Java) + ; (Procedure NullabilitySuggest.checker, Config.Clang) ] ) ; ( "quandary" , Config.quandary , [ (Procedure JavaTaintAnalysis.checker, Config.Java) diff --git a/infer/tests/codetoanalyze/cpp/nullable/Makefile b/infer/tests/codetoanalyze/cpp/nullable/Makefile new file mode 100644 index 000000000..0adceab72 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/nullable/Makefile @@ -0,0 +1,21 @@ +# 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. + +TESTS_DIR = ../../.. + +ANALYZER = checkers +# see explanations in cpp/errors/Makefile for the custom isystem +CLANG_OPTIONS = -x c++ -std=c++11 -nostdinc++ -isystem$(ROOT_DIR) -isystem$(CLANG_INCLUDES)/c++/v1/ -c +INFER_OPTIONS = --biabduction --suggest-nullable --debug-exceptions --project-root $(TESTS_DIR) +INFER_OPTIONS += --debug +INFERPRINT_OPTIONS = --issues-tests + +SOURCES = $(wildcard *.cpp) + +include $(TESTS_DIR)/clang.make + +infer-out/report.json: $(MAKEFILE_LIST) diff --git a/infer/tests/codetoanalyze/cpp/nullable/example.cpp b/infer/tests/codetoanalyze/cpp/nullable/example.cpp new file mode 100644 index 000000000..602819782 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/nullable/example.cpp @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2017 - 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. + */ +class T { + private: + int* _Nullable nullable_field; + int* unnanotated_field; + + public: + void assign_nullable_field_to_null_okay() { nullable_field = nullptr; } + + public: + void assign_unnanotated_field_to_null_bad() { unnanotated_field = nullptr; } + + public: + void test_nullable_field_for_null_okay() { + if (nullable_field == nullptr) { + } + } + + public: + void test_unnanotated_field_for_null_bad() { + if (unnanotated_field == nullptr) { + } + } + + public: + void FN_dereference_nullable_field_bad() { *nullable_field = 42; } +}; diff --git a/infer/tests/codetoanalyze/cpp/nullable/issues.exp b/infer/tests/codetoanalyze/cpp/nullable/issues.exp new file mode 100644 index 000000000..37bedb616 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/nullable/issues.exp @@ -0,0 +1,4 @@ +codetoanalyze/cpp/nullable/example.cpp, T_assign_nullable_field_to_null_okay, 0, FIELD_SHOULD_BE_NULLABLE, [Field nullable_field is assigned null here] +codetoanalyze/cpp/nullable/example.cpp, T_assign_unnanotated_field_to_null_bad, 0, FIELD_SHOULD_BE_NULLABLE, [Field unnanotated_field is assigned null here] +codetoanalyze/cpp/nullable/example.cpp, T_test_nullable_field_for_null_okay, 1, FIELD_SHOULD_BE_NULLABLE, [Field nullable_field is compared to null here] +codetoanalyze/cpp/nullable/example.cpp, T_test_unnanotated_field_for_null_bad, 1, FIELD_SHOULD_BE_NULLABLE, [Field unnanotated_field is compared to null here]