[hoisting] Invalidate all dependencies of invalidated parameters

Reviewed By: mbouaziz

Differential Revision: D10356056

fbshipit-source-id: a5d3b4bbe

infer/src/checkers/loopInvariant.ml

@@ -100,25 +100,41 @@ let get_vars_in_loop loop_nodes =
     loop_nodes VarsInLoop.empty
 
 
-let get_loaded_object var node invalidated_vars =
+(* get all the ptr variables (and their dependencies) occurring on the
+   RHS of the definition of a given variable. *)
+let get_ptr_vars_in_defn_path node var =
+  let rec aux node var =
+    let invalidate_exp exp_rhs init =
+      Var.get_all_vars_in_exp exp_rhs
+      |> Sequence.fold ~init ~f:(fun acc var ->
+             List.fold_left ~init:(InvalidatedVars.add var acc)
+               ~f:(fun acc node -> InvalidatedVars.union (aux node var) acc)
+               (node :: Procdesc.Node.get_preds node) )
+    in
     Procdesc.Node.get_instrs node
-  |> Instrs.fold
-       ~f:(fun acc instr ->
+    |> Instrs.fold ~init:InvalidatedVars.empty ~f:(fun acc instr ->
            match instr with
-         | Sil.Load (id, Lvar pvar, 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) && Typ.is_pointer typ
              ->
-             InvalidatedVars.add (Var.of_pvar pvar) acc
+               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 ->
+               invalidate_exp exp_rhs acc
            | _ ->
                acc )
-       ~init:invalidated_vars
+  in
+  aux node var
 
 
 let get_vars_to_invalidate node params invalidated_vars : InvalidatedVars.t =
   List.fold ~init:invalidated_vars
-    ~f:(fun acc (arg_exp, _) ->
+    ~f:(fun acc (arg_exp, typ) ->
       Var.get_all_vars_in_exp arg_exp
       |> Sequence.fold ~init:acc ~f:(fun acc var ->
-             get_loaded_object var node (InvalidatedVars.add var acc) ) )
+             if Typ.is_pointer typ then
+               let dep_vars = get_ptr_vars_in_defn_path node var in
+               InvalidatedVars.union dep_vars (InvalidatedVars.add var acc)
+             else acc ) )
     params
 
 

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

@@ -139,6 +139,16 @@ class HoistIndirect {
     return d;
   }
 
+  int independent_hoist(int size, Test t) {
+    int d = 0;
+    for (int i = 0; i < size; i++) {
+      set_ith(i, array);
+      t.foo(size); // hoist call to foo
+      d += get_ith(size, array); // don't hoist since array changes
+    }
+    return d;
+  }
+
   static int regionFirst(int[] region) {
     return region[0];
   }
@@ -147,10 +157,35 @@ class HoistIndirect {
     dest[0] = source[0] + 1;
   }
 
-  void nested_change_dont_hoist_FP(int[][] nextRegionM, int p, int[] tempRegion) {
+  static void sumDest(int[] source, int[] dest, int x) {
+    dest[0] = source[0] + x;
+  }
+
+  void irvar_change_dont_hoist(int[][] nextRegionM, int p, int[] tempRegion) {
     for (int i = 0; i < 10; i++) {
       if (i < regionFirst(nextRegionM[p])) {
-        incrDest(tempRegion, nextRegionM[p]);
+        incrDest(tempRegion, nextRegionM[p]); // invalidate nextRegionM
+      }
+    }
+  }
+
+  void tmp_irvar_change_dont_hoist(int[][] nextRegionM, int p, int[] tempRegion) {
+    for (int i = 0; i < 10; i++) {
+      if (i < regionFirst(nextRegionM[p])) {
+        int[] arr = nextRegionM[p];
+        incrDest(tempRegion, arr); // invalidate nextRegionM
+      }
+    }
+  }
+
+  int double_me(int p) {
+    return 2 * p;
+  }
+
+  void irvar_independent_hoist(int[][] nextRegionM, int p, int[] tempRegion) {
+    for (int i = 0; i < 10; i++) {
+      if (i < regionFirst(nextRegionM[p]) + double_me(p)) { // double_me(p) can be hoisted
+        sumDest(tempRegion, nextRegionM[p], i); // invalidate nextRegionM
       }
     }
   }

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

@@ -27,9 +27,11 @@ codetoanalyze/java/hoisting/HoistIndirect.java, HoistIndirect$Test.get_test(Hois
 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.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.nested_change_dont_hoist_FP(int[][],int,int[]):void, 2, INVARIANT_CALL, no_bucket, ERROR, [Loop-invariant call to int HoistIndirect.regionFirst(int[]) at line 152]
+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.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/HoistNoIndirectMod.java, HoistNoIndirectMod.calcNext():int, 0, PURE_FUNCTION, no_bucket, ERROR, [Side-effect free function int HoistNoIndirectMod.calcNext()]