Move `llvm::Initialize*` calls to `main`. (#3449)

Per their documentation, the `llvm::Initialize*` functions are only
supposed to be called by the main program, not by a library like
toolchain/codegen. Fixes a hang due to a data race in multithreaded
autoupdate.

Add a utility class `Carbon::InitLLVM` to do the common LLVM
initialization shared by all Carbon tools, optionally including
initializing the LLVM targets. Because the LLVM targets add a lot of
binary size, only initialize them for binaries that opt in by depending
on a new target `//common:all_llvm_targets`.

Also fix `//explorer:file_test` and `//explorer:file_test.trace` to
share a binary rather than linking an identical binary twice.

---------

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>

bazel/cc_toolchains/clang_cc_toolchain_config.bzl

@@ -686,8 +686,7 @@ def _impl(ctx):
                             ),
                             flag_group(
                                 flags = ["-Wl,-whole-archive"],
-                                expand_if_true =
-                                    "libraries_to_link.is_whole_archive",
+                                expand_if_true = "libraries_to_link.is_whole_archive",
                             ),
                             flag_group(
                                 flags = ["%{libraries_to_link.object_files}"],
@@ -757,6 +756,137 @@ def _impl(ctx):
         ],
     )
 
+    macos_link_libraries_feature = feature(
+        name = "macos_link_libraries",
+        enabled = True,
+        flag_sets = [
+            flag_set(
+                actions = all_link_actions,
+                flag_groups = [
+                    flag_group(
+                        flags = ["%{linkstamp_paths}"],
+                        iterate_over = "linkstamp_paths",
+                        expand_if_available = "linkstamp_paths",
+                    ),
+                    flag_group(
+                        iterate_over = "libraries_to_link",
+                        flag_groups = [
+                            flag_group(
+                                flags = ["-Wl,--start-lib"],
+                                expand_if_equal = variable_with_value(
+                                    name = "libraries_to_link.type",
+                                    value = "object_file_group",
+                                ),
+                            ),
+                            flag_group(
+                                iterate_over = "libraries_to_link.object_files",
+                                expand_if_equal = variable_with_value(
+                                    name = "libraries_to_link.type",
+                                    value = "object_file_group",
+                                ),
+                                flag_groups = [
+                                    flag_group(
+                                        flags = ["%{libraries_to_link.object_files}"],
+                                        expand_if_false = "libraries_to_link.is_whole_archive",
+                                    ),
+                                    flag_group(
+                                        flags = ["-Wl,-force_load,%{libraries_to_link.object_files}"],
+                                        expand_if_true = "libraries_to_link.is_whole_archive",
+                                    ),
+                                ],
+                            ),
+                            flag_group(
+                                expand_if_equal = variable_with_value(
+                                    name = "libraries_to_link.type",
+                                    value = "object_file",
+                                ),
+                                flag_groups = [
+                                    flag_group(
+                                        flags = ["%{libraries_to_link.name}"],
+                                        expand_if_false = "libraries_to_link.is_whole_archive",
+                                    ),
+                                    flag_group(
+                                        flags = ["-Wl,-force_load,%{libraries_to_link.name}"],
+                                        expand_if_true = "libraries_to_link.is_whole_archive",
+                                    ),
+                                ],
+                            ),
+                            flag_group(
+                                expand_if_equal = variable_with_value(
+                                    name = "libraries_to_link.type",
+                                    value = "interface_library",
+                                ),
+                                flag_groups = [
+                                    flag_group(
+                                        flags = ["%{libraries_to_link.name}"],
+                                        expand_if_false = "libraries_to_link.is_whole_archive",
+                                    ),
+                                    flag_group(
+                                        flags = ["-Wl,-force_load,%{libraries_to_link.name}"],
+                                        expand_if_true = "libraries_to_link.is_whole_archive",
+                                    ),
+                                ],
+                            ),
+                            flag_group(
+                                expand_if_equal = variable_with_value(
+                                    name = "libraries_to_link.type",
+                                    value = "static_library",
+                                ),
+                                flag_groups = [
+                                    flag_group(
+                                        flags = ["%{libraries_to_link.name}"],
+                                        expand_if_false = "libraries_to_link.is_whole_archive",
+                                    ),
+                                    flag_group(
+                                        flags = ["-Wl,-force_load,%{libraries_to_link.name}"],
+                                        expand_if_true = "libraries_to_link.is_whole_archive",
+                                    ),
+                                ],
+                            ),
+                            flag_group(
+                                flags = ["-l%{libraries_to_link.name}"],
+                                expand_if_equal = variable_with_value(
+                                    name = "libraries_to_link.type",
+                                    value = "dynamic_library",
+                                ),
+                            ),
+                            flag_group(
+                                flags = ["-l:%{libraries_to_link.name}"],
+                                expand_if_equal = variable_with_value(
+                                    name = "libraries_to_link.type",
+                                    value = "versioned_dynamic_library",
+                                ),
+                            ),
+                            flag_group(
+                                expand_if_true = "libraries_to_link.is_whole_archive",
+                                flag_groups = [
+                                    flag_group(
+                                        expand_if_false = "macos_flags",
+                                        flags = ["-Wl,-no-whole-archive"],
+                                    ),
+                                ],
+                            ),
+                            flag_group(
+                                flags = ["-Wl,--end-lib"],
+                                expand_if_equal = variable_with_value(
+                                    name = "libraries_to_link.type",
+                                    value = "object_file_group",
+                                ),
+                            ),
+                        ],
+                        expand_if_available = "libraries_to_link",
+                    ),
+                    # Note that the params file comes at the end, after the
+                    # libraries to link above.
+                    flag_group(
+                        expand_if_available = "linker_param_file",
+                        flags = ["@%{linker_param_file}"],
+                    ),
+                ],
+            ),
+        ],
+    )
+
     # Place user provided compile flags after all the features so that these
     # flags can override or customize behavior. The only thing user flags
     # cannot override is the output file as Bazel depends on that.
@@ -913,10 +1043,11 @@ def _impl(ctx):
         features.append(x86_64_cpu_flags)
 
     # Finally append the libraries to link and any final flags.
-    features += [
-        default_link_libraries_feature,
-        final_flags_feature,
-    ]
+    if ctx.attr.target_cpu in ["darwin", "darwin_arm64"]:
+        features.append(macos_link_libraries_feature)
+    else:
+        features.append(default_link_libraries_feature)
+    features.append(final_flags_feature)
 
     return cc_common.create_cc_toolchain_config_info(
         ctx = ctx,

common/BUILD

@@ -165,6 +165,32 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "init_llvm",
+    srcs = ["init_llvm.cpp"],
+    hdrs = ["init_llvm.h"],
+    deps = [
+        "@llvm-project//llvm:Support",
+    ],
+)
+
+# Link against this to cause `:init_llvm` to pull in all LLVM targets.
+#
+# Be careful when depending on this: it pulls in several hundred megabytes of
+# LLVM binary size in -c fastbuild. This should only be depended on by a
+# `cc_binary` or `cc_test` target, never a `cc_library`.
+cc_library(
+    name = "all_llvm_targets",
+    srcs = ["all_llvm_targets.cpp"],
+    deps = [
+        ":init_llvm",
+        "@llvm-project//llvm:AllTargetsAsmParsers",
+        "@llvm-project//llvm:AllTargetsCodeGens",
+        "@llvm-project//llvm:Support",
+    ],
+    alwayslink = 1,
+)
+
 cc_library(
     name = "ostream",
     hdrs = ["ostream.h"],

common/all_llvm_targets.cpp

@@ -0,0 +1,22 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "common/init_llvm.h"
+#include "llvm/Support/TargetSelect.h"
+
+namespace Carbon {
+
+static auto InitLLVMTargets() -> void {
+  llvm::InitializeAllTargetInfos();
+  llvm::InitializeAllTargets();
+  llvm::InitializeAllTargetMCs();
+  llvm::InitializeAllAsmParsers();
+  llvm::InitializeAllAsmPrinters();
+}
+
+// On program startup, set `InitLLVM::InitializeTargets` to be our
+// initialization function so that `InitLLVM` can call it at the right moment.
+char InitLLVM::RegisterTargets = (InitializeTargets = &InitLLVMTargets, 0);
+
+}  // namespace Carbon

common/init_llvm.cpp

@@ -0,0 +1,39 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "common/init_llvm.h"
+
+#include "llvm/Support/TargetSelect.h"
+
+namespace Carbon {
+
+InitLLVM::InitLLVM(int& argc, char**& argv)
+    : init_llvm_(argc, argv),
+      // LLVM assumes that argc and argv won't change, and registers them with
+      // an `llvm::PrettyStackTraceProgram` that will crash if an argv element
+      // gets nulled out, which for example `testing::InitGoogleTest` does. So
+      // make a copy of the argv that LLVM produces in order to support
+      // mutation.
+      args_(argv, argv + argc) {
+  // Return our mutable copy of argv for the program to use.
+  argc = args_.size();
+  argv = args_.data();
+
+  // `argv[argc]` is expected to be a null pointer.
+  args_.push_back(0);
+
+  llvm::setBugReportMsg(
+      "Please report issues to "
+      "https://github.com/carbon-language/carbon-lang/issues and include the "
+      "crash backtrace.\n");
+
+  // Initialize LLVM targets if //common:all_llvm_targets was linked in.
+  if (InitializeTargets) {
+    InitializeTargets();
+  }
+}
+
+auto (*InitLLVM::InitializeTargets)() -> void = nullptr;
+
+}  // namespace Carbon

common/init_llvm.h

@@ -0,0 +1,42 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_COMMON_INIT_LLVM_H_
+#define CARBON_COMMON_INIT_LLVM_H_
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/InitLLVM.h"
+
+namespace Carbon {
+
+// A RAII class to handle initializing LLVM and shutting it down. An instance of
+// this class should be created in the `main` function of each Carbon binary
+// that interacts with LLVM, before `argc` and `argv` are first inspected.
+class InitLLVM {
+ public:
+  // Initializes LLVM for use by a Carbon binary. On Windows, `argc` and `argv`
+  // are updated to refer to properly-encoded UTF-8 versions of the command line
+  // arguments.
+  explicit InitLLVM(int& argc, char**& argv);
+
+  // Shuts down LLVM.
+  ~InitLLVM() = default;
+
+ private:
+  llvm::InitLLVM init_llvm_;
+  llvm::SmallVector<char*> args_;
+
+  // A pointer to the LLVM target initialization function, if :all_llvm_targets
+  // is linked in. Otherwise nullptr.
+  static auto (*InitializeTargets)() -> void;
+
+  // The initializer of this static data member populates `InitializeTargets`.
+  // Defined only if :all_llvm_targets is linked in. This is a member so that
+  // it has access to `InitializeTargets`.
+  static char RegisterTargets;
+};
+
+}  // namespace Carbon
+
+#endif  // CARBON_COMMON_INIT_LLVM_H_

explorer/BUILD

@@ -42,8 +42,8 @@ cc_binary(
     ],
 )
 
-cc_library(
-    name = "file_test_common",
+cc_binary(
+    name = "file_test",
     testonly = 1,
     srcs = ["file_test.cpp"],
     deps = [
@@ -57,20 +57,18 @@ cc_library(
 )
 
 file_test(
-    name = "file_test",
+    name = "file_test.notrace",
     size = "small",
+    prebuilt_binary = ":file_test",
     shard_count = 20,
     tests = glob(["testdata/**/*.carbon"]),
-    deps = [
-        ":file_test_common",
-        "@com_googlesource_code_re2//:re2",
-    ],
 )
 
 file_test(
     name = "file_test.trace",
     size = "small",
     args = ["--trace"],
+    prebuilt_binary = ":file_test",
     shard_count = 30,
     tests = glob(
         ["testdata/**/*.carbon"],
@@ -86,7 +84,6 @@ file_test(
             "testdata/linked_list/typed_linked_list.carbon",
         ],
     ),
-    deps = [":file_test_common"],
 )
 
 glob_sh_run(

testing/base/BUILD

@@ -20,8 +20,8 @@ cc_library(
     testonly = 1,
     srcs = ["gtest_main.cpp"],
     deps = [
+        "//common:init_llvm",
         "@com_google_googletest//:gtest",
-        "@llvm-project//llvm:Support",
     ],
 )
 

testing/base/gtest_main.cpp

@@ -4,14 +4,10 @@
 
 #include <gtest/gtest.h>
 
-#include "llvm/Support/InitLLVM.h"
+#include "common/init_llvm.h"
 
 auto main(int argc, char** argv) -> int {
+  Carbon::InitLLVM init_llvm(argc, argv);
   testing::InitGoogleTest(&argc, argv);
-  llvm::setBugReportMsg(
-      "Please report issues to "
-      "https://github.com/carbon-language/carbon-lang/issues and include the "
-      "crash backtrace.\n");
-  llvm::InitLLVM init_llvm(argc, argv);
   return RUN_ALL_TESTS();
 }

testing/file_test/BUILD

@@ -33,6 +33,7 @@ cc_library(
         ":autoupdate",
         "//common:check",
         "//common:error",
+        "//common:init_llvm",
         "//common:ostream",
         "@com_google_absl//absl/flags:flag",
         "@com_google_absl//absl/flags:parse",

testing/file_test/file_test_base.cpp

@@ -14,10 +14,10 @@
 #include "absl/flags/parse.h"
 #include "common/check.h"
 #include "common/error.h"
+#include "common/init_llvm.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/FormatVariadic.h"
-#include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
@@ -712,16 +712,17 @@ static auto GetTests() -> llvm::SmallVector<std::string> {
 
 // Implements main() within the Carbon::Testing namespace for convenience.
 static auto Main(int argc, char** argv) -> int {
-  absl::ParseCommandLine(argc, argv);
+  Carbon::InitLLVM init_llvm(argc, argv);
   testing::InitGoogleTest(&argc, argv);
-  llvm::setBugReportMsg(
-      "Please report issues to "
-      "https://github.com/carbon-language/carbon-lang/issues and include the "
-      "crash backtrace.\n");
-  llvm::InitLLVM init_llvm(argc, argv);
-
-  if (argc > 1) {
-    llvm::errs() << "Unexpected arguments starting at: " << argv[1] << "\n";
+  auto args = absl::ParseCommandLine(argc, argv);
+
+  if (args.size() > 1) {
+    llvm::errs() << "Unexpected arguments:";
+    for (char* arg : llvm::ArrayRef(args).drop_front()) {
+      llvm::errs() << " ";
+      llvm::errs().write_escaped(arg);
+    }
+    llvm::errs() << "\n";
     return EXIT_FAILURE;
   }
 

testing/file_test/rules.bzl

@@ -38,7 +38,13 @@ _tests_as_input_file_rule = rule(
     implementation = _tests_as_input_file_rule_impl,
 )
 
-def file_test(name, tests, data = [], args = [], **kwargs):
+def file_test(
+        name,
+        tests,
+        data = [],
+        args = [],
+        prebuilt_binary = None,
+        **kwargs):
     """Generates tests using the file_test base.
 
     There will be one main test using `name` that can be sharded, and includes
@@ -50,6 +56,8 @@ def file_test(name, tests, data = [], args = [], **kwargs):
       tests: The list of test files to use as data, typically a glob.
       data: Passed to cc_test.
       args: Passed to cc_test.
+      prebuilt_binary: If set, specifies a prebuilt test binary to use instead
+                       of building a new one.
       **kwargs: Passed to cc_test.
     """
 
@@ -60,9 +68,21 @@ def file_test(name, tests, data = [], args = [], **kwargs):
         data = tests,
         testonly = 1,
     )
-    cc_test(
-        name = name,
-        data = [tests_file] + tests + data,
-        args = ["--test_targets_file=$(rootpath :{0})".format(tests_file)] + args,
-        **kwargs
-    )
+    args = ["--test_targets_file=$(rootpath :{0})".format(tests_file)] + args
+    data = [tests_file] + tests + data
+
+    if prebuilt_binary:
+        native.sh_test(
+            name = name,
+            srcs = [prebuilt_binary],
+            data = data,
+            args = args,
+            **kwargs
+        )
+    else:
+        cc_test(
+            name = name,
+            data = data,
+            args = args,
+            **kwargs
+        )

toolchain/codegen/BUILD

@@ -16,8 +16,6 @@ cc_library(
     srcs = ["codegen.cpp"],
     hdrs = ["codegen.h"],
     deps = [
-        "@llvm-project//llvm:AllTargetsAsmParsers",
-        "@llvm-project//llvm:AllTargetsCodeGens",
         "@llvm-project//llvm:Core",
         "@llvm-project//llvm:MC",
         "@llvm-project//llvm:Support",

toolchain/codegen/codegen.cpp

@@ -8,7 +8,6 @@
 
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/MC/TargetRegistry.h"
-#include "llvm/Support/TargetSelect.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/TargetParser/Host.h"
 
@@ -17,13 +16,6 @@ namespace Carbon {
 auto CodeGen::Create(llvm::Module& module, llvm::StringRef target_triple,
                      llvm::raw_pwrite_stream& errors)
     -> std::optional<CodeGen> {
-  // Initialize the target registry etc.
-  llvm::InitializeAllTargetInfos();
-  llvm::InitializeAllTargets();
-  llvm::InitializeAllTargetMCs();
-  llvm::InitializeAllAsmParsers();
-  llvm::InitializeAllAsmPrinters();
-
   std::string error;
   const llvm::Target* target =
       llvm::TargetRegistry::lookupTarget(target_triple, error);

toolchain/driver/BUILD

@@ -44,6 +44,7 @@ cc_test(
     srcs = ["driver_test.cpp"],
     deps = [
         ":driver",
+        "//common:all_llvm_targets",
         "//testing/base:gtest_main",
         "//testing/base:test_raw_ostream",
         "//toolchain/diagnostics:diagnostic_emitter",
@@ -73,7 +74,9 @@ cc_binary(
     env = cc_env(),
     deps = [
         ":driver",
+        "//common:all_llvm_targets",
         "//common:bazel_working_dir",
+        "//common:init_llvm",
         "@llvm-project//llvm:Support",
     ],
 )

toolchain/driver/driver_main.cpp

@@ -5,24 +5,20 @@
 #include <cstdlib>
 
 #include "common/bazel_working_dir.h"
+#include "common/init_llvm.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/InitLLVM.h"
 #include "toolchain/driver/driver.h"
 
 auto main(int argc, char** argv) -> int {
+  Carbon::InitLLVM init_llvm(argc, argv);
+
   if (argc < 1) {
     return EXIT_FAILURE;
   }
 
   Carbon::SetWorkingDirForBazel();
 
-  llvm::setBugReportMsg(
-      "Please report issues to "
-      "https://github.com/carbon-language/carbon-lang/issues and include the "
-      "crash backtrace.\n");
-  llvm::InitLLVM init_llvm(argc, argv);
-
   // Printing to stderr should flush stdout. This is most noticeable when stderr
   // is piped to stdout.
   llvm::errs().tie(&llvm::outs());

toolchain/testing/BUILD

@@ -21,6 +21,7 @@ file_test(
         "//toolchain/parse:testdata",
     ],
     deps = [
+        "//common:all_llvm_targets",
         "//testing/file_test:file_test_base",
         "//toolchain/driver",
         "@llvm-project//llvm:Support",