From 39b11619b82fd47693cc11c142255b11e7b63ace Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Mon, 21 Jan 2019 05:28:17 -0800 Subject: [PATCH] [classloads] overhaul test infra & fix treatment of self-class loading in method calls Reviewed By: ddino Differential Revision: D13622717 fbshipit-source-id: 233cc5d74 --- .gitignore | 2 + infer/src/concurrency/classLoads.ml | 45 +++++++++++++-- infer/src/concurrency/classLoadsDomain.ml | 17 ++---- infer/src/concurrency/classLoadsDomain.mli | 8 ++- .../codetoanalyze/java/classloads/Basic.java | 16 +----- .../codetoanalyze/java/classloads/Makefile | 55 ++++++++++++++++++- .../codetoanalyze/java/classloads/issues.exp | 1 - infer/tests/javac.make | 2 +- 8 files changed, 109 insertions(+), 37 deletions(-) delete mode 100644 infer/tests/codetoanalyze/java/classloads/issues.exp diff --git a/.gitignore b/.gitignore index 60fd31629..9d75e6842 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,8 @@ duplicates.txt /infer/tests/build_systems/buck_flavors_deterministic/capture_hash-*.sha /infer/tests/build_systems/genrule/report.json /infer/tests/build_systems/java_test_determinator/*.test +/infer/tests/codetoanalyze/java/classloads/*.loads +/infer/tests/codetoanalyze/java/classloads/loads.exp /_release /infer-source diff --git a/infer/src/concurrency/classLoads.ml b/infer/src/concurrency/classLoads.ml index b7729e28f..560d88f0f 100644 --- a/infer/src/concurrency/classLoads.ml +++ b/infer/src/concurrency/classLoads.ml @@ -14,6 +14,13 @@ module Payload = SummaryPayload.Make (struct let of_payloads (payloads : Payloads.t) = payloads.class_loads end) +let get_java_class = function + | Typ.Procname.Java java_pname -> + Some (Typ.Procname.Java.get_class_name java_pname) + | _ -> + None + + let exec_instr pdesc astate _ (instr : Sil.instr) = match instr with | Call (_, Const (Cfun callee), _, loc, _) -> @@ -24,22 +31,48 @@ let exec_instr pdesc astate _ (instr : Sil.instr) = let report_loads proc_desc summary astate = - let report_load ({ClassLoadsDomain.Event.loc} as event) = + let report_load ({ClassLoadsDomain.Event.loc; elem} as event) = let ltr = ClassLoadsDomain.Event.make_loc_trace event in - Reporting.log_warning summary ~loc ~ltr IssueType.class_load "Class load" + let msg = Format.asprintf "Class %s loaded" elem in + Reporting.log_warning summary ~loc ~ltr IssueType.class_load msg in let pname = Procdesc.get_proc_name proc_desc in - ClassLoadsDomain.get_java_class pname + get_java_class pname |> Option.iter ~f:(fun clazz -> let method_strname = Typ.Procname.get_method pname in let fullname = clazz ^ "." ^ method_strname in - (* Logging.debug_dev "Pname = %s" method_strname ; *) if String.Set.mem Config.class_loads_roots fullname then ClassLoadsDomain.iter report_load astate ) +(* if [pdesc] is *not* a class initializer (to avoid infinite recursion), return the + class initializer of [pdesc]'s class *) +let class_initializer_of_method pdesc = + let open Typ.Procname in + match Procdesc.get_proc_name pdesc with + | Java java_pname when Java.is_class_initializer java_pname -> + None + | Java java_pname -> + Some (Java Java.(replace_method_name class_initializer_method_name java_pname)) + | _ -> + assert false + + let analyze_procedure {Callbacks.proc_desc; summary} = - let init = ClassLoadsDomain.empty in - let post = Procdesc.fold_instrs proc_desc ~init ~f:(exec_instr proc_desc) in + let proc_name = Procdesc.get_proc_name proc_desc in + let loc = Procdesc.get_loc proc_desc in + (* add a load for the method's class *) + let init = + get_java_class proc_name + |> Option.fold ~init:ClassLoadsDomain.empty ~f:(ClassLoadsDomain.add_load loc) + in + (* add loads done by the static initialization of this method's class *) + let after_class_init = + class_initializer_of_method proc_desc + |> Option.bind ~f:(Payload.read proc_desc) + (* pretend there is a call to class initializer before the method body *) + |> Option.fold ~init ~f:(ClassLoadsDomain.integrate_summary proc_name loc) + in + let post = Procdesc.fold_instrs proc_desc ~init:after_class_init ~f:(exec_instr proc_desc) in report_loads proc_desc summary post ; Payload.update_summary post summary diff --git a/infer/src/concurrency/classLoadsDomain.ml b/infer/src/concurrency/classLoadsDomain.ml index 3d65b56ee..76dd4e883 100644 --- a/infer/src/concurrency/classLoadsDomain.ml +++ b/infer/src/concurrency/classLoadsDomain.ml @@ -27,20 +27,15 @@ let add ({Event.trace} as x) astate = let union xs ys = fold add xs ys -let get_java_class = function - | Typ.Procname.Java java_pname -> - Some (Typ.Procname.Java.get_class_name java_pname) - | _ -> - None +let add_load loc astate clazz = + let new_event = Event.make clazz loc in + add new_event astate let integrate_summary callee_pname loc astate callee_summary = - get_java_class callee_pname - |> Option.value_map ~default:astate ~f:(fun clazz -> - let new_event = Event.make clazz loc in - let callsite = CallSite.make callee_pname loc in - let summary = with_callsite callee_summary callsite in - add new_event astate |> union summary ) + let callsite = CallSite.make callee_pname loc in + let summary = with_callsite callee_summary callsite in + union astate summary type summary = t diff --git a/infer/src/concurrency/classLoadsDomain.mli b/infer/src/concurrency/classLoadsDomain.mli index e3933423a..69462bce1 100644 --- a/infer/src/concurrency/classLoadsDomain.mli +++ b/infer/src/concurrency/classLoadsDomain.mli @@ -9,14 +9,16 @@ module F = Format module ClassLoad : ExplicitTrace.Element with type t = string -val get_java_class : Typ.Procname.t -> string option - module Event : ExplicitTrace.TraceElem with type elem_t = ClassLoad.t -include AbstractDomain.FiniteSetS with type elt = Event.t +include AbstractDomain.WithBottom type summary = t val pp_summary : F.formatter -> summary -> unit +val add_load : Location.t -> t -> string -> t + val integrate_summary : Typ.Procname.t -> Location.t -> t -> summary -> t + +val iter : (Event.t -> unit) -> t -> unit diff --git a/infer/tests/codetoanalyze/java/classloads/Basic.java b/infer/tests/codetoanalyze/java/classloads/Basic.java index 9b7134df9..407c17414 100644 --- a/infer/tests/codetoanalyze/java/classloads/Basic.java +++ b/infer/tests/codetoanalyze/java/classloads/Basic.java @@ -6,19 +6,9 @@ */ class Basic { - void foo_report(Other o) { - o.bar(); - } - - void baz_no_report(Another a) { - a.baz(); + public static void main(String args[]) { + new BasicObj(); } } -class Other { - void bar() {} -} - -class Another { - void baz() {} -} +class BasicObj {} diff --git a/infer/tests/codetoanalyze/java/classloads/Makefile b/infer/tests/codetoanalyze/java/classloads/Makefile index 9837fa64e..a608eca3b 100644 --- a/infer/tests/codetoanalyze/java/classloads/Makefile +++ b/infer/tests/codetoanalyze/java/classloads/Makefile @@ -3,10 +3,61 @@ # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. +# Convention: each Java file must have a single [main] method in the eponymous public class +# This is the root for class loads analysis, and the one executed in the JVM to log loads. +# There is no .exp file committed, the loads are generated by JVM during testing. + TESTS_DIR = ../../.. -INFER_OPTIONS = --class-loads-only --class-loads-roots "Basic.foo_report" --debug-exceptions INFERPRINT_OPTIONS = --issues-tests + SOURCES = $(wildcard *.java) +LOADS = $(patsubst %.java,%.loads,$(SOURCES)) +OBJECTS = $(patsubst %.java,%.class,$(SOURCES)) +CLASS_LOADS_OPT = $(patsubst %.java,--class-loads-roots %.main,$(SOURCES)) +INFER_OPTIONS = -j 1 --class-loads-only $(CLASS_LOADS_OPT) --debug-exceptions +CLEAN_EXTRA = *.class *.loads loads.exp loads.exp.test + +include $(TESTS_DIR)/java.make + +INFER_OUT ?= infer-out$(TEST_SUFFIX) + +include $(TESTS_DIR)/base.make + +.PHONY: clean +clean: + $(REMOVE_DIR) codetoanalyze com issues.exp.test$(TEST_SUFFIX) $(OBJECTS) $(CLEAN_EXTRA) +ifneq ($(INFER_OUT),.) + $(REMOVE_DIR) $(INFER_OUT) +endif + +PROJECT_ROOT ?= $(TESTS_DIR) + +$(OBJECTS): $(SOURCES) + $(QUIET)$(JAVAC) -cp $(CLASSPATH) $(SOURCES) + +issues.exp.test$(TEST_SUFFIX): $(INFER_OUT)/report.json $(INFER_BIN) + $(QUIET)$(INFER_BIN) report -q --results-dir $( $@ + +loads.exp: $(LOADS) + $(QUIET)for F in $(LOADS) ; do sed -e "s#^#$(TEST_REL_DIR)/$${F%.*}.java, #" $$F ; done | sort > loads.exp + +# FIXME change domain so that -u is not needed below +loads.exp.test: issues.exp.test$(TEST_SUFFIX) + $(QUIET)cat $< | sed 's/^\([^,]*\),.*[,\[]\([^,]*\)\]$$/\1, \2/' | sort -u > $@ + +.PHONY: test +test: loads.exp loads.exp.test + $(QUIET)cd $(TESTS_DIR) && \ + $(call check_no_diff,$(TEST_REL_DIR)/loads.exp,$(TEST_REL_DIR)/loads.exp.test) -include $(TESTS_DIR)/javac.make diff --git a/infer/tests/codetoanalyze/java/classloads/issues.exp b/infer/tests/codetoanalyze/java/classloads/issues.exp deleted file mode 100644 index 491b62b21..000000000 --- a/infer/tests/codetoanalyze/java/classloads/issues.exp +++ /dev/null @@ -1 +0,0 @@ -codetoanalyze/java/classloads/Basic.java, Basic.foo_report(Other):void, 1, CLASS_LOAD, no_bucket, WARNING, [Other] diff --git a/infer/tests/javac.make b/infer/tests/javac.make index f48293aa1..81d80b654 100644 --- a/infer/tests/javac.make +++ b/infer/tests/javac.make @@ -17,7 +17,7 @@ include $(TESTS_DIR)/infer.make PROJECT_ROOT ?= $(TESTS_DIR) $(OBJECTS): $(SOURCES) - $(JAVAC) -cp $(CLASSPATH) $(SOURCES) + $(QUIET)$(JAVAC) -cp $(CLASSPATH) $(SOURCES) infer-out/report.json: $(JAVA_DEPS) $(SOURCES) $(MAKEFILE_LIST) $(QUIET)$(call silent_on_success,Testing infer/java in $(TEST_REL_DIR),\