From 52746fd9ebebb1eeaa7993c2aa79283e455f90e6 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Thu, 10 Aug 2017 07:53:53 -0700 Subject: [PATCH] [checkers] add ---only options Summary: This makes it easier to test a single checker. Also refactor the code to make it harder to mess up the list of default/all checkers. Reviewed By: sblackshear Differential Revision: D5583209 fbshipit-source-id: 7c919b2 --- infer/src/base/Config.ml | 99 ++++++++----------- infer/src/base/Config.mli | 2 - infer/src/labs/lab.md | 4 +- .../tests/codetoanalyze/cpp/liveness/Makefile | 2 +- 4 files changed, 44 insertions(+), 63 deletions(-) diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index acfb5500f..08d5cb2a3 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -597,7 +597,6 @@ and ( annotation_reachability , biabduction , bufferoverrun , crashcontext - , default_checkers , eradicate , fragment_retains_view , immutable_cast @@ -608,85 +607,71 @@ and ( annotation_reachability , siof , threadsafety , suggest_nullable ) = + let all_checkers = ref [] in + let default_checkers = ref [] in + let mk_checker ?(default= false) ~long doc = + let var = CLOpt.mk_bool ~long ~in_help:CLOpt.([(Analyze, manual_generic)]) ~default doc in + all_checkers := (var, long) :: !all_checkers ; + if default then default_checkers := (var, long) :: !default_checkers ; + var + in let annotation_reachability = - CLOpt.mk_bool ~long:"annotation-reachability" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - ~default:true + mk_checker ~default:true ~long:"annotation-reachability" "the annotation reachability checker. Given a pair of source and sink annotation, e.g. @PerformanceCritical and @Expensive, this checker will warn whenever some method annotated with @PerformanceCritical calls, directly or indirectly, another method annotated with @Expensive" and biabduction = - CLOpt.mk_bool ~long:"biabduction" - ~in_help:CLOpt.([(Analyze, manual_generic)]) + mk_checker ~long:"biabduction" "the separation logic based bi-abduction analysis using the checkers framework" - and bufferoverrun = - CLOpt.mk_bool ~long:"bufferoverrun" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - "the buffer overrun analysis" + and bufferoverrun = mk_checker ~long:"bufferoverrun" "the buffer overrun analysis" and crashcontext = - CLOpt.mk_bool ~long:"crashcontext" - ~in_help:CLOpt.([(Analyze, manual_generic)]) + mk_checker ~long:"crashcontext" "the crashcontext checker for Java stack trace context reconstruction" and eradicate = - CLOpt.mk_bool ~long:"eradicate" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - "the eradicate @Nullable checker for Java annotations" + mk_checker ~long:"eradicate" "the eradicate @Nullable checker for Java annotations" and fragment_retains_view = - CLOpt.mk_bool ~long:"fragment-retains-view" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - ~default:true + mk_checker ~long:"fragment-retains-view" ~default:true "detects when Android fragments are not explicitly nullified before becoming unreabable" and immutable_cast = - CLOpt.mk_bool ~long:"immutable-cast" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - ~default:true + mk_checker ~long:"immutable-cast" ~default:true "the detection of object cast from immutable type to mutable type. For instance, it will detect cast from ImmutableList to List, ImmutableMap to Map, and ImmutableSet to Set." and liveness = - CLOpt.mk_bool ~long:"liveness" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - ~default:true "the detection of dead stores and unused variables" + mk_checker ~long:"liveness" ~default:true "the detection of dead stores and unused variables" and printf_args = - CLOpt.mk_bool ~long:"printf-args" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - ~default:true + mk_checker ~long:"printf-args" ~default:true "the detection of mismatch between the Java printf format strings and the argument types For, example, this checker will warn about the type error in `printf(\"Hello %d\", \"world\")`" - and repeated_calls = - CLOpt.mk_bool ~long:"repeated-calls" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - "check for repeated calls" - and quandary = - CLOpt.mk_bool ~long:"quandary" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - ~default:true "the quandary taint analysis" + and repeated_calls = mk_checker ~long:"repeated-calls" "check for repeated calls" + and quandary = mk_checker ~long:"quandary" ~default:true "the quandary taint analysis" and siof = - CLOpt.mk_bool ~long:"siof" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - ~default:true "the Static Initialization Order Fiasco analysis (C++ only)" - and threadsafety = - CLOpt.mk_bool ~long:"threadsafety" - ~in_help:CLOpt.([(Analyze, manual_generic)]) - ~default:true "the thread safety analysis" + mk_checker ~long:"siof" ~default:true + "the Static Initialization Order Fiasco analysis (C++ only)" + and threadsafety = mk_checker ~long:"threadsafety" ~default:true "the thread safety analysis" and suggest_nullable = - CLOpt.mk_bool ~long:"suggest-nullable" ~default:false + mk_checker ~long:"suggest-nullable" ~default:false "Nullable annotation sugesstions analysis (experimental)" in - (* IMPORTANT: keep in sync with the checkers that have ~default:true above *) - let default_checkers = + let mk_only (var, long) = + let _ : bool ref = + CLOpt.mk_bool_group ~long:(long ^ "-only") + ~in_help:CLOpt.([(Analyze, manual_generic)]) + (Printf.sprintf "Enable $(b,--%s) and disable all other checkers" long) [var] + (List.map ~f:fst !all_checkers) + in + () + in + List.iter ~f:mk_only !all_checkers ; + let _default_checkers : bool ref = CLOpt.mk_bool_group ~long:"default-checkers" ~in_help:CLOpt.([(Analyze, manual_generic)]) ~default:true - "Default checkers: $(b,--annotation-reachability), $(b,--fragments-retains-view), $(b,--immutable-cast), $(b,--printf-args), $(b,--quandary), $(b,--siof), $(b,--threadsafety)" - [ annotation_reachability - ; fragment_retains_view - ; immutable_cast - ; printf_args - ; quandary - ; siof - ; threadsafety ] [] + ( "Default checkers: " + ^ ( List.map ~f:(fun (_, s) -> Printf.sprintf "$(b,--%s)" s) !default_checkers + |> String.concat ~sep:", " ) ) + (List.map ~f:fst !default_checkers) + [] in ( annotation_reachability , biabduction , bufferoverrun , crashcontext - , default_checkers , eradicate , fragment_retains_view , immutable_cast @@ -1243,7 +1228,7 @@ and linters_def_folder = ~in_help:CLOpt.([(Capture, manual_clang_linters)]) ~meta:"dir" "Specify the folder containing linters files with extension .al" in - let _ = + let () = CLOpt.mk_set linters_def_folder [] ~long:"reset-linters-def-folder" "Reset the list of folders containing linters definitions to be empty (see $(b,linters-def-folder))." in @@ -1547,7 +1532,7 @@ and specs_library = CLOpt.mk_path_list ~deprecated:["lib"] ~long:"specs-library" ~short:'L' ~meta:"dir|jar" "Search for .spec files in given directory or jar file" in - let _ = + let _ : string ref = (* Given a filename with a list of paths, convert it into a list of string iff they are absolute *) let read_specs_dir_list_file fname = @@ -1951,8 +1936,6 @@ and debug_exceptions = !debug_exceptions and debug_mode = !debug -and default_checkers = !default_checkers - and default_linters = !default_linters and dependency_mode = !dependencies diff --git a/infer/src/base/Config.mli b/infer/src/base/Config.mli index 8c764d87b..5d395328f 100644 --- a/infer/src/base/Config.mli +++ b/infer/src/base/Config.mli @@ -106,8 +106,6 @@ val relative_cpp_models_dir : string val csl_analysis : bool -val default_checkers : bool - val default_failure_name : string val default_in_zip_results_dir : string diff --git a/infer/src/labs/lab.md b/infer/src/labs/lab.md index 43c86a963..a1d595a69 100644 --- a/infer/src/labs/lab.md +++ b/infer/src/labs/lab.md @@ -12,7 +12,7 @@ The `ResourceLeaks.ml` and `ResourceLeaksDomain.ml` files have comments containi (c) Add a new test method containing a resource leak to `Leaks.java`, then run the tests again. The tests should now fail, and the error message should indicate that the failure is due to the new lreeak that you added. As the message suggests, run `make replace` to update the expected output (this simply copies the actual output `issues.exp.test` to the expected output `issues.exp`). Re-run the tests; they should now pass again. -(d) Run the analyzer on a single test file to produce the debug HTML: `infer --debug -a checkers --no-default-checkers --resource-leak -- javac Leaks.java`. Then, open the debug HTML: `open infer-out/captured/*.html`. This helpful artifact shows the Infer warnings alongside the code they are complaining about. It also displays the CFG node(s) associated with each line of code. Clicking a CFG node shows the Infer IR associated with the node, and the pre/post state computed while analyzing the instruction. Come back to the debug HTML early and often when you can't understand what your analysis is doing--it will help! +(d) Run the analyzer on a single test file to produce the debug HTML: `infer --debug -a checkers --resource-leak-only -- javac Leaks.java`. Then, open the debug HTML: `open infer-out/captured/*.html`. This helpful artifact shows the Infer warnings alongside the code they are complaining about. It also displays the CFG node(s) associated with each line of code. Clicking a CFG node shows the Infer IR associated with the node, and the pre/post state computed while analyzing the instruction. Come back to the debug HTML early and often when you can't understand what your analysis is doing--it will help! (e) The `Logging.d_str`/`Logging.d_strln` functions print to the debug HTML. The logging is contextual: it will print to the log for the CFG node that's being analyzed at the time the logging statement execute. Try adding `Logging.d_strln "Hi Infer";` inside of the case for `Call`, recompiling/re-running. You should see the text you printed inside the appropriate CFG node log in the debug HTML. @@ -132,7 +132,7 @@ void closeInCatchBad() { ``` (b) Try running on real code! The instructions [here](http://fm.csl.sri.com/SSFT17/infer-instr.html) have several suggestions for open-source Android apps to point your analysis at. Try `./gradlew assembleDebug -x test` first to make sure everything builds correctly without Infer (if not, you are probably missing some dependencies--the error messages should guide you). Once that's working, try -`./gradlew clean; infer -a checkers --no-default-checkers --resource-leak -- ./gradlew assembleDebug -x test`. +`./gradlew clean; infer -a checkers --resource-leak-only -- ./gradlew assembleDebug -x test`. - Found a real bug? Bonus points! Send a pull request to fix it! Very frequently, the best fix is to use try-with-resources. - Found a false positive in your analysis? Try re-running Infer with `--debug` and see if you can narrow down the root cause/fix it? - How does your analysis compare to Infer's production resource leak analysis? Run with `infer -- ` to see if your analysis finds bugs that Infer misses, or vice versa. diff --git a/infer/tests/codetoanalyze/cpp/liveness/Makefile b/infer/tests/codetoanalyze/cpp/liveness/Makefile index b0f325e6b..7781a2b25 100644 --- a/infer/tests/codetoanalyze/cpp/liveness/Makefile +++ b/infer/tests/codetoanalyze/cpp/liveness/Makefile @@ -10,7 +10,7 @@ TESTS_DIR = ../../.. ANALYZER = checkers # see explanations in cpp/errors/Makefile for the custom isystem CLANG_OPTIONS = -x c++ -std=c++11 -nostdinc++ -isystem$(MODELS_DIR)/cpp/include -isystem$(CLANG_INCLUDES)/c++/v1/ -c -INFER_OPTIONS = --ml-buckets cpp --debug-exceptions --project-root $(TESTS_DIR) --no-keep-going +INFER_OPTIONS = --liveness --ml-buckets cpp --debug-exceptions --project-root $(TESTS_DIR) --no-keep-going INFERPRINT_OPTIONS = --issues-tests SOURCES = $(wildcard *.cpp)