[hoisting] Invalidate arguments of type structs

Reviewed By: mbouaziz

Differential Revision: D12924850

fbshipit-source-id: b442d37be

infer/src/IR/Typ.ml

@@ -539,6 +539,8 @@ let is_pointer typ = match typ.desc with Tptr _ -> true | _ -> false
 
 let is_reference typ = match typ.desc with Tptr (_, Pk_reference) -> true | _ -> false
 
+let is_struct typ = match typ.desc with Tstruct _ -> true | _ -> false
+
 let is_pointer_to_cpp_class typ = match typ.desc with Tptr (t, _) -> is_cpp_class t | _ -> false
 
 let is_pointer_to_void typ = match typ.desc with Tptr ({desc= Tvoid}, _) -> true | _ -> false

infer/src/IR/Typ.mli

@@ -288,6 +288,8 @@ val is_pointer : t -> bool
 
 val is_reference : t -> bool
 
+val is_struct : t -> bool
+
 val has_block_prefix : string -> bool
 
 val unsome : string -> t option -> t

infer/src/checkers/loopInvariant.ml

@@ -37,6 +37,8 @@ let is_fun_pure tenv ~is_inv_by_default callee_pname params =
         is_inv_by_default )
 
 
+let is_non_primitive typ = Typ.is_pointer typ || Typ.is_struct typ
+
 (* check if the def of var is unique and invariant *)
 let is_def_unique_and_satisfy tenv var (loop_nodes : LoopNodes.t) ~is_inv_by_default
     is_exp_invariant =
@@ -114,11 +116,11 @@ let get_ptr_vars_in_defn_path node var =
     Procdesc.Node.get_instrs node
     |> Instrs.fold ~init:InvalidatedVars.empty ~f:(fun acc instr ->
            match instr with
-           | Sil.Load (id, exp_rhs, typ, _) when Var.equal var (Var.of_id id) && Typ.is_pointer typ
-             ->
+           | Sil.Load (id, exp_rhs, typ, _)
+             when Var.equal var (Var.of_id id) && is_non_primitive typ ->
                invalidate_exp exp_rhs acc
            | Sil.Store (Exp.Lvar pvar, typ, exp_rhs, _)
-             when Var.equal var (Var.of_pvar pvar) && Typ.is_pointer typ ->
+             when Var.equal var (Var.of_pvar pvar) && is_non_primitive typ ->
                invalidate_exp exp_rhs acc
            | _ ->
                acc )
@@ -131,7 +133,7 @@ let get_vars_to_invalidate node params invalidated_vars : InvalidatedVars.t =
     ~f:(fun acc (arg_exp, typ) ->
       Var.get_all_vars_in_exp arg_exp
       |> Sequence.fold ~init:acc ~f:(fun acc var ->
-             if Typ.is_pointer typ then
+             if is_non_primitive typ then
                let dep_vars = get_ptr_vars_in_defn_path node var in
                InvalidatedVars.union dep_vars (InvalidatedVars.add var acc)
              else acc ) )

infer/tests/codetoanalyze/java/hoisting/HoistIndirect.java

@@ -14,6 +14,8 @@ class HoistIndirect {
 
     int a = 0;
 
+    int[] test_array;
+
     int foo(int x) {
       return x + 10;
     }
@@ -149,6 +151,15 @@ class HoistIndirect {
     return d;
   }
 
+  int modified_inner_array_dont_hoist(int size, Test t) {
+    int d = 0;
+    for (int i = 0; i < size; i++) {
+      set_ith(i, t.test_array);
+      d += t.foo(t.test_array[0]); // don't hoist since t.test_array changes
+    }
+    return d;
+  }
+
   static int regionFirst(int[] region) {
     return region[0];
   }

infer/tests/codetoanalyze/java/hoisting/issues.exp

@@ -25,15 +25,15 @@ codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect$Test.foo(int):int,
 codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect$Test.get_sum_test(HoistIndirect$Test,int):int, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function int HoistIndirect$Test.get_sum_test(HoistIndirect$Test,int)]
 codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect$Test.get_test(HoistIndirect$Test):int, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function int HoistIndirect$Test.get_test(HoistIndirect$Test)]
 codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect$Test.return_only(HoistIndirect$Test):HoistIndirect$Test, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function HoistIndirect$Test HoistIndirect$Test.return_only(HoistIndirect$Test)]
-codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.arg_modification_hoist(int,HoistIndirect$Test):int, 3, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.get() at line 119]
-codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.direct_this_modification_dont_hoist_FP(int):int, 4, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.get() at line 101]
+codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.arg_modification_hoist(int,HoistIndirect$Test):int, 3, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.get() at line 121]
+codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.direct_this_modification_dont_hoist_FP(int):int, 4, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.get() at line 103]
 codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.double_me(int):int, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function int HoistIndirect.double_me(int)]
 codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.get():int, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function int HoistIndirect.get()]
 codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.get_ith(int,int[]):int, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function int HoistIndirect.get_ith(int,int[])]
-codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.independent_hoist(int,HoistIndirect$Test):int, 4, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect$Test.foo(int) at line 146]
-codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.irvar_independent_hoist(int[][],int,int[]):void, 2, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.double_me(int) at line 187]
+codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.independent_hoist(int,HoistIndirect$Test):int, 4, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect$Test.foo(int) at line 148]
+codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.irvar_independent_hoist(int[][],int,int[]):void, 2, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.double_me(int) at line 198]
 codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.regionFirst(int[]):int, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function int HoistIndirect.regionFirst(int[])]
-codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.this_modification_outside_hoist(int):int, 4, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.get() at line 111]
+codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect.this_modification_outside_hoist(int):int, 4, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.get() at line 113]
 codetoanalyze/java/hoisting/HoistNoIndirectMod.java, HoistNoIndirectMod.calcNext():int, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function int HoistNoIndirectMod.calcNext()]
 codetoanalyze/java/hoisting/HoistNoIndirectMod.java, HoistNoIndirectMod.calcSame():int, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function int HoistNoIndirectMod.calcSame()]
 codetoanalyze/java/hoisting/HoistNoIndirectMod.java, HoistNoIndirectMod.increment_dont_hoist_FP(int):int, 2, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistNoIndirectMod.calcNext() at line 27]