Skip to content

[Clang] Add resource_dir_EQ flag to CC1Option group #140870

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

Merged

Conversation

wenju-he
Copy link
Contributor

@wenju-he wenju-he commented May 21, 2025

This PR fixes ClangTool error in -cc1 mode in function injectResourceDir:
error: unknown argument: '-resource-dir=

This PR fixes ClangTool error in -cc1 mode:
error: unknown argument: '-resource-dir=
@llvmbot llvmbot added the clang Clang issues not falling into any other category label May 21, 2025
@llvmbot
Copy link
Member

llvmbot commented May 21, 2025

@llvm/pr-subscribers-clang

Author: Wenju He (wenju-he)

Changes

This PR fixes ClangTool error in -cc1 mode:
error: unknown argument: '-resource-dir=


Full diff: https://github.com/llvm/llvm-project/pull/140870.diff

2 Files Affected:

  • (modified) clang/lib/Tooling/Tooling.cpp (+5-3)
  • (modified) clang/unittests/Tooling/ToolingTest.cpp (+19)
diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp
index 3c72f52040142..87a984672662b 100644
--- a/clang/lib/Tooling/Tooling.cpp
+++ b/clang/lib/Tooling/Tooling.cpp
@@ -510,9 +510,11 @@ static void injectResourceDir(CommandLineArguments &Args, const char *Argv0,
       return;
 
   // If there's no override in place add our resource dir.
-  Args = getInsertArgumentAdjuster(
-      ("-resource-dir=" + CompilerInvocation::GetResourcesPath(Argv0, MainAddr))
-          .c_str())(Args, "");
+  CommandLineArguments Extra = {
+      "-resource-dir", CompilerInvocation::GetResourcesPath(Argv0, MainAddr)};
+
+  Args =
+      getInsertArgumentAdjuster(Extra, ArgumentInsertPosition::END)(Args, "");
 }
 
 int ClangTool::run(ToolAction *Action) {
diff --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp
index 8cdfffb54390e..07104ccf9835f 100644
--- a/clang/unittests/Tooling/ToolingTest.cpp
+++ b/clang/unittests/Tooling/ToolingTest.cpp
@@ -771,6 +771,25 @@ TEST(ClangToolTest, BaseVirtualFileSystemUsage) {
   EXPECT_EQ(0, Tool.run(Action.get()));
 }
 
+// Check -cc1 command doesn't crash.
+TEST(ClangToolTest, CC1Arg) {
+  FixedCompilationDatabase Compilations("/", {"-cc1"});
+  llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFileSystem(
+      new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem()));
+  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
+      new llvm::vfs::InMemoryFileSystem);
+  OverlayFileSystem->pushOverlay(InMemoryFileSystem);
+
+  InMemoryFileSystem->addFile(
+      "a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
+
+  ClangTool Tool(Compilations, std::vector<std::string>(1, "a.cpp"),
+                 std::make_shared<PCHContainerOperations>(), OverlayFileSystem);
+  std::unique_ptr<FrontendActionFactory> Action(
+      newFrontendActionFactory<SyntaxOnlyAction>());
+  EXPECT_EQ(0, Tool.run(Action.get()));
+}
+
 // Check getClangStripDependencyFileAdjuster doesn't strip args after -MD/-MMD.
 TEST(ClangToolTest, StripDependencyFileAdjuster) {
   FixedCompilationDatabase Compilations("/", {"-MD", "-c", "-MMD", "-w"});

@wenju-he wenju-he requested a review from kadircet May 25, 2025 23:29
Copy link
Member

@kadircet kadircet left a comment

Choose a reason for hiding this comment

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

do we know why https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Driver/Options.td#L5894-L5901 doesn't declare _EQ version for cc1?

i think instead of working around this in various places, we should unify the two if possible.

@wenju-he wenju-he changed the title [ClangTool] Use CC1Option flag resource-dir in injectResourceDir [Clang] Add resource_dir_EQ flag to CC1Option group May 26, 2025
@wenju-he
Copy link
Contributor Author

do we know why https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Driver/Options.td#L5894-L5901 doesn't declare _EQ version for cc1?

Git history shows the earliest commit f3e624c already have the difference. I don't see obvious reason why resource_dir_EQ should not be cc1 option.

i think instead of working around this in various places, we should unify the two if possible.

done, thanks for the suggestion.

@wenju-he wenju-he requested a review from kadircet May 26, 2025 23:29
@kadircet
Copy link
Member

LGTM, but cc @llvm/clang-area-team, in case they have any concerns here

@wenju-he wenju-he merged commit c7b8d6e into llvm:main May 30, 2025
11 checks passed
@wenju-he wenju-he deleted the ClangTool-injectResourceDir-use-CC1Option branch May 30, 2025 07:20
@nathanchance
Copy link
Member

I bisected a crash that I see when running check-clang to this change.

# bad: [9ff30d4f1c2595f038c4c0cf873387a0c32c7c7b] MCParse: Disallow @ specifier in symbol equating
# good: [fbbae9ea2bd1c89e39deb62c8a2f549a33e51fae] [LoongArch] Only report the first range error if there is actually more than one for __builtin_loongarch_cacop_[wd]
git bisect start '9ff30d4f1c2595f038c4c0cf873387a0c32c7c7b' 'fbbae9ea2bd1c89e39deb62c8a2f549a33e51fae'
# bad: [8eadbea8e3aac52608880f0e0ae64e8403f2609f] [HLSL] Diagnose overlapping resource bindings (#140982)
git bisect bad 8eadbea8e3aac52608880f0e0ae64e8403f2609f
# bad: [aaa9c19328b23319eb1b50789d9478f31dee09b1] [LLDB] Remove redundant check in DemangledNameInfo::hasBasename (#142139)
git bisect bad aaa9c19328b23319eb1b50789d9478f31dee09b1
# bad: [37edd2c1c32bf7599af28b96ae5adf909e96fc58] [X86] combineEXTRACT_SUBVECTOR - generalize extract_subvector(broadcast(x),c) fold with IsElementEquivalent (#141963)
git bisect bad 37edd2c1c32bf7599af28b96ae5adf909e96fc58
# good: [1e81e800508fbec5bdd394754e6005af536aa2f3] [IR] Avoid UB in `SymbolTableListTraits` (#139096)
git bisect good 1e81e800508fbec5bdd394754e6005af536aa2f3
# good: [d490526a81586c7b2fe674ce520276570c9881e2] [clangd] [Modules] Fixes to correctly handle module dependencies (#142090)
git bisect good d490526a81586c7b2fe674ce520276570c9881e2
# good: [b71255705bab3bba231c045a9fd101df438a7a72] [libc++] P2165R4: Update deduction guides for map containers and container adaptors (#136011)
git bisect good b71255705bab3bba231c045a9fd101df438a7a72
# bad: [417e43ad43d706c8a932adf702a55de97e65fb37] [LV] Set PhiTy once in adjustRecipesForReductions (NFC).
git bisect bad 417e43ad43d706c8a932adf702a55de97e65fb37
# bad: [c7b8d6e1dcb0a277f95496cbc6bb27143afb80a6] [Clang] Add resource_dir_EQ flag to CC1Option group (#140870)
git bisect bad c7b8d6e1dcb0a277f95496cbc6bb27143afb80a6
# first bad commit: [c7b8d6e1dcb0a277f95496cbc6bb27143afb80a6] [Clang] Add resource_dir_EQ flag to CC1Option group (#140870)
$ git sh -s --format='%h ("%s")'
832a7bb46061 ("[AArch64] Add missing Neon Types (#126945)")

$ cmake \
	-B build \
	-G Ninja \
	-S llvm \
	-Wno-dev \
	-DCLANG_ENABLE_STATIC_ANALYZER=OFF \
	-DCLANG_PLUGIN_SUPPORT=OFF \
	-DCMAKE_BUILD_TYPE=Release \
	-DCMAKE_CXX_COMPILER=clang++ \
	-DCMAKE_C_COMPILER=clang \
	-DLLVM_ENABLE_ASSERTIONS=ON \
	-DLLVM_ENABLE_BINDINGS=OFF \
	-DLLVM_ENABLE_LIBXML2=OFF \
	-DLLVM_ENABLE_OCAMLDOC=OFF \
	-DLLVM_ENABLE_PROJECTS=clang \
	-DLLVM_ENABLE_WARNINGS=OFF \
	-DLLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR= \
	-DLLVM_INCLUDE_DOCS=OFF \
	-DLLVM_INCLUDE_EXAMPLES=OFF \
	-DLLVM_USE_LINKER=lld
and ninja -C build check-clang
...
[----------] 1 test from Rewriter
[ RUN      ] Rewriter.ReplaceTextRangeTypes
[       OK ] Rewriter.ReplaceTextRangeTypes (2 ms)
[----------] 1 test from Rewriter (2 ms total)

[----------] 1 test from ClangToolTest
[ RUN      ] ClangToolTest.StripDependencyFileAdjusterShowIncludes
[       OK ] ClangToolTest.StripDependencyFileAdjusterShowIncludes (2 ms)
[----------] 1 test from ClangToolTest (2 ms total)

[----------] 1 test from CodeGenTest
[ RUN      ] CodeGenTest.TestNonAlterTest
AllClangUnitTests: .../llvm-project/llvm/lib/CodeGen/CodeGenTargetMachineImpl.cpp:48: void llvm::CodeGenTargetMachineImpl::initAsmInfo(): Assertion `MRI && "Unable to create reg info"' failed.
 #0 0x000055f7c928d468 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x5ebb468)
 #1 0x000055f7c928b0cc llvm::sys::RunSignalHandlers() (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x5eb90cc)
 #2 0x000055f7c928dc91 SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
 #3 0x00007fb92f84def0 (/usr/lib/libc.so.6+0x3def0)
 #4 0x00007fb92f8a774c (/usr/lib/libc.so.6+0x9774c)
 #5 0x00007fb92f84ddc0 raise (/usr/lib/libc.so.6+0x3ddc0)
 #6 0x00007fb92f83557a abort (/usr/lib/libc.so.6+0x2557a)
 #7 0x00007fb92f8354e3 __assert_perror_fail (/usr/lib/libc.so.6+0x254e3)
 #8 0x000055f7cd69de44 llvm::CodeGenTargetMachineImpl::initAsmInfo() (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0xa2cbe44)
 #9 0x000055f7ca6f23b6 llvm::X86TargetMachine::X86TargetMachine(llvm::Target const&, llvm::Triple const&, llvm::StringRef, llvm::StringRef, llvm::TargetOptions const&, std::optional<llvm::Reloc::Model>, std::optional<llvm::CodeModel::Model>, llvm::CodeGenOptLevel, bool) X86TargetMachine.cpp:0:0
#10 0x000055f7ca6f3cc4 llvm::RegisterTargetMachine<llvm::X86TargetMachine>::Allocator(llvm::Target const&, llvm::Triple const&, llvm::StringRef, llvm::StringRef, llvm::TargetOptions const&, std::optional<llvm::Reloc::Model>, std::optional<llvm::CodeModel::Model>, llvm::CodeGenOptLevel, bool) X86TargetMachine.cpp:0:0
#11 0x000055f7ccca8b0e clang::emitBackendOutput(clang::CompilerInstance&, clang::CodeGenOptions&, llvm::StringRef, llvm::Module*, clang::BackendAction, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>, clang::BackendConsumer*) (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x98d6b0e)
#12 0x000055f7ccc9fb53 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x98cdb53)
#13 0x000055f7cc4a36dc clang::MultiplexConsumer::HandleTranslationUnit(clang::ASTContext&) (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x90d16dc)
#14 0x000055f7cb72c879 clang::ParseAST(clang::Sema&, bool, bool) (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x835a879)
#15 0x000055f7cc40b2d6 clang::FrontendAction::Execute() (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x90392d6)
#16 0x000055f7cc37a6cd clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x8fa86cd)
#17 0x000055f7cc6dceea clang::tooling::FrontendActionFactory::runInvocation(std::shared_ptr<clang::CompilerInvocation>, clang::FileManager*, std::shared_ptr<clang::PCHContainerOperations>, clang::DiagnosticConsumer*) (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x930aeea)
#18 0x000055f7cc6dcc2a clang::tooling::ToolInvocation::runInvocation(char const*, clang::driver::Compilation*, std::shared_ptr<clang::CompilerInvocation>, std::shared_ptr<clang::PCHContainerOperations>) (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x930ac2a)
#19 0x000055f7cc6db5a5 clang::tooling::ToolInvocation::run() (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x93095a5)
#20 0x000055f7cc6dae00 clang::tooling::runToolOnCodeWithArgs(std::unique_ptr<clang::FrontendAction, std::default_delete<clang::FrontendAction>>, llvm::Twine const&, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&, llvm::Twine const&, llvm::Twine const&, std::shared_ptr<clang::PCHContainerOperations>) (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x9308e00)
#21 0x000055f7cc6da995 clang::tooling::runToolOnCodeWithArgs(std::unique_ptr<clang::FrontendAction, std::default_delete<clang::FrontendAction>>, llvm::Twine const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&, llvm::Twine const&, llvm::Twine const&, std::shared_ptr<clang::PCHContainerOperations>, std::vector<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>> const&) (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x9308995)
#22 0x000055f7c90c9f03 (anonymous namespace)::CodeGenTest_TestNonAlterTest_Test::TestBody() NoAlterCodeGenActionTest.cpp:0:0
#23 0x000055f7cad83fc0 testing::Test::Run() (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x79b1fc0)
#24 0x000055f7cad854d0 testing::TestInfo::Run() (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x79b34d0)
#25 0x000055f7cad860ef testing::TestSuite::Run() (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x79b40ef)
#26 0x000055f7cad96dd4 testing::internal::UnitTestImpl::RunAllTests() (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x79c4dd4)
#27 0x000055f7cad961c9 testing::UnitTest::Run() (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x79c41c9)
#28 0x000055f7cad6eabc main (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x799cabc)
#29 0x00007fb92f8376b5 (/usr/lib/libc.so.6+0x276b5)
#30 0x00007fb92f837769 __libc_start_main (/usr/lib/libc.so.6+0x27769)
#31 0x000055f7c821db65 _start (.../llvm-project/build/tools/clang/unittests/./AllClangUnitTests+0x4e4bb65)

--
exit: -6
--
shard JSON output does not exist: .../llvm-project/build/tools/clang/unittests/./AllClangUnitTests-Clang-Unit-4039843-179-182.json
********************
********************
Failed Tests (1):
  Clang-Unit :: ./AllClangUnitTests/179/182


Testing Time: 69.21s

Total Discovered Tests: 45688
  Skipped          :     1 (0.00%)
  Unsupported      :  1406 (3.08%)
  Passed           : 44256 (96.87%)
  Expectedly Failed:    24 (0.05%)
  Failed           :     1 (0.00%)
FAILED: tools/clang/test/CMakeFiles/check-clang .../llvm-project/build/tools/clang/test/CMakeFiles/check-clang
cd .../llvm-project/build/tools/clang/test && /usr/bin/python3.13 .../llvm-project/build/./bin/llvm-lit -sv --param USE_Z3_SOLVER=0 .../llvm-project/build/tools/clang/test

@wenju-he
Copy link
Contributor Author

wenju-he commented Jun 3, 2025

I bisected a crash that I see when running check-clang to this change.

thanks @nathanchance for the bisecting.
I have created a revert PR at #142509 please review.
I'll investigate later why the test fail.

sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
This PR fixes ClangTool error in -cc1 mode in function
injectResourceDir:
error: unknown argument: '-resource-dir=
wenju-he added a commit that referenced this pull request Jun 7, 2025
#142509)

This reverts commit c7b8d6e.
Fix check-clang CodeGenTest.TestNonAlterTest regression, see
#140870 (comment)
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 7, 2025
…p (#140870)" (#142509)

This reverts commit c7b8d6e.
Fix check-clang CodeGenTest.TestNonAlterTest regression, see
llvm/llvm-project#140870 (comment)
rorth pushed a commit to rorth/llvm-project that referenced this pull request Jun 11, 2025
)" (llvm#142509)

This reverts commit c7b8d6e.
Fix check-clang CodeGenTest.TestNonAlterTest regression, see
llvm#140870 (comment)
DhruvSrivastavaX pushed a commit to DhruvSrivastavaX/lldb-for-aix that referenced this pull request Jun 12, 2025
)" (llvm#142509)

This reverts commit c7b8d6e.
Fix check-clang CodeGenTest.TestNonAlterTest regression, see
llvm#140870 (comment)
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
)" (llvm#142509)

This reverts commit c7b8d6e.
Fix check-clang CodeGenTest.TestNonAlterTest regression, see
llvm#140870 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants