[frontend,pulse] Avoid dereference of C struct

Summary:
This diff avoids dereference of C struct, in its frontend and its semantics of Pulse. In SIL, C
struct is not first-class value, thus dereferencing on it does not make sense.

Reviewed By: ezgicicek

Differential Revision: D27953258

fbshipit-source-id: 348d56338

infer/src/clang/cTrans_utils.ml

@@ -560,11 +560,16 @@ let cast_operation ?objc_bridge_cast_kind cast_kind ((exp, typ) as exp_typ) cast
   | `BitCast | `IntegralCast | `IntegralToBoolean ->
       (* This is treated as a nop by returning the same expressions exps*)
       ([], (exp, cast_typ))
-  | `LValueToRValue ->
+  | `LValueToRValue -> (
+    match typ with
+    | {Typ.desc= Tstruct _} ->
+        (* Avoid dereference of C struct on cast *)
+        ([], exp_typ)
+    | _ ->
         (* Takes an LValue and allow it to use it as RValue. *)
         (* So we assign the LValue to a temp and we pass it to the parent.*)
         let instr, deref_exp = dereference_var_sil (exp, cast_typ) sil_loc in
-      ([instr], (deref_exp, cast_typ))
+        ([instr], (deref_exp, cast_typ)) )
   | `NullToPointer ->
       if Exp.is_zero exp then ([], (Exp.null, cast_typ)) else ([], (exp, cast_typ))
   | `ToVoid ->

infer/src/pulse/PulseCallOperations.ml

@@ -198,9 +198,9 @@ let call_aux tenv caller_proc_desc call_loc callee_pname ret actuals callee_proc
   in
   let captured_vars =
     Procdesc.get_captured callee_proc_desc
-    |> List.map ~f:(fun {CapturedVar.name; capture_mode} ->
+    |> List.map ~f:(fun {CapturedVar.name; capture_mode; typ} ->
            let pvar = Pvar.mk name callee_pname in
-           (Var.of_pvar pvar, capture_mode) )
+           (Var.of_pvar pvar, capture_mode, typ) )
   in
   let<*> astate, captured_vars_with_actuals =
     match actuals with

infer/src/pulse/PulseInterproc.ml

@@ -205,13 +205,22 @@ let rec materialize_pre_from_address callee_proc_name call_location ~pre ~addr_p
               ~addr_hist_caller:addr_hist_dest_caller call_state ) )
 
 
+let deref_non_c_struct addr typ astate =
+  match typ.Typ.desc with
+  | Tstruct _ ->
+      Some addr
+  | _ ->
+      BaseMemory.find_edge_opt addr Dereference astate |> Option.map ~f:fst
+
+
 (** materialize subgraph of [pre] rooted at the address represented by a [formal] parameter that has
     been instantiated with the corresponding [actual] into the current state [call_state.astate] *)
-let materialize_pre_from_actual callee_proc_name call_location ~pre ~formal ~actual call_state =
+let materialize_pre_from_actual callee_proc_name call_location ~pre ~formal ~actual:(actual, typ)
+    call_state =
   L.d_printfln "Materializing PRE from [%a <- %a]" Var.pp formal AbstractValue.pp (fst actual) ;
   (let open IOption.Let_syntax in
   let* addr_formal_pre, _ = BaseStack.find_opt formal pre.BaseDomain.stack in
-  let+ formal_pre, _ = BaseMemory.find_edge_opt addr_formal_pre Dereference pre.BaseDomain.heap in
+  let+ formal_pre = deref_non_c_struct addr_formal_pre typ pre.BaseDomain.heap in
   materialize_pre_from_address callee_proc_name call_location ~pre ~addr_pre:formal_pre
     ~addr_hist_caller:actual call_state)
   |> function Some result -> result | None -> Ok call_state
@@ -236,7 +245,7 @@ let materialize_pre_for_parameters callee_proc_name call_location pre_post ~form
      call [materialize_pre_from] on them.  Give up if calling the function introduces aliasing.
   *)
   match
-    IList.fold2_result formals actuals ~init:call_state ~f:(fun call_state formal (actual, _) ->
+    IList.fold2_result formals actuals ~init:call_state ~f:(fun call_state formal actual ->
         materialize_pre_from_actual callee_proc_name call_location
           ~pre:(pre_post.AbductiveDomain.pre :> BaseDomain.t)
           ~formal ~actual call_state )
@@ -468,15 +477,16 @@ let rec record_post_for_address callee_proc_name call_loc ({AbductiveDomain.pre;
               ~addr_hist_caller:addr_hist_curr_dest call_state ) )
 
 
-let record_post_for_actual callee_proc_name call_loc pre_post ~formal ~actual call_state =
+let record_post_for_actual callee_proc_name call_loc pre_post ~formal ~actual:(actual, typ)
+    call_state =
   L.d_printfln_escaped "Recording POST from [%a] <-> %a" Var.pp formal AbstractValue.pp (fst actual) ;
   match
     let open IOption.Let_syntax in
     let* addr_formal_pre, _ =
       BaseStack.find_opt formal (pre_post.AbductiveDomain.pre :> BaseDomain.t).BaseDomain.stack
     in
-    let+ formal_pre, _ =
-      BaseMemory.find_edge_opt addr_formal_pre Dereference
+    let+ formal_pre =
+      deref_non_c_struct addr_formal_pre typ
         (pre_post.AbductiveDomain.pre :> BaseDomain.t).BaseDomain.heap
     in
     record_post_for_address callee_proc_name call_loc pre_post ~addr_callee:formal_pre
@@ -525,7 +535,7 @@ let apply_post_for_parameters callee_proc_name call_location pre_post ~formals ~
      between pre and post since it's unreliable, eg replace value read in pre with same value in
      post but nuke other fields in the meantime? is that possible?). *)
   match
-    List.fold2 formals actuals ~init:call_state ~f:(fun call_state formal (actual, _) ->
+    List.fold2 formals actuals ~init:call_state ~f:(fun call_state formal actual ->
         record_post_for_actual callee_proc_name call_location pre_post ~formal ~actual call_state )
   with
   | Unequal_lengths ->

infer/src/pulse/PulseInterproc.mli

@@ -14,7 +14,7 @@ val apply_prepost :
   -> Procname.t
   -> Location.t
   -> callee_prepost:AbductiveDomain.t
-  -> captured_vars_with_actuals:(Var.t * (AbstractValue.t * ValueHistory.t)) list
+  -> captured_vars_with_actuals:(Var.t * ((AbstractValue.t * ValueHistory.t) * Typ.t)) list
   -> formals:Var.t list
   -> actuals:((AbstractValue.t * ValueHistory.t) * Typ.t) list
   -> AbductiveDomain.t

infer/src/pulse/PulseOperations.ml

@@ -561,12 +561,12 @@ let get_captured_actuals location ~captured_vars ~actual_closure astate =
   let* astate, this_value_addr = eval_access Read location actual_closure Dereference astate in
   let+ _, astate, captured_vars_with_actuals =
     List.fold_result captured_vars ~init:(0, astate, [])
-      ~f:(fun (id, astate, captured) (var, mode) ->
+      ~f:(fun (id, astate, captured) (var, mode, typ) ->
         let+ astate, captured_actual =
           eval_access Read location this_value_addr
             (FieldAccess (Closures.mk_fake_field ~id mode))
             astate
         in
-        (id + 1, astate, (var, captured_actual) :: captured) )
+        (id + 1, astate, (var, (captured_actual, typ)) :: captured) )
   in
   (astate, captured_vars_with_actuals)

infer/src/pulse/PulseOperations.mli

@@ -204,7 +204,7 @@ val check_address_escape :
 
 val get_captured_actuals :
      Location.t
-  -> captured_vars:(Var.t * Pvar.capture_mode) list
+  -> captured_vars:(Var.t * Pvar.capture_mode * Typ.t) list
   -> actual_closure:AbstractValue.t * ValueHistory.t
   -> t
-  -> (t * (Var.t * (AbstractValue.t * ValueHistory.t)) list) AccessResult.t
+  -> (t * (Var.t * ((AbstractValue.t * ValueHistory.t) * Typ.t)) list) AccessResult.t

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

@@ -70,7 +70,6 @@ codetoanalyze/cpp/pulse/optional.cpp, inside_try_catch_FP, 3, OPTIONAL_EMPTY_ACC
 codetoanalyze/cpp/pulse/optional.cpp, none_copy_bad, 3, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),is optional empty,use-after-lifetime part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),passed as argument to `folly::Optional::Optional(folly::Optional<Value> arg)` (modelled),return from call to `folly::Optional::Optional(folly::Optional<Value> arg)` (modelled),invalid access occurs here]
 codetoanalyze/cpp/pulse/optional.cpp, none_no_check_bad, 2, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),is optional empty,use-after-lifetime part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),invalid access occurs here]
 codetoanalyze/cpp/pulse/optional.cpp, not_none_check_value_ok_FP, 5, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),is optional empty,use-after-lifetime part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),invalid access occurs here]
-codetoanalyze/cpp/pulse/optional.cpp, operator_arrow_bad, 0, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),is optional empty,use-after-lifetime part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),when calling `emplace` here,parameter `state` of emplace,passed as argument to `folly::Optional::operator->()` (modelled),return from call to `folly::Optional::operator->()` (modelled),invalid access occurs here]
 codetoanalyze/cpp/pulse/optional.cpp, std_assign2_bad, 4, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `std::optional::operator=(None)` (modelled),return from call to `std::optional::operator=(None)` (modelled),is optional empty,use-after-lifetime part of the trace starts here,passed as argument to `std::optional::operator=(None)` (modelled),return from call to `std::optional::operator=(None)` (modelled),invalid access occurs here]
 codetoanalyze/cpp/pulse/optional.cpp, std_assign_bad, 5, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `std::optional::optional(=nullopt)` (modelled),return from call to `std::optional::optional(=nullopt)` (modelled),is optional empty,use-after-lifetime part of the trace starts here,passed as argument to `std::optional::optional(=nullopt)` (modelled),return from call to `std::optional::optional(=nullopt)` (modelled),passed as argument to `std::optional::operator=(std::optional<Value> arg)` (modelled),return from call to `std::optional::operator=(std::optional<Value> arg)` (modelled),invalid access occurs here]
 codetoanalyze/cpp/pulse/optional.cpp, std_none_copy_bad, 3, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `std::optional::optional(=nullopt)` (modelled),return from call to `std::optional::optional(=nullopt)` (modelled),is optional empty,use-after-lifetime part of the trace starts here,passed as argument to `std::optional::optional(=nullopt)` (modelled),return from call to `std::optional::optional(=nullopt)` (modelled),passed as argument to `std::optional::optional(std::optional<Value> arg)` (modelled),return from call to `std::optional::optional(std::optional<Value> arg)` (modelled),invalid access occurs here]

infer/tests/codetoanalyze/cpp/pulse/optional.cpp

@@ -148,7 +148,8 @@ void emplace(folly::Optional<State> state) {
   auto pos = state->vec.begin();
 }
 
-void operator_arrow_bad() { emplace(folly::none); }
+/* There is a bug in the frontend T89443328 */
+void operator_arrow_bad_FN() { emplace(folly::none); }
 
 void get_pointer_check_none_check_ok() {
   folly::Optional<int> foo{folly::none};

infer/tests/codetoanalyze/objc/frontend/property/Property_getter.m.dot

@@ -15,7 +15,7 @@ digraph cfg {
 	
 
 	 "getMyStructField:my_struct:#A(class A)#class.35da25720e80922866aa41dc70b313cb_4" -> "getMyStructField:my_struct:#A(class A)#class.35da25720e80922866aa41dc70b313cb_2" ;
-"getMyStructField:my_struct:#A(class A)#class.35da25720e80922866aa41dc70b313cb_5" [label="5:  Message Call: setS: \n   n$8=*&target:A* [line 27, column 3]\n  n$7=*&my_struct:my_struct [line 27, column 14]\n  n$9=_fun_A.setS:(n$8:A*,n$7:my_struct) [line 27, column 10]\n " shape="box"]
+"getMyStructField:my_struct:#A(class A)#class.35da25720e80922866aa41dc70b313cb_5" [label="5:  Message Call: setS: \n   n$7=*&target:A* [line 27, column 3]\n  n$8=_fun_A.setS:(n$7:A*,&my_struct:my_struct) [line 27, column 10]\n " shape="box"]
 	
 
 	 "getMyStructField:my_struct:#A(class A)#class.35da25720e80922866aa41dc70b313cb_5" -> "getMyStructField:my_struct:#A(class A)#class.35da25720e80922866aa41dc70b313cb_3" ;

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

@@ -9,5 +9,6 @@ codetoanalyze/objc/pulse/MemoryLeaks.m, call_cfrelease_interproc_leak_ok_FP, 2,
 codetoanalyze/objc/pulse/MemoryLeaksInBlocks.m, block_captured_var_leak_bad, 6, MEMORY_LEAK, no_bucket, ERROR, [allocation part of the trace starts here,allocated by call to `malloc_no_fail` (modelled),allocation part of the trace ends here,memory becomes unreachable here]
 codetoanalyze/objc/pulse/NPEBlocks.m, Singleton.dispatch_once_no_npe_good_FP, 6, NULLPTR_DEREFERENCE, no_bucket, ERROR, [source of the null value part of the trace starts here,assigned,is the null pointer,null pointer dereference part of the trace starts here,assigned,invalid access occurs here]
 codetoanalyze/objc/pulse/NPEBlocks.m, captured_npe_bad, 5, NULLPTR_DEREFERENCE, no_bucket, ERROR, [source of the null value part of the trace starts here,assigned,is the null pointer,null pointer dereference part of the trace starts here,assigned,when calling `objc_blockcaptured_npe_bad_3` here,parameter `x` of objc_blockcaptured_npe_bad_3,invalid access occurs here]
+codetoanalyze/objc/pulse/uninit.m, Uninit.call_setter_c_struct_bad, 3, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [struct field address `x` created,when calling `Uninit.setS:` here,parameter `s` of Uninit.setS:,read to uninitialized value occurs here]
 codetoanalyze/objc/pulse/use_after_free.m, PulseTest.use_after_free_simple_in_objc_method_bad:, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidation part of the trace starts here,parameter `x` of PulseTest.use_after_free_simple_in_objc_method_bad:,was invalidated by call to `free()`,use-after-lifetime part of the trace starts here,parameter `x` of PulseTest.use_after_free_simple_in_objc_method_bad:,invalid access occurs here]
 codetoanalyze/objc/pulse/use_after_free.m, use_after_free_simple_bad, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidation part of the trace starts here,parameter `x` of use_after_free_simple_bad,was invalidated by call to `free()`,use-after-lifetime part of the trace starts here,parameter `x` of use_after_free_simple_bad,invalid access occurs here]

infer/tests/codetoanalyze/objc/pulse/uninit.m

@@ -130,10 +130,9 @@ dispatch_queue_t queue;
   return [Uninit getter_c_struct:obj];
 }
 
-/* FN due to the frontend is incorrect when passing C struct value as a function
-   paraemeter when calling C struct setter. */
-+ (void)call_setter_c_struct_bad_FN:(Uninit*)obj {
++ (void)call_setter_c_struct_bad {
   struct my_struct s;
+  Uninit* obj = [Uninit new];
   obj.s = s; // setter is called
 }