[buck-db] add flavour more robustly

Summary:
Two issues in the previous code:
1. We'd just tack on `#compilation-database` so if there already are `#flavors`
  in a Buck target then we get a malformed `#flavors#compilation-database`
  instead of `#flavors,compilation-database`. There's already code to deal
  with that for other flavors (`#infer-capture-all`), so extend and reuse that.
2. The code didn't work if there are spaces in target names (sigh). Harden the
  code a bit so that it works. Unfortunately Buck doesn't escape spaces at all
  in its `buck targets --show-output` output, so the parsing still relies on
  there being no spaces in flavors to work correctly. Sample output:
```
$ buck targets --show-full-output //clang_compilation_database:Hel\ lo#compilation-database
Not using buckd because watchman isn't installed.
[+] PROCESSING BUCK FILES...0.1s [100%] 🐳  New buck daemon
//clang_compilation_database:Hel lo#compilation-database /home/jul/infer/infer/tests/build_systems/codetoanalyze/buck-out/gen/clang_compilation_database/__Hel lo#compilation-database.json
```

Also, some code in codetoanalyze/clang-compilation-database/ didn't compile, so
fix that.

Reviewed By: dulmarod

Differential Revision: D4714338

fbshipit-source-id: b8ae324

Makefile

@@ -39,6 +39,9 @@ BUILD_SYSTEMS_TESTS += \
   utf8_in_procname \
   waf \
 
+ifneq ($(BUCK),no)
+BUILD_SYSTEMS_TESTS += buck-clang-db
+endif
 ifneq ($(CMAKE),no)
 BUILD_SYSTEMS_TESTS += clang_compilation_db cmake
 endif

infer/src/base/Config.ml

@@ -572,6 +572,7 @@ and buck_build_args =
 
 and buck_compilation_database =
   CLOpt.mk_symbol_opt ~long:"buck-compilation-database" ~deprecated:["-use-compilation-database"]
+    ~parse_mode:CLOpt.(Infer [Driver])
     "Buck integration using the compilation database, with or without dependencies."
     ~symbols:[("deps", `Deps); ("no-deps", `NoDeps)]
 

infer/src/integration/Buck.ml

@@ -36,23 +36,26 @@ let no_targets_found_error_and_exit buck_cmd =
     (String.concat ~sep:" " buck_cmd)
 
 let add_flavor_to_target target =
-  let infer_flavor =
+  let add flavor =
-    match Config.analyzer with
+    if List.mem ~equal:String.equal target.flavors flavor then
-    | Compile ->
-        None
-    | Linters | Capture ->
-        Some "infer-capture-all"
-    | Checkers | Infer ->
-        Some "infer"
-    | Eradicate | Tracing | Crashcontext | Quandary | Threadsafety | Bufferoverrun ->
-        failwithf "Unsupported infer analyzer with Buck flavors: %s"
-          (Config.string_of_analyzer Config.analyzer) in
-  if List.exists ~f:(String.is_prefix ~prefix:"infer") target.flavors then
       (* there's already an infer flavor associated to the target, do nothing *)
       target
     else
-    { target with flavors = (Option.to_list infer_flavor) @ target.flavors }
+      { target with flavors = flavor::target.flavors } in
-
+  match Config.buck_compilation_database, Config.analyzer with
+  | Some `Deps, _ ->
+      add "uber-compilation-database"
+  | Some `NoDeps, _ ->
+      add "compilation-database"
+  | None, Compile ->
+      target
+  | None, (Linters | Capture) ->
+      add "infer-capture-all"
+  | None, (Checkers | Infer) ->
+      add "infer"
+  | None, (Eradicate | Tracing | Crashcontext | Quandary | Threadsafety | Bufferoverrun) ->
+      failwithf "Unsupported infer analyzer with Buck flavors: %s"
+        (Config.string_of_analyzer Config.analyzer)
 
 let add_flavors_to_buck_command build_cmd =
   let add_infer_if_target s (cmd, found_one_target) =

infer/src/integration/CaptureCompilationDatabase.ml

@@ -27,23 +27,6 @@ let should_capture_file_from_index () =
   | Some files_set ->
       function source_file -> SourceFile.Set.mem source_file files_set
 
-(** The buck targets are assumed to start with //, aliases are not supported. *)
-let check_args_for_targets args =
-  if not (List.exists ~f:Buck.is_target_string args) then
-    Buck.no_targets_found_error_and_exit args
-
-let add_flavor_to_targets args =
-  let flavor =
-    match Config.buck_compilation_database with
-    | Some `Deps -> "#uber-compilation-database"
-    | Some `NoDeps -> "#compilation-database"
-    | _ -> assert false (* cannot happen *) in
-  let process_arg arg =
-    (* Targets are assumed to start with //, aliases are not allowed *)
-    if String.is_prefix ~prefix:"//" arg then arg ^ flavor
-    else arg in
-  List.map ~f:process_arg args
-
 let create_files_stack compilation_database should_capture_file =
   let stack = Stack.create () in
   let add_to_stack file _ = if should_capture_file file then
@@ -98,27 +81,35 @@ let run_compilation_database compilation_database should_capture_file =
 (** Computes the compilation database files. *)
 let get_compilation_database_files_buck () =
   let cmd = List.rev_append Config.rest (List.rev Config.buck_build_args) in
-  match cmd with
+  match Buck.add_flavors_to_buck_command cmd with
-  | buck :: build :: args ->
+  | buck :: build :: args_with_flavor -> (
-      (check_args_for_targets args;
+      let build_args = build :: "--config" :: "*//cxx.pch_enabled=false" :: args_with_flavor in
-       let args_with_flavor = add_flavor_to_targets args in
+      Process.create_process_and_wait ~prog:buck ~args:build_args;
-       let args = build :: "--config" :: "*//cxx.pch_enabled=false" :: args_with_flavor in
+      let buck_targets_shell =
-       Process.create_process_and_wait ~prog:buck ~args;
+        buck :: "targets" :: "--show-output" :: args_with_flavor
-       let buck_targets_list = buck :: "targets" :: "--show-output" :: args_with_flavor in
+        |> List.map ~f:(Printf.sprintf "'%s'")
-       let buck_targets = String.concat ~sep:" " buck_targets_list in
+        |> String.concat ~sep:" " in
       try
-         match fst @@ Utils.with_process_in buck_targets In_channel.input_lines with
+        match fst @@ Utils.with_process_in buck_targets_shell In_channel.input_lines with
         | [] -> Logging.stdout "There are no files to process, exiting."; exit 0
         | lines ->
             Logging.out "Reading compilation database from:@\n%s@\n"
               (String.concat ~sep:"\n" lines);
-             let scan_output compilation_database_files chan =
+            (* this assumes that flavors do not contain spaces *)
-               Scanf.sscanf chan "%s %s" (fun _ file -> `Raw file::compilation_database_files) in
+            let split_regex = Str.regexp "#[^ ]* " in
+            let scan_output compilation_database_files line =
+              match Str.bounded_split split_regex line 2 with
+              | _::filename::[] ->
+                  `Raw filename::compilation_database_files
+              | _ ->
+                  failwithf
+                    "Failed to parse `buck targets --show-output ...` line of output:@\n%s" line in
             List.fold ~f:scan_output ~init:[] lines
       with Unix.Unix_error (err, _, _) ->
         Process.print_error_and_exit
           "Cannot execute %s\n%!"
-           (buck_targets ^ " " ^ (Unix.error_message err)))
+          (buck_targets_shell ^ " " ^ (Unix.error_message err))
+    )
   | _ ->
       let cmd = String.concat ~sep:" " cmd in
       Process.print_error_and_exit "Incorrect buck command: %s. Please use buck build <targets>" cmd

infer/tests/build_systems/buck-clang-db/Makefile

@@ -0,0 +1,25 @@
+# Copyright (c) 2017 - 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.
+
+TESTS_DIR = ../..
+
+SOURCE_DIR = ../codetoanalyze
+
+ANALYZER = infer
+INFER_OPTIONS = --report-custom-error --developer-mode --project-root $(SOURCE_DIR) --no-failures-allowed
+SOURCES = $(wildcard $(SOURCE_DIR)/*.cpp $(SOURCE_DIR)/*.h)
+OBJECTS = '$(SOURCE_DIR)/buck-out/gen/clang_compilation_database/__Hel lo\#compilation-database,x86_64.json'
+INFERPRINT_OPTIONS = --issues-tests
+CLEAN_EXTRA = $(SOURCE_DIR)/buck-out
+
+include $(TESTS_DIR)/base.make
+
+infer-out/report.json: $(CLANG_DEPS) $(SOURCES)
+	cd $(SOURCE_DIR) && \
+	$(call silent_on_success,\
+	  $(INFER_BIN) -a $(ANALYZER) --stats $(INFER_OPTIONS) -o $(CURDIR)/$(@D) \
+	  --buck-compilation-database no-deps -- buck build '//clang_compilation_database:Hel lo#x86_64')

infer/tests/build_systems/buck-clang-db/issues.exp

@@ -0,0 +1,4 @@
+clang_compilation_database/hello.cpp, test0, 2, NULL_DEREFERENCE, [start of procedure test0()]
+clang_compilation_database/hello.cpp, test1, 2, NULL_DEREFERENCE, [start of procedure test1(),start of procedure deref1()]
+clang_compilation_database/hello.cpp, test2, 2, NULL_DEREFERENCE, [start of procedure test2(),start of procedure deref2()]
+clang_compilation_database/lib3.h, test, 1, NULL_DEREFERENCE, [start of procedure test(),start of procedure deref3()]

infer/tests/build_systems/codetoanalyze/clang_compilation_database/BUCK

@@ -0,0 +1,9 @@
+cxx_library(
+  name = 'Hel lo',
+  srcs = [
+    'hello.cpp', 'lib1.cpp', 'lib2.cpp', 'lib3.cpp',
+  ],
+  compiler_flags = [
+    '-std=c++11',
+  ],
+)

infer/tests/build_systems/codetoanalyze/clang_compilation_database/hello.cpp

@@ -20,7 +20,7 @@ int test1() {
   return deref1(s); // should be nullderef
 }
 
-void test2() {
+int test2() {
   int* s = nullptr;
   return deref2(s); // should be nullderef, but will be skipped
   //  because of --changed-files-index option