Python script to compile and link the integration tests using llvm's clang-cl

syzygy/integration_tests/make_integration_tests_clang.py

Index: syzygy/integration_tests/make_integration_tests_clang.py
diff --git a/syzygy/integration_tests/make_integration_tests_clang.py b/syzygy/integration_tests/make_integration_tests_clang.py
new file mode 100644
index 0000000000000000000000000000000000000000..9cc8fe2f3c38d8189007fcbca104cbd91c9a63a1
--- /dev/null
+++ b/syzygy/integration_tests/make_integration_tests_clang.py
@@ -0,0 +1,106 @@
+#!/usr/bin/env python
+# 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.

[Sébastien Marchand, 2017/06/22 19:44:48]

Add a comment to describe what this script does, e.g.:
https://github.com/google/syzygy/blob/master/syzygy/scripts/asan/minidump_sym...

[njanevsk, 2017/06/22 21:16:32]

Done.

+

[Sébastien Marchand, 2017/06/22 19:44:48]

Add a BL here.

Basically each "section" is separated by 2 BLs.

[njanevsk, 2017/06/22 21:16:33]

Done.

+import optparse
+import os
+import re
+import subprocess
+import sys
+

[Sébastien Marchand, 2017/06/22 19:34:17]

Add an extra BL here.

[njanevsk, 2017/06/22 21:16:32]

Done.

+_SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__))
+_SRC_DIR = os.path.abspath(os.path.join(os.path.join(_SCRIPT_DIR, os.pardir),

[Sébastien Marchand, 2017/06/22 19:34:17]

you can do os.path.abspath(os.path.join(_SCRIPT_DIR, os.pardir, os.pardir)) , no
need for the nested join as it can take any number of args.

[njanevsk, 2017/06/22 21:16:33]

Done.

+  os.pardir))

[Sébastien Marchand, 2017/06/22 19:34:17]

Indent at +4.

[njanevsk, 2017/06/22 21:16:32]

Done.

+_CLANG_PATH = (_SRC_DIR,

[Sébastien Marchand, 2017/06/22 19:34:16]

I'd call this _CLANG_CL_PATH

[njanevsk, 2017/06/22 21:16:32]

Done.

+  "/third_party/llvm-build/Release+Asserts/bin/clang-cl.exe")
+

[Sébastien Marchand, 2017/06/22 19:34:17]

Add an extra BL here.

[njanevsk, 2017/06/22 21:16:32]

Done.

+def compile(clang_path, source_files, src_dir):
+  """ Only compiles the integration tests but does not link.

[Sébastien Marchand, 2017/06/22 19:34:17]

Remove the space before "Only", also I'll just say "Compiles with Clang", this
isn't specific to the integration_tests.

Maybe call this compile_with_asan as the Asan flags are used?

[njanevsk, 2017/06/22 21:16:32]

Done.

+

[Sébastien Marchand, 2017/06/22 19:34:17]

Remove one of these 2 BLs.

[njanevsk, 2017/06/22 21:16:33]

Done.

+
+  Only compiles the files and isntruments them with LLVM's Asan but does not
+  link them. The linking is done separately in the method link.
+
+  Args:
+    clang_path: Path to the clang-cl compiler in the syzygy project.
+    source_files: The source files to be compiled.
+    src_dir: The repository where the syzgy src is located.
+  """
+  compiler_flags = ['-c',

[Sébastien Marchand, 2017/06/22 19:34:17]

Nit, I prefer this form:
foo = [
    'bar',
    'baz',
]

i.e., no value one the first line, and the closing bracket on its own line.

[njanevsk, 2017/06/22 21:16:32]

Done.

+                    '-fsanitize=address',
+                    '-mllvm',
+                    '-asan-instrumentation-with-call-threshold=0',
+                    '-mllvm',
+                    '-asan-stack=0',
+                    '-DUNICODE',
+                    '-D_UNICODE',
+                    '-DNOMINMAX',
+                    '-D_CRT_SECURE_NO_WARNINGS',
+                    '-I' + src_dir]
+                    
+  compile_command = [clang_path]
+  compile_command.extend(compiler_flags)
+  compile_command.extend(source_files)
+  
+  ret = subprocess.call(compile_command)
+  if ret != 0:
+    print "Failed compiling the integration_tests using clang-cl"
+  return ret
+

[Sébastien Marchand, 2017/06/22 19:44:48]

2 BLs between each function :)

[njanevsk, 2017/06/22 21:16:32]

Done.

+def link(clang_path, source_files, build_dir):
+  """ Links the object files and produces the integration_tests_clang_dll.dll.
+

[Sébastien Marchand, 2017/06/22 19:34:17]

Remove one of these BLs.

[njanevsk, 2017/06/22 21:16:33]

Done.

+
+  Links the object files and produces the dll. The object files have to be
+  produced by the compile method above.
+
+  Args:
+    clang_path: Path to the clang-cl compiler in the syzygy project.
+    source_files: The source file names which are converted to obj filenames.
+    build_dir: The directory where to produce the linked dll.
+  """
+  object_files = ([os.path.splitext(filename)[0] +
+    ".o" for filename in source_files])

[Sébastien Marchand, 2017/06/22 19:34:17]

Indent +2. When you continue a line over the next one you should have a 4 spaces
indentation.

[njanevsk, 2017/06/22 21:16:32]

Done.

+
+  compiler_controlled_linker_flags = ['-o',
+                                      (build_dir,

[Sébastien Marchand, 2017/06/22 19:34:17]

Why the parenthesis? Should you use os.path.join here?

[njanevsk, 2017/06/22 21:16:33]

To join the two. I tried os.path.join but I get an error when running the
script. I think it is not joining them correctly cause build_dir has ../.. in
it.

[Sébastien Marchand, 2017/06/23 18:01:54]

os.path.join handles '..', you should use it.

+                                      '\integration_tests_clang_dll.dll')]
+  
+  linker_flags = ['/link',
+                  '/dll',
+                  build_dir + '\export_dll.dll.lib',
+                  build_dir + '\syzyasan_rtl.dll.lib',
+                  '-defaultlib:libcmt']
+  
+  linker_command = [clang_path, '-m32']
+  linker_command.extend(object_files)
+  linker_command.extend(compiler_controlled_linker_flags)
+  linker_command.extend(linker_flags)
+  ret = subprocess.call(linker_command)
+  
+  if ret != 0:
+    print "Failed to link the integration_tests using clang-cl"
+
+def main():
+  parser = optparse.OptionParser(usage='%prog [options]')
+  parser.add_option('--build-dir', help='Syzygy build directory.')

[Sébastien Marchand, 2017/06/22 19:44:48]

help="Path to the build directory"? There's a directory called "build" in the
Syzygy repo.

[njanevsk, 2017/06/22 21:16:32]

To the Release directory not the build directory. Updated

[Sébastien Marchand, 2017/06/23 18:01:53]

You also want this to work in debug :), let's just call this --output-dir,
"Output directory where the compiled binary should be written." ?

+  parser.add_option('--input-files', help='files to be compiled and linked')
+  
+  options, _ = parser.parse_args()
+  
+  if not options.build_dir:
+    parser.error('--build-dir is required')
+  if not options.input_files:
+    parser.error('--input-files is required')
+
+  source_files = re.findall(r'\b(\w+.cc)\b', options.input_files)

[Sébastien Marchand, 2017/06/22 19:34:17]

using a Regex is overkill, you can use glob:
https://docs.python.org/2/library/glob.html

[Sébastien Marchand, 2017/06/22 19:44:48]

And as discussed in a previous patchset we should have 2 lists in the gyp file:
- the source files that should be compiled with Clang.
- the other ones.

+  ret = compile(_CLANG_PATH, source_files, _SRC_DIR)
+  if ret == 0:
+    link(_CLANG_PATH, source_files, options.build_dir)
+  else:
+    print "Compilation of integration_tests_clang_dll failed."
+    print "Skipping link step."

[Sébastien Marchand, 2017/06/22 19:34:17]

You should also return something !0 on failure so Gyp will know if it has failed
or not (in this case it'll consider that it always works even if it fails
because the return value is always 0).

[njanevsk, 2017/06/22 21:16:32]

Done.

+
+if __name__ == '__main__':
+  sys.exit(main())