From 9a38987929eb92402fc8d7fb894bcb90e9429bdb Mon Sep 17 00:00:00 2001 From: Sungkeun Cho Date: Thu, 27 Feb 2020 07:12:02 -0800 Subject: [PATCH] [cost] Do not report `EXPENSIVE_EXECUTION_TIME` when a cost of a node is top Summary: > We don't report when the cost is Top as it corresponds to subsequent 'don't know's. Instead, we > report Top cost only at the top level per function The previous code just ignored top costed nodes, so it was able to report a non-top cost that was from another node. For example, ``` void foo() { linear-cost(); top-cost(); } ``` It reported inconsistent reports: `EXPENSIVE_EXECUTION_TIME` with a linear cost and `INFINITE_EXECUTION_TIME` at the same time. This diff fixes it not to report `EXPENSIVE_EXECUTION_TIME` when there is a node with the top cost. Reviewed By: ezgicicek Differential Revision: D20139408 fbshipit-source-id: 9fedd4aec --- infer/src/cost/cost.ml | 18 +++++++++--------- .../codetoanalyze/java/performance/issues.exp | 2 -- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/infer/src/cost/cost.ml b/infer/src/cost/cost.ml index 924453b24..341085ae1 100644 --- a/infer/src/cost/cost.ml +++ b/infer/src/cost/cost.ml @@ -140,6 +140,7 @@ module ThresholdReports = struct type threshold_or_report = | Threshold of BasicCost.t | ReportOn of {location: Location.t; cost: BasicCost.t} + | NoReport type t = threshold_or_report CostIssues.CostKindMap.t @@ -163,10 +164,6 @@ module WorstCaseCost = struct (** We don't report when the cost is Top as it corresponds to subsequent 'don't know's. Instead, we report Top cost only at the top level per function. *) - let should_report_cost cost ~threshold = - (not (BasicCost.is_top cost)) && not (BasicCost.leq ~lhs:cost ~rhs:threshold) - - let exec_node tenv {costs; reports} extras instr_node = let {get_node_nb_exec} = extras in let node_cost = @@ -183,13 +180,16 @@ module WorstCaseCost = struct CostIssues.CostKindMap.merge (fun _kind threshold_or_report_opt cost_opt -> match (threshold_or_report_opt, cost_opt) with - | None, _ -> - None + | Some ThresholdReports.NoReport, _ -> + threshold_or_report_opt + | Some ThresholdReports.(Threshold _ | ReportOn _), Some cost when BasicCost.is_top cost + -> + Some ThresholdReports.NoReport | Some (ThresholdReports.Threshold threshold), Some cost - when should_report_cost cost ~threshold -> + when not (BasicCost.leq ~lhs:cost ~rhs:threshold) -> Some (ThresholdReports.ReportOn {location= InstrCFG.Node.loc instr_node; cost}) | Some (ThresholdReports.ReportOn {cost= prev}), Some cost - when (not (BasicCost.is_top cost)) && BasicCost.compare_by_degree prev cost < 0 -> + when BasicCost.compare_by_degree prev cost < 0 -> Some (ThresholdReports.ReportOn {location= InstrCFG.Node.loc instr_node; cost}) | _ -> threshold_or_report_opt ) @@ -267,7 +267,7 @@ module Check = struct if not (Procname.is_java_access_method pname) then ( CostIssues.CostKindMap.iter2 CostIssues.enabled_cost_map reports ~f:(fun _kind (CostIssues.{name; threshold} as kind_spec) -> function - | ThresholdReports.Threshold _ -> + | ThresholdReports.Threshold _ | ThresholdReports.NoReport -> () | ThresholdReports.ReportOn {location; cost} -> report_threshold pname summary ~name ~location ~cost kind_spec diff --git a/infer/tests/codetoanalyze/java/performance/issues.exp b/infer/tests/codetoanalyze/java/performance/issues.exp index 2d9c3ca60..2fe87cbe6 100644 --- a/infer/tests/codetoanalyze/java/performance/issues.exp +++ b/infer/tests/codetoanalyze/java/performance/issues.exp @@ -4,7 +4,6 @@ codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array. codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.binary_search_log(java.lang.String[]):int, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 2 + log(arr.length), O(log(arr.length)), degree = 0 + 1⋅log,{arr.length},Modeled call to Arrays.binarySearch] codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.bsearch_log(int):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 5 + log(i), O(log(i)), degree = 0 + 1⋅log,{i},Modeled call to Arrays.binarySearch] codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.call_gen_and_iter_types(int):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 13 + 5 ⋅ x, O(x), degree = 1,{x},call to java.lang.String[] Array.gen_and_iter_types(int),Loop at line 76] -codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.call_gen_and_iter_types_linear_FP(int,int):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 13 + 5 ⋅ x, O(x), degree = 1,{x},call to java.lang.String[] Array.gen_and_iter_types(int),Loop at line 76] codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.call_gen_and_iter_types_linear_FP(int,int):void, 0, INFINITE_EXECUTION_TIME, no_bucket, ERROR, [Unbounded loop,Loop at line 88] codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.call_gen_and_iter_types_linear_FP(int,int):void, 2, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [,Assignment,Binary operation: ([0, +oo] + 1):signed32] codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.copyOf_linear(java.lang.String[]):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 4 + arr.length, O(arr.length), degree = 1,{arr.length},Modeled call to Arrays.copyOf] @@ -172,7 +171,6 @@ codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops. codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.string_concat_linear(java.lang.String,java.lang.String):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 8 + 5 ⋅ (p.length + s.length) + 3 ⋅ (p.length + s.length + 1), O((p.length + s.length)), degree = 1,{p.length + s.length + 1},Loop at line 103,{p.length + s.length},Loop at line 103] codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.string_length_linear(java.lang.String):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 2 + 5 ⋅ s.length + 3 ⋅ (s.length + 1), O(s.length), degree = 1,{s.length + 1},Loop at line 98,{s.length},Loop at line 98] codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.unboundedSymbol():void, 0, INFINITE_EXECUTION_TIME, no_bucket, ERROR, [Unbounded value x,call to void Loops.linear(int),Loop at line 86] -codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.unboundedSymbol():void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 6996, O(1), degree = 0] codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.unboundedSymbol():void, 2, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [,Assignment,,Assignment,Binary operation: ([-oo, +oo] × [-oo, +oo]):signed32] codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.unboundedSymbol():void, 4, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [Assignment,Call,,Assignment,Binary operation: ([0, +oo] + 1):signed32 by call to `void Loops.linear(int)` ] codetoanalyze/java/performance/MapTest.java, MapTest.entrySet_linear(java.util.Map):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 7 + 8 ⋅ map.length + 3 ⋅ (map.length + 1), O(map.length), degree = 1,{map.length + 1},Loop at line 17,{map.length},Loop at line 17]