From d579b2be519647c559f90e140b888735829e381c Mon Sep 17 00:00:00 2001 From: jrm Date: Thu, 17 Dec 2015 08:52:44 -0800 Subject: [PATCH] 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 --- .../main/java/infer/other/MainActivity.java | 26 +++++++ examples/android_hello/gradle_report.json | 5 ++ infer/src/java/jClasspath.ml | 75 ++++++++++++++++--- infer/src/java/jClasspath.mli | 9 ++- infer/src/java/jFrontend.ml | 27 +++++-- infer/src/java/jFrontend.mli | 2 +- infer/src/java/jMain.ml | 32 +++++--- infer/src/java/jTransType.mli | 2 + 8 files changed, 145 insertions(+), 33 deletions(-) create mode 100644 examples/android_hello/app/src/main/java/infer/other/MainActivity.java diff --git a/examples/android_hello/app/src/main/java/infer/other/MainActivity.java b/examples/android_hello/app/src/main/java/infer/other/MainActivity.java new file mode 100644 index 000000000..9251c998c --- /dev/null +++ b/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(); + } + +} diff --git a/examples/android_hello/gradle_report.json b/examples/android_hello/gradle_report.json index 0f3b2137e..96c057e74 100644 --- a/examples/android_hello/gradle_report.json +++ b/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", diff --git a/infer/src/java/jClasspath.ml b/infer/src/java/jClasspath.ml index c9d59e6da..58e1eb480 100644 --- a/infer/src/java/jClasspath.ml +++ b/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 diff --git a/infer/src/java/jClasspath.mli b/infer/src/java/jClasspath.mli index a90188a43..2fb48bf9d 100644 --- a/infer/src/java/jClasspath.mli +++ b/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 diff --git a/infer/src/java/jFrontend.ml b/infer/src/java/jFrontend.ml index da793ad16..6f3acc13e 100644 --- a/infer/src/java/jFrontend.ml +++ b/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 diff --git a/infer/src/java/jFrontend.mli b/infer/src/java/jFrontend.mli index 10cd28b4a..72c329892 100644 --- a/infer/src/java/jFrontend.mli +++ b/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 *) diff --git a/infer/src/java/jMain.ml b/infer/src/java/jMain.ml index d087c84a1..2b99d4872 100644 --- a/infer/src/java/jMain.ml +++ b/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 + 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 (package_opt, source_file) map in StringMap.fold - (fun basename 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) + (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 diff --git a/infer/src/java/jTransType.mli b/infer/src/java/jTransType.mli index 6b739feee..10ac3e24c 100644 --- a/infer/src/java/jTransType.mli +++ b/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