From 8182514f35280afe296a836a424e14f61d726cfe Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Tue, 15 Oct 2019 03:37:08 -0700 Subject: [PATCH] [impurity] clarify string parameter of `ImpurityDomain.add_to_errlog` Summary: Instead of a string argument named `~str` pass `Formal | Global` and let `add_to_errlog` figure out how to print it. Reviewed By: ezgicicek Differential Revision: D17907657 fbshipit-source-id: ed09aab72 --- infer/src/checkers/impurity.ml | 9 +++++---- infer/src/checkers/impurityDomain.ml | 20 ++++++++++++++----- infer/src/checkers/impurityDomain.mli | 4 +++- .../codetoanalyze/cpp/impurity/issues.exp | 12 +++++------ 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/infer/src/checkers/impurity.ml b/infer/src/checkers/impurity.ml index 49f8f148a..a93373094 100644 --- a/infer/src/checkers/impurity.ml +++ b/infer/src/checkers/impurity.ml @@ -125,13 +125,14 @@ let report_errors summary modified_opt = impure_fun_desc | Some (ImpurityDomain.{modified_globals; modified_params} as astate) -> if Purity.should_report pdesc && not (ImpurityDomain.is_pure astate) then - let modified_ltr ~str set acc = - ImpurityDomain.ModifiedVarSet.fold (ImpurityDomain.add_to_errlog ~nesting:1 ~str) set acc + let modified_ltr param_source set acc = + ImpurityDomain.ModifiedVarSet.fold + (ImpurityDomain.add_to_errlog ~nesting:1 param_source) + set acc in let ltr = impure_fun_ltr - :: modified_ltr ~str:"parameter" modified_params - (modified_ltr ~str:"global var" modified_globals []) + :: modified_ltr Formal modified_params (modified_ltr Global modified_globals []) in Reporting.log_error summary ~loc:pname_loc ~ltr IssueType.impure_function impure_fun_desc ) ; diff --git a/infer/src/checkers/impurityDomain.ml b/infer/src/checkers/impurityDomain.ml index e8fd4437c..a550a8b57 100644 --- a/infer/src/checkers/impurityDomain.ml +++ b/infer/src/checkers/impurityDomain.ml @@ -43,13 +43,23 @@ let join astate1 astate2 = astate1 astate2 -let add_to_errlog ~nesting ~str ModifiedVar.{var; trace_list} errlog = +type param_source = Formal | Global + +let pp_param_source fmt = function + | Formal -> + F.pp_print_string fmt "parameter" + | Global -> + F.pp_print_string fmt "global variable" + + +let add_to_errlog ~nesting param_source ModifiedVar.{var; trace_list} errlog = let rec aux ~nesting rev_errlog action = match action with | WrittenTo (PulseDomain.InterprocAction.Immediate {location}) -> let rev_errlog = Errlog.make_trace_element nesting location - (F.asprintf "%s '%a' is modified at %a" str Var.pp var Location.pp location) + (F.asprintf "%a '%a' is modified at %a" pp_param_source param_source Var.pp var + Location.pp location) [] :: rev_errlog in @@ -57,14 +67,14 @@ let add_to_errlog ~nesting ~str ModifiedVar.{var; trace_list} errlog = | WrittenTo (PulseDomain.InterprocAction.ViaCall {action; f; location}) -> aux ~nesting:(nesting + 1) ( Errlog.make_trace_element nesting location - (F.asprintf "%s '%a' is modified when calling %a at %a" str Var.pp var - PulseDomain.CallEvent.describe f Location.pp location) + (F.asprintf "%a '%a' is modified when calling %a at %a" pp_param_source param_source + Var.pp var PulseDomain.CallEvent.describe f Location.pp location) [] :: rev_errlog ) (WrittenTo action) | Invalid trace -> PulseDomain.Trace.add_to_errlog - ~header:(F.asprintf "%s '%a'" str Var.pp var) + ~header:(F.asprintf "%a '%a'" pp_param_source param_source Var.pp var) (fun f invalidation -> F.fprintf f "%a here" PulseDomain.Invalidation.describe invalidation ) trace rev_errlog diff --git a/infer/src/checkers/impurityDomain.mli b/infer/src/checkers/impurityDomain.mli index fc1c34784..9babc56b0 100644 --- a/infer/src/checkers/impurityDomain.mli +++ b/infer/src/checkers/impurityDomain.mli @@ -27,9 +27,11 @@ val pure : t val is_pure : t -> bool +type param_source = Formal | Global + val add_to_errlog : nesting:int - -> str:string + -> param_source -> ModifiedVar.t -> Errlog.loc_trace_elem list -> Errlog.loc_trace_elem list diff --git a/infer/tests/codetoanalyze/cpp/impurity/issues.exp b/infer/tests/codetoanalyze/cpp/impurity/issues.exp index c1bcbd3e4..ad4243d46 100644 --- a/infer/tests/codetoanalyze/cpp/impurity/issues.exp +++ b/infer/tests/codetoanalyze/cpp/impurity/issues.exp @@ -6,15 +6,15 @@ codetoanalyze/cpp/impurity/array_test.cpp, array_mod_impure, 0, IMPURE_FUNCTION, codetoanalyze/cpp/impurity/array_test.cpp, call_array_mod_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function call_array_mod_impure,parameter 'a' is modified when calling `array_mod_impure()` at line 22, column 3,parameter 'a' is modified at line 9, column 3] codetoanalyze/cpp/impurity/array_test.cpp, modify_direct_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function modify_direct_impure,parameter 'array' is modified at line 40, column 60] codetoanalyze/cpp/impurity/array_test.cpp, modify_ptr_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function modify_ptr_impure,parameter 'array' is modified at line 44, column 3] -codetoanalyze/cpp/impurity/global_test.cpp, call_modify_global, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function call_modify_global,global var 'a' is modified when calling `modify_global_array_impure()` at line 16, column 3,global var 'a' is modified at line 12, column 37,global var 'x' is modified when calling `modify_global_primitive_impure()` at line 15, column 3,global var 'x' is modified at line 10, column 41] -codetoanalyze/cpp/impurity/global_test.cpp, local_static_var_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function local_static_var_impure,global var 'local_static_var_impure.arr' is modified at line 26, column 3,global var 'local_static_var_impure.arr' is modified at line 27, column 3] -codetoanalyze/cpp/impurity/global_test.cpp, modify_global_array_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function modify_global_array_impure,global var 'a' is modified at line 12, column 37] -codetoanalyze/cpp/impurity/global_test.cpp, modify_global_inside_lamda_impure::lambda_global_test.cpp:33:14::operator(), 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function modify_global_inside_lamda_impure::lambda_global_test.cpp:33:14::operator(),global var 'x' is modified at line 33, column 22] -codetoanalyze/cpp/impurity/global_test.cpp, modify_global_primitive_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function modify_global_primitive_impure,global var 'x' is modified at line 10, column 41] +codetoanalyze/cpp/impurity/global_test.cpp, call_modify_global, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function call_modify_global,global variable 'a' is modified when calling `modify_global_array_impure()` at line 16, column 3,global variable 'a' is modified at line 12, column 37,global variable 'x' is modified when calling `modify_global_primitive_impure()` at line 15, column 3,global variable 'x' is modified at line 10, column 41] +codetoanalyze/cpp/impurity/global_test.cpp, local_static_var_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function local_static_var_impure,global variable 'local_static_var_impure.arr' is modified at line 26, column 3,global variable 'local_static_var_impure.arr' is modified at line 27, column 3] +codetoanalyze/cpp/impurity/global_test.cpp, modify_global_array_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function modify_global_array_impure,global variable 'a' is modified at line 12, column 37] +codetoanalyze/cpp/impurity/global_test.cpp, modify_global_inside_lamda_impure::lambda_global_test.cpp:33:14::operator(), 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function modify_global_inside_lamda_impure::lambda_global_test.cpp:33:14::operator(),global variable 'x' is modified at line 33, column 22] +codetoanalyze/cpp/impurity/global_test.cpp, modify_global_primitive_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function modify_global_primitive_impure,global variable 'x' is modified at line 10, column 41] codetoanalyze/cpp/impurity/invalid_test.cpp, delete_param_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function delete_param_impure,parameter 's',was invalidated by `delete` here] codetoanalyze/cpp/impurity/invalid_test.cpp, double_free_global_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function double_free_global_impure] codetoanalyze/cpp/impurity/invalid_test.cpp, double_free_global_impure, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidation part of the trace starts here,when calling `free_global_pointer_impure` here,memory was invalidated by call to `free()` here,use-after-lifetime part of the trace starts here,when calling `free_global_pointer_impure` here,invalid access occurs here] -codetoanalyze/cpp/impurity/invalid_test.cpp, free_global_pointer_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function free_global_pointer_impure,global var 'global_pointer',was invalidated by call to `free()` here] +codetoanalyze/cpp/impurity/invalid_test.cpp, free_global_pointer_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function free_global_pointer_impure,global variable 'global_pointer',was invalidated by call to `free()` here] codetoanalyze/cpp/impurity/invalid_test.cpp, free_param_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function free_param_impure,parameter 'x',was invalidated by call to `free()` here] codetoanalyze/cpp/impurity/invalid_test.cpp, reassign_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function reassign_impure,parameter 's' is modified at line 34, column 3] codetoanalyze/cpp/impurity/param_test.cpp, create_cycle_impure, 0, IMPURE_FUNCTION, no_bucket, ERROR, [Impure function create_cycle_impure,parameter 'x' is modified at line 23, column 44]