From 84ff14f2e7cc3f495c373deb23062a105e0c45b8 Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Mon, 14 Sep 2020 00:55:22 -0700 Subject: [PATCH] [sqlite] enforce attribute.is_defined iff procdesc is not NULL Summary: The `attr_kind` column has now exactly two possible values: defined and undefined (since D22187238 (https://github.com/facebook/infer/commit/61ae2d1e1bb9a6eb6aec24ee5b0a30ee80702278)). This is also what should be stored in the field `is_defined` in the procedure attributes. Finally, the cfg can be `NULL` or not. This diff makes all three of these agree, in order to allow for the removal of the `attr_kind` column up the stack, since it will be equivalent to `cfg is NOT NULL`. The changes in the tests indicate improved correctness: previously, specialising a modelled callee would result in a dummy entry in the procedures table, because all clang procedures were given a procdesc, so the specialisation would specialise the empty procdesc. Now, the model is not shadowed by the specialised dummy procdesc, and is specialised itself (and that's why it also ends up in the capture DB, which is slightly counter-intuitive). Reviewed By: jvillard Differential Revision: D23536931 fbshipit-source-id: 983216cb6 --- infer/src/IR/Attributes.ml | 7 +++++++ infer/src/IR/Cfg.ml | 2 +- infer/src/biabduction/SymExec.ml | 2 +- infer/tests/codetoanalyze/objc/biabduction/issues.exp | 4 +++- .../objc/biabduction/npe/ObjCMethodCallInCondition.m | 2 +- .../objc/biabduction/npe/skip_method_with_nil_object.m | 4 ++-- 6 files changed, 15 insertions(+), 6 deletions(-) diff --git a/infer/src/IR/Attributes.ml b/infer/src/IR/Attributes.ml index 07b974a36..7261aead3 100644 --- a/infer/src/IR/Attributes.ml +++ b/infer/src/IR/Attributes.ml @@ -67,6 +67,13 @@ let find = let load pname = find (Procname.to_unique_id pname) let store ~proc_desc (attr : ProcAttributes.t) = + if attr.is_defined && Option.is_none proc_desc then + Logging.die InternalError "Was given DEFINED procedure without procdesc: %a@." ProcAttributes.pp + attr ; + if (not attr.is_defined) && Option.is_some proc_desc then + Logging.die InternalError + "Was given UNDEFINED procedure WITH procdesc:@\nAttributes:%a@\nProcdesc:%a@." + ProcAttributes.pp attr (Pp.option Procdesc.pp_signature) proc_desc ; let pname = attr.proc_name in let akind = proc_kind_of_attr attr in let proc_uid = Procname.to_unique_id pname in diff --git a/infer/src/IR/Cfg.ml b/infer/src/IR/Cfg.ml index 248603f14..19a14f92a 100644 --- a/infer/src/IR/Cfg.ml +++ b/infer/src/IR/Cfg.ml @@ -52,7 +52,7 @@ let store source_file cfg = {attributes with loc= loc'; translation_unit= source_file} in Procdesc.set_attributes proc_desc attributes' ; - Attributes.store ~proc_desc:(Some proc_desc) attributes' + Attributes.store ~proc_desc:(Option.some_if attributes.is_defined proc_desc) attributes' in Procname.Hash.iter save_proc cfg diff --git a/infer/src/biabduction/SymExec.ml b/infer/src/biabduction/SymExec.ml index cb8acd014..6f0d22768 100644 --- a/infer/src/biabduction/SymExec.ml +++ b/infer/src/biabduction/SymExec.ml @@ -374,7 +374,7 @@ let reason_to_skip ~callee_desc : string option = if Option.is_some attr_reason then attr_reason else Some "function or method not found" | `ProcName callee_pname -> let pname_reason = reason_from_pname callee_pname in - if Option.is_some pname_reason then pname_reason else Some "function or method not found" + if Option.is_some pname_reason then pname_reason else Some "method has no implementation" (** In case of constant string dereference, return the result immediately *) diff --git a/infer/tests/codetoanalyze/objc/biabduction/issues.exp b/infer/tests/codetoanalyze/objc/biabduction/issues.exp index f92183704..44dac57fc 100644 --- a/infer/tests/codetoanalyze/objc/biabduction/issues.exp +++ b/infer/tests/codetoanalyze/objc/biabduction/issues.exp @@ -79,6 +79,7 @@ codetoanalyze/objc/biabduction/subtyping/KindOfClassExample.m, shouldThrowDivide codetoanalyze/objc/biabduction/subtyping/KindOfClassExample.m, shouldThrowDivideByZero2, 2, DIVIDE_BY_ZERO, no_bucket, ERROR, [start of procedure shouldThrowDivideByZero2(),start of procedure init,return from a call to Base.init,start of procedure returnsZero2(),Taking false branch,return from a call to returnsZero2] codetoanalyze/objc/biabduction/subtyping/KindOfClassExample.m, shouldThrowDivideByZero3, 3, DIVIDE_BY_ZERO, no_bucket, ERROR, [start of procedure shouldThrowDivideByZero3(),start of procedure init,return from a call to Derived.init,Taking true branch] codetoanalyze/objc/biabduction/variadic_methods/premature_nil_termination.m, PrematureNilTermA.nilInArrayWithObjects, 5, PREMATURE_NIL_TERMINATION_ARGUMENT, B1, WARNING, [start of procedure nilInArrayWithObjects] +objc/src/CADisplayLink.m, CADisplayLink.displayLinkWithTarget:selector:, 3, Missing_fld, no_bucket, ERROR, [start of procedure displayLinkWithTarget:selector:] codetoanalyze/objc/biabduction/memory_leaks_benchmark/NSData_models_tests.m, NSData_models_tests.macForIV:, 2, BIABDUCTION_MEMORY_LEAK, no_bucket, ERROR, [start of procedure macForIV:] codetoanalyze/objc/biabduction/memory_leaks_benchmark/NSString_models_tests.m, StringInitA.hexStringValue, 11, BIABDUCTION_MEMORY_LEAK, no_bucket, ERROR, [start of procedure hexStringValue,Skipping CFStringCreateWithBytesNoCopy(): method has no implementation,Taking false branch] codetoanalyze/objc/biabduction/memory_leaks_benchmark/RetainCycleLength3.m, strongcycle, 6, BIABDUCTION_ANALYSIS_STOPS, no_bucket, WARNING, [start of procedure strongcycle(),start of procedure setB:,return from a call to AA.setB:,start of procedure setC:,return from a call to BB.setC:,start of procedure setA:,return from a call to CC.setA:] @@ -97,7 +98,8 @@ codetoanalyze/objc/biabduction/npe/block.m, BlockA.foo7, 2, IVAR_NOT_NULL_CHECKE codetoanalyze/objc/biabduction/npe/dynamic_dispatch.m, DynamicDispatchMain.npe_bad, 2, NULL_DEREFERENCE, B5, ERROR, [start of procedure npe_bad,start of procedure get_ddclass_from:,start of procedure get_ddclass,return from a call to PInstance.get_ddclass,return from a call to DynamicDispatchMain.get_ddclass_from:] codetoanalyze/objc/biabduction/npe/dynamic_dispatch.m, objc_blockDynamicDispatchMain.dispatch_async_block_npe_bad_1, 3, NULL_DEREFERENCE, B5, ERROR, [start of procedure block,start of procedure get_ddclass_from:,start of procedure get_ddclass,return from a call to PInstance.get_ddclass,return from a call to DynamicDispatchMain.get_ddclass_from:] codetoanalyze/objc/biabduction/npe/ivar_blocks.m, MyClass.ivar_npe, 1, IVAR_NOT_NULL_CHECKED, B1, WARNING, [start of procedure ivar_npe] -codetoanalyze/objc/biabduction/npe/skip_method_with_nil_object.m, SkipMethodNilA.testBug:, 6, PARAMETER_NOT_NULL_CHECKED, B2, WARNING, [start of procedure testBug:,Message get_a with receiver nil returns nil.,Message skip_method with receiver nil returns nil.,Taking false branch] +codetoanalyze/objc/biabduction/npe/skip_method_with_nil_object.m, SkipMethodNilA.testBug:, 6, PARAMETER_NOT_NULL_CHECKED, B2, WARNING, [start of procedure testBug:,Message get_a with receiver nil returns nil.,Skipping skip_method: method has no implementation,Taking false branch] +codetoanalyze/objc/biabduction/npe/skip_method_with_nil_object.m, SkipMethodNilA.testOk_FP, 4, NULL_DEREFERENCE, B1, ERROR, [start of procedure testOk_FP,Skipping skip_method: method has no implementation,Taking true branch] codetoanalyze/objc/biabduction/property/main.c, property_main, 3, BIABDUCTION_MEMORY_LEAK, no_bucket, ERROR, [start of procedure property_main(),Skipping aProperty: method has no implementation] codetoanalyze/objc/biabduction/resource_leaks/Dispatch_sources.m, ProcessContentsOfFile, 35, RESOURCE_LEAK, no_bucket, ERROR, [start of procedure ProcessContentsOfFile(),Taking false branch,Skipping dispatch_get_global_queue(): method has no implementation,Skipping dispatch_source_create(): method has no implementation,Taking false branch] codetoanalyze/objc/biabduction/resource_leaks/Dispatch_sources.m, objc_blockProcessContentsOfFile_2, 6, BIABDUCTION_MEMORY_LEAK, no_bucket, ERROR, [start of procedure block,Skipping dispatch_source_get_data(): method has no implementation,Taking true branch,Skipping MyProcessFileData(): method has no implementation] diff --git a/infer/tests/codetoanalyze/objc/biabduction/npe/ObjCMethodCallInCondition.m b/infer/tests/codetoanalyze/objc/biabduction/npe/ObjCMethodCallInCondition.m index bed205ff4..f399ea4a8 100644 --- a/infer/tests/codetoanalyze/objc/biabduction/npe/ObjCMethodCallInCondition.m +++ b/infer/tests/codetoanalyze/objc/biabduction/npe/ObjCMethodCallInCondition.m @@ -21,7 +21,7 @@ - (void)testLength:(NSData*)imageData { unsigned char* pixels = (unsigned char*)[imageData bytes]; - if (imageData.length > 0) { + if (imageData && imageData.length > 0) { pixels[0] = 255; } } diff --git a/infer/tests/codetoanalyze/objc/biabduction/npe/skip_method_with_nil_object.m b/infer/tests/codetoanalyze/objc/biabduction/npe/skip_method_with_nil_object.m index 89526f8a5..5335932ea 100644 --- a/infer/tests/codetoanalyze/objc/biabduction/npe/skip_method_with_nil_object.m +++ b/infer/tests/codetoanalyze/objc/biabduction/npe/skip_method_with_nil_object.m @@ -23,8 +23,8 @@ return [SkipMethodNilA new]; } -- (int)testOk:(SkipMethodNilA*)person { - SkipMethodNilA* personID = [person get_a]; +- (int)testOk_FP { + SkipMethodNilA* personID = nil; NSString* lastRecord = [personID skip_method]; if (lastRecord) { personID->x = 6;