[llvm-branch-commits] [clang] release/19.x: [clang-scan-deps] Don't inspect Args[0] as an option (#109050) (PR #109865)
Tobias Hieta via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Sep 30 23:53:12 PDT 2024
Martin =?utf-8?q?Storsjö?= <martin at martin.st>,
Martin =?utf-8?q?Storsjö?= <martin at martin.st>,
Martin =?utf-8?q?Storsjö?= <martin at martin.st>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/109865 at github.com>
https://github.com/tru updated https://github.com/llvm/llvm-project/pull/109865
>From 7d1f2065d68795b6fc6de4953f9f0ac719cf1c65 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Thu, 12 Sep 2024 22:20:14 +0300
Subject: [PATCH 1/4] [clang-scan-deps] Infer the target from the executable
name (#108189)
This allows clang-scan-deps to work correctly when using cross compilers
with names like <triple>-clang.
(cherry picked from commit 87e1104cf0e2de0d04bee2944893fa7897277b2f)
---
clang/test/ClangScanDeps/implicit-target.c | 31 +++++++++++++++++++
clang/tools/clang-scan-deps/ClangScanDeps.cpp | 5 +++
2 files changed, 36 insertions(+)
create mode 100644 clang/test/ClangScanDeps/implicit-target.c
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/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index a8f6150dd3493d..cd6dd2620152a6 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,8 @@ 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));
+
// The command options are rewritten to run Clang in preprocessor only mode.
auto AdjustingCompilations =
std::make_unique<tooling::ArgumentsAdjustingCompilations>(
>From a0fc8a2b2b85a70c8c523ff2d1fe4ef2e86cda7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Thu, 12 Sep 2024 23:11:27 +0300
Subject: [PATCH 2/4] [clang-scan-deps] Fix builds with BUILD_SHARED_LIBS=ON
This fixes building in this configuration after
87e1104cf0e2de0d04bee2944893fa7897277b2f.
(cherry picked from commit aa3465793a250faa5426ac626989375465256658)
---
clang/tools/clang-scan-deps/CMakeLists.txt | 1 +
1 file changed, 1 insertion(+)
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
>From 2b6c23303f7c3f6397003cdac4be6e9e6b78d957 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Fri, 13 Sep 2024 23:18:10 +0300
Subject: [PATCH 3/4] [clang-scan-deps] Infer the tool locations from PATH
(#108539)
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 a26ec542371652e1d774696e90016fd5b0b1c191)
---
.../clang/Tooling/CompilationDatabase.h | 6 ++
clang/lib/Tooling/CMakeLists.txt | 1 +
.../Tooling/LocateToolCompilationDatabase.cpp | 71 +++++++++++++++++++
.../ClangScanDeps/modules-extern-submodule.c | 3 +-
.../modules-full-output-tu-order.c | 6 +-
.../modules-has-include-umbrella-header.c | 3 +-
.../ClangScanDeps/modules-header-sharing.m | 3 +-
.../modules-implementation-module-map.c | 3 +-
.../modules-implementation-private.m | 3 +-
.../ClangScanDeps/modules-priv-fw-from-pub.m | 3 +-
.../ClangScanDeps/resolve-executable-path.c | 32 +++++++++
clang/tools/clang-scan-deps/ClangScanDeps.cpp | 2 +
12 files changed, 120 insertions(+), 16 deletions(-)
create mode 100644 clang/lib/Tooling/LocateToolCompilationDatabase.cpp
create mode 100644 clang/test/ClangScanDeps/resolve-executable-path.c
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/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/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index cd6dd2620152a6..0f581e73cdfe4b 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -815,6 +815,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
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>(
>From 997b66e566886b8a395b852db46e7930f757b818 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Thu, 19 Sep 2024 20:46:09 +0300
Subject: [PATCH 4/4] [clang-scan-deps] Don't inspect Args[0] as an option
(#109050)
Since a26ec542371652e1d774696e90016fd5b0b1c191, 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 cead9044a995910306e2e64b426fcc8042d7e0ef)
---
clang/tools/clang-scan-deps/ClangScanDeps.cpp | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 0f581e73cdfe4b..867df19c863fe5 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -837,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".
More information about the llvm-branch-commits
mailing list