Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add utilities for managing a toolchain install, and install and use LLD. #3993

Merged
merged 4 commits into from
Jun 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions bazel/carbon_rules/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ def carbon_binary(name, srcs):
#
# TODO: This is a hack; replace with something better once the toolchain
# supports doing so.
#
# TODO: Switch to the `prefix_root` based rule similar to linking when
# the prelude moves there.
out = src + ".o"
srcs_reordered = [s for s in srcs if s != src] + [src]
run_binary(
Expand All @@ -40,14 +43,13 @@ def carbon_binary(name, srcs):
#
# TODO: This will need to be revisited eventually.
objs = [s + ".o" for s in srcs]
run_binary(
native.genrule(
name = name + ".link",
tool = "//toolchain/driver:carbon",
args = (["link"] +
["$(location %s)" % s for s in objs] +
["--output=$(location %s)" % name]),
tools = [
"//toolchain/install:prefix_root/bin/carbon",
"//toolchain/install:install_data",
],
cmd = "$(execpath //toolchain/install:prefix_root/bin/carbon) link --output=$@ $(SRCS)",
srcs = objs,
outs = [name],
# `link` has a dependency on ld, which should be in /usr/bin.
env = {"PATH": "/usr/bin"},
)
12 changes: 11 additions & 1 deletion bazel/check_deps/check_non_test_cc_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,13 @@

# Other packages in the LLVM project shouldn't be accidentally used
# in Carbon. We can expand the above list if use cases emerge.
if package not in ("llvm", "lld", "clang", "clang-tools-extra/clangd"):
if package not in (
"llvm",
"lld",
"clang",
"clang-tools-extra/clangd",
"libunwind",
):
sys.exit(
"ERROR: unexpected dependency into the LLVM project: %s" % dep
)
Expand All @@ -69,6 +75,10 @@
if repo_base == "@@rules_cc" and rule == ":link_extra_lib":
continue

# An utility library provided by Bazel that is under a compatible license.
if repo_base == "@@bazel_tools" and rule == "tools/cpp/runfiles:runfiles":
continue

# These are stubs wrapping system libraries for LLVM. They aren't
# distributed and so should be fine.
if repo_base in (
Expand Down
3 changes: 2 additions & 1 deletion explorer/file_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ namespace {

class ExplorerFileTest : public FileTestBase {
public:
explicit ExplorerFileTest(llvm::StringRef test_name)
explicit ExplorerFileTest(llvm::StringRef /*exe_path*/,
llvm::StringRef test_name)
: FileTestBase(test_name),
prelude_line_re_(R"(prelude.carbon:(\d+))"),
timing_re_(R"((Time elapsed in \w+: )\d+(ms))") {
Expand Down
1 change: 1 addition & 0 deletions testing/file_test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ cc_library(
":autoupdate",
"//common:check",
"//common:error",
"//common:exe_path",
"//common:init_llvm",
"//common:ostream",
"@abseil-cpp//absl/flags:flag",
Expand Down
22 changes: 14 additions & 8 deletions testing/file_test/file_test_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "absl/flags/parse.h"
#include "common/check.h"
#include "common/error.h"
#include "common/exe_path.h"
#include "common/init_llvm.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/Twine.h"
Expand Down Expand Up @@ -826,6 +827,8 @@ static auto Main(int argc, char** argv) -> int {
return EXIT_FAILURE;
}

std::string exe_path = FindExecutablePath(argv[0]);

// Tests might try to read from stdin. Ensure those reads fail by closing
// stdin and reopening it as /dev/null. Note that STDIN_FILENO doesn't exist
// on Windows, but POSIX requires it to be 0.
Expand Down Expand Up @@ -854,8 +857,9 @@ static auto Main(int argc, char** argv) -> int {
std::mutex errs_mutex;

for (const auto& test_name : tests) {
pool.async([&test_factory, &errs_mutex, test_name] {
std::unique_ptr<FileTestBase> test(test_factory.factory_fn(test_name));
pool.async([&test_factory, &errs_mutex, &exe_path, test_name] {
std::unique_ptr<FileTestBase> test(
test_factory.factory_fn(exe_path, test_name));
auto result = test->Autoupdate();

// Guard access to llvm::errs, which is not thread-safe.
Expand All @@ -872,7 +876,8 @@ static auto Main(int argc, char** argv) -> int {
return EXIT_SUCCESS;
} else if (absl::GetFlag(FLAGS_dump_output)) {
for (const auto& test_name : tests) {
std::unique_ptr<FileTestBase> test(test_factory.factory_fn(test_name));
std::unique_ptr<FileTestBase> test(
test_factory.factory_fn(exe_path, test_name));
auto result = test->DumpOutput();
if (!result.ok()) {
llvm::errs() << "\n" << result.error().message() << "\n";
Expand All @@ -882,11 +887,12 @@ static auto Main(int argc, char** argv) -> int {
return EXIT_SUCCESS;
} else {
for (llvm::StringRef test_name : tests) {
testing::RegisterTest(test_factory.name, test_name.data(), nullptr,
test_name.data(), __FILE__, __LINE__,
[&test_factory, test_name = test_name]() {
return test_factory.factory_fn(test_name);
});
testing::RegisterTest(
test_factory.name, test_name.data(), nullptr, test_name.data(),
__FILE__, __LINE__,
[&test_factory, &exe_path, test_name = test_name]() {
return test_factory.factory_fn(exe_path, test_name);
});
}
return RUN_ALL_TESTS();
}
Expand Down
12 changes: 8 additions & 4 deletions testing/file_test/file_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,9 @@ struct FileTestFactory {
const char* name;

// A factory function for tests.
std::function<FileTestBase*(llvm::StringRef path)> factory_fn;
std::function<FileTestBase*(llvm::StringRef exe_path,
llvm::StringRef test_name)>
factory_fn;
};

// Must be implemented by the individual file_test to initialize tests.
Expand All @@ -198,9 +200,11 @@ struct FileTestFactory {
extern auto GetFileTestFactory() -> FileTestFactory;

// Provides a standard GetFileTestFactory implementation.
#define CARBON_FILE_TEST_FACTORY(Name) \
auto GetFileTestFactory() -> FileTestFactory { \
return {#Name, [](llvm::StringRef path) { return new Name(path); }}; \
#define CARBON_FILE_TEST_FACTORY(Name) \
auto GetFileTestFactory() -> FileTestFactory { \
return {#Name, [](llvm::StringRef exe_path, llvm::StringRef test_name) { \
return new Name(exe_path, test_name); \
}}; \
}

} // namespace Carbon::Testing
Expand Down
3 changes: 2 additions & 1 deletion testing/file_test/file_test_base_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ namespace {

class FileTestBaseTest : public FileTestBase {
public:
using FileTestBase::FileTestBase;
FileTestBaseTest(llvm::StringRef /*exe_path*/, llvm::StringRef test_name)
: FileTestBase(test_name) {}

auto Run(const llvm::SmallVector<llvm::StringRef>& test_args,
llvm::vfs::InMemoryFileSystem& fs, llvm::raw_pwrite_stream& stdout,
Expand Down
4 changes: 3 additions & 1 deletion toolchain/check/check_fuzzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
llvm::MemoryBuffer::getMemBuffer(data_ref, /*BufferName=*/TestFileName,
/*RequiresNullTerminator=*/false)));

// TODO: We should try to thread the executable path into here.
const auto install_paths = InstallPaths::Make("");
llvm::raw_null_ostream null_ostream;
Driver driver(fs, "", null_ostream, null_ostream);
Driver driver(fs, &install_paths, "", null_ostream, null_ostream);

// TODO: Get checking to a point where it can handle invalid parse trees
// without crashing.
Expand Down
13 changes: 12 additions & 1 deletion toolchain/driver/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ cc_library(
name = "clang_runner",
srcs = ["clang_runner.cpp"],
hdrs = ["clang_runner.h"],
data = [
"//toolchain/install:llvm_link_data",
],
deps = [
"//common:command_line",
"//common:ostream",
"//common:vlog",
"//toolchain/install:install_paths",
"@llvm-project//clang:basic",
"@llvm-project//clang:driver",
"@llvm-project//clang:frontend",
Expand Down Expand Up @@ -52,7 +56,10 @@ cc_library(
name = "driver",
srcs = ["driver.cpp"],
hdrs = ["driver.h"],
data = ["//core:prelude"],
data = [
"//core:prelude",
"//toolchain/install:install_lib_data",
],
textual_hdrs = ["flags.def"],
deps = [
":clang_runner",
Expand All @@ -63,6 +70,7 @@ cc_library(
"//toolchain/codegen",
"//toolchain/diagnostics:diagnostic_emitter",
"//toolchain/diagnostics:sorting_diagnostic_consumer",
"//toolchain/install:install_paths",
"//toolchain/lex",
"//toolchain/lower",
"//toolchain/parse",
Expand All @@ -86,6 +94,7 @@ cc_test(
"//testing/base:gtest_main",
"//testing/base:test_raw_ostream",
"//toolchain/diagnostics:diagnostic_emitter",
"//toolchain/install:install_paths",
"//toolchain/lex:tokenized_buffer_test_helpers",
"//toolchain/testing:yaml_test_helpers",
"@googletest//:gtest",
Expand All @@ -102,6 +111,7 @@ cc_fuzz_test(
deps = [
":driver",
"//testing/base:test_raw_ostream",
"//toolchain/install:install_paths",
"@llvm-project//llvm:Support",
],
)
Expand All @@ -116,6 +126,7 @@ cc_binary(
"//common:bazel_working_dir",
"//common:exe_path",
"//common:init_llvm",
"//toolchain/install:install_paths",
"@llvm-project//llvm:Support",
],
)
36 changes: 17 additions & 19 deletions toolchain/driver/clang_runner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,9 @@

namespace Carbon {

static auto GetExecutablePath(llvm::StringRef exe_name) -> std::string {
// If the `exe_name` isn't already a valid path, look it up.
if (!llvm::sys::fs::exists(exe_name)) {
if (llvm::ErrorOr<std::string> path_result =
llvm::sys::findProgramByName(exe_name)) {
return *path_result;
}
}

return exe_name.str();
}

ClangRunner::ClangRunner(llvm::StringRef exe_name, llvm::StringRef target,
llvm::raw_ostream* vlog_stream)
: exe_name_(exe_name),
exe_path_(GetExecutablePath(exe_name)),
ClangRunner::ClangRunner(const InstallPaths* install_paths,
llvm::StringRef target, llvm::raw_ostream* vlog_stream)
: installation_(install_paths),
target_(target),
vlog_stream_(vlog_stream),
diagnostic_ids_(new clang::DiagnosticIDs()) {}
Expand All @@ -70,7 +57,10 @@ auto ClangRunner::Run(llvm::ArrayRef<llvm::StringRef> args) -> bool {
// Render the arguments into null-terminated C-strings for use by the Clang
// driver. Command lines can get quite long in build systems so this tries to
// minimize the memory allocation overhead.
std::array<llvm::StringRef, 1> exe_arg = {exe_name_};

// Start with a dummy executable name. We'll manually set the install
// directory below.
std::array<llvm::StringRef, 1> exe_arg = {"clang-runner"};
auto args_range =
llvm::concat<const llvm::StringRef>(exe_arg, maybe_v_arg, args);
int total_size = 0;
Expand All @@ -91,7 +81,7 @@ auto ClangRunner::Run(llvm::ArrayRef<llvm::StringRef> args) -> bool {
cstr_arg_storage[i] = '\0';
++i;
}
for (const char* cstr_arg : llvm::ArrayRef(cstr_args).drop_front()) {
for (const char* cstr_arg : llvm::ArrayRef(cstr_args)) {
CARBON_VLOG() << " '" << cstr_arg << "'\n";
}

Expand All @@ -113,7 +103,15 @@ auto ClangRunner::Run(llvm::ArrayRef<llvm::StringRef> args) -> bool {
/*ShouldOwnClient=*/false);
clang::ProcessWarningOptions(diagnostics, *diagnostic_options);

clang::driver::Driver driver(exe_path_, target_, diagnostics);
clang::driver::Driver driver("clang-runner", target_, diagnostics);

// Configure the install directory to find other tools and data files.
//
// We directly override the detected directory as we use a synthetic path
// above. This makes it appear that our binary was in the installed binaries
// directory, and allows finding tools relative to it.
driver.Dir = installation_->llvm_install_bin();
CARBON_VLOG() << "Setting bin directory to: " << driver.Dir << "\n";

// TODO: Directly run in-process rather than using a subprocess. This is both
// more efficient and makes debugging (much) easier. Needs code like:
Expand Down
7 changes: 4 additions & 3 deletions toolchain/driver/clang_runner.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "common/ostream.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/StringRef.h"
#include "toolchain/install/install_paths.h"

namespace Carbon {

Expand Down Expand Up @@ -40,15 +41,15 @@ class ClangRunner {
//
// If `verbose` is passed as true, will enable verbose logging to the
// `err_stream` both from the runner and Clang itself.
ClangRunner(llvm::StringRef exe_name, llvm::StringRef target,
ClangRunner(const InstallPaths* install_paths, llvm::StringRef target,
llvm::raw_ostream* vlog_stream = nullptr);

// Run Clang with the provided arguments.
auto Run(llvm::ArrayRef<llvm::StringRef> args) -> bool;

private:
llvm::StringRef exe_name_;
std::string exe_path_;
const InstallPaths* installation_;

llvm::StringRef target_;
llvm::raw_ostream* vlog_stream_;

Expand Down
14 changes: 10 additions & 4 deletions toolchain/driver/clang_runner_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Program.h"
#include "llvm/TargetParser/Host.h"
#include "testing/base/gtest_main.h"
#include "testing/base/test_raw_ostream.h"

namespace Carbon {
Expand Down Expand Up @@ -54,8 +55,10 @@ static auto RunWithCapturedOutput(std::string& out, std::string& err,

TEST(ClangRunnerTest, Version) {
TestRawOstream test_os;
const auto install_paths =
InstallPaths::MakeForBazelRunfiles(Testing::GetTestExePath());
std::string target = llvm::sys::getDefaultTargetTriple();
ClangRunner runner("./toolchain/driver/run_clang_test", target, &test_os);
ClangRunner runner(&install_paths, target, &test_os);

std::string out;
std::string err;
Expand All @@ -72,8 +75,9 @@ TEST(ClangRunnerTest, Version) {
EXPECT_THAT(out, HasSubstr("clang version"));
// The target should match what we provided.
EXPECT_THAT(out, HasSubstr((llvm::Twine("Target: ") + target).str()));
// The installation should come from the above path of the test binary.
EXPECT_THAT(out, HasSubstr("InstalledDir: ./toolchain/driver"));
// Clang's install should be our private LLVM install bin directory.
EXPECT_THAT(out, HasSubstr(std::string("InstalledDir: ") +
install_paths.llvm_install_bin()));
}

// Utility to write a test file. We don't need the full power provided here yet,
Expand Down Expand Up @@ -120,10 +124,12 @@ TEST(ClangRunnerTest, LinkCommandEcho) {
std::filesystem::path foo_file = WriteTestFile("foo.o", "");
std::filesystem::path bar_file = WriteTestFile("bar.o", "");

const auto install_paths =
InstallPaths::MakeForBazelRunfiles(Testing::GetTestExePath());
std::string verbose_out;
llvm::raw_string_ostream verbose_os(verbose_out);
std::string target = llvm::sys::getDefaultTargetTriple();
ClangRunner runner("./toolchain/driver/run_clang_test", target, &verbose_os);
ClangRunner runner(&install_paths, target, &verbose_os);
std::string out;
std::string err;
EXPECT_TRUE(RunWithCapturedOutput(out, err,
Expand Down
5 changes: 4 additions & 1 deletion toolchain/driver/driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,9 @@ auto Driver::Link(const LinkOptions& options,
// We link using a C++ mode of the driver.
clang_args.push_back("--driver-mode=g++");

// Use LLD, which we provide in our install directory, for linking.
clang_args.push_back("-fuse-ld=lld");

// Add OS-specific flags based on the target.
AddOSFlags(codegen_options.target, clang_args);

Expand All @@ -951,7 +954,7 @@ auto Driver::Link(const LinkOptions& options,
clang_args.append(options.object_filenames.begin(),
options.object_filenames.end());

ClangRunner runner("FIXME", codegen_options.target, vlog_stream_);
ClangRunner runner(installation_, codegen_options.target, vlog_stream_);
return {.success = runner.Run(clang_args)};
}

Expand Down
Loading
Loading