Add Clang static analyzer to Clang toolchain defs in GN

build/toolchain/clang_static_analyzer_wrapper.py

+#!/usr/bin/env python
+# Copyright 2016 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.
+"""
+
+import argparse
+import fnmatch
+import os
+import sys
+
+import wrapper_utils
+
+def run_command(command, args, env=os.environ):
+  returncode, stderr = wrapper_utils.CaptureCommandStderr(
+      wrapper_utils.CommandToRun([command] + args), env)
+  sys.stderr.write(stderr)
+  return returncode
+
+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.',
+                      metavar='PATH')
+  args, compile_args = parser.parse_known_args()
+
+  is_cxx = False
+  if args.clang_cxx_path != None:

[Wez, 2017/01/10 02:41:48]

Should we assert that clang_cc_path is None if so?

I'd suggest assigning to e.g. clang_command here, so that you'd have:

is_cxx = (args.clang_cxx_path != None)
clang_command = args.clang_cxx_path or args.clang_cc_path

Then most of the logic below can be expressed in terms of clang_command, with
just a couple of conditionals on is_cxx.

[Kevin M, 2017/01/10 19:12:27]

Did something different - moved the one-of validation to the top to clean up the
logic a bit.

+    is_cxx = True
+
+  command = None
+  env = os.environ
+  env['CCC_ANALYZER_FORCE_ANALYZE_DEBUG_CODE'] = '0'
+  clang_path = None
+
+  if args.clang_cc_path != None:
+    env['CCC_CC'] = args.clang_cc_path
+    env['CLANG'] = args.clang_cc_path
+    clang_path = args.clang_cc_path
+  if args.clang_cxx_path != None:
+    env['CCC_CXX'] = args.clang_cxx_path
+    env['CLANG'] = args.clang_cxx_path
+    clang_path = args.clang_cc_path

[Wez, 2017/01/10 02:41:48]

typo cc -> cxx

[Kevin M, 2017/01/10 19:12:27]

Done.

+
+  # Only one of CCC_CC or CCC_CXX must be set.
+  assert (env.has_key('CCC_CC') or env.has_key('CCC_CXX'))
+  assert (env.has_key('CCC_CC') != env.has_key('CCC_CXX'))
+
+  returncode = run_command(args.analyzer, compile_args, env)
+  if returncode == 0:
+    return 0
+  else:
+    sys.stderr.write('WARNING: static analysis failed with return code %d.' %
+                     returncode)
+
+  if is_cxx:
+    command = [args.clang_cxx_path] + compile_args

[Wez, 2017/01/10 02:41:48]

You don't seem to use |command|?

[Kevin M, 2017/01/10 19:12:27]

Done.

+    return run_command(args.clang_cxx_path, compile_args)

[Wez, 2017/01/10 02:41:48]

Why not have a single run_command that uses clang_path?

That said, it looks like you're simultaneously assuming that clang_cc/cxx_path
are directory paths for Clang, and that they're the full paths including the
actually command - which is it? Should you be using dirname() when you sassign
to env['CLANG'], for instance?

[Kevin M, 2017/01/10 19:12:27]

Need a fallback for the time being - there were a couple files that failed
analysis as a workaround for this minor bug in ccc-analyzer (observed when
building third_party/libxml). We could potentially send a fix pull request but
there's no need to block on it right now:

Use of uninitialized value $HtmlDir in concatenation (.) or string at
../../third_party/llvm/tools/clang/tools/scan-build/libexec/ccc-analyzer line
150, <$fh> line 7.
mkdir /failures: Permission denied at
../../third_party/llvm/tools/clang/tools/scan-build/libexec/ccc-analyzer line
151.
WARNING: static analysis failed with return code 13.Traceback (most recent call
last):
I don't understand - they are treated as fully qualified paths to the clang
executable. The list concatenation that I'm performing prior to running a
command is just following Python convention for joining command line arguments.
Instead of using spaces in a string as a delimiter, it breaks the arguments up
as discrete list items.

[Wez, 2017/01/14 00:04:58]

My comment was based on the fact that you were setting an env variable CLANG to
either clang_cxx_path or clang_cc, which given that the env variable was not
language specific implied it expected a directory path, not a filename.

In part I think the confusion is in the _path suffix, which is ambiguous in
meaning but which in the context of running shell commands I would tend to think
of as the directory in which the command resides, not the full path to the
command.

+  else:
+    return run_command(args.clang_cc_path, compile_args)
+
+if __name__ == "__main__":
+  sys.exit(main())