getting started on a Nullable annotation checker for obj-c

Summary: public This only supports parameters for now, but should be easy to extend to return values and fields. The work of this diff is all in the translation--the task of finding annotations and doing the actual checking is handled by existing code.

Reviewed By: akotulski

Differential Revision: D2706791

fb-gh-sync-id: 0d706a8

infer/src/backend/localise.ml

@@ -437,11 +437,12 @@ let dereference_string deref_str value_str access_opt loc =
     | Some Initialized_automatically ->
         ["initialized automatically"] in
   let problem_desc =
+    let nullable_text = if !Config.curr_language = Config.Java then "@Nullable" else "__nullable" in
     let problem_str =
       match Tags.get !tags Tags.nullable_src with
       | Some nullable_src ->
-          if nullable_src = value_str then "is annotated with @Nullable and is dereferenced without a null check"
-          else "is indirectly marked @Nullable (source: " ^ nullable_src ^ ") and is dereferenced without a null check"
+          if nullable_src = value_str then "is annotated with " ^ nullable_text ^ " and is dereferenced without a null check"
+          else "is indirectly marked " ^ nullable_text ^ " (source: " ^ nullable_src ^ ") and is dereferenced without a null check"
       | None -> deref_str.problem_str in
     [(problem_str ^ " " ^ at_line tags loc)] in
   value_desc:: access_desc @ problem_desc, None, !tags

infer/src/backend/rearrange.ml

@@ -967,8 +967,8 @@ let check_dereference_error pdesc (prop : Prop.normal Prop.t) lexp loc =
   let is_deref_of_nullable =
     let is_definitely_non_null exp prop =
       Prover.check_disequal prop exp Sil.exp_zero in
-    !Config.report_nullable_inconsistency && !Config.curr_language = Config.Java &&
-    not (is_definitely_non_null root prop) && is_only_pt_by_nullable_fld_or_param root in
+    !Config.report_nullable_inconsistency && not (is_definitely_non_null root prop)
+    && is_only_pt_by_nullable_fld_or_param root in
   let relevant_attributes_getters = [
     Prop.get_resource_attribute;
     Prop.get_undef_attribute;

infer/src/checkers/annotations.mli

@@ -52,6 +52,8 @@ val get_annotated_signature : ProcAttributes.t -> annotated_signature
 val get_field_type_and_annotation :
   Ident.fieldname -> Sil.typ -> (Sil.typ * Sil.item_annotation) option
 
+val nullable : string
+
 val ia_contains : Sil.item_annotation -> string -> bool
 
 val ia_has_annotation_with : Sil.item_annotation -> (Sil.annotation -> bool) -> bool

infer/src/clang/cFrontend_utils.ml

@@ -353,12 +353,18 @@ struct
     | _ -> assert false
 
   (*TODO take the attributes into account too. To be done after we get the attribute's arguments. *)
-  let is_type_nonnull type_ptr attributes =
+  let is_type_nonnull type_ptr =
     let open Clang_ast_t in
     match get_type type_ptr with
     | Some AttributedType (_, attr_info) -> attr_info.ati_attr_kind = `Nonnull
     | _ -> false
 
+  let is_type_nullable type_ptr =
+    let open Clang_ast_t in
+    match get_type type_ptr with
+    | Some AttributedType (_, attr_info) -> attr_info.ati_attr_kind = `Nullable
+    | _ -> false
+
   let string_of_type_ptr type_ptr =
     match get_desugared_type type_ptr with
     | Some typ -> (Clang_ast_proj.get_type_tuple typ).Clang_ast_t.ti_raw

infer/src/clang/cFrontend_utils.mli

@@ -64,7 +64,9 @@ sig
 
   val is_copy : Clang_ast_t.property_attribute option -> bool
 
-  val is_type_nonnull : Clang_ast_t.type_ptr -> Clang_ast_t.attribute list -> bool
+  val is_type_nonnull : Clang_ast_t.type_ptr -> bool
+
+  val is_type_nullable : Clang_ast_t.type_ptr -> bool
 
   val get_fresh_pointer : unit -> string
 

infer/src/clang/cMethod_signature.mli

@@ -31,9 +31,9 @@ val ms_get_lang : method_signature -> CFrontend_config.lang
 
 val ms_get_pointer_to_parent : method_signature -> Clang_ast_t.pointer option
 
-val make_ms : Procname.t -> (string * Clang_ast_t.type_ptr) list ->
-  Clang_ast_t.type_ptr -> Clang_ast_t.attribute list -> Clang_ast_t.source_range -> bool ->
-  bool -> CFrontend_config.lang -> Clang_ast_t.pointer option -> method_signature
+val make_ms : Procname.t -> (string * Clang_ast_t.type_ptr) list -> Clang_ast_t.type_ptr
+  -> Clang_ast_t.attribute list -> Clang_ast_t.source_range -> bool -> bool -> CFrontend_config.lang
+  -> Clang_ast_t.pointer option -> method_signature
 
 val replace_name_ms : method_signature -> Procname.t -> method_signature
 

infer/src/clang/cMethod_trans.ml

@@ -86,22 +86,22 @@ let get_return_type function_method_decl_info =
   | Block_decl_info (_, typ, _) -> CTypes.return_type_of_function_type typ
   | ObjC_Meth_decl_info (method_decl_info, _) -> method_decl_info.Clang_ast_t.omdi_result_type
 
-let build_method_signature decl_info procname function_method_decl_info is_anonym_block is_generated
-    parent_pointer =
+let build_method_signature decl_info procname function_method_decl_info is_anonym_block
+    is_generated parent_pointer =
   let source_range = decl_info.Clang_ast_t.di_source_range in
   let tp = get_return_type function_method_decl_info in
   let is_instance_method = is_instance_method function_method_decl_info in
   let parameters = get_parameters function_method_decl_info in
   let attributes = decl_info.Clang_ast_t.di_attributes in
   let lang = get_language function_method_decl_info in
-  CMethod_signature.make_ms procname parameters tp attributes source_range is_instance_method
-    is_generated lang parent_pointer
+  CMethod_signature.make_ms
+    procname parameters tp attributes source_range is_instance_method is_generated
+    lang parent_pointer
 
-let get_assume_not_null_calls ms param_decls =
-  let attributes = CMethod_signature.ms_get_attributes ms in
+let get_assume_not_null_calls param_decls =
   let do_one_param decl = match decl with
     | Clang_ast_t.ParmVarDecl (decl_info, name, tp, var_decl_info)
-      when CFrontend_utils.Ast_utils.is_type_nonnull tp attributes ->
+      when CFrontend_utils.Ast_utils.is_type_nonnull tp ->
         let assume_call = Ast_expressions.create_assume_not_null_call decl_info name tp in
         [(`ClangStmt assume_call)]
     | _ -> [] in
@@ -121,7 +121,7 @@ let method_signature_of_decl meth_decl block_data_opt =
       let function_info = Some (decl_info, fdi) in
       let procname = General_utils.mk_procname_from_function name function_info tp language in
       let ms = build_method_signature decl_info procname func_decl false false None in
-      let extra_instrs = get_assume_not_null_calls ms fdi.Clang_ast_t.fdi_parameters in
+      let extra_instrs = get_assume_not_null_calls fdi.Clang_ast_t.fdi_parameters in
       ms, fdi.Clang_ast_t.fdi_body, extra_instrs
   | CXXMethodDecl (decl_info, name_info, tp, fdi, mdi), _
   | CXXConstructorDecl (decl_info, name_info, tp, fdi, mdi), _ ->
@@ -131,7 +131,7 @@ let method_signature_of_decl meth_decl block_data_opt =
       let method_decl = Cpp_Meth_decl_info (fdi, mdi, class_name, tp)  in
       let parent_pointer = decl_info.Clang_ast_t.di_parent_pointer in
       let ms = build_method_signature decl_info procname method_decl false false parent_pointer in
-      let non_null_instrs = get_assume_not_null_calls ms fdi.Clang_ast_t.fdi_parameters in
+      let non_null_instrs = get_assume_not_null_calls fdi.Clang_ast_t.fdi_parameters in
       let init_list_instrs = get_init_list_instrs mdi in (* it will be empty for methods *)
       ms, fdi.Clang_ast_t.fdi_body, (init_list_instrs @ non_null_instrs)
   | ObjCMethodDecl (decl_info, name_info, mdi), _ ->
@@ -143,14 +143,14 @@ let method_signature_of_decl meth_decl block_data_opt =
       let method_decl = ObjC_Meth_decl_info (mdi, class_name) in
       let is_generated = Ast_utils.is_generated name_info in
       let parent_pointer = decl_info.Clang_ast_t.di_parent_pointer in
-      let ms = build_method_signature decl_info procname method_decl false is_generated
-          parent_pointer in
-      let extra_instrs = get_assume_not_null_calls ms mdi.omdi_parameters in
+      let ms =
+        build_method_signature decl_info procname method_decl false is_generated parent_pointer in
+      let extra_instrs = get_assume_not_null_calls mdi.omdi_parameters in
       ms, mdi.omdi_body, extra_instrs
   | BlockDecl (decl_info, bdi), Some (outer_context, tp, procname, _) ->
       let func_decl = Block_decl_info (bdi, tp, outer_context) in
       let ms = build_method_signature decl_info procname func_decl true false None in
-      let extra_instrs = get_assume_not_null_calls ms bdi.bdi_parameters in
+      let extra_instrs = get_assume_not_null_calls bdi.bdi_parameters in
       ms, bdi.bdi_body, extra_instrs
   | _ -> raise Invalid_declaration
 
@@ -282,12 +282,28 @@ let should_create_procdesc cfg procname defined generated =
       else false
   | None -> true
 
+let sil_method_annotation_of_args args : Sil.method_annotation =
+  let default_visibility = true in
+  let mk_annot param_name annot_name =
+    let annot = { Sil.class_name = annot_name; Sil.parameters = [param_name]; } in
+    annot, default_visibility in
+  let arg_to_sil_annot acc (arg_name, type_ptr) =
+    if CFrontend_utils.Ast_utils.is_type_nullable type_ptr then
+      [mk_annot arg_name Annotations.nullable] :: acc
+    else acc in
+  let param_annots = IList.fold_left arg_to_sil_annot [] args in
+  (* TODO: parse annotations on return value *)
+  let retval_annot = [] in
+  retval_annot, param_annots
+
 (** Creates a procedure description. *)
 let create_local_procdesc cfg tenv ms fbody captured is_objc_inst_method =
   let defined = not ((IList.length fbody) == 0) in
   let proc_name = CMethod_signature.ms_get_name ms in
   let pname = Procname.to_string proc_name in
   let attributes = sil_func_attributes_of_attributes (CMethod_signature.ms_get_attributes ms) in
+  let method_annotation =
+    sil_method_annotation_of_args (CMethod_signature.ms_get_args ms) in
   let is_generated = CMethod_signature.ms_is_generated ms in
   let is_cpp_inst_method = CMethod_signature.ms_is_instance ms
                            && CMethod_signature.ms_get_lang ms = CFrontend_config.CPP in
@@ -316,6 +332,7 @@ let create_local_procdesc cfg tenv ms fbody captured is_objc_inst_method =
           is_objc_instance_method = is_objc_inst_method;
           is_cpp_instance_method = is_cpp_inst_method;
           loc = loc_start;
+          method_annotation;
           ret_type;
         } in
       Cfg.Procdesc.create {

infer/tests/codetoanalyze/objc/errors/npe/nullable.m

@@ -0,0 +1,31 @@
+/*
+* Copyright (c) 2015 - 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.
+*/
+
+#import <Foundation/NSString.h>
+
+@interface A : NSObject {
+    @public int x;
+}
+
+@end
+
+int derefNullableParamDirect(A * __nullable param) {
+  return param->x;
+}
+
+int derefNullableParamIndirect(A * __nullable param) {
+  A* local = param;
+  return local->x;
+}
+
+A * derefNullableParamOk(A * __nullable param) {
+  if (!param) param = [A new];
+  param->x = 7;
+  return param;
+}

infer/tests/endtoend/objc/NullableTest.java

@@ -0,0 +1,66 @@
+/*
+* Copyright (c) 2013 - 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.objc;
+
+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 NullableTest {
+
+  public static final String NPE_FILE = "infer/tests/codetoanalyze/objc/errors/npe/nullable.m";
+
+  private static ImmutableList<String> inferCmdNPD;
+
+  public static final String NULL_DEREFERENCE = "NULL_DEREFERENCE";
+
+  @ClassRule
+  public static DebuggableTemporaryFolder folderNPD = new DebuggableTemporaryFolder();
+
+  @BeforeClass
+  public static void runInfer() throws InterruptedException, IOException {
+    inferCmdNPD = InferRunner.createiOSInferCommandWithMLBuckets(
+        folderNPD,
+        NPE_FILE,
+        "cf",
+        true);
+  }
+
+  @Test
+  public void nullDereferenceTest() throws InterruptedException, IOException, InferException {
+    InferResults inferResults = InferRunner.runInferC(inferCmdNPD);
+    String[] procedures = {
+      "derefNullableParamDirect",
+      "derefNullableParamIndirect"
+    };
+    assertThat(
+        "Results should contain null pointer dereference error",
+        inferResults,
+        containsExactly(
+            NULL_DEREFERENCE,
+            NPE_FILE,
+            procedures
+        )
+    );
+  }
+
+}