Skip to content

[clang] Register all LLVM targets in AllClangUnitTest main #144428

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jun 25, 2025
+29 −0
Merged

[clang] Register all LLVM targets in AllClangUnitTest main #144428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
afb0501
[clang] Register all LLVM targets in AllClangUnitTest main
rnk Jun 16, 2025
f5accc3
reorder headers to pacify format checker
rnk Jun 16, 2025
1f57ce2
Add additional target registration hooks to match cc1
rnk Jun 17, 2025
5d9b647
pacify formatter
rnk Jun 17, 2025
893096a
Rewrite this to use a dynamic initializer to register all target laye…
rnk Jun 19, 2025
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
28 changes: 28 additions & 0 deletions clang/unittests/AllClangUnitTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//===- clang/unittests/AllClangUnitTests.cpp ------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "llvm/Support/TargetSelect.h"

namespace {
struct RegisterAllLLVMTargets {
RegisterAllLLVMTargets();
} gv;
} // namespace

// This dynamic initializer initializes all layers (TargetInfo, MC, CodeGen,
// AsmPrinter, etc) of all LLVM targets. This matches what cc1_main does on
// startup, and prevents tests from initializing some of the Target layers,
// which can interfere with tests that assume that lower target layers are
// registered if the TargetInfo is registered.
RegisterAllLLVMTargets::RegisterAllLLVMTargets() {
llvm::InitializeAllTargetInfos();
llvm::InitializeAllTargets();
llvm::InitializeAllTargetMCs();
llvm::InitializeAllAsmPrinters();
llvm::InitializeAllAsmParsers();
}
1 change: 1 addition & 0 deletions clang/unittests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ get_property(LINK_LIBS GLOBAL PROPERTY CLANG_UNITTEST_LINK_LIBS)
get_property(LLVM_COMPONENTS GLOBAL PROPERTY CLANG_UNITTEST_LLVM_COMPONENTS)
add_distinct_clang_unittest(AllClangUnitTests
${SRCS}
AllClangUnitTests.cpp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we're still linking against llvm_gtest_main, https://github.com/llvm/llvm-project/blob/main/third-party/unittest/UnitTestMain/TestMain.cpp. we probably need to do some fiddling in the build rules as well to make sure, we only link against llvm_gtest. (not sure if you want to have other msvc related magic in here as well)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spot checked two other unit tests that have custom mains, and they don't do anything special:

  1. llvm-cfi-verify main , cmake
  2. LiveIntervalTest main, cmake

I think archive semantics are pretty portable between platforms, so I think this just works. The sources used to build the unit test executable override the main provided by the gtest library.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't feel great about relying on linking order and linker not loading llvm_gtest_main solely because it wasn't referenced, but I guess that's a theoretical concern that won't matter much in practice (since llvm_gtest_main doesn't really define symbols apart from TestMainArgv0, at least yet). so leaving this up to you.

but can we at least leave a comment here, mentioning that we're also linking against llvm_gtest_main to make it easier for people to investigate if they encounter any such issues?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, on second thought, there's a lot more going on in UnitTestMain than I thought. I rewrote this to use a dynamic initializer to register the targets instead. Those will reliably run before test fixtures. The only risk here is that we encounter intialization-order-fiasco issues, but I spot-checked x86, and it seems to be resilient to that... I will test with ASan locally to build confidence, but PTAL at the new approach.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot, this is really neat! and yeah i think initialization-order-fiasco should be less of a concern here.

CLANG_LIBS
${CLANG_LIBS}
LINK_LIBS
Expand Down
Loading