From c02b3ca034ce5be3ccceeccccfd4bd1801d40b66 Mon Sep 17 00:00:00 2001 From: Dulma Rodriguez Date: Tue, 8 Dec 2015 10:10:32 -0800 Subject: [PATCH] Add correct ivar name to the getters and save the ivar to the tenv if not available Summary: public The ivar corresponding to the property is only available in the ast when the implementation of the peroperty is available. Otherwise we add an ivar with the correct type and the default name to the tenv and use it in the getter (and later in the setter). This was not causing crashes because the generated code was swallowing the Missing_fld exception. Now it flags it. Reviewed By: akotulski Differential Revision: D2734217 fb-gh-sync-id: 21c62af --- infer/src/backend/symExec.ml | 2 +- infer/src/clang/cField_decl.ml | 24 ++++++- infer/src/clang/cField_decl.mli | 6 ++ infer/src/clang/cMethod_trans.ml | 11 +-- .../objc/errors/property/ExplicitIvarName.m | 47 +++++++++++++ .../objc/ExplicitIvarNameInGetterTest.java | 67 +++++++++++++++++++ 6 files changed, 148 insertions(+), 9 deletions(-) create mode 100644 infer/tests/codetoanalyze/objc/errors/property/ExplicitIvarName.m create mode 100644 infer/tests/endtoend/objc/ExplicitIvarNameInGetterTest.java diff --git a/infer/src/backend/symExec.ml b/infer/src/backend/symExec.ml index 327930cbe..2cf024b91 100644 --- a/infer/src/backend/symExec.ml +++ b/infer/src/backend/symExec.ml @@ -1365,7 +1365,7 @@ and sym_exec_objc_getter field_name ret_typ_opt tenv cfg ret_ids pdesc callee_na | _ -> 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 true cfg tenv pdesc [ret_instr] [(_prop, path)] + sym_exec_generated false cfg tenv pdesc [ret_instr] [(_prop, path)] | _ -> 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 diff --git a/infer/src/clang/cField_decl.ml b/infer/src/clang/cField_decl.ml index 138197ba2..c811ba0b4 100644 --- a/infer/src/clang/cField_decl.ml +++ b/infer/src/clang/cField_decl.ml @@ -36,7 +36,8 @@ let fields_superclass tenv interface_decl_info = | _ -> []) | _ -> [] -let build_sil_field type_ptr_to_sil_type tenv field_name type_ptr prop_atts = +let build_sil_field type_ptr_to_sil_type tenv field_name type_ptr prop_attributes = + let prop_atts = IList.map Clang_ast_j.string_of_property_attribute prop_attributes in let annotation_from_type t = match t with | Sil.Tptr (_, Sil.Pk_objc_weak) -> [Config.weak] @@ -72,8 +73,7 @@ let build_sil_field_property type_ptr_to_sil_type curr_class tenv field_name typ match att_opt with | Some prop_attributes -> prop_attributes | None -> ivar_property curr_class field_name in - let atts_str = IList.map Clang_ast_j.string_of_property_attribute prop_attributes in - build_sil_field type_ptr_to_sil_type tenv field_name type_ptr atts_str + build_sil_field type_ptr_to_sil_type tenv field_name type_ptr prop_attributes (* Given a list of declarations in an interface returns a list of fields *) let rec get_fields type_ptr_to_sil_type tenv curr_class decl_list = @@ -127,3 +127,21 @@ let is_ivar_atomic ivar fields = with Not_found -> ( Printing.log_out "NOT Found field ivar = '%s' " (Ident.fieldname_to_string ivar); false) + +let get_property_corresponding_ivar tenv type_ptr_to_sil_type class_name property_decl = + let open Clang_ast_t in + match property_decl with + | ObjCPropertyDecl (decl_info, named_decl_info, obj_c_property_decl_info) -> + (let ivar_decl_ref = obj_c_property_decl_info.Clang_ast_t.opdi_ivar_decl in + match Ast_utils.get_decl_opt_with_decl_ref ivar_decl_ref with + | Some ObjCIvarDecl (decl_info, named_decl_info, type_ptr, _, _) -> + General_utils.mk_class_field_name named_decl_info + | _ -> (* Ivar is not known, so add a default one to the tenv *) + let type_ptr = obj_c_property_decl_info.Clang_ast_t.opdi_type_ptr in + let field_name_str = Ast_utils.generated_ivar_name named_decl_info in + let prop_attributes = obj_c_property_decl_info.Clang_ast_t.opdi_property_attributes in + let field_name, typ, attr = build_sil_field type_ptr_to_sil_type tenv + field_name_str type_ptr prop_attributes in + ignore (add_missing_fields tenv class_name [(field_name, typ, attr)]); + field_name) + | _ -> assert false diff --git a/infer/src/clang/cField_decl.mli b/infer/src/clang/cField_decl.mli index dd0ba7993..4c9616f19 100644 --- a/infer/src/clang/cField_decl.mli +++ b/infer/src/clang/cField_decl.mli @@ -20,6 +20,9 @@ val get_fields : Ast_utils.type_ptr_to_sil_type -> Sil.tenv -> CContext.curr_cla val fields_superclass : Sil.tenv -> Clang_ast_t.obj_c_interface_decl_info -> field_type list +val build_sil_field : Ast_utils.type_ptr_to_sil_type -> Sil.tenv -> Clang_ast_t.named_decl_info -> + Clang_ast_t.type_ptr -> Clang_ast_t.property_attribute list -> field_type + val build_sil_field_property : Ast_utils.type_ptr_to_sil_type -> CContext.curr_class -> Sil.tenv -> Clang_ast_t.named_decl_info -> Clang_ast_t.type_ptr -> Clang_ast_t.property_attribute list option -> field_type @@ -27,3 +30,6 @@ val build_sil_field_property : Ast_utils.type_ptr_to_sil_type -> CContext.curr_c val add_missing_fields : Sil.tenv -> string -> field_type list -> unit val is_ivar_atomic : Ident.fieldname -> Sil.struct_fields -> bool + +val get_property_corresponding_ivar : Sil.tenv -> Ast_utils.type_ptr_to_sil_type -> string -> + Clang_ast_t.decl -> Ident.fieldname diff --git a/infer/src/clang/cMethod_trans.ml b/infer/src/clang/cMethod_trans.ml index c030c4527..0a60bfa39 100644 --- a/infer/src/clang/cMethod_trans.ml +++ b/infer/src/clang/cMethod_trans.ml @@ -240,13 +240,14 @@ let get_objc_method_data obj_c_message_expr_info = | `Class _ | `SuperClass -> (selector, pointer, MCStatic) -let get_objc_property_accessor ms = +let get_objc_property_accessor tenv ms = let open Clang_ast_t in let pointer_to_property_opt = CMethod_signature.ms_get_pointer_to_property_opt ms in match Ast_utils.get_decl_opt pointer_to_property_opt with - | Some ObjCPropertyDecl (decl_info, named_decl_info, obj_c_property_decl_info) -> - let field_name_str = Ast_utils.generated_ivar_name named_decl_info in - let field_name = General_utils.mk_class_field_name field_name_str in + | Some (ObjCPropertyDecl (decl_info, named_decl_info, obj_c_property_decl_info) as d) -> + let class_name = Procname.c_get_class (CMethod_signature.ms_get_name ms) in + let field_name = CField_decl.get_property_corresponding_ivar tenv + CTypes_decl.type_ptr_to_sil_type class_name d in if CMethod_signature.ms_is_getter ms then Some (ProcAttributes.Objc_getter field_name) else None (* Setter TODO *) @@ -341,7 +342,7 @@ let create_local_procdesc cfg tenv ms fbody captured is_objc_inst_method = let proc_attributes = { (ProcAttributes.default proc_name Config.C_CPP) with ProcAttributes.captured = captured'; - ProcAttributes.objc_accessor = get_objc_property_accessor ms; + ProcAttributes.objc_accessor = get_objc_property_accessor tenv ms; formals; func_attributes = attributes; is_defined = defined; diff --git a/infer/tests/codetoanalyze/objc/errors/property/ExplicitIvarName.m b/infer/tests/codetoanalyze/objc/errors/property/ExplicitIvarName.m new file mode 100644 index 000000000..95eb1793d --- /dev/null +++ b/infer/tests/codetoanalyze/objc/errors/property/ExplicitIvarName.m @@ -0,0 +1,47 @@ +/* +* 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 + +@property (nonatomic) int x; + +@end + +@interface B : NSObject + +@property (nonatomic) int y; + +@end + +@implementation A + +@synthesize x; + + +-(int) test { + int* p = 0; + self->x = 5; + if (self.x == 5) { // If NPE is found, means that getter is using the correct ivar name x + //rather than the default _x + return *p; + }; +} + +-(int) testDefaultName { + int* p = 0; + B *b = [[B alloc] init]; + b.y = 5; + if (b.y == 5) { // If NPE is found, means that getter is using default name _y that is + // added to the tenv, so there is no Missing_fld beforehand. + return *p; + }; +} +@end diff --git a/infer/tests/endtoend/objc/ExplicitIvarNameInGetterTest.java b/infer/tests/endtoend/objc/ExplicitIvarNameInGetterTest.java new file mode 100644 index 000000000..a9eab8bff --- /dev/null +++ b/infer/tests/endtoend/objc/ExplicitIvarNameInGetterTest.java @@ -0,0 +1,67 @@ +/* +* 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 ExplicitIvarNameInGetterTest { + + public static final String FILE = + "infer/tests/codetoanalyze/objc/errors/property/ExplicitIvarName.m"; + + + private static ImmutableList inferCmd; + + public static final String NULL_DEREFERENCE = "NULL_DEREFERENCE"; + + @ClassRule + public static DebuggableTemporaryFolder folder = new DebuggableTemporaryFolder(); + + @BeforeClass + public static void runInfer() throws InterruptedException, IOException { + inferCmd = InferRunner.createObjCInferCommandWithMLBuckets( + folder, + FILE, + "cf", + true); + } + + @Test + public void whenInferRunsOnAtomicProperty() + throws InterruptedException, IOException, InferException { + InferResults inferResults = InferRunner.runInferObjC(inferCmd); + assertThat( + "Results should contain null dereference", + inferResults, + containsExactly( + NULL_DEREFERENCE, + FILE, + new String[]{ + "test", + "testDefaultName" + } + ) + ); + } +}