avoid name collision when two or more files have the same basename

Summary:
public
Infer would previously give confusing reports in the following case: two classes `foo.MyClass` defined in `MyClass.java` under directory `foo/` and `bar.MyClass` defined in file `MyClass.java` under `bar/` are compiled together in a single call to the Java compiler. Then the errors in `foo/MyClass.java` could potentially be reported in `bar/MyClass.java`, or the other way around.

The reason is: Infer starts the translation from the bytecode which only contains information about the base filename in the metadata. For example, both `foo.MyClass` and `bar.MyClass` will contains the information that the source file is `MyClass.java` but not the full path to the actual source file (hopefully).

In order to cope with this issue, this diff adds the possibility to read the package declaration from the source file so that we can map classes to the source files these classes are defined without ambiguity. In order to avoid having to open and read the source files when not necessary, the code will behave as before as long as no name conflict is found. Otherwise, it will only load and search for the package declaration when two or more sources files have the same basename but are defined in different subdirectories.

Closes t9395275

Reviewed By: jberdine

Differential Revision: D2763775

fb-gh-sync-id: 0adc1ac

examples/android_hello/app/src/main/java/infer/other/MainActivity.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2015 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+package infer.other;
+
+import android.support.v7.app.ActionBarActivity;
+import android.os.Bundle;
+
+public class MainActivity extends ActionBarActivity {
+
+  Object source() {
+    return null;
+  }
+
+  @Override
+  protected void onCreate(Bundle savedInstanceState) {
+    source().toString();
+  }
+
+}

examples/android_hello/gradle_report.json

@@ -4,6 +4,11 @@
     "file": "app/src/main/java/infer/inferandroidexample/MainActivity.java",
     "procedure": "void MainActivity.onCreate(Bundle)"
   },
+  {
+    "bug_type": "NULL_DEREFERENCE",
+    "file": "app/src/main/java/infer/other/MainActivity.java",
+    "procedure": "void MainActivity.onCreate(Bundle)"
+  },
   {
     "bug_type": "RESOURCE_LEAK",
     "file": "app/src/main/java/infer/inferandroidexample/MainActivity.java",

infer/src/java/jClasspath.ml

@@ -109,23 +109,74 @@ let append_path classpath path =
     classpath
 
 
+type file_entry =
+  | Singleton of DB.source_file
+  | Duplicate of (string * DB.source_file) list
+
+
+(* Open the source file and search for the package declaration.
+   Only the case where the package is declared in a single line is supported *)
+let read_package_declaration source_file =
+  let path = DB.source_file_to_string source_file in
+  let file_in = open_in path in
+  let remove_trailing_semicolon =
+    Str.replace_first (Str.regexp ";") "" in
+  let empty_package = "" in
+  let rec loop () =
+    try
+      let line = remove_trailing_semicolon (input_line file_in) in
+      match Str.split (Str.regexp "[ \t]+") line with
+      | [] -> loop ()
+      | hd::package::[] when hd = "package" -> package
+      | _ -> loop ()
+    with End_of_file ->
+      close_in file_in;
+      empty_package in
+  loop ()
+
+
+let add_source_file path map =
+  let convert_to_absolute p =
+    if Filename.is_relative p then
+      Filename.concat (Sys.getcwd ()) p
+    else
+      p in
+  let basename = Filename.basename path in
+  let entry =
+    let current_source_file =
+      java_source_file_from_path (convert_to_absolute path) in
+    try
+      match StringMap.find basename map with
+      | Singleton previous_source_file ->
+          (* Another source file with the same base name has been found.
+             Reading the package from the source file to resolve the ambiguity
+             only happens in this case *)
+          let previous_package = read_package_declaration previous_source_file
+          and current_package = read_package_declaration current_source_file in
+          let source_list = [
+            (current_package, current_source_file);
+            (previous_package, previous_source_file)] in
+          Duplicate source_list
+      | Duplicate previous_source_files ->
+          (* Two or more source file with the same base name have been found *)
+          let current_package = read_package_declaration current_source_file in
+          Duplicate ((current_package, current_source_file) :: previous_source_files)
+    with Not_found ->
+      (* Most common case: there is no conflict with the base name of the source file *)
+      Singleton current_source_file in
+  StringMap.add basename entry map
+
+
 let load_sources_and_classes () =
   let file_in = open_in !javac_verbose_out in
-  let cwd = Sys.getcwd () in
-  let convert_filename fname =
-    if Filename.is_relative fname then
-      Filename.concat cwd fname
-    else
-      fname in
   let rec loop paths roots sources classes =
     try
       let lexbuf = Lexing.from_string (input_line file_in) in
       match JVerboseParser.line JVerboseLexer.token lexbuf with
-      | JVerbose.Source fname ->
-          let source_file = java_source_file_from_path (convert_filename fname) in
-          loop paths roots (StringMap.add (Filename.basename fname) source_file sources) classes
-      | JVerbose.Class fname ->
-          let cn, root_info = Javalib.extract_class_name_from_file fname in
+      | JVerbose.Source path ->
+          loop paths roots (add_source_file path sources) classes
+      | JVerbose.Class path ->
+          let cn, root_info = Javalib.extract_class_name_from_file path in
           let root_dir = if root_info = "" then Filename.current_dir_name else root_info in
           let updated_roots =
             if IList.exists (fun p -> p = root_dir) roots then roots
@@ -225,7 +276,7 @@ let classmap_of_classpath classpath =
   IList.fold_left collect_classes JBasics.ClassMap.empty jar_filenames
 
 
-let load_program classpath classes arg_source_files =
+let load_program classpath classes =
   JUtils.log "loading program ... %!";
   let models =
     if !models_jar = "" then JBasics.ClassMap.empty

infer/src/java/jClasspath.mli

@@ -30,9 +30,14 @@ val java_source_file_from_path : string -> DB.source_file
 
 val split_classpath : string -> string list
 
+(** map entry for source files with potential basname collision within the same compiler call *)
+type file_entry =
+  | Singleton of DB.source_file
+  | Duplicate of (string * DB.source_file) list
+
 (** load the list of source files and the list of classes from the javac verbose file *)
 val load_sources_and_classes : unit ->
-  string * DB.source_file Utils.StringMap.t * JBasics.ClassSet.t
+  string * file_entry Utils.StringMap.t * JBasics.ClassSet.t
 
 type classmap = JCode.jcode Javalib.interface_or_class JBasics.ClassMap.t
 
@@ -45,7 +50,7 @@ val get_models : program -> classmap
 val cleanup : program -> unit
 
 (** load a java program *)
-val load_program : string -> JBasics.ClassSet.t -> DB.source_file Utils.StringMap.t -> program
+val load_program : string -> JBasics.ClassSet.t -> program
 
 (** retrive a Java node from the classname *)
 val lookup_node : JBasics.class_name -> program -> JCode.jcode Javalib.interface_or_class option

infer/src/java/jFrontend.ml

@@ -152,7 +152,7 @@ let is_classname_cached cn =
 (* Given a source file and a class, translates the code of this class.
    In init - mode, finds out whether this class contains initializers at all,
    in this case translates it. In standard mode, all methods are translated *)
-let create_icfg never_null_matcher linereader program icfg source_file cn node =
+let create_icfg never_null_matcher linereader program icfg cn node =
   JUtils.log "\tclassname: %s@." (JBasics.cn_name cn);
   cache_classname cn;
   let cfg = icfg.JContext.cfg in
@@ -181,18 +181,29 @@ type capture_status =
   | Unknown
 
 (* returns true for the set of classes that are selected to be translated *)
-let should_capture classes source_basename node =
+let should_capture classes package_opt source_basename node =
   let classname = Javalib.get_name node in
   let temporary_skip =
     (* TODO (#6341744): remove this *)
     IList.exists
       (fun part -> part = "graphschema")
       (JBasics.cn_package classname) in
+  let match_package pkg cn =
+    match JTransType.package_to_string (JBasics.cn_package cn) with
+    | None -> pkg = ""
+    | Some found_pkg -> found_pkg = pkg in
   if JBasics.ClassSet.mem classname classes && not temporary_skip then
     begin
       match Javalib.get_sourcefile node with
       | None -> false
-      | Some source -> source = source_basename
+      | Some found_basename ->
+          begin
+            match package_opt with
+            | None -> found_basename = source_basename
+            | Some pkg ->
+                match_package pkg classname
+                && found_basename = source_basename
+          end
     end
   else false
 
@@ -201,13 +212,13 @@ let should_capture classes source_basename node =
    In the standard - mode, it translated all the classes that correspond to this
    source file. *)
 let compute_source_icfg
-    never_null_matcher linereader classes program tenv source_basename source_file =
+    never_null_matcher linereader classes program tenv
+    source_basename package_opt =
   let icfg =
     { JContext.cg = Cg.create ();
       JContext.cfg = Cfg.Node.create_cfg ();
       JContext.tenv = tenv } in
   let select test procedure cn node =
-    (* let () = JUtils.log "translating: %s@." (JBasics.cn_name (JProgram.get_name node)) in *)
     if test node then
       try
         procedure cn node
@@ -217,8 +228,8 @@ let compute_source_icfg
   let () =
     JBasics.ClassMap.iter
       (select
-         (should_capture classes source_basename)
-         (create_icfg never_null_matcher linereader program icfg source_file))
+         (should_capture classes package_opt source_basename)
+         (create_icfg never_null_matcher linereader program icfg))
       (JClasspath.get_classmap program) in
   (icfg.JContext.cg, icfg.JContext.cfg)
 
@@ -230,7 +241,7 @@ let compute_class_icfg never_null_matcher linereader program tenv node fake_sour
   begin
     try
       create_icfg
-        never_null_matcher linereader program icfg fake_source_file (Javalib.get_name node) node
+        never_null_matcher linereader program icfg (Javalib.get_name node) node
     with
     | Bir.Subroutine -> ()
     | e -> raise e

infer/src/java/jFrontend.mli

@@ -28,7 +28,7 @@ val compute_source_icfg :
   JClasspath.program ->
   Sil.tenv ->
   string ->
-  DB.source_file ->
+  string option ->
   Cg.t * Cfg.cfg
 
 (** Compute the CFG for a class *)

infer/src/java/jMain.ml

@@ -80,12 +80,14 @@ let store_icfg tenv cg cfg source_file =
 (* Given a source file, its code is translated, and the call-graph, control-flow-graph and type *)
 (* environment are obtained and saved. *)
 let do_source_file
-    never_null_matcher linereader classes program tenv source_basename source_file proc_file_map =
+    never_null_matcher linereader classes program tenv
+    source_basename (package_opt, source_file) proc_file_map =
   JUtils.log "\nfilename: %s (%s)@."
     (DB.source_file_to_string source_file) source_basename;
   let call_graph, cfg =
     JFrontend.compute_source_icfg
-      never_null_matcher linereader classes program tenv source_basename source_file in
+      never_null_matcher linereader classes program tenv
+      source_basename package_opt in
   store_icfg tenv call_graph cfg source_file;
   if !JConfig.create_harness then
     IList.fold_left
@@ -143,7 +145,7 @@ let do_all_files classpath sources classes =
   JUtils.log "Translating %d source files (%d classes)@."
     (StringMap.cardinal sources)
     (JBasics.ClassSet.cardinal classes);
-  let program = JClasspath.load_program classpath classes sources in
+  let program = JClasspath.load_program classpath classes in
   let tenv = load_tenv program in
   let linereader = Printer.LineReader.create () in
   let skip_translation_matcher =
@@ -151,14 +153,24 @@ let do_all_files classpath sources classes =
   let never_null_matcher =
     Inferconfig.NeverReturnNull.load_matcher (Inferconfig.inferconfig ()) in
   let proc_file_map =
-    let skip filename =
-      skip_translation_matcher filename Procname.empty in
-    StringMap.fold
-      (fun basename source_file map ->
+    let skip source_file =
+      skip_translation_matcher source_file Procname.empty in
+    let translate_source_file basename (package_opt, source_file) source_file map =
       init_global_state source_file;
       if skip source_file then map
       else do_source_file
-             never_null_matcher linereader classes program tenv basename source_file map)
+          never_null_matcher linereader classes program tenv
+          basename (package_opt, source_file) map in
+    StringMap.fold
+      (fun basename file_entry map ->
+         match file_entry with
+         | JClasspath.Singleton source_file ->
+             translate_source_file basename (None, source_file) source_file map
+         | JClasspath.Duplicate source_files ->
+             IList.fold_left
+               (fun accu (package, source_file) ->
+                  translate_source_file basename (Some package, source_file) source_file accu)
+               map source_files)
       sources
       Procname.Map.empty in
   if !JConfig.dependency_mode then

infer/src/java/jTransType.mli

@@ -62,6 +62,8 @@ val expr_type : JContext.t -> JBir.expr -> Sil.typ
 (** translates a conversion type from Java to Sil. *)
 val cast_type : JBir.conv -> Sil.typ
 
+val package_to_string : string list -> string option
+
 (** [create_array_type typ dim] creates an array type with dimension dim and content typ *)
 val create_array_type : Sil.typ -> int -> Sil.typ