[classloads] overhaul test infra & fix treatment of self-class loading in method calls

Reviewed By: ddino

Differential Revision: D13622717

fbshipit-source-id: 233cc5d74

.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
 

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

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 )
+  union astate summary
 
 
 type summary = t

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

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 {}

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 $(<D) \
+	   $(INFERPRINT_OPTIONS) $@ --from-json-report $<
+	   
+infer-out/report.json: $(JAVA_DEPS) $(SOURCES) $(MAKEFILE_LIST)
+	$(QUIET)$(call silent_on_success,Testing infer/java in $(TEST_REL_DIR),\
+	  $(INFER_BIN) --project-root $(PROJECT_ROOT) \
+	    $(INFER_OPTIONS) -- \
+	    $(JAVAC) -cp $(CLASSPATH) $(SOURCES))
+
+%.loads: %.class
+	$(QUIET)java -verbose:class $* | grep "^\[Loaded" | cut -f2 -d' ' | grep -vE "^(java|sun)" > $@
+
+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

infer/tests/codetoanalyze/java/classloads/issues.exp

@@ -1 +0,0 @@
-codetoanalyze/java/classloads/Basic.java, Basic.foo_report(Other):void, 1, CLASS_LOAD, no_bucket, WARNING, [Other]

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),\