From 9be6939ee9490823f9fbafddee3d07000ebccf89 Mon Sep 17 00:00:00 2001
From: Jon Ross-Perkins <jperkins@google.com>
Date: Tue, 20 Sep 2022 14:21:02 -0700
Subject: [PATCH] Fix LLVM compilation issues (#2198)

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).
---
 .github/workflows/tests.yaml                  | 33 +++++++++-------
 .../clang_cc_toolchain_config.bzl             |  4 ++
 bazel/cc_toolchains/clang_configuration.bzl   | 39 ++++++++++++-------
 .../clang_detected_variables.tpl.bzl          |  1 +
 4 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
index 1db77f739c6c..f13ebe946097 100644
--- a/.github/workflows/tests.yaml
+++ b/.github/workflows/tests.yaml
@@ -59,26 +59,19 @@ 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'
@@ -86,6 +79,16 @@ jobs:
           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: |
@@ -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.
diff --git a/bazel/cc_toolchains/clang_cc_toolchain_config.bzl b/bazel/cc_toolchains/clang_cc_toolchain_config.bzl
index 8db25228df84..342b5a7f9c57 100644
--- a/bazel/cc_toolchains/clang_cc_toolchain_config.bzl
+++ b/bazel/cc_toolchains/clang_cc_toolchain_config.bzl
@@ -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",
@@ -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(
diff --git a/bazel/cc_toolchains/clang_configuration.bzl b/bazel/cc_toolchains/clang_configuration.bzl
index f20f07cc5120..a2cd75d5154c 100644
--- a/bazel/cc_toolchains/clang_configuration.bzl
+++ b/bazel/cc_toolchains/clang_configuration.bzl
@@ -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'.
+    # 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]
+
+    # 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:
-        return None
+    if version_dot != -1 and version_dot < version_newline:
+        clang_version = int(version_output[version_start:version_dot])
 
-    # Make sure the dot was on the same line as the version.
-    if version_output.find("\n", version_start) < version_dot:
-        return None
-
-    # 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.
@@ -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."""
@@ -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.
@@ -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],
diff --git a/bazel/cc_toolchains/clang_detected_variables.tpl.bzl b/bazel/cc_toolchains/clang_detected_variables.tpl.bzl
index 955456a5e99f..372474694cfc 100644
--- a/bazel/cc_toolchains/clang_detected_variables.tpl.bzl
+++ b/bazel/cc_toolchains/clang_detected_variables.tpl.bzl
@@ -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}"