From be2fc1d90aeeafc1870837c445e47991606a1024 Mon Sep 17 00:00:00 2001 From: Alexandre Ganea Date: Tue, 13 Aug 2024 15:23:25 -0400 Subject: [PATCH 1/2] [llvm-config] Optionally quote and escape paths with a flag If any of the printed paths by llvm-config contains quotes, spaces, backslashes or dollar sign characters, these paths can be quoted if using --quote-paths and the corresponding characters will be escaped. Following discussion in #76304 Fixes #28117 --- llvm/docs/ReleaseNotes.rst | 4 ++ llvm/test/tools/llvm-config/paths.test | 16 +++++ llvm/tools/llvm-config/llvm-config.cpp | 92 ++++++++++++++++++++------ 3 files changed, 90 insertions(+), 22 deletions(-) diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index 67175e13a8e9f..9c1616c975408 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -163,6 +163,10 @@ Changes to the Debug Info Changes to the LLVM tools --------------------------------- +* llvm-config gained a new flag --quote-paths which quotes and escapes paths + emitted on 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..61d86f7eb0ba1 100644 --- a/llvm/test/tools/llvm-config/paths.test +++ b/llvm/test/tools/llvm-config/paths.test @@ -4,18 +4,34 @@ RUN: llvm-config --bindir 2>&1 | FileCheck --check-prefix=CHECK-BINDIR %s CHECK-BINDIR: {{.*}}{{/|\\}}bin CHECK-BINDIR-NOT: error: CHECK-BINDIR-NOT: warning +RUN: llvm-config --bindir --quote-paths 2>&1 | FileCheck --check-prefix=CHECK-BINDIR2 %s +CHECK-BINDIR2: {{.*}}{{/|\\\\}}bin +CHECK-BINDIR2-NOT: error: +CHECK-BINDIR2-NOT: warning RUN: llvm-config --includedir 2>&1 | FileCheck --check-prefix=CHECK-INCLUDEDIR %s CHECK-INCLUDEDIR: {{.*}}{{/|\\}}include CHECK-INCLUDEDIR-NOT: error: CHECK-INCLUDEDIR-NOT: warning +RUN: llvm-config --includedir --quote-paths 2>&1 | FileCheck --check-prefix=CHECK-INCLUDEDIR2 %s +CHECK-INCLUDEDIR2: {{.*}}{{/|\\\\}}include +CHECK-INCLUDEDIR2-NOT: error: +CHECK-INCLUDEDIR2-NOT: warning RUN: llvm-config --libdir 2>&1 | FileCheck --check-prefix=CHECK-LIBDIR %s CHECK-LIBDIR: {{.*}}{{/|\\}}lib{{.*}} CHECK-LIBDIR-NOT: error: CHECK-LIBDIR-NOT: warning +RUN: llvm-config --libdir --quote-paths 2>&1 | FileCheck --check-prefix=CHECK-LIBDIR2 %s +CHECK-LIBDIR2: {{.*}}{{/|\\\\}}lib{{.*}} +CHECK-LIBDIR2-NOT: error: +CHECK-LIBDIR2-NOT: warning RUN: llvm-config --cmakedir 2>&1 | FileCheck --check-prefix=CHECK-CMAKEDIR %s CHECK-CMAKEDIR: {{.*}}{{/|\\}}cmake{{/|\\}}llvm CHECK-CMAKEDIR-NOT: error: CHECK-CMAKEDIR-NOT: warning +RUN: llvm-config --cmakedir --quote-paths 2>&1 | FileCheck --check-prefix=CHECK-CMAKEDIR2 %s +CHECK-CMAKEDIR2: {{.*}}{{/|\\\\}}cmake{{/|\\\\}}llvm +CHECK-CMAKEDIR2-NOT: error: +CHECK-CMAKEDIR2-NOT: warning diff --git a/llvm/tools/llvm-config/llvm-config.cpp b/llvm/tools/llvm-config/llvm-config.cpp index d5b76b1bb6c16..8cf42d32df01a 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" @@ -219,6 +220,7 @@ Options:\n\ --components List of all possible components.\n\ --cppflags C preprocessor flags for files that include LLVM headers.\n\ --cxxflags C++ compiler flags for files that include LLVM headers.\n\ + --quote-paths Quote and escape paths when needed.\n\ --has-rtti Print whether or not LLVM was built with rtti (YES or NO).\n\ --help Print a summary of llvm-config arguments.\n\ --host-target Target triple used to configure LLVM.\n\ @@ -326,7 +328,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; @@ -352,8 +354,8 @@ int main(int argc, char **argv) { } // We need to include files from both the source and object trees. - ActiveIncludeOption = - ("-I" + ActiveIncludeDir + " " + "-I" + ActiveObjRoot + "/include"); + ActiveIncludeOptions.push_back(ActiveIncludeDir); + ActiveIncludeOptions.push_back(ActiveObjRoot + "/include"); } else { ActivePrefix = CurrentExecPrefix; { @@ -372,7 +374,7 @@ int main(int argc, char **argv) { sys::fs::make_absolute(ActivePrefix, Path); ActiveCMakeDir = std::string(Path); } - ActiveIncludeOption = "-I" + ActiveIncludeDir; + ActiveIncludeOptions.push_back(ActiveIncludeDir); } /// We only use `shared library` mode in cases where the static library form @@ -401,8 +403,9 @@ 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(), '/', - '\\'); + std::replace(ActiveIncludeDir.begin(), ActiveIncludeDir.end(), '/', '\\'); + for (auto &Include : ActiveIncludeOptions) + std::replace(Include.begin(), Include.end(), '/', '\\'); } SharedDir = ActiveBinDir; StaticDir = ActiveLibDir; @@ -504,6 +507,36 @@ int main(int argc, char **argv) { }; raw_ostream &OS = outs(); + + // Check if we want quoting and escaping. + bool QuotePaths = false; + for (int i = 1; i != argc; ++i) { + if (StringRef(argv[i]) == "--quote-paths") { + QuotePaths = true; + break; + } + } + + auto MaybePrintQuoted = [&](StringRef Str) { + if (QuotePaths) + sys::printArg(OS, Str, /*Quote=*/false); // only add quotes if necessary + else + OS << Str; + }; + + // Render include paths and associated flags + auto RenderFlags = [&](StringRef Flags) { + bool First = true; + for (auto &Include : ActiveIncludeOptions) { + if (!First) + OS << ' '; + std::string FlagsStr = "-I" + Include; + MaybePrintQuoted(FlagsStr); + First = false; + } + OS << ' ' << Flags << '\n'; + }; + for (int i = 1; i != argc; ++i) { StringRef Arg = argv[i]; @@ -512,24 +545,32 @@ int main(int argc, char **argv) { if (Arg == "--version") { OS << PACKAGE_VERSION << '\n'; } else if (Arg == "--prefix") { - OS << ActivePrefix << '\n'; + MaybePrintQuoted(ActivePrefix); + OS << '\n'; } else if (Arg == "--bindir") { - OS << ActiveBinDir << '\n'; + MaybePrintQuoted(ActiveBinDir); + OS << '\n'; } else if (Arg == "--includedir") { - OS << ActiveIncludeDir << '\n'; + MaybePrintQuoted(ActiveIncludeDir); + OS << '\n'; } else if (Arg == "--libdir") { - OS << ActiveLibDir << '\n'; + MaybePrintQuoted(ActiveLibDir); + OS << '\n'; } else if (Arg == "--cmakedir") { - OS << ActiveCMakeDir << '\n'; + MaybePrintQuoted(ActiveCMakeDir); + 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") - << ActiveLibDir << ' ' << LLVM_LDFLAGS << '\n'; + std::string LDFlags = + HostTriple.isWindowsMSVCEnvironment() ? "-LIBPATH:" : "-L"; + LDFlags += ActiveLibDir; + MaybePrintQuoted(LDFlags); + OS << ' ' << LLVM_LDFLAGS << '\n'; } else if (Arg == "--system-libs") { PrintSystemLibs = true; } else if (Arg == "--libs") { @@ -590,7 +631,8 @@ int main(int argc, char **argv) { } else if (Arg == "--shared-mode") { PrintSharedMode = true; } else if (Arg == "--obj-root") { - OS << ActivePrefix << '\n'; + MaybePrintQuoted(ActivePrefix); + OS << '\n'; } else if (Arg == "--ignore-libllvm") { LinkDyLib = false; LinkMode = BuiltSharedLibs ? LinkModeShared : LinkModeAuto; @@ -600,6 +642,8 @@ int main(int argc, char **argv) { LinkMode = LinkModeStatic; } else if (Arg == "--help") { usage(false); + } else if (Arg == "--quote-paths") { + // Was already handled above this loop. } else { usage(); } @@ -695,26 +739,30 @@ 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 { + LibFileName = "-l"; StringRef LibName; if (GetComponentLibraryNameSlice(Lib, LibName)) { // Extract library name (remove prefix and suffix). - OS << "-l" << LibName; + LibFileName += LibName; } else { // Lib is already a library name without prefix and suffix. - OS << "-l" << Lib; + LibFileName += Lib; } } } + if (!LibFileName.empty()) + MaybePrintQuoted(LibFileName); }; if (LinkMode == LinkModeShared && LinkDyLib) { From 49cef7910ddc921e28a2918ad96e2b07a5df5186 Mon Sep 17 00:00:00 2001 From: Alexandre Ganea Date: Tue, 13 Aug 2024 16:53:12 -0400 Subject: [PATCH 2/2] Use llvm::any_of --- llvm/tools/llvm-config/llvm-config.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/llvm/tools/llvm-config/llvm-config.cpp b/llvm/tools/llvm-config/llvm-config.cpp index 8cf42d32df01a..0d29852ed118c 100644 --- a/llvm/tools/llvm-config/llvm-config.cpp +++ b/llvm/tools/llvm-config/llvm-config.cpp @@ -509,13 +509,9 @@ int main(int argc, char **argv) { raw_ostream &OS = outs(); // Check if we want quoting and escaping. - bool QuotePaths = false; - for (int i = 1; i != argc; ++i) { - if (StringRef(argv[i]) == "--quote-paths") { - QuotePaths = true; - break; - } - } + bool QuotePaths = std::any_of(&argv[0], &argv[argc], [](const char *Arg) { + return StringRef(Arg) == "--quote-paths"; + }); auto MaybePrintQuoted = [&](StringRef Str) { if (QuotePaths)