Skip to content

Commit

Permalink
[infer][clang] port the nullable suggestion on fields on C++
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jeremydubreil authored and facebook-github-bot committed Sep 15, 2017
1 parent 919b926 commit 4ec5440
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 3 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion infer/src/checkers/NullabilitySuggest.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion infer/src/checkers/registerCheckers.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 21 additions & 0 deletions infer/tests/codetoanalyze/cpp/nullable/Makefile
Original file line number Diff line number Diff line change
@@ -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)
34 changes: 34 additions & 0 deletions infer/tests/codetoanalyze/cpp/nullable/example.cpp
Original file line number Diff line number Diff line change
@@ -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; }
};
4 changes: 4 additions & 0 deletions infer/tests/codetoanalyze/cpp/nullable/issues.exp
Original file line number Diff line number Diff line change
@@ -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]

0 comments on commit 4ec5440

Please sign in to comment.