[pulse][objcpp][nullptr] Link self with return to preserve the trace

Summary: We were missing a part of the trace if it was going through a nil summary as the invalidation was set in the nil summary. Instead of creating a fresh value for return in the nil summary {self=0}{return=0}, we return self {self=0}{return=self}. This way we keep all the information about invalidation in the trace.

Reviewed By: jvillard

Differential Revision: D26871098

fbshipit-source-id: 6eb175e68

infer/src/pulse/PulseObjectiveCSummary.ml

@@ -6,7 +6,6 @@
  *)
 
 open! IStd
-open PulseBasicInterface
 open PulseOperations.Import
 
 let mk_objc_self_pvar proc_desc =
@@ -15,18 +14,16 @@ let mk_objc_self_pvar proc_desc =
 
 
 let mk_objc_method_nil_summary_aux proc_desc astate =
+  (* Constructs summary {self = 0} {return = self}.
+     This allows us to connect invalidation with invalid access in the trace *)
   let location = Procdesc.get_loc proc_desc in
   let self = mk_objc_self_pvar proc_desc in
   let* astate, self_value = PulseOperations.eval_deref location (Lvar self) astate in
   let astate = PulseArithmetic.prune_eq_zero (fst self_value) astate in
   let ret_var = Procdesc.get_ret_var proc_desc in
-  let ret_addr = AbstractValue.mk_fresh () in
-  let ret_value = (ret_addr, []) in
   let* astate, ret_var_addr_hist = PulseOperations.eval Write location (Lvar ret_var) astate in
-  let* astate = PulseOperations.write_deref location ~ref:ret_var_addr_hist ~obj:ret_value astate in
-  let astate = PulseArithmetic.and_eq_int ret_addr IntLit.zero astate in
   let+ astate =
-    PulseOperations.invalidate location (ConstantDereference IntLit.zero) ret_value astate
+    PulseOperations.write_deref location ~ref:ret_var_addr_hist ~obj:self_value astate
   in
   [astate]
 

infer/tests/codetoanalyze/objcpp/pulse/NPEBasic.mm

@@ -19,6 +19,8 @@
 
 - (int)add:(int)param1 andParam2:(int)param2;
 
+- (int*)getXPtr;
+
 @end
 
 @implementation SomeObject
@@ -35,6 +37,10 @@
   return _x + param1 + param2;
 }
 
+- (int*)getXPtr {
+  return &_x;
+}
+
 @end
 
 int dereferenceNilBad() {
@@ -97,3 +103,9 @@ int testParamsRemainTheSameOk() {
     return *int_ptr;
   }
 }
+
+int testTraceBad() {
+  SomeObject* obj = nil;
+  int* ptr = [obj getXPtr];
+  return *ptr;
+}

infer/tests/codetoanalyze/objcpp/pulse/issues.exp

@@ -1,4 +1,5 @@
 codetoanalyze/objcpp/pulse/AllocPatternMemLeak.mm, A.create_no_release_leak_bad, 1, MEMORY_LEAK, no_bucket, ERROR, [allocation part of the trace starts here,allocated by call to `ABFDataCreate` (modelled),allocation part of the trace ends here,memory becomes unreachable here]
 codetoanalyze/objcpp/pulse/NPEBasic.mm, dereferenceNilBad, 2, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,invalid access occurs here]
+codetoanalyze/objcpp/pulse/NPEBasic.mm, testTraceBad, 3, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,passed as argument to `SomeObject.getXPtr`,return from call to `SomeObject.getXPtr`,assigned,invalid access occurs here]
 codetoanalyze/objcpp/pulse/use_after_delete.mm, PulseTest.deref_deleted_in_objc_method_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `new` (modelled),return from call to `new` (modelled),assigned,was invalidated by `delete`,use-after-lifetime part of the trace starts here,passed as argument to `new` (modelled),return from call to `new` (modelled),assigned,when calling `Simple::Simple` here,parameter `__param_0` of Simple::Simple,invalid access occurs here]
 codetoanalyze/objcpp/pulse/use_after_delete.mm, deref_deleted_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `new` (modelled),return from call to `new` (modelled),assigned,was invalidated by `delete`,use-after-lifetime part of the trace starts here,passed as argument to `new` (modelled),return from call to `new` (modelled),assigned,when calling `Simple::Simple` here,parameter `__param_0` of Simple::Simple,invalid access occurs here]