Skip to content

Commit

Permalink
Fix LLVM compilation issues (#2198)
Browse files Browse the repository at this point in the history
We've been having issues with asan builds on linux. This change should fix all of that. A build run can be found at:
https://github.com/carbon-language/carbon-lang/actions/runs/3093378863/jobs/5005683059

(currently in progress, but I'm expecting it to succeed at this point)

It may be that the issues with asan builds were actually related to caching. That is, maybe the brew build command didn't change enough between v14 and v15 that the cache hits were still an issue. We did notice this with 15.0.0 versus 15.0.1 include paths (that is, bazel wasn't happy using the cached results of a 15.0.0 build due to the skew in include paths). In order to address this, I've added CACHE_VERSION to the remote_cache setup. I've also set up corresponding buckets in Cloud.

However, I'm also switching Linux to llvm-15 and apt. I'd originally been looking at this because the issues were linux-specific, and we've previously had linux-specific issues with Homebrew. Although it may have been the cache all along, I would prefer to keep this setup (if nothing else, it made the caching issues more obvious, even though we were still confused by the include path manifestation).
  • Loading branch information
jonmeow authored Sep 20, 2022
1 parent 174aff1 commit 9be6939
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 26 deletions.
33 changes: 20 additions & 13 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,33 +59,36 @@ jobs:
go get github.com/bazelbuild/bazelisk
echo "$(go env GOPATH)/bin" >> $GITHUB_PATH
# Setup to the latest LLVM and Clang release.
#
# Ideally we would use the pre-installed versions in the image, but the
# debian packages for LLVM-15 currently don't work for fastbuild.
#
# For now, we rely on Homebrew to manage installing a correctly built
# toolchain. We also take some care to be as resilient as possible to
# issues fetching and installing the toolchain.
- name: Install Clang/LLVM using brew
# On macOS, use Homebrew to install a recent LLVM and Clang.
- name: Setup LLVM and Clang (macOS)
if: matrix.os == 'macos-latest'
env:
HOMEBREW_NO_INSTALL_CLEANUP: 1
HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK: 1
# Use llvm@14 because llvm (15) has sanitizer issues.
run: |
echo '*** Updating brew'
brew update
echo '*** Installing LLVM deps'
brew install --force-bottle --only-dependencies llvm@14
brew install --force-bottle --only-dependencies llvm
echo '*** Installing LLVM itself'
brew install --force-bottle --force --verbose llvm@14
brew install --force-bottle --force --verbose llvm
echo '*** brew info llvm'
brew info llvm
echo '*** brew config'
brew config
echo '*** Updating PATH'
echo "$(brew --prefix llvm)/bin" >> $GITHUB_PATH
# On Ubuntu, use apt.llvm.org to install a recent LLVM and Clang.
- name: Setup LLVM and Clang (Ubuntu)
if: matrix.os == 'ubuntu-latest'
run: |
wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
sudo ./llvm.sh 15 all
rm llvm.sh
echo "/usr/lib/llvm-15/bin" >> $GITHUB_PATH
# Print the various tool paths and versions to help in debugging.
- name: Print tool debugging info
run: |
Expand Down Expand Up @@ -114,10 +117,14 @@ jobs:
# Add our bazel configuration and print basic info to ease debugging.
- name: Configure Bazel and print info
env:
# Add a cache version for changes that bazel won't otherwise detect,
# like llvm version changes.
CACHE_VERSION: 1
run: |
cat >user.bazelrc <<EOF
# Enable remote cache for our CI.
build --remote_cache=https://storage.googleapis.com/carbon-builds-github-${{ matrix.os }}
build --remote_cache=https://storage.googleapis.com/carbon-builds-github-v${CACHE_VERSION}-${{ matrix.os }}
build --google_credentials=$HOME/gcp-builds-service-account.json
# General build options.
Expand Down
4 changes: 4 additions & 0 deletions bazel/cc_toolchains/clang_cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ load(
":clang_detected_variables.bzl",
"clang_bindir",
"clang_version",
"clang_version_for_cache",
"clang_include_dirs_list",
"clang_resource_dir",
"llvm_bindir",
Expand Down Expand Up @@ -191,6 +192,9 @@ def _impl(ctx):
"-D__DATE__=\"redacted\"",
"-D__TIMESTAMP__=\"redacted\"",
"-D__TIME__=\"redacted\"",
# Pass the clang version as a define so that bazel
# caching is more likely to notice version changes.
"-DCLANG_VERSION_FOR_CACHE=\"%s\"" % clang_version_for_cache,
],
),
flag_group(
Expand Down
39 changes: 26 additions & 13 deletions bazel/cc_toolchains/clang_configuration.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,34 @@ def _run(repository_ctx, cmd):
return exec_result

def _clang_version(version_output):
"""Returns clang's major version number, or None if not found."""
"""Returns version information, or a (None, "unknown") tuple if not found.
Returns both the major version number (14) and the full version number for
caching.
"""
clang_version = None
clang_version_for_cache = "unknown"

version_prefix = "clang version "
version_start = version_output.find(version_prefix)
if version_start == -1:
# No version
return None
return (clang_version, clang_version_for_cache)
version_start += len(version_prefix)

# Find a dot to indicate something like 'clang version 14.0.6'.
version_dot = version_output.find(".", version_start)
if version_dot == -1:
return None
# Find the newline.
version_newline = version_output.find("\n", version_start)
if version_newline == -1:
return (clang_version, clang_version_for_cache)
clang_version_for_cache = version_output[version_start:version_newline]

# Make sure the dot was on the same line as the version.
if version_output.find("\n", version_start) < version_dot:
return None
# Find a dot to indicate something like 'clang version 14.0.6', and grab the
# major version.
version_dot = version_output.find(".", version_start)
if version_dot != -1 and version_dot < version_newline:
clang_version = int(version_output[version_start:version_dot])

# Return the version as int.
return int(version_output[version_start:version_dot])
return (clang_version, clang_version_for_cache)

def _detect_system_clang(repository_ctx):
"""Detects whether the system-provided clang can be used.
Expand All @@ -59,7 +68,8 @@ def _detect_system_clang(repository_ctx):
version_output = _run(repository_ctx, [cc_path, "--version"]).stdout
if "clang" not in version_output:
fail("Searching for clang or CC (%s), and found (%s), which is not a Clang compiler" % (cc, cc_path))
return (cc_path, _clang_version(version_output))
clang_version, clang_version_for_cache = _clang_version(version_output)
return (cc_path, clang_version, clang_version_for_cache)

def _compute_clang_resource_dir(repository_ctx, clang):
"""Runs the `clang` binary to get its resource dir."""
Expand Down Expand Up @@ -152,7 +162,9 @@ def _configure_clang_toolchain_impl(repository_ctx):
# here as the other LLVM tools may not be symlinked into the PATH even if
# `clang` is. We also insist on finding the basename of `clang++` as that is
# important for C vs. C++ compiles.
(clang, clang_version) = _detect_system_clang(repository_ctx)
(clang, clang_version, clang_version_for_cache) = _detect_system_clang(
repository_ctx,
)
clang = clang.realpath.dirname.get_child("clang++")

# Compute the various directories used by Clang.
Expand Down Expand Up @@ -190,6 +202,7 @@ def _configure_clang_toolchain_impl(repository_ctx):
"{LLVM_BINDIR}": str(arpath.dirname),
"{CLANG_BINDIR}": str(clang.dirname),
"{CLANG_VERSION}": str(clang_version),
"{CLANG_VERSION_FOR_CACHE}": clang_version_for_cache.replace('"', "_").replace("\\", "_"),
"{CLANG_RESOURCE_DIR}": resource_dir,
"{CLANG_INCLUDE_DIRS_LIST}": str(
[str(path) for path in include_dirs],
Expand Down
1 change: 1 addition & 0 deletions bazel/cc_toolchains/clang_detected_variables.tpl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ This file gets processed by a repository rule, substituting the
llvm_bindir = "{LLVM_BINDIR}"
clang_bindir = "{CLANG_BINDIR}"
clang_version = {CLANG_VERSION}
clang_version_for_cache = "{CLANG_VERSION_FOR_CACHE}"
clang_resource_dir = "{CLANG_RESOURCE_DIR}"
clang_include_dirs_list = {CLANG_INCLUDE_DIRS_LIST}
sysroot_dir = "{SYSROOT}"

0 comments on commit 9be6939

Please sign in to comment.