Remove dependency on scan-build wrapper script for analysis builds.

This CL replaces the Clang 'c++-analyzer' Perl script with logic added to the Python wrapper script. This gives us the hooks we need to run analysis builds with "gomacc" (necessary for distributed builds) and gives us a convenient place to specify analyzer config flags which aren't exposed by scan-build.

Adds a warning suppression rule which prevents the analyzer from raising false positives from stdlib code (code w/namespace "std").

Other changes:
* Modify Goma portions of the GCC toolchain GNI to build paths to
  the Clang static analyzer.
* Create an exception for ASM tool invocations, which don't play
  well with the analysis command line flags.
* Removed 'scan-build' DEP.

BUG=687245
R=thakis@chromium.org,wez@chromium.org

Review-Url: https://codereview.chromium.org/2667853004
Cr-Commit-Position: refs/heads/master@{#456136}

DEPS

@@ -106,10 +106,6 @@ vars = {
   # and whatever else without interference from each other.
   'devtools_node_modules_revision': '6226d6cd80aaf2e5295ed460cf73ef6a582e4d78',
   # Three lines of non-changing comments so that
-  # the commit queue can handle CLs rolling libFuzzer
-  # and whatever else without interference from each other.
-  'scanbuild_revision': '15bd7ca2934162c51654ddffc52933e45f95e7ef',
-  # Three lines of non-changing comments so that
   # the commit queue can handle CLs rolling libprotobuf-mutator
   # and whatever else without interference from each other.
   'libprotobuf-mutator': '2d609dfa69f670f6a1daf80eaa2d538f96120b2d',
@@ -314,9 +310,6 @@ deps = {
 
   'src/third_party/visualmetrics/src':
     Var('chromium_git') + '/external/github.com/WPO-Foundation/visualmetrics.git' + '@' +  '1edde9d2fe203229c895b648fdec355917200ad6',
-
-  'src/third_party/scan-build/src':
-    Var('chromium_git') + '/chromium/llvm-project/cfe/tools/scan-build.git' + '@' +  Var('scanbuild_revision'),
 }
 
 

build/toolchain/clang_static_analyzer_wrapper.py

 #!/usr/bin/env python
-# Copyright 2016 The Chromium Authors. All rights reserved.
+# Copyright 2017 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""Invokes the Clang static analysis command using arguments provided on the
-command line.
+"""Adds an analysis build step to invocations of the Clang C/C++ compiler.
+
+Usage: clang_static_analyzer_wrapper.py <compiler> [args...]
 """
 
 import argparse
 import fnmatch
+import itertools
 import os
-import shutil
 import sys
-import tempfile
-
 import wrapper_utils
 
+# Flags used to enable analysis for Clang invocations.
+analyzer_enable_flags = [
+    '--analyze',
+    '-fdiagnostics-show-option',
+]
+
+# Flags used to configure the analyzer's behavior.
+analyzer_option_flags = [
+    '-analyzer-checker=cplusplus',
+    '-analyzer-opt-analyze-nested-blocks',
+    '-analyzer-eagerly-assume',
+    '-analyzer-output=text',
+    '-analyzer-config',
+    'suppress-c++-stdlib=true',
+
+# List of checkers to execute.
+# The full list of checkers can be found at
+# https://clang-analyzer.llvm.org/available_checks.html.
+    '-analyzer-checker=core',
+    '-analyzer-checker=unix',
+    '-analyzer-checker=deadcode',
+]
+
 
 def main():
-  parser = argparse.ArgumentParser(description=__doc__)
-  parser.add_argument('--clang-cc-path',
-                      help='Path to the clang compiler.',
-                      metavar='PATH')
-  parser.add_argument('--clang-cxx-path',
-                      help='Path to the clang++ compiler',
-                      metavar='PATH')
-  parser.add_argument('--analyzer',
-                      help='Path to the language-specific Clang analysis tool.',
-                      required=True,
-                      metavar='PATH')
-  args, compile_args = parser.parse_known_args()
-
-  # Check that only one of --clang-cc-path or --clang-cxx-path are set.
-  assert ((args.clang_cc_path != None) != (args.clang_cxx_path != None))
-
-  is_cxx = args.clang_cxx_path != None
-  env = os.environ
-  env['CCC_ANALYZER_FORCE_ANALYZE_DEBUG_CODE'] = '0'
-  env['CCC_ANALYZER_OUTPUT_FORMAT'] = 'text'
-  clang_path = args.clang_cxx_path or args.clang_cc_path
-  if is_cxx:
-    env['CCC_CXX'] = clang_path
-    env['CLANG_CXX'] = clang_path
-  else:
-    env['CCC_CC'] = clang_path
-    env['CLANG'] = clang_path
-
-  # TODO(kmarshall): Place the summarized output in a useful directory.
-  temp_dir = tempfile.mkdtemp()
-  try:
-    env['CCC_ANALYZER_HTML'] = temp_dir
-    returncode, stderr = wrapper_utils.CaptureCommandStderr(
-        wrapper_utils.CommandToRun([args.analyzer] + compile_args), env)
-    sys.stderr.write(stderr)
+  args = sys.argv[1:]
+  assert args
+
+  # Build the object file and proceed with analysis if it is buildable.
+  returncode, stderr = wrapper_utils.CaptureCommandStderr(
+    wrapper_utils.CommandToRun(args))
+  sys.stderr.write(stderr)
+  if returncode != 0:
     return returncode
-  finally:
-    shutil.rmtree(temp_dir)
 
-if __name__ == "__main__":
+  # Now run the analyzer.
+
+  # Interleave 'analyzer_option_flags' flags w/'-Xanalyzer' so that Clang
+  # passes them to the analysis tool.
+  # e.g. ['-analyzer-foo', '-analyzer-bar'] => ['-Xanalyzer', '-analyzer-foo',
+  #                                             '-Xanalyzer', '-analyzer-bar']
+  interleaved_analyzer_flags = list(sum(zip(
+      ['-Xanalyzer'] * len(analyzer_option_flags),
+      analyzer_option_flags), ()))
+  returncode, stderr = wrapper_utils.CaptureCommandStderr(
+      wrapper_utils.CommandToRun(args + analyzer_enable_flags +
+                                 interleaved_analyzer_flags))
+  sys.stderr.write(stderr)
+  if returncode != 0:
+    sys.stderr.write(
+        """WARNING! The Clang static analyzer exited with error code %d.
+         Please share the error details in crbug.com/695243 if this looks like
+         a new regression.\n""" % (returncode))
+
+  return 0
+
+if __name__ == '__main__':
   sys.exit(main())

build/toolchain/gcc_toolchain.gni

@@ -12,6 +12,13 @@ import("//build/toolchain/clang_static_analyzer.gni")
 import("//build/toolchain/goma.gni")
 import("//build/toolchain/toolchain.gni")
 
+# Path to the Clang static analysis wrapper script.
+# REVIEWERS: can you suggest a better location for this?
+# GN is really picky about dead stores of variables except at the global scope.
+analyzer_wrapper =
+    rebase_path("//build/toolchain/clang_static_analyzer_wrapper.py",
+                root_build_dir)
+
 # This template defines a toolchain for something that works like gcc
 # (including clang).
 #
@@ -126,22 +133,49 @@ template("gcc_toolchain") {
     } else {
       toolchain_cc_wrapper = cc_wrapper
     }
+    assert(!(toolchain_cc_wrapper != "" && toolchain_uses_goma),
+           "Goma and cc_wrapper can't be used together.")
 
-    # Compute the compiler prefix.
+    # When the invoker has explicitly overridden use_goma or cc_wrapper in the
+    # toolchain args, use those values, otherwise default to the global one.
+    # This works because the only reasonable override that toolchains might
+    # supply for these values are to force-disable them.
     if (toolchain_uses_goma) {
-      assert(toolchain_cc_wrapper == "",
-             "Goma and cc_wrapper can't be used together.")
-      compiler_prefix = "$goma_dir/gomacc "
-    } else if (toolchain_cc_wrapper != "") {
-      compiler_prefix = toolchain_cc_wrapper + " "
+      goma_path = "$goma_dir/gomacc"
+
+      # Use the static analysis script if static analysis is turned on
+      # AND the tool has not opted out by setting
+      # 'is_clang_static_analysis_supported' to false.
+      if (is_clang && use_clang_static_analyzer &&
+          (!defined(invoker.is_clang_analysis_supported) ||
+           invoker.is_clang_analysis_supported)) {
+        compiler_prefix = "${analyzer_wrapper} ${goma_path} "
+
+        # Create a distinct variable for "asm", since analysis runs pass
+        # a bunch of flags to clang/clang++ that are nonsensical on assembler
+        # runs.
+        asm = "${goma_path} ${invoker.cc}"
+      } else {
+        compiler_prefix = "${goma_path} "
+      }
     } else {
-      compiler_prefix = ""
+      if (is_clang && use_clang_static_analyzer &&
+          (!defined(invoker.is_clang_analysis_supported) ||
+           invoker.is_clang_analysis_supported)) {
+        compiler_prefix = "${analyzer_wrapper} "
+        asm = invoker.cc
+      } else {
+        compiler_prefix = "${toolchain_cc_wrapper} "
+      }
     }
 
     cc = compiler_prefix + invoker.cc
     cxx = compiler_prefix + invoker.cxx
     ar = invoker.ar
     ld = invoker.ld
+    if (!defined(asm)) {
+      asm = cc
+    }
     if (defined(invoker.readelf)) {
       readelf = invoker.readelf
     } else {
@@ -263,7 +297,7 @@ template("gcc_toolchain") {
     tool("asm") {
       # For GCC we can just use the C compiler to compile assembly.
       depfile = "{{output}}.d"
-      command = "$cc -MMD -MF $depfile ${rebuild_string}{{defines}} {{include_dirs}} {{asmflags}} -c {{source}} -o {{output}}"
+      command = "$asm -MMD -MF $depfile ${rebuild_string}{{defines}} {{include_dirs}} {{asmflags}} -c {{source}} -o {{output}}"
       depsformat = "gcc"
       description = "ASM {{output}}"
       outputs = [
@@ -518,34 +552,15 @@ template("clang_toolchain") {
     cc = "$prefix/clang"
     cxx = "$prefix/clang++"
     ld = cxx
-
-    if (use_clang_static_analyzer) {
-      # Static analysis isn't supported under GOMA. See crbug.com/687245
-      # for progress on this issue.
-      assert(!use_goma, "'use_clang_static_analyzer' cannot be used with GOMA.")
-
-      # Call "ccc-analyzer" or "c++-analyzer" instead of directly calling Clang.
-      # |wrapper_tool| sets the environment variables which are read by the
-      # analyzer tools.
-      analyzer_wrapper =
-          rebase_path("//build/toolchain/clang_static_analyzer_wrapper.py",
-                      root_build_dir)
-      cc = analyzer_wrapper + " --clang-cc-path=${cc} --analyzer=" +
-           rebase_path("//third_party/scan-build/src/libexec/ccc-analyzer",
-                       root_build_dir)
-      cxx = analyzer_wrapper + " --clang-cxx-path=${cxx} --analyzer=" +
-            rebase_path("//third_party/scan-build/src/libexec/c++-analyzer",
-                        root_build_dir)
-    }
-
     readelf = "${toolprefix}readelf"
     ar = "${toolprefix}ar"
     nm = "${toolprefix}nm"
 
     forward_variables_from(invoker,
                            [
-                             "enable_linker_map",
                              "strip",
+                             "is_clang_analysis_supported",
+                             "enable_linker_map",
                            ])
 
     toolchain_args = {

build/toolchain/nacl/BUILD.gn

@@ -74,13 +74,16 @@ template("pnacl_toolchain") {
     if (defined(invoker.strip)) {
       strip = scriptprefix + toolprefix + invoker.strip + scriptsuffix
     }
+    forward_variables_from(invoker,
+                           [
+                             "executable_extension",
+                             "is_clang_analysis_supported",
+                           ])
 
     # Note this is not the usual "ld = cxx" because "ld" uses are
     # never run via goma, so this needs scriptprefix.
     ld = scriptprefix + toolprefix + "clang++" + scriptsuffix
 
-    executable_extension = invoker.executable_extension
-
     toolchain_args = {
       is_clang = true
       current_cpu = "pnacl"
@@ -104,6 +107,10 @@ pnacl_toolchain("newlib_pnacl") {
 pnacl_toolchain("newlib_pnacl_nonsfi") {
   executable_extension = ""
   strip = "strip"
+
+  if (use_clang_static_analyzer) {
+    is_clang_analysis_supported = false
+  }
 }
 
 template("nacl_glibc_toolchain") {

build/toolchain/nacl_toolchain.gni

@@ -32,6 +32,7 @@ template("nacl_toolchain") {
                              "cc",
                              "cxx",
                              "deps",
+                             "is_clang_analysis_supported",
                              "ld",
                              "link_outputs",
                              "nm",

docs/clang_static_analyzer.md

@@ -17,6 +17,3 @@ use_clang_static_analyzer = true
 The next time you rebuild, you should see static analysis warnings appear inline
 with the usual Clang build warnings and errors.
 
-## Future plans/potential issues
-* Support for running under GOMA is untested, but will be added shortly if
-  feasible.