Skip to content

Commit

Permalink
[Vim/YCM] Fix hang/crash when no Clang command line is available.
Browse files Browse the repository at this point in the history
Previously, we tried to determine the command line for building a source
file by looking at the first output of said source file. This doesn't
work if the first output doesn't yield a clang command line. This patch
attempts to resolve this issue by going through all the build outputs of
a source file until one is found that yields a clang command line.

It is still possible to not find a Clang command line. In this case,
patch causes a graceful failure, rather than a crash.

R=jbroman@chromium.org,johnme@chromium.org
BUG=497787

Review URL: https://codereview.chromium.org/1156223007

Cr-Commit-Position: refs/heads/master@{#336467}
  • Loading branch information
asankah authored and Commit bot committed Jun 26, 2015
1 parent fc047f3 commit 234b8d2
Show file tree
Hide file tree
Showing 5 changed files with 477 additions and 27 deletions.
12 changes: 12 additions & 0 deletions tools/vim/PRESUBMIT.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright 2015 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.

"""Presubmit tests for /tools/vim.
Runs Python unit tests in /tools/vim/tests on upload.
"""

def CheckChangeOnUpload(input_api, output_api):
return input_api.canned_checks.RunUnitTestsInDirectory(
input_api, output_api, 'tests', whitelist=r'.*test.py')
119 changes: 92 additions & 27 deletions tools/vim/chromium.ycm_extra_conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,19 +169,18 @@ def FindChromeSrcFromFilename(filename):
return curdir


def GetDefaultCppFile(chrome_root, filename):
"""Returns the default target file to use for |filename|.
def GetDefaultSourceFile(chrome_root, filename):
"""Returns the default source file to use as an alternative to |filename|.
The default target is some source file that is known to exist and loosely
related to |filename|. Compile flags used to build the default target is
assumed to be a close-enough approximation for building |filename|.
Compile flags used to build the default source file is assumed to be a
close-enough approximation for building |filename|.
Args:
chrome_root: (String) Absolute path to the root of Chromium checkout.
filename: (String) Absolute path to the target source file.
filename: (String) Absolute path to the source file.
Returns:
(String) Absolute path to substitute target file.
(String) Absolute path to substitute source file.
"""
blink_root = os.path.join(chrome_root, 'third_party', 'WebKit')
if filename.startswith(blink_root):
Expand All @@ -190,15 +189,20 @@ def GetDefaultCppFile(chrome_root, filename):
return os.path.join(chrome_root, 'base', 'logging.cc')


def GetBuildTargetForSourceFile(chrome_root, filename):
"""Returns a build target corresponding to |filename|.
def GetBuildableSourceFile(chrome_root, filename):
"""Returns a buildable source file corresponding to |filename|.
A buildable source file is one which is likely to be passed into clang as a
source file during the build. For .h files, returns the closest matching .cc,
.cpp or .c file. If no such file is found, returns the same as
GetDefaultSourceFile().
Args:
chrome_root: (String) Absolute path to the root of Chromium checkout.
filename: (String) Absolute path to the target source file.
Returns:
(String) Absolute path to build target.
(String) Absolute path to source file.
"""
if filename.endswith('.h'):
# Header files can't be built. Instead, try to match a header file to its
Expand All @@ -209,46 +213,104 @@ def GetBuildTargetForSourceFile(chrome_root, filename):
if os.path.exists(alt_name):
return alt_name

# Failing that, build a default file instead and assume that the resulting
# commandline options are valid for the .h file.
return GetDefaultCppFile(chrome_root, filename)
return GetDefaultSourceFile(chrome_root, filename)

return filename


def GetClangCommandLineFromNinjaForFilename(out_dir, filename):
"""Returns the Clang command line for building |filename|
def GetNinjaBuildOutputsForSourceFile(out_dir, filename):
"""Returns a list of build outputs for filename.
Asks ninja for the list of commands used to build |filename| and returns the
final Clang invocation.
The list is generated by invoking 'ninja -t query' tool to retrieve a list of
inputs and outputs of |filename|. This list is then filtered to only include
.o and .obj outputs.
Args:
out_dir: (String) Absolute path to ninja build output directory.
filename: (String) Absolute path to source file.
Returns:
(String) Clang command line or None if command line couldn't be determined.
(List of Strings) List of target names. Will return [] if |filename| doesn't
yield any .o or .obj outputs.
"""
# Ninja needs the path to the source file relative to the output build
# directory.
rel_filename = os.path.relpath(os.path.realpath(filename), out_dir)

# Ask ninja how it would build our source file.
p = subprocess.Popen(['ninja', '-v', '-C', out_dir, '-t',
'commands', rel_filename + '^'],
p = subprocess.Popen(['ninja', '-C', out_dir, '-t', 'query', rel_filename],
stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
stdout, _ = p.communicate()
if p.returncode:
return []

# The output looks like:
# ../../relative/path/to/source.cc:
# outputs:
# obj/reative/path/to/target.source.o
# obj/some/other/target2.source.o
# another/target.txt
#
outputs_text = stdout.partition('\n outputs:\n')[2]
output_lines = [line.strip() for line in outputs_text.split('\n')]
return [target for target in output_lines
if target and (target.endswith('.o') or target.endswith('.obj'))]


def GetClangCommandLineForNinjaOutput(out_dir, build_target):
"""Returns the Clang command line for building |build_target|
Asks ninja for the list of commands used to build |filename| and returns the
final Clang invocation.
Args:
out_dir: (String) Absolute path to ninja build output directory.
build_target: (String) A build target understood by ninja
Returns:
(String or None) Clang command line or None if a Clang command line couldn't
be determined.
"""
p = subprocess.Popen(['ninja', '-v', '-C', out_dir,
'-t', 'commands', build_target],
stdout=subprocess.PIPE)
stdout, stderr = p.communicate()
if p.returncode:
return None

# Ninja might execute several commands to build something. We want the last
# clang command.
# Ninja will return multiple build steps for all dependencies up to
# |build_target|. The build step we want is the last Clang invocation, which
# is expected to be the one that outputs |build_target|.
for line in reversed(stdout.split('\n')):
if 'clang' in line:
return line
return None


def GetClangCommandLineFromNinjaForSource(out_dir, filename):
"""Returns a Clang command line used to build |filename|.
The same source file could be built multiple times using different tool
chains. In such cases, this command returns the first Clang invocation. We
currently don't prefer one toolchain over another. Hopefully the tool chain
corresponding to the Clang command line is compatible with the Clang build
used by YCM.
Args:
out_dir: (String) Absolute path to Chromium checkout.
filename: (String) Absolute path to source file.
Returns:
(String or None): Command line for Clang invocation using |filename| as a
source. Returns None if no such command line could be found.
"""
build_targets = GetNinjaBuildOutputsForSourceFile(out_dir, filename)
for build_target in build_targets:
command_line = GetClangCommandLineForNinjaOutput(out_dir, build_target)
if command_line:
return command_line
return None


def GetNormalizedClangCommand(command, out_dir):
"""Gets the normalized Clang binary path if |command| is a Clang command.
Expand Down Expand Up @@ -363,14 +425,17 @@ def GetClangOptionsFromNinjaForFilename(chrome_root, filename):
from ninja_output import GetNinjaOutputDirectory
out_dir = os.path.realpath(GetNinjaOutputDirectory(chrome_root))

clang_line = GetClangCommandLineFromNinjaForFilename(
out_dir, GetBuildTargetForSourceFile(chrome_root, filename))
clang_line = GetClangCommandLineFromNinjaForSource(
out_dir, GetBuildableSourceFile(chrome_root, filename))
if not clang_line:
# If ninja didn't know about filename or it's companion files, then try a
# default build target. It is possible that the file is new, or build.ninja
# is stale.
clang_line = GetClangCommandLineFromNinjaForFilename(
out_dir, GetDefaultCppFile(chrome_root, filename))
clang_line = GetClangCommandLineFromNinjaForSource(
out_dir, GetDefaultSourceFile(chrome_root, filename))

if not clang_line:
return (additional_flags, [])

return GetClangOptionsFromCommandLine(clang_line, out_dir, additional_flags)

Expand Down
Loading

0 comments on commit 234b8d2

Please sign in to comment.