From b40d654d35514066cd7681254e16c6d89ead692c Mon Sep 17 00:00:00 2001 From: Alexandre Ganea Date: Mon, 1 Jul 2024 10:47:19 -0400 Subject: [PATCH 1/3] [llvm-config] Quote and escape paths if necessary --- llvm/tools/llvm-config/llvm-config.cpp | 50 +++++++++++++++++--------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/llvm/tools/llvm-config/llvm-config.cpp b/llvm/tools/llvm-config/llvm-config.cpp index d5b76b1bb6c16..7e46bf45073b8 100644 --- a/llvm/tools/llvm-config/llvm-config.cpp +++ b/llvm/tools/llvm-config/llvm-config.cpp @@ -24,6 +24,7 @@ #include "llvm/Config/config.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" +#include "llvm/Support/Program.h" #include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" #include "llvm/TargetParser/Triple.h" @@ -352,8 +353,11 @@ int main(int argc, char **argv) { } // We need to include files from both the source and object trees. - ActiveIncludeOption = - ("-I" + ActiveIncludeDir + " " + "-I" + ActiveObjRoot + "/include"); + raw_string_ostream OS(ActiveIncludeOption); + OS << "-I"; + sys::printArg(OS, ActiveIncludeDir, /*Quote=*/true); + OS << " -I"; + sys::printArg(OS, ActiveObjRoot + "/include", /*Quote=*/true); } else { ActivePrefix = CurrentExecPrefix; { @@ -372,7 +376,9 @@ int main(int argc, char **argv) { sys::fs::make_absolute(ActivePrefix, Path); ActiveCMakeDir = std::string(Path); } - ActiveIncludeOption = "-I" + ActiveIncludeDir; + raw_string_ostream OS(ActiveIncludeOption); + OS << "-I"; + sys::printArg(OS, ActiveIncludeDir, /*Quote=*/true); } /// We only use `shared library` mode in cases where the static library form @@ -512,15 +518,20 @@ int main(int argc, char **argv) { if (Arg == "--version") { OS << PACKAGE_VERSION << '\n'; } else if (Arg == "--prefix") { - OS << ActivePrefix << '\n'; + sys::printArg(OS, ActivePrefix, /*Quote=*/true); + OS << '\n'; } else if (Arg == "--bindir") { - OS << ActiveBinDir << '\n'; + sys::printArg(OS, ActiveBinDir, /*Quote=*/true); + OS << '\n'; } else if (Arg == "--includedir") { - OS << ActiveIncludeDir << '\n'; + sys::printArg(OS, ActiveIncludeDir, /*Quote=*/true); + OS << '\n'; } else if (Arg == "--libdir") { - OS << ActiveLibDir << '\n'; + sys::printArg(OS, ActiveLibDir, /*Quote=*/true); + OS << '\n'; } else if (Arg == "--cmakedir") { - OS << ActiveCMakeDir << '\n'; + sys::printArg(OS, ActiveCMakeDir, /*Quote=*/true); + OS << '\n'; } else if (Arg == "--cppflags") { OS << ActiveIncludeOption << ' ' << LLVM_CPPFLAGS << '\n'; } else if (Arg == "--cflags") { @@ -528,8 +539,9 @@ int main(int argc, char **argv) { } else if (Arg == "--cxxflags") { OS << ActiveIncludeOption << ' ' << LLVM_CXXFLAGS << '\n'; } else if (Arg == "--ldflags") { - OS << ((HostTriple.isWindowsMSVCEnvironment()) ? "-LIBPATH:" : "-L") - << ActiveLibDir << ' ' << LLVM_LDFLAGS << '\n'; + OS << ((HostTriple.isWindowsMSVCEnvironment()) ? "-LIBPATH:" : "-L"); + sys::printArg(OS, ActiveLibDir, /*Quote=*/true); + OS << ' ' << LLVM_LDFLAGS << '\n'; } else if (Arg == "--system-libs") { PrintSystemLibs = true; } else if (Arg == "--libs") { @@ -590,7 +602,8 @@ int main(int argc, char **argv) { } else if (Arg == "--shared-mode") { PrintSharedMode = true; } else if (Arg == "--obj-root") { - OS << ActivePrefix << '\n'; + sys::printArg(OS, ActivePrefix, /*Quote=*/true); + OS << '\n'; } else if (Arg == "--ignore-libllvm") { LinkDyLib = false; LinkMode = BuiltSharedLibs ? LinkModeShared : LinkModeAuto; @@ -695,26 +708,31 @@ int main(int argc, char **argv) { auto PrintForLib = [&](const StringRef &Lib) { const bool Shared = LinkMode == LinkModeShared; + std::string LibFileName; if (PrintLibNames) { - OS << GetComponentLibraryFileName(Lib, Shared); + LibFileName = GetComponentLibraryFileName(Lib, Shared); } else if (PrintLibFiles) { - OS << GetComponentLibraryPath(Lib, Shared); + LibFileName = GetComponentLibraryPath(Lib, Shared); } else if (PrintLibs) { // On Windows, output full path to library without parameters. // Elsewhere, if this is a typical library name, include it using -l. if (HostTriple.isWindowsMSVCEnvironment()) { - OS << GetComponentLibraryPath(Lib, Shared); + LibFileName = GetComponentLibraryPath(Lib, Shared); } else { StringRef LibName; if (GetComponentLibraryNameSlice(Lib, LibName)) { // Extract library name (remove prefix and suffix). - OS << "-l" << LibName; + OS << "-l"; + LibFileName = LibName; } else { // Lib is already a library name without prefix and suffix. - OS << "-l" << Lib; + OS << "-l"; + LibFileName = Lib; } } } + if (!LibFileName.empty()) + sys::printArg(OS, LibFileName, /*Quote=*/true); }; if (LinkMode == LinkModeShared && LinkDyLib) { From cb2629e200c3b74a7b17963831be783b8f18952f Mon Sep 17 00:00:00 2001 From: Alexandre Ganea Date: Mon, 1 Jul 2024 10:57:54 -0400 Subject: [PATCH 2/3] Simplify --- llvm/tools/llvm-config/llvm-config.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/tools/llvm-config/llvm-config.cpp b/llvm/tools/llvm-config/llvm-config.cpp index 7e46bf45073b8..749835a36d4db 100644 --- a/llvm/tools/llvm-config/llvm-config.cpp +++ b/llvm/tools/llvm-config/llvm-config.cpp @@ -719,14 +719,13 @@ int main(int argc, char **argv) { if (HostTriple.isWindowsMSVCEnvironment()) { LibFileName = GetComponentLibraryPath(Lib, Shared); } else { + OS << "-l"; StringRef LibName; if (GetComponentLibraryNameSlice(Lib, LibName)) { // Extract library name (remove prefix and suffix). - OS << "-l"; LibFileName = LibName; } else { // Lib is already a library name without prefix and suffix. - OS << "-l"; LibFileName = Lib; } } From e02fdff859447499cf6edbeb783eae2bbbf145de Mon Sep 17 00:00:00 2001 From: Alexandre Ganea Date: Tue, 2 Jul 2024 10:26:31 -0400 Subject: [PATCH 3/3] Fix test and add release notes --- llvm/docs/ReleaseNotes.rst | 4 +++ llvm/test/tools/llvm-config/paths.test | 8 +++--- llvm/tools/llvm-config/llvm-config.cpp | 37 ++++++++++++++++---------- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index a110671bacf34..89a45de06fbaf 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -369,6 +369,10 @@ Changes to the LLVM tools jumping in reverse direction with shift+L/R/B). (`#95662 `). +* llvm-config now quotes and escapes paths emitted in stdout, to account for + spaces or other special characters in path. + (`#97305 `). + Changes to LLDB --------------------------------- diff --git a/llvm/test/tools/llvm-config/paths.test b/llvm/test/tools/llvm-config/paths.test index 419f155ae1f83..6fa43a1561521 100644 --- a/llvm/test/tools/llvm-config/paths.test +++ b/llvm/test/tools/llvm-config/paths.test @@ -1,21 +1,21 @@ # Check directory options for obvious issues. RUN: llvm-config --bindir 2>&1 | FileCheck --check-prefix=CHECK-BINDIR %s -CHECK-BINDIR: {{.*}}{{/|\\}}bin +CHECK-BINDIR: {{.*}}{{/|\\\\}}bin CHECK-BINDIR-NOT: error: CHECK-BINDIR-NOT: warning RUN: llvm-config --includedir 2>&1 | FileCheck --check-prefix=CHECK-INCLUDEDIR %s -CHECK-INCLUDEDIR: {{.*}}{{/|\\}}include +CHECK-INCLUDEDIR: {{.*}}{{/|\\\\}}include CHECK-INCLUDEDIR-NOT: error: CHECK-INCLUDEDIR-NOT: warning RUN: llvm-config --libdir 2>&1 | FileCheck --check-prefix=CHECK-LIBDIR %s -CHECK-LIBDIR: {{.*}}{{/|\\}}lib{{.*}} +CHECK-LIBDIR: {{.*}}{{/|\\\\}}lib{{.*}} CHECK-LIBDIR-NOT: error: CHECK-LIBDIR-NOT: warning RUN: llvm-config --cmakedir 2>&1 | FileCheck --check-prefix=CHECK-CMAKEDIR %s -CHECK-CMAKEDIR: {{.*}}{{/|\\}}cmake{{/|\\}}llvm +CHECK-CMAKEDIR: {{.*}}{{/|\\\\}}cmake{{/|\\\\}}llvm CHECK-CMAKEDIR-NOT: error: CHECK-CMAKEDIR-NOT: warning diff --git a/llvm/tools/llvm-config/llvm-config.cpp b/llvm/tools/llvm-config/llvm-config.cpp index 749835a36d4db..db92d462807d0 100644 --- a/llvm/tools/llvm-config/llvm-config.cpp +++ b/llvm/tools/llvm-config/llvm-config.cpp @@ -327,7 +327,7 @@ int main(int argc, char **argv) { // information. std::string ActivePrefix, ActiveBinDir, ActiveIncludeDir, ActiveLibDir, ActiveCMakeDir; - std::string ActiveIncludeOption; + std::vector ActiveIncludeOptions; if (IsInDevelopmentTree) { ActiveIncludeDir = std::string(LLVM_SRC_ROOT) + "/include"; ActivePrefix = CurrentExecPrefix; @@ -353,11 +353,8 @@ int main(int argc, char **argv) { } // We need to include files from both the source and object trees. - raw_string_ostream OS(ActiveIncludeOption); - OS << "-I"; - sys::printArg(OS, ActiveIncludeDir, /*Quote=*/true); - OS << " -I"; - sys::printArg(OS, ActiveObjRoot + "/include", /*Quote=*/true); + ActiveIncludeOptions.push_back(ActiveIncludeDir); + ActiveIncludeOptions.push_back(ActiveObjRoot + "/include"); } else { ActivePrefix = CurrentExecPrefix; { @@ -376,9 +373,7 @@ int main(int argc, char **argv) { sys::fs::make_absolute(ActivePrefix, Path); ActiveCMakeDir = std::string(Path); } - raw_string_ostream OS(ActiveIncludeOption); - OS << "-I"; - sys::printArg(OS, ActiveIncludeDir, /*Quote=*/true); + ActiveIncludeOptions.push_back(ActiveIncludeDir); } /// We only use `shared library` mode in cases where the static library form @@ -407,8 +402,8 @@ int main(int argc, char **argv) { std::replace(ActiveBinDir.begin(), ActiveBinDir.end(), '/', '\\'); std::replace(ActiveLibDir.begin(), ActiveLibDir.end(), '/', '\\'); std::replace(ActiveCMakeDir.begin(), ActiveCMakeDir.end(), '/', '\\'); - std::replace(ActiveIncludeOption.begin(), ActiveIncludeOption.end(), '/', - '\\'); + for (auto &Include : ActiveIncludeOptions) + std::replace(Include.begin(), Include.end(), '/', '\\'); } SharedDir = ActiveBinDir; StaticDir = ActiveLibDir; @@ -510,6 +505,20 @@ int main(int argc, char **argv) { }; raw_ostream &OS = outs(); + + // Render include paths and associated flags + auto RenderFlags = [&](StringRef Flags) { + bool First = true; + for (auto &Include : ActiveIncludeOptions) { + if (!First) + OS << ' '; + OS << "-I"; + sys::printArg(OS, Include, /*Quote=*/true); + First = false; + } + OS << ' ' << Flags << '\n'; + }; + for (int i = 1; i != argc; ++i) { StringRef Arg = argv[i]; @@ -533,11 +542,11 @@ int main(int argc, char **argv) { sys::printArg(OS, ActiveCMakeDir, /*Quote=*/true); OS << '\n'; } else if (Arg == "--cppflags") { - OS << ActiveIncludeOption << ' ' << LLVM_CPPFLAGS << '\n'; + RenderFlags(LLVM_CPPFLAGS); } else if (Arg == "--cflags") { - OS << ActiveIncludeOption << ' ' << LLVM_CFLAGS << '\n'; + RenderFlags(LLVM_CFLAGS); } else if (Arg == "--cxxflags") { - OS << ActiveIncludeOption << ' ' << LLVM_CXXFLAGS << '\n'; + RenderFlags(LLVM_CXXFLAGS); } else if (Arg == "--ldflags") { OS << ((HostTriple.isWindowsMSVCEnvironment()) ? "-LIBPATH:" : "-L"); sys::printArg(OS, ActiveLibDir, /*Quote=*/true);