Framework to emit warning from frontend

Reviewed By: dulmarod

Differential Revision: D2564879

fb-gh-sync-id: 46e0a03

infer/bin/inferlib.py

@@ -286,7 +286,8 @@ def should_report(analyzer, row):
         'ASSERTION_FAILURE',
         'CONTEXT_LEAK',
         'BAD_POINTER_COMPARISON',
-        # 'CHECKERS_PRINTF_ARGS'
+        'STRONG_DELEGATE_WARNING'
+        #'CHECKERS_PRINTF_ARGS'
         # TODO (#8030397): revert this once all the checkers are moved to Infer
     ]
 

infer/src/backend/errdesc.ml

@@ -981,6 +981,10 @@ let explain_tainted_value_reaching_sensitive_function e loc =
 let explain_return_statement_missing loc =
   Localise.desc_return_statement_missing loc
 
+(** explain a fronend warning *)
+let explain_frontend_warning loc =
+  Localise.desc_frontend_warning loc
+
 (** explain a comparing floats for equality *)
 let explain_comparing_floats_for_equality loc =
   Localise.desc_comparing_floats_for_equality loc

infer/src/backend/errdesc.mli

@@ -96,6 +96,9 @@ val explain_condition_always_true_false :
 val explain_stack_variable_address_escape :
   Location.t -> Sil.pvar -> Sil.dexp option -> Localise.error_desc
 
+(** explain frontend warning *)
+val explain_frontend_warning : string -> string -> string -> Localise.error_desc
+
 (** explain a return statement missing *)
 val explain_return_statement_missing : Location.t -> Localise.error_desc
 

infer/src/backend/exceptions.ml

@@ -51,6 +51,7 @@ exception Deallocation_mismatch of Localise.error_desc * ml_location
 exception Divide_by_zero of Localise.error_desc * ml_location
 exception Eradicate of string * Localise.error_desc
 exception Field_not_null_checked of Localise.error_desc * ml_location
+exception Frontend_warning of string * Localise.error_desc * ml_location
 exception Checkers of string * Localise.error_desc
 exception Inherently_dangerous_function of Localise.error_desc
 exception Internal_error of Localise.error_desc
@@ -64,7 +65,7 @@ exception Parameter_not_null_checked of Localise.error_desc * ml_location
 exception Pointer_size_mismatch of Localise.error_desc * ml_location
 exception Precondition_not_found of Localise.error_desc * ml_location
 exception Precondition_not_met of Localise.error_desc * ml_location
-exception Retain_cycle of Prop.normal Prop.t * Sil.hpred *Localise.error_desc * ml_location
+exception Retain_cycle of Prop.normal Prop.t * Sil.hpred * Localise.error_desc * ml_location
 exception Return_expression_required of Localise.error_desc * ml_location
 exception Return_statement_missing of Localise.error_desc * ml_location
 exception Return_value_ignored of Localise.error_desc * ml_location
@@ -140,6 +141,8 @@ let recognize_exception exn =
         (Localise.from_string kind_s, desc, None, Exn_user, High, None, Prover)
     | Field_not_null_checked (desc, mloc) ->
         (Localise.field_not_null_checked, desc, Some mloc, Exn_user, Medium, Some Kwarning, Nocat)
+    | Frontend_warning (name, desc, mloc) ->
+        (Localise.from_string name, desc, Some mloc, Exn_user, Medium, None, Nocat)
     | Checkers (kind_s, desc) ->
         (Localise.from_string kind_s, desc, None, Exn_user, High, None, Prover)
     | Null_dereference (desc, mloc) ->

infer/src/backend/exceptions.mli

@@ -52,6 +52,7 @@ exception Divide_by_zero of Localise.error_desc * ml_location
 exception Field_not_null_checked of Localise.error_desc * ml_location
 exception Eradicate of string * Localise.error_desc
 exception Checkers of string * Localise.error_desc
+exception Frontend_warning of string * Localise.error_desc * ml_location
 exception Inherently_dangerous_function of Localise.error_desc
 exception Internal_error of Localise.error_desc
 exception Java_runtime_exception of Mangled.t * string * Localise.error_desc

infer/src/backend/localise.ml

@@ -573,6 +573,10 @@ let desc_divide_by_zero expr_str loc =
       (at_line tags loc) in
   [description], None, !tags
 
+let desc_frontend_warning desc sugg loc =
+  let tags = Tags.create () in
+  [desc ^" at line "^ loc ^". "^ sugg], None, !tags
+
 let desc_leak value_str_opt resource_opt resource_action_opt loc bucket_opt =
   let tags = Tags.create () in
   let () = match bucket_opt with
@@ -655,7 +659,7 @@ let desc_retain_cycle prop cycle loc =
     match Str.split_delim (Str.regexp_string "&old_") s with
     | [_; s'] -> s'
     | _ -> s in
-  let do_edge ((se,_), f, se') =
+  let do_edge ((se, _), f, se') =
     match se with
     | Sil.Eexp(Sil.Lvar pvar, _) when Sil.pvar_equal pvar Sil.block_pvar ->
         str_cycle:=!str_cycle^" ("^(string_of_int !ct)^") a block capturing "^(Ident.fieldname_to_string f)^"; ";

infer/src/backend/localise.mli

@@ -187,6 +187,8 @@ val desc_deallocate_static_memory : string -> Procname.t -> Location.t -> error_
 
 val desc_divide_by_zero : string -> Location.t -> error_desc
 
+val desc_frontend_warning : string -> string -> string -> error_desc
+
 val desc_leak :
   string option -> Sil.resource option -> Sil.res_action option ->
   Location.t -> string option -> error_desc

infer/src/backend/reporting.ml

@@ -10,8 +10,7 @@
 open Utils
 module L = Logging
 
-type log_issue =
+type log_t =
-  Procname.t ->
   ?loc: Location.t option ->
   ?node_id: (int * int) option ->
   ?session: int option ->
@@ -20,18 +19,19 @@ type log_issue =
   exn ->
   unit
 
-let log_issue
+type log_issue = Procname.t -> log_t
+
+type log_issue_from_errlog = Errlog.t -> log_t
+
+let log_issue_from_errlog
     err_kind
-    proc_name
+    err_log
     ?(loc = None)
     ?(node_id = None)
     ?(session = None)
     ?(ltr = None)
     ?(pre = None)
     exn =
-  match Specs.get_summary proc_name with
-  | Some summary ->
-      let err_log = summary.Specs.attributes.ProcAttributes.err_log in
   let loc = match loc with
     | None -> State.get_loc ()
     | Some loc -> loc in
@@ -45,8 +45,27 @@ let log_issue
     | None -> State.get_loc_trace ()
     | Some ltr -> ltr in
   Errlog.log_issue err_kind err_log loc node_id session ltr pre exn
+
+let log_issue
+    err_kind
+    proc_name
+    ?(loc = None)
+    ?(node_id = None)
+    ?(session = None)
+    ?(ltr = None)
+    ?(pre = None)
+    exn =
+  match Specs.get_summary proc_name with
+  | Some summary ->
+      let err_log = summary.Specs.attributes.ProcAttributes.err_log in
+      log_issue_from_errlog err_kind err_log ~loc:loc ~node_id:node_id
+        ~session:session ~ltr:ltr ~pre:pre exn
   | None -> ()
 
 let log_error = log_issue Exceptions.Kerror
 let log_warning = log_issue Exceptions.Kwarning
 let log_info = log_issue Exceptions.Kinfo
+
+let log_error_from_errlog = log_issue_from_errlog Exceptions.Kerror
+let log_warning_from_errlog = log_issue_from_errlog Exceptions.Kwarning
+let log_info_from_errlog = log_issue_from_errlog Exceptions.Kinfo

infer/src/backend/reporting.mli

@@ -8,8 +8,8 @@
  *)
 
 (** Type of functions to report issues to the error_log in a spec. *)
-type log_issue =
+
-  Procname.t ->
+type log_t =
   ?loc: Location.t option ->
   ?node_id: (int * int) option ->
   ?session: int option ->
@@ -18,6 +18,10 @@ type log_issue =
   exn ->
   unit
 
+type log_issue = Procname.t -> log_t
+
+type log_issue_from_errlog = Errlog.t -> log_t
+
 (** Report an error in the given procedure. *)
 val log_error : log_issue
 
@@ -26,3 +30,12 @@ val log_warning : log_issue
 
 (** Report an info in the given procedure. *)
 val log_info : log_issue
+
+(** Report an error in the given error log. *)
+val log_error_from_errlog : log_issue_from_errlog
+
+(** Report a warning in the given error log. *)
+val log_warning_from_errlog : log_issue_from_errlog
+
+(** Report an info in the given error log. *)
+val log_info_from_errlog : log_issue_from_errlog

infer/src/clang/cFrontend.ml

@@ -79,7 +79,8 @@ let rec translate_one_declaration tenv cg cfg namespace parent_dec dec =
       let curr_class = ObjcInterface_decl.get_curr_class_impl idi in
       let type_ptr_to_sil_type = CTypes_decl.type_ptr_to_sil_type in
       ignore (ObjcInterface_decl.interface_impl_declaration type_ptr_to_sil_type tenv dec);
-      CMethod_declImpl.process_methods tenv cg cfg curr_class namespace decl_list
+      CMethod_declImpl.process_methods tenv cg cfg curr_class namespace decl_list;
+      CFrontend_errors.check_for_property_errors cfg curr_class
 
   | CXXMethodDecl (decl_info, name_info, type_ptr, function_decl_info, _)
   | CXXConstructorDecl (decl_info, name_info, type_ptr, function_decl_info, _)
@@ -90,7 +91,7 @@ let rec translate_one_declaration tenv cg cfg namespace parent_dec dec =
         | Some ptr -> Ast_utils.get_decl ptr
         | None -> Some parent_dec in
       (match class_decl with
-       | Some (CXXRecordDecl _ as d)->
+       | Some (CXXRecordDecl _ as d) ->
            let class_name = CTypes_decl.get_record_name d in
            let curr_class = CContext.ContextCls(class_name, None, []) in
            if !CFrontend_config.testing_mode then

infer/src/clang/cFrontend_checkers.ml

@@ -0,0 +1,30 @@
+(*
+ * 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.
+ *)
+
+open CFrontend_utils
+open General_utils
+
+
+(* === Warnings on properties === *)
+
+(* Strong Delegate Warning: a property with name delegate should not be declared strong *)
+let checker_strong_delegate_warning pname ptype =
+  Printing.log_out "Checking for STRONG_DELEGATE property warning\n";
+  let delegate_regexp = Str.regexp_string_case_fold "delegate" in
+  let pname_contains_delegate = try
+      Str.search_forward delegate_regexp pname.Clang_ast_t.ni_name 0 >= 0
+    with Not_found -> false in
+  let condition = pname_contains_delegate
+                  && ObjcProperty_decl.is_strong_property ptype in
+  let warning_desc= {name = "STRONG_DELEGATE_WARNING";
+                     description = "Property or ivar "^pname.Clang_ast_t.ni_name^" declared strong";
+                     suggestion = "In general delegates should be declared weak or assign";
+                     loc = string_of_int (ObjcProperty_decl.property_line ptype);
+                    } in
+  (condition, warning_desc)

infer/src/clang/cFrontend_checkers.mli

@@ -0,0 +1,16 @@
+(*
+ * 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.
+ *)
+
+
+(* === Warnings on properties === *)
+
+(* Strong Delegate Warning: a property with name delegate should not be declared strong *)
+val checker_strong_delegate_warning : Clang_ast_t.named_decl_info ->
+  ObjcProperty_decl.property_type ->
+  (bool * CFrontend_utils.General_utils.warning_desc)

infer/src/clang/cFrontend_errors.ml

@@ -0,0 +1,55 @@
+(*
+ * 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.
+ *)
+
+(* Module for warnings detected at translation time by the frontend
+ * To specify a checker define the following:
+ * - condition: a boolean condition when the warning should be flagged
+ * - description: a string describing the warning
+ * - loc: the location where is occurs
+*)
+
+open Utils
+open CFrontend_utils
+open General_utils
+
+(* === Warnings on properties === *)
+
+let property_checkers_list = [CFrontend_checkers.checker_strong_delegate_warning]
+
+(* Add a frontend warning with a description desc at location loc to the errlog of a proc desc *)
+let log_frontend_warning pdesc warn_desc =
+  let errlog = Cfg.Procdesc.get_err_log pdesc in
+  let err_desc =
+    Errdesc.explain_frontend_warning warn_desc.description warn_desc.suggestion
+      warn_desc.loc in
+  let exn = (Exceptions.Frontend_warning
+               (warn_desc.name, err_desc,
+                try assert false with Assert_failure x -> x)) in
+  Reporting.log_error_from_errlog errlog exn
+
+(* Call all checkers on properties of class c *)
+let check_for_property_errors cfg c =
+  let qual_setter setter_name =
+    let cname = CContext.get_curr_class_name c in
+    mk_procname_from_objc_method cname setter_name Procname.Instance_objc_method in
+  let do_one_property (pname, property) =
+    let (_, _, _, _, (setter_name, _), ndi) = property in
+    let qual_setter = qual_setter setter_name in
+    match Cfg.Procdesc.find_from_name cfg qual_setter with
+    | Some pdesc_setter -> (* Property warning will be added to the proc desc of the setter *)
+        Printing.log_out "Found setter for property   %s\n" pname.Clang_ast_t.ni_name;
+        IList.iter (fun checker ->
+            let (condition, warning_desc) = checker pname property in
+            if condition then log_frontend_warning pdesc_setter warning_desc) property_checkers_list
+    | None ->
+        Printing.log_out "NO Setter Found for property %s\n" pname.Clang_ast_t.ni_name;
+        () in
+  let properties = ObjcProperty_decl.find_properties_class c in
+  Printing.log_out "Retrieved all properties of the class...\n";
+  IList.iter do_one_property properties

infer/src/clang/cFrontend_errors.mli

@@ -0,0 +1,15 @@
+(*
+ * 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.
+ *)
+
+
+(* Module for warnings detected at translation time by the frontend *)
+
+
+(* Checks for warnings on properties of class c *)
+val check_for_property_errors : Cfg.cfg -> CContext.curr_class -> unit

infer/src/clang/cFrontend_utils.ml

@@ -356,6 +356,13 @@ let get_fresh_block_index () =
 module General_utils =
 struct
 
+  type warning_desc = {
+    name : string;
+    description : string;
+    suggestion : string; (* an optional suggestion or correction *)
+    loc : string;
+  }
+
   type var_info = Clang_ast_t.decl_info * Clang_ast_t.type_ptr * Clang_ast_t.var_decl_info * bool
 
   let rec swap_elements_list l =

infer/src/clang/cFrontend_utils.mli

@@ -113,6 +113,14 @@ end
 
 module General_utils :
 sig
+
+  type warning_desc = {
+    name : string;
+    description : string;
+    suggestion : string; (* an optional suggestion or correction *)
+    loc : string;
+  }
+
   type var_info = Clang_ast_t.decl_info * Clang_ast_t.type_ptr * Clang_ast_t.var_decl_info * bool
 
   val string_from_list : string list -> string

infer/src/clang/objcProperty_decl.ml

@@ -220,6 +220,19 @@ let method_is_property_accesor cls method_name =
           else None in
   IList.fold_right method_is_getter properties_class None
 
+let is_strong_property property_type =
+  let _, attrs, _, _, _, _ = property_type in
+  IList.exists (fun a -> match a with
+      | `Strong -> true
+      | _ -> false) attrs
+
+let property_line property_type =
+  let _, attrs, decl_info, _, _, _ = property_type in
+  let src_range = fst decl_info.Clang_ast_t.di_source_range in
+  match src_range.Clang_ast_t.sl_line with
+  | Some l -> l
+  | _ -> -1
+
 let prepare_dynamic_property curr_class decl_info property_impl_decl_info =
   let pname = Ast_utils.property_name property_impl_decl_info in
   let prop_name = pname.Clang_ast_t.ni_name in
@@ -366,7 +379,7 @@ let add_properties_to_table curr_class decl_list =
     | _ -> () in
   IList.iter add_property_to_table decl_list
 
-(* Given a list of declarations in an interface returns list of methods)*)
+(* Given a list of declarations in an interface returns list of methods *)
 let get_methods curr_class decl_list =
   let class_name = CContext.get_curr_class_name curr_class in
   add_properties_to_table curr_class decl_list;

infer/src/clang/objcProperty_decl.mli

@@ -79,3 +79,9 @@ val method_is_property_accesor : CContext.curr_class -> string ->
 
 val get_ivar_name : Clang_ast_t.named_decl_info -> Clang_ast_t.named_decl_info option ->
   Clang_ast_t.named_decl_info
+
+(* Given a property type returns whether the property is strong *)
+val is_strong_property : property_type -> bool
+
+(* Given a property type returns whether the property line *)
+val property_line : property_type -> int

infer/tests/codetoanalyze/objc/warnings/strong_delegate.m

@@ -0,0 +1,36 @@
+/*
+* 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/NSObject.h>
+
+@interface A : NSObject
+- (void) bla;
+
+@property (nonatomic,strong) id delegated;
+
+@property (nonatomic,strong) id delegation;
+
+@property (nonatomic,strong) id delegate;
+
+@property (nonatomic,strong) id fileDelegate;
+
+@property (nonatomic,strong) id delegateFile;
+
+@property (nonatomic,strong) id OneDelegateFile;
+
+@end
+
+@implementation A {
+    BOOL _b;
+}
+
+- (void) bla {
+}
+
+@end

infer/tests/endtoend/objc/StrongDelegateTest.java

@@ -0,0 +1,69 @@
+/*
+* 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.
+*/
+
+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 StrongDelegateTest {
+
+  public static final String FILE =
+      "infer/tests/codetoanalyze/objc/warnings/strong_delegate.m";
+
+  private static ImmutableList<String> inferCmdFraction;
+
+  public static final String STRONG_DELEGATE_WARNING = "STRONG_DELEGATE_WARNING";
+
+  @ClassRule
+  public static DebuggableTemporaryFolder folder =
+      new DebuggableTemporaryFolder();
+
+  @BeforeClass
+  public static void runInfer() throws InterruptedException, IOException {
+    inferCmdFraction = InferRunner.createObjCInferCommand(
+        folder,
+        FILE);
+  }
+
+  @Test
+  public void whenInferRunsOnStrongDelegate()
+      throws InterruptedException, IOException, InferException {
+    InferResults inferResults = InferRunner.runInferObjC(inferCmdFraction);
+    assertThat(
+        "Results should contain a strong delegate warning",
+        inferResults,
+        containsExactly(
+            STRONG_DELEGATE_WARNING,
+            FILE,
+            new String[]{
+                "setDelegate:",
+                "setDelegateFile:",
+                "setDelegated:",
+                "setFileDelegate:",
+                "setOneDelegateFile:"
+            }
+        )
+    );
+  }
+
+}