diff --git a/infer/bin/inferlib.py b/infer/bin/inferlib.py index e8c0089e2..2bf42f2f5 100644 --- a/infer/bin/inferlib.py +++ b/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 ] diff --git a/infer/src/backend/errdesc.ml b/infer/src/backend/errdesc.ml index 56fb52a04..51e4d94cf 100644 --- a/infer/src/backend/errdesc.ml +++ b/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 diff --git a/infer/src/backend/errdesc.mli b/infer/src/backend/errdesc.mli index 6f8f3cb76..4411af73a 100644 --- a/infer/src/backend/errdesc.mli +++ b/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 diff --git a/infer/src/backend/exceptions.ml b/infer/src/backend/exceptions.ml index c4265f725..2fec19012 100644 --- a/infer/src/backend/exceptions.ml +++ b/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) -> diff --git a/infer/src/backend/exceptions.mli b/infer/src/backend/exceptions.mli index c85d8105e..a4f6caced 100644 --- a/infer/src/backend/exceptions.mli +++ b/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 diff --git a/infer/src/backend/localise.ml b/infer/src/backend/localise.ml index 319d23d81..bb4fe8253 100644 --- a/infer/src/backend/localise.ml +++ b/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)^"; "; diff --git a/infer/src/backend/localise.mli b/infer/src/backend/localise.mli index 55abc9103..fa9a5ea8e 100644 --- a/infer/src/backend/localise.mli +++ b/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 diff --git a/infer/src/backend/reporting.ml b/infer/src/backend/reporting.ml index f20f6d00e..e9d7dd99a 100644 --- a/infer/src/backend/reporting.ml +++ b/infer/src/backend/reporting.ml @@ -10,8 +10,7 @@ open Utils module L = Logging -type log_issue = - Procname.t -> +type log_t = ?loc: Location.t option -> ?node_id: (int * int) option -> ?session: int option -> @@ -20,6 +19,33 @@ type log_issue = exn -> unit +type log_issue = Procname.t -> log_t + +type log_issue_from_errlog = Errlog.t -> log_t + +let log_issue_from_errlog + err_kind + err_log + ?(loc = None) + ?(node_id = None) + ?(session = None) + ?(ltr = None) + ?(pre = None) + exn = + let loc = match loc with + | None -> State.get_loc () + | Some loc -> loc in + let node_id = match node_id with + | None -> State.get_node_id_key () + | Some node_id -> node_id in + let session = match session with + | None -> State.get_session () + | Some session -> session in + let ltr = match ltr with + | 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 @@ -32,21 +58,14 @@ let log_issue 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 - let node_id = match node_id with - | None -> State.get_node_id_key () - | Some node_id -> node_id in - let session = match session with - | None -> State.get_session () - | Some session -> session in - let ltr = match ltr with - | None -> State.get_loc_trace () - | Some ltr -> ltr in - Errlog.log_issue err_kind err_log loc node_id session ltr pre exn + 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 diff --git a/infer/src/backend/reporting.mli b/infer/src/backend/reporting.mli index 376981b1b..6c7966aa7 100644 --- a/infer/src/backend/reporting.mli +++ b/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 diff --git a/infer/src/clang/cFrontend.ml b/infer/src/clang/cFrontend.ml index e19375c7f..1b8b0019a 100644 --- a/infer/src/clang/cFrontend.ml +++ b/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 diff --git a/infer/src/clang/cFrontend_checkers.ml b/infer/src/clang/cFrontend_checkers.ml new file mode 100644 index 000000000..647408ffc --- /dev/null +++ b/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) diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli new file mode 100644 index 000000000..a88f5a16b --- /dev/null +++ b/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) diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml new file mode 100644 index 000000000..e29abb11c --- /dev/null +++ b/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 diff --git a/infer/src/clang/cFrontend_errors.mli b/infer/src/clang/cFrontend_errors.mli new file mode 100644 index 000000000..d2bcc3302 --- /dev/null +++ b/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 diff --git a/infer/src/clang/cFrontend_utils.ml b/infer/src/clang/cFrontend_utils.ml index 58af20984..2c2138020 100644 --- a/infer/src/clang/cFrontend_utils.ml +++ b/infer/src/clang/cFrontend_utils.ml @@ -343,7 +343,7 @@ struct ignore (type_ptr_to_sil_type tenv (`DeclPtr dr.Clang_ast_t.dr_decl_pointer)) in IList.iter add_elem decl_ref_list -end +end (* Global counter for anonymous block*) let block_counter = ref 0 @@ -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 = diff --git a/infer/src/clang/cFrontend_utils.mli b/infer/src/clang/cFrontend_utils.mli index df4c3a5a6..7f571e768 100644 --- a/infer/src/clang/cFrontend_utils.mli +++ b/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 diff --git a/infer/src/clang/objcProperty_decl.ml b/infer/src/clang/objcProperty_decl.ml index 4452aaab1..e695d4d1b 100644 --- a/infer/src/clang/objcProperty_decl.ml +++ b/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; diff --git a/infer/src/clang/objcProperty_decl.mli b/infer/src/clang/objcProperty_decl.mli index 9ad75bde8..b0a25eff5 100644 --- a/infer/src/clang/objcProperty_decl.mli +++ b/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 diff --git a/infer/tests/codetoanalyze/objc/warnings/strong_delegate.m b/infer/tests/codetoanalyze/objc/warnings/strong_delegate.m new file mode 100644 index 000000000..3da97d5b3 --- /dev/null +++ b/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 + +@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 diff --git a/infer/tests/endtoend/objc/StrongDelegateTest.java b/infer/tests/endtoend/objc/StrongDelegateTest.java new file mode 100644 index 000000000..cbaaf6363 --- /dev/null +++ b/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 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:" + } + ) + ); + } + +}