release/19.x: [clang-scan-deps] Don't inspect Args[0] as an option (#109050) #109865
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@jansvoboda11 @jansvoboda11 @jansvoboda11 What do you think about merging this PR to the release branch? |
|
@llvm/pr-subscribers-clang Author: None (llvmbot) ChangesBackport 87e1104 aa34657 a26ec54 cead904 Requested by: @mstorsjo Full diff: https://github.com/llvm/llvm-project/pull/109865.diff 14 Files Affected:
diff --git a/clang/include/clang/Tooling/CompilationDatabase.h b/clang/include/clang/Tooling/CompilationDatabase.h
index fee584acb48623..36fe0812ebe974 100644
--- a/clang/include/clang/Tooling/CompilationDatabase.h
+++ b/clang/include/clang/Tooling/CompilationDatabase.h
@@ -234,6 +234,12 @@ std::unique_ptr<CompilationDatabase>
std::unique_ptr<CompilationDatabase>
inferTargetAndDriverMode(std::unique_ptr<CompilationDatabase> Base);
+/// Returns a wrapped CompilationDatabase that will transform argv[0] to an
+/// absolute path, if it currently is a plain tool name, looking it up in
+/// PATH.
+std::unique_ptr<CompilationDatabase>
+inferToolLocation(std::unique_ptr<CompilationDatabase> Base);
+
/// Returns a wrapped CompilationDatabase that will expand all rsp(response)
/// files on commandline returned by underlying database.
std::unique_ptr<CompilationDatabase>
diff --git a/clang/lib/Tooling/CMakeLists.txt b/clang/lib/Tooling/CMakeLists.txt
index 93a9e707a134cf..fc1f1f9f9d367e 100644
--- a/clang/lib/Tooling/CMakeLists.txt
+++ b/clang/lib/Tooling/CMakeLists.txt
@@ -25,6 +25,7 @@ add_clang_library(clangTooling
GuessTargetAndModeCompilationDatabase.cpp
InterpolatingCompilationDatabase.cpp
JSONCompilationDatabase.cpp
+ LocateToolCompilationDatabase.cpp
Refactoring.cpp
RefactoringCallbacks.cpp
StandaloneExecution.cpp
diff --git a/clang/lib/Tooling/LocateToolCompilationDatabase.cpp b/clang/lib/Tooling/LocateToolCompilationDatabase.cpp
new file mode 100644
index 00000000000000..033f69f3760c6d
--- /dev/null
+++ b/clang/lib/Tooling/LocateToolCompilationDatabase.cpp
@@ -0,0 +1,71 @@
+//===- GuessTargetAndModeCompilationDatabase.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 "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include <memory>
+
+namespace clang {
+namespace tooling {
+
+namespace {
+class LocationAdderDatabase : public CompilationDatabase {
+public:
+ LocationAdderDatabase(std::unique_ptr<CompilationDatabase> Base)
+ : Base(std::move(Base)) {
+ assert(this->Base != nullptr);
+ }
+
+ std::vector<std::string> getAllFiles() const override {
+ return Base->getAllFiles();
+ }
+
+ std::vector<CompileCommand> getAllCompileCommands() const override {
+ return addLocation(Base->getAllCompileCommands());
+ }
+
+ std::vector<CompileCommand>
+ getCompileCommands(StringRef FilePath) const override {
+ return addLocation(Base->getCompileCommands(FilePath));
+ }
+
+private:
+ std::vector<CompileCommand>
+ addLocation(std::vector<CompileCommand> Cmds) const {
+ for (auto &Cmd : Cmds) {
+ if (Cmd.CommandLine.empty())
+ continue;
+ std::string &Driver = Cmd.CommandLine.front();
+ // If the driver name already is absolute, we don't need to do anything.
+ if (llvm::sys::path::is_absolute(Driver))
+ continue;
+ // If the name is a relative path, like bin/clang, we assume it's
+ // possible to resolve it and don't do anything about it either.
+ if (llvm::any_of(Driver,
+ [](char C) { return llvm::sys::path::is_separator(C); }))
+ continue;
+ auto Absolute = llvm::sys::findProgramByName(Driver);
+ // If we found it in path, update the entry in Cmd.CommandLine
+ if (Absolute && llvm::sys::path::is_absolute(*Absolute))
+ Driver = std::move(*Absolute);
+ }
+ return Cmds;
+ }
+ std::unique_ptr<CompilationDatabase> Base;
+};
+} // namespace
+
+std::unique_ptr<CompilationDatabase>
+inferToolLocation(std::unique_ptr<CompilationDatabase> Base) {
+ return std::make_unique<LocationAdderDatabase>(std::move(Base));
+}
+
+} // namespace tooling
+} // namespace clang
diff --git a/clang/test/ClangScanDeps/implicit-target.c b/clang/test/ClangScanDeps/implicit-target.c
new file mode 100644
index 00000000000000..cf757f937331a6
--- /dev/null
+++ b/clang/test/ClangScanDeps/implicit-target.c
@@ -0,0 +1,31 @@
+// Check that we can detect an implicit target when clang is invoked as
+// <triple->clang. Using an implicit triple requires that the target actually
+// is available, too.
+// REQUIRES: x86-registered-target
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
+
+// Check that we can deduce this both when using a compilation database, and when using
+// a literal command line.
+
+// RUN: clang-scan-deps -format experimental-full -compilation-database %t/cdb.json | FileCheck %s
+
+// RUN: clang-scan-deps -format experimental-full -- x86_64-w64-mingw32-clang %t/source.c -o %t/source.o | FileCheck %s
+
+// CHECK: "-triple",
+// CHECK-NEXT: "x86_64-w64-windows-gnu",
+
+
+//--- cdb.json.in
+[
+ {
+ "directory": "DIR"
+ "command": "x86_64-w64-mingw32-clang -c DIR/source.c -o DIR/source.o"
+ "file": "DIR/source.c"
+ },
+]
+
+//--- source.c
+void func(void) {}
diff --git a/clang/test/ClangScanDeps/modules-extern-submodule.c b/clang/test/ClangScanDeps/modules-extern-submodule.c
index 92f2c749dd2c30..6e9082b0165fdf 100644
--- a/clang/test/ClangScanDeps/modules-extern-submodule.c
+++ b/clang/test/ClangScanDeps/modules-extern-submodule.c
@@ -112,8 +112,7 @@ module third {}
// CHECK: "-fmodule-map-file=[[PREFIX]]/first/first/module.modulemap",
// CHECK: "-fmodule-file=first=[[PREFIX]]/cache/{{.*}}/first-{{.*}}.pcm",
// CHECK: ],
-// CHECK-NEXT: "executable": "clang",
-// CHECK-NEXT: "file-deps": [
+// CHECK: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/tu.m"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
diff --git a/clang/test/ClangScanDeps/modules-full-output-tu-order.c b/clang/test/ClangScanDeps/modules-full-output-tu-order.c
index 04939826817fc1..065b8ac8ae07f0 100644
--- a/clang/test/ClangScanDeps/modules-full-output-tu-order.c
+++ b/clang/test/ClangScanDeps/modules-full-output-tu-order.c
@@ -35,8 +35,7 @@
// CHECK: "-D"
// CHECK-NEXT: "ONE"
// CHECK: ],
-// CHECK-NEXT: "executable": "clang",
-// CHECK-NEXT: "file-deps": [
+// CHECK: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/tu.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
@@ -52,8 +51,7 @@
// CHECK: "-D"
// CHECK-NEXT: "TWO"
// CHECK: ],
-// CHECK-NEXT: "executable": "clang",
-// CHECK-NEXT: "file-deps": [
+// CHECK: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/tu.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
diff --git a/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c b/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c
index e9363b2e14b07a..022c59ca65db2e 100644
--- a/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c
+++ b/clang/test/ClangScanDeps/modules-has-include-umbrella-header.c
@@ -64,8 +64,7 @@ module Dependency { header "dependency.h" }
// CHECK: ],
// CHECK-NEXT: "command-line": [
// CHECK: ],
-// CHECK-NEXT: "executable": "clang",
-// CHECK-NEXT: "file-deps": [
+// CHECK: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/tu.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
diff --git a/clang/test/ClangScanDeps/modules-header-sharing.m b/clang/test/ClangScanDeps/modules-header-sharing.m
index ec94923ae8eeee..31ef351ec38b73 100644
--- a/clang/test/ClangScanDeps/modules-header-sharing.m
+++ b/clang/test/ClangScanDeps/modules-header-sharing.m
@@ -77,8 +77,7 @@
// CHECK: "-fmodule-map-file=[[PREFIX]]/frameworks/A.framework/Modules/module.modulemap",
// CHECK: "-fmodule-name=A",
// CHECK: ],
-// CHECK-NEXT: "executable": "clang",
-// CHECK-NEXT: "file-deps": [
+// CHECK: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/tu.m",
// CHECK-NEXT: "[[PREFIX]]/shared/H.h"
// CHECK-NEXT: ],
diff --git a/clang/test/ClangScanDeps/modules-implementation-module-map.c b/clang/test/ClangScanDeps/modules-implementation-module-map.c
index d76d3157004699..b7637d0c9143a6 100644
--- a/clang/test/ClangScanDeps/modules-implementation-module-map.c
+++ b/clang/test/ClangScanDeps/modules-implementation-module-map.c
@@ -27,8 +27,7 @@ framework module FWPrivate { header "private.h" }
// CHECK: "-fmodule-map-file=[[PREFIX]]/frameworks/FW.framework/Modules/module.private.modulemap",
// CHECK: "-fmodule-name=FWPrivate",
// CHECK: ],
-// CHECK-NEXT: "executable": "clang",
-// CHECK-NEXT: "file-deps": [
+// CHECK: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/tu.m"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m"
diff --git a/clang/test/ClangScanDeps/modules-implementation-private.m b/clang/test/ClangScanDeps/modules-implementation-private.m
index acc01017d66403..b376073f4b9ee9 100644
--- a/clang/test/ClangScanDeps/modules-implementation-private.m
+++ b/clang/test/ClangScanDeps/modules-implementation-private.m
@@ -62,8 +62,7 @@
// CHECK-NEXT: ],
// CHECK-NEXT: "command-line": [
// CHECK: ],
-// CHECK-NEXT: "executable": "clang",
-// CHECK-NEXT: "file-deps": [
+// CHECK: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/tu.m",
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/PrivateHeaders/Missed.h",
// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Headers/FW.h"
diff --git a/clang/test/ClangScanDeps/modules-priv-fw-from-pub.m b/clang/test/ClangScanDeps/modules-priv-fw-from-pub.m
index 4847fedac3bf6f..e9613088463630 100644
--- a/clang/test/ClangScanDeps/modules-priv-fw-from-pub.m
+++ b/clang/test/ClangScanDeps/modules-priv-fw-from-pub.m
@@ -110,8 +110,7 @@
// CHECK-NEXT: ],
// CHECK-NEXT: "command-line": [
// CHECK: ],
-// CHECK-NEXT: "executable": "clang",
-// CHECK-NEXT: "file-deps": [
+// CHECK: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/tu.m"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m"
diff --git a/clang/test/ClangScanDeps/resolve-executable-path.c b/clang/test/ClangScanDeps/resolve-executable-path.c
new file mode 100644
index 00000000000000..63e34ca256a8b4
--- /dev/null
+++ b/clang/test/ClangScanDeps/resolve-executable-path.c
@@ -0,0 +1,32 @@
+// UNSUPPORTED: system-windows
+
+// Check that we expand the executable name to an absolute path, when invoked
+// with a plain executable name, which is implied to be found in PATH.
+// REQUIRES: x86-registered-target
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/bin
+// RUN: ln -s %clang %t/bin/x86_64-w64-mingw32-clang
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
+
+// Check that we can deduce this both when using a compilation database, and when using
+// a literal command line.
+
+// RUN: env "PATH=%t/bin:%PATH%" clang-scan-deps -format experimental-full -compilation-database %t/cdb.json | FileCheck %s -DBASE=%/t
+
+// RUN: env "PATH=%t/bin:%PATH%" clang-scan-deps -format experimental-full -- x86_64-w64-mingw32-clang %t/source.c -o %t/source.o | FileCheck %s -DBASE=%/t
+
+// CHECK: "executable": "[[BASE]]/bin/x86_64-w64-mingw32-clang"
+
+//--- cdb.json.in
+[
+ {
+ "directory": "DIR"
+ "command": "x86_64-w64-mingw32-clang -c DIR/source.c -o DIR/source.o"
+ "file": "DIR/source.c"
+ },
+]
+
+//--- source.c
+void func(void) {}
diff --git a/clang/tools/clang-scan-deps/CMakeLists.txt b/clang/tools/clang-scan-deps/CMakeLists.txt
index f0be6a546ff882..10bc0ff23c5482 100644
--- a/clang/tools/clang-scan-deps/CMakeLists.txt
+++ b/clang/tools/clang-scan-deps/CMakeLists.txt
@@ -1,4 +1,5 @@
set(LLVM_LINK_COMPONENTS
+ ${LLVM_TARGETS_TO_BUILD}
Core
Option
Support
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index a8f6150dd3493d..867df19c863fe5 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -15,6 +15,7 @@
#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
#include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
#include "clang/Tooling/JSONCompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/CommandLine.h"
@@ -24,6 +25,7 @@
#include "llvm/Support/LLVMDriver.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/Signals.h"
+#include "llvm/Support/TargetSelect.h"
#include "llvm/Support/ThreadPool.h"
#include "llvm/Support/Threading.h"
#include "llvm/Support/Timer.h"
@@ -795,6 +797,7 @@ getCompilationDatabase(int argc, char **argv, std::string &ErrorMessage) {
}
int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
+ llvm::InitializeAllTargetInfos();
std::string ErrorMessage;
std::unique_ptr<tooling::CompilationDatabase> Compilations =
getCompilationDatabase(argc, argv, ErrorMessage);
@@ -810,6 +813,10 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
Compilations = expandResponseFiles(std::move(Compilations),
llvm::vfs::getRealFileSystem());
+ Compilations = inferTargetAndDriverMode(std::move(Compilations));
+
+ Compilations = inferToolLocation(std::move(Compilations));
+
// The command options are rewritten to run Clang in preprocessor only mode.
auto AdjustingCompilations =
std::make_unique<tooling::ArgumentsAdjustingCompilations>(
@@ -830,7 +837,12 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
// Reverse scan, starting at the end or at the element before "--".
auto R = std::make_reverse_iterator(FlagsEnd);
- for (auto I = R, E = Args.rend(); I != E; ++I) {
+ auto E = Args.rend();
+ // Don't include Args[0] in the iteration; that's the executable, not
+ // an option.
+ if (E != R)
+ E--;
+ for (auto I = R; I != E; ++I) {
StringRef Arg = *I;
if (ClangCLMode) {
// Ignore arguments that are preceded by "-Xclang".
|
|
Admittedly, these changes aren't regression fixes, but fixes using clang-scan-deps without requiring a wrapper script in certain distribution setups. The changes are somewhat small and localized, and have been cooking in git main for a couple weeks now soon, without any complaints or feedback. |
) This allows clang-scan-deps to work correctly when using cross compilers with names like <triple>-clang. (cherry picked from commit 87e1104)
This allows the clang driver to know which tool is meant to be executed, which allows the clang driver to load the right clang config files, and allows clang to find colocated sysroots. This makes sure that doing `clang-scan-deps -- <tool> ...` looks up things in the same way as if one just would execute `<tool> ...`, when `<tool>` isn't an absolute or relative path. (cherry picked from commit a26ec54)
Since a26ec54, we expand the executable name to an absolute path, if it isn't already one, if found in path. This broke a couple tests in some environments; when the clang workdir resides in a path under e.g. /opt. Tests that only use a tool name like "clang-cl" would get expanded to the absolute path in the build tree. The loop for finding the last "-o" like option for clang-cl command lines would inspect all arguments, including Args[0] which is the executable name itself. As an /opt path matches Arg.starts_with("/o"), this would get detected as an object file output name in cases where there was no other explicit output argument. Thus, this fixes those tests in workdirs under e.g. /opt. (cherry picked from commit cead904)
|
@mstorsjo (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport 87e1104 aa34657 a26ec54 cead904
Requested by: @mstorsjo