From 6b9e1fc9d7f404cc3a8e79d45f4144653c892fff Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Thu, 17 Dec 2015 09:45:53 -0800 Subject: [PATCH] Stop reporting false alarms due to __nullable on Obj-C property getters and setters Summary: public Did this by adding an option to rearrange that turns of error reporting. Reviewed By: dulmarod Differential Revision: D2768396 fb-gh-sync-id: 4898d2d --- infer/src/backend/rearrange.ml | 6 ++- infer/src/backend/rearrange.mli | 2 +- infer/src/backend/symExec.ml | 42 +++++++++---------- .../codetoanalyze/objc/errors/npe/nullable.m | 29 +++++++++++-- 4 files changed, 51 insertions(+), 28 deletions(-) diff --git a/infer/src/backend/rearrange.ml b/infer/src/backend/rearrange.ml index 4f9e49354..608ac1821 100644 --- a/infer/src/backend/rearrange.ml +++ b/infer/src/backend/rearrange.ml @@ -1091,7 +1091,9 @@ let check_call_to_objc_block_error pdesc prop fun_exp loc = (** [rearrange lexp prop] rearranges [prop] into the form [prop' * lexp|->strexp:typ]. It returns an iterator with [lexp |-> strexp: typ] as current predicate and the path (an [offsetlist]) which leads to [lexp] as the iterator state. *) -let rearrange pdesc tenv lexp typ prop loc : (Sil.offset list) Prop.prop_iter list = +let rearrange ?(report_deref_errors=true) pdesc tenv lexp typ prop loc + : (Sil.offset list) Prop.prop_iter list = + let nlexp = match Prop.exp_normalize_prop prop lexp with | Sil.BinOp(Sil.PlusPI, ep, e) -> (* array access with pointer arithmetic *) Sil.Lindex(ep, e) @@ -1102,7 +1104,7 @@ let rearrange pdesc tenv lexp typ prop loc : (Sil.offset list) Prop.prop_iter li L.d_strln ".... Rearrangement Start ...."; L.d_str "Exp: "; Sil.d_exp nlexp; L.d_ln (); L.d_str "Prop: "; L.d_ln(); Prop.d_prop prop; L.d_ln (); L.d_ln (); - check_dereference_error pdesc prop nlexp (State.get_loc ()); + if report_deref_errors then check_dereference_error pdesc prop nlexp (State.get_loc ()); let pname = Cfg.Procdesc.get_proc_name pdesc in match Prop.prop_iter_create prop with | None -> diff --git a/infer/src/backend/rearrange.mli b/infer/src/backend/rearrange.mli index e475c52ef..21f22c2b1 100644 --- a/infer/src/backend/rearrange.mli +++ b/infer/src/backend/rearrange.mli @@ -26,6 +26,6 @@ val check_call_to_objc_block_error : It returns an iterator with [lexp |-> strexp: typ] as current predicate and the path (an [offsetlist]) which leads to [lexp] as the iterator state. *) val rearrange : - Cfg.Procdesc.t -> Sil.tenv -> Sil.exp -> + ?report_deref_errors:bool -> Cfg.Procdesc.t -> Sil.tenv -> Sil.exp -> Sil.typ -> Prop.normal Prop.t -> Location.t -> (Sil.offset list) Prop.prop_iter list diff --git a/infer/src/backend/symExec.ml b/infer/src/backend/symExec.ml index e2fb415ad..5a7c13228 100644 --- a/infer/src/backend/symExec.ml +++ b/infer/src/backend/symExec.ml @@ -835,7 +835,7 @@ let add_constraints_on_retval pdesc prop ret_exp typ callee_pname callee_loc = else prop'' else add_ret_non_null ret_exp typ prop -let execute_letderef pname pdesc tenv id rhs_exp typ loc prop_ = +let execute_letderef ?(report_deref_errors=true) pname pdesc tenv id rhs_exp typ loc prop_ = let execute_letderef_ pdesc tenv id rhs_exp loc acc_in iter = let iter_ren = Prop.prop_iter_make_id_primed id iter in let prop_ren = Prop.prop_iter_to_prop iter_ren in @@ -887,7 +887,8 @@ let execute_letderef pname pdesc tenv id rhs_exp typ loc prop_ = add_constraints_on_retval pdesc prop n_rhs_exp' typ callee_pname callee_loc | _ -> prop else prop in - let iter_list = Rearrange.rearrange pdesc tenv n_rhs_exp' typ prop' loc in + let iter_list = + Rearrange.rearrange ~report_deref_errors pdesc tenv n_rhs_exp' typ prop' loc in IList.rev (IList.fold_left (execute_letderef_ pdesc tenv id n_rhs_exp' loc) [] iter_list) with Rearrange.ARRAY_ACCESS -> if (!Config.array_level = 0) then assert false @@ -895,7 +896,7 @@ let execute_letderef pname pdesc tenv id rhs_exp typ loc prop_ = let undef = Sil.exp_get_undefined false in [Prop.conjoin_eq (Sil.Var id) undef prop_] -let execute_set pname pdesc tenv lhs_exp typ rhs_exp loc prop_ = +let execute_set ?(report_deref_errors=true) pname pdesc tenv lhs_exp typ rhs_exp loc prop_ = let execute_set_ pdesc tenv rhs_exp acc_in iter = let (lexp, strexp, typ, st, offlist) = match Prop.prop_iter_current iter with @@ -918,7 +919,7 @@ let execute_set pname pdesc tenv lhs_exp typ rhs_exp loc prop_ = let n_rhs_exp, prop = exp_norm_check_arith pname _prop' rhs_exp in let prop = Prop.replace_objc_null prop n_lhs_exp n_rhs_exp in let n_lhs_exp' = Prop.exp_collapse_consecutive_indices_prop prop typ n_lhs_exp in - let iter_list = Rearrange.rearrange pdesc tenv n_lhs_exp' typ prop loc in + let iter_list = Rearrange.rearrange ~report_deref_errors pdesc tenv n_lhs_exp' typ prop loc in IList.rev (IList.fold_left (execute_set_ pdesc tenv n_rhs_exp) [] iter_list) with Rearrange.ARRAY_ACCESS -> if (!Config.array_level = 0) then assert false @@ -1344,8 +1345,7 @@ and sym_exe_check_variadic_sentinel_if_present cfg pdesc tenv prop path (IList.length formals) actual_params sentinel_arg callee_pname loc -and sym_exec_objc_getter field_name ret_typ_opt tenv cfg ret_ids pdesc callee_name loc args _prop - path : Builtin.ret_typ = +and sym_exec_objc_getter field_name ret_typ_opt tenv cfg ret_ids pdesc pname loc args prop = L.d_strln ("No custom getter found. Executing the ObjC builtin getter with ivar "^ (Ident.fieldname_to_string field_name)^"."); let ret_id = @@ -1363,12 +1363,11 @@ and sym_exec_objc_getter field_name ret_typ_opt tenv cfg ret_ids pdesc callee_na | Sil.Tptr (t, _) -> Sil.expand_type tenv t | _ -> assert false) in let field_access_exp = Sil.Lfield (lexp, field_name, typ') in - let ret_instr = Sil.Letderef (ret_id, field_access_exp, ret_typ, loc) in - sym_exec_generated false cfg tenv pdesc [ret_instr] [(_prop, path)] + execute_letderef + ~report_deref_errors:false pname pdesc tenv ret_id field_access_exp ret_typ loc prop | _ -> raise (Exceptions.Wrong_argument_number (try assert false with Assert_failure x -> x)) -and sym_exec_objc_setter field_name ret_typ_opt tenv cfg ret_ids pdesc callee_name loc args _prop - path : Builtin.ret_typ = +and sym_exec_objc_setter field_name ret_typ_opt tenv cfg ret_ids pdesc pname loc args prop = L.d_strln ("No custom setter found. Executing the ObjC builtin setter with ivar "^ (Ident.fieldname_to_string field_name)^"."); match args with @@ -1378,19 +1377,20 @@ and sym_exec_objc_setter field_name ret_typ_opt tenv cfg ret_ids pdesc callee_na | Sil.Tptr (t, _) -> Sil.expand_type tenv t | _ -> assert false) in let field_access_exp = Sil.Lfield (lexp1, field_name, typ1') in - let set_instr = Sil.Set (field_access_exp, typ2, lexp2, loc) in - sym_exec_generated false cfg tenv pdesc [set_instr] [(_prop, path)] + execute_set ~report_deref_errors:false pname pdesc tenv field_access_exp typ2 lexp2 loc prop | _ -> raise (Exceptions.Wrong_argument_number (try assert false with Assert_failure x -> x)) -and sym_exec_objc_accessor property_accesor ret_typ_opt tenv cfg ret_ids pdesc callee_name loc args - _prop path : Builtin.ret_typ = - match property_accesor with - | ProcAttributes.Objc_getter field_name -> - sym_exec_objc_getter field_name ret_typ_opt tenv cfg ret_ids pdesc callee_name loc args _prop - path - | ProcAttributes.Objc_setter field_name -> - sym_exec_objc_setter field_name ret_typ_opt tenv cfg ret_ids pdesc callee_name loc args _prop - path +and sym_exec_objc_accessor property_accesor ret_typ_opt tenv cfg ret_ids pdesc callee_pname loc args + prop path : Builtin.ret_typ = + let f_accessor = + match property_accesor with + | ProcAttributes.Objc_getter field_name -> sym_exec_objc_getter field_name + | ProcAttributes.Objc_setter field_name -> sym_exec_objc_setter field_name in + (* we want to execute in the context of the current procedure, not in the context of callee_pname, + since this is the procname of the setter/getter method *) + let cur_pname = Cfg.Procdesc.get_proc_name pdesc in + f_accessor ret_typ_opt tenv cfg ret_ids pdesc cur_pname loc args prop + |> IList.map (fun p -> (p, path)) (** Perform symbolic execution for a function call *) and sym_exec_call cfg pdesc tenv pre path ret_ids actual_pars summary loc = diff --git a/infer/tests/codetoanalyze/objc/errors/npe/nullable.m b/infer/tests/codetoanalyze/objc/errors/npe/nullable.m index 8a5835c94..6c707b739 100644 --- a/infer/tests/codetoanalyze/objc/errors/npe/nullable.m +++ b/infer/tests/codetoanalyze/objc/errors/npe/nullable.m @@ -10,22 +10,43 @@ #import @interface A : NSObject { - @public int x; + @public int fld; } @end +@interface B : NSObject + +@property int prop; + +- (void) method; + +@end + + int derefNullableParamDirect(A * __nullable param) { - return param->x; + return param->fld; } int derefNullableParamIndirect(A * __nullable param) { A* local = param; - return local->x; + return local->fld; } A * derefNullableParamOk(A * __nullable param) { if (!param) param = [A new]; - param->x = 7; + param->fld = 7; return param; } + +int readNullableParamPropertyOk(B * __nullable param) { + return param.prop; +} + +void writeNullableParamPropertyOk(B * __nullable param) { + param.prop = 7; +} + +void methodCallOnNullableParamOk(B * __nullable param) { + [param method]; +}