diff --git a/infer/src/checkers/loopInvariant.ml b/infer/src/checkers/loopInvariant.ml index 6f20f5cbf..bc2c9cf16 100644 --- a/infer/src/checkers/loopInvariant.ml +++ b/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 = - Procdesc.Node.get_instrs node - |> Instrs.fold - ~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 - -> - InvalidatedVars.add (Var.of_pvar pvar) acc - | _ -> - acc ) - ~init: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 ~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 + -> + 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 ) + 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 diff --git a/infer/tests/codetoanalyze/java/hoisting/HoistIndirect.java b/infer/tests/codetoanalyze/java/hoisting/HoistIndirect.java index 6d30ac727..a669acc68 100644 --- a/infer/tests/codetoanalyze/java/hoisting/HoistIndirect.java +++ b/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 } } } diff --git a/infer/tests/codetoanalyze/java/hoisting/issues.exp b/infer/tests/codetoanalyze/java/hoisting/issues.exp index 08a6106bd..5cb412391 100644 --- a/infer/tests/codetoanalyze/java/hoisting/issues.exp +++ b/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()]