From 503721047731e8f6566bc51d6eadc7d24161c129 Mon Sep 17 00:00:00 2001 From: jsmall-nvidia Date: Wed, 12 Jun 2019 16:18:07 -0400 Subject: CommandLine arg escaping (#980) * Work in progress to be able to invoke VS from within code. * First pass at windows version of refactor of OSProcessSpawner * Closer to getting VS path lookup working. * Make OSString assignable/ctor able * Work out program files directory directly, so don't have to expand %%. * WIP: Improve handling of process spawning. * Add support for splitting input by line. * * Correctly locates visual studio install * Added functionality to invoke vs via cmd * Add option to execute the command line. * Handle in ProcessUtil for windows -> WinHandle. * Rename files slang-win-visual-studio-util.cpp/.h and slang-process-util.h * First pass at unix/linux version of ProcessUtil. * Fix reading Visual Studio path from the registry. * Get compiling on linux with. * Fix vcvarsall.bat name * Use ProcessUtil to execute external code. * Remove OSProcessSpawner. * Remove includes for "os.h" where no longer needed. * Fix tabbing issue in premake5.lua Remove test code from slang-test-main.cpp * Fix premake4.lua tabbing issue. * Small fixes to slang-process-util.h Init ExecuteResult on Win execute. * Improve comments. * Fix bug in StringUtil::calcLines - with oddly terminated source input being able to read past end. Make slang-generate use StringUtil over it's own impl. * Fix off by one bug in working out Visual Studio version. * Fix bug in calculating Visual Studio Version * Fix compilation on linux with string parameter being passed to messageFormat. * Remove erroneous use of kOSError codes - use Result. * First effort to generate standard compiler options. * Initial efforts in compiling source code in test framework for VisualStudio. * Testing compiling c code on VisualStudio on Windows. * Fix warning on linux. * Fix clang on linux warning (and therefore failing) returning a StringBuilder as String. * Disable return-std-move on clang. * CommandLine arguments are now tagged if they are escaped or not. That it is the clients responsibility to escape command lines that cannot be automatically escaped. * Add checks on unix/linux that command line args are all unescaped. --- source/core/windows/slang-win-process-util.cpp | 56 ++++------------------ .../core/windows/slang-win-visual-studio-util.cpp | 26 +++------- 2 files changed, 15 insertions(+), 67 deletions(-) (limited to 'source/core/windows') diff --git a/source/core/windows/slang-win-process-util.cpp b/source/core/windows/slang-win-process-util.cpp index 680864acb..ac6468109 100644 --- a/source/core/windows/slang-win-process-util.cpp +++ b/source/core/windows/slang-win-process-util.cpp @@ -154,35 +154,7 @@ static DWORD WINAPI _readerThreadProc(LPVOID threadParam) return UnownedStringSlice::fromLiteral(".exe"); } -static bool _isLetter(char c) -{ - return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); -} - -/* Special case handling for command line switches of the form /LINKPATH: that it is immediately followed -potentially by a path. This detects that kind of switch, and returns the index of the first character after -the :. If doesn't have the right format returns -1, to indicate that it's not this style of switch */ -static int _calcIndexOfColonSwitch(const UnownedStringSlice& slice) -{ - if (!slice.startsWith("/")) - { - return -1; - } - int index = slice.indexOf(':'); - if (index > 0) - { - for (int i = 1; i < index; ++i) - { - if (!_isLetter(slice[i])) - { - return -1; - } - } - } - return index + 1; -} - -static void _appendCommandLineEscaped(const UnownedStringSlice& slice, StringBuilder& out) +/* static */void ProcessUtil::appendCommandLineEscaped(const UnownedStringSlice& slice, StringBuilder& out) { // TODO(JS): This escaping is not complete... ! @@ -218,23 +190,6 @@ static void _appendCommandLineEscaped(const UnownedStringSlice& slice, StringBui } } -/* static */void ProcessUtil::appendCommandLineEscaped(const UnownedStringSlice& slice, StringBuilder& out) -{ - // Check if starts with a colon ending switch, as we need to special case escaping in this case - const int index = _calcIndexOfColonSwitch(slice); - if (index >= 0) - { - // Append the switch prefix as is (ie /linkpath:) - out << UnownedStringSlice(slice.begin(), index); - // Append the stuff after the prefix escaping if needed - _appendCommandLineEscaped(UnownedStringSlice(slice.begin() + index, slice.end()), out); - } - else - { - _appendCommandLineEscaped(slice, out); - } -} - /* static */String ProcessUtil::getCommandLineString(const CommandLine& commandLine) { StringBuilder cmd; @@ -242,7 +197,14 @@ static void _appendCommandLineEscaped(const UnownedStringSlice& slice, StringBui for (const auto& arg : commandLine.m_args) { cmd << " "; - appendCommandLineEscaped(arg.getUnownedSlice(), cmd); + if (arg.type == CommandLine::ArgType::Unescaped) + { + appendCommandLineEscaped(arg.value.getUnownedSlice(), cmd); + } + else + { + cmd << arg.value; + } } return cmd.ToString(); } diff --git a/source/core/windows/slang-win-visual-studio-util.cpp b/source/core/windows/slang-win-visual-studio-util.cpp index 5fc56d091..b7de7607c 100644 --- a/source/core/windows/slang-win-visual-studio-util.cpp +++ b/source/core/windows/slang-win-visual-studio-util.cpp @@ -373,9 +373,7 @@ static SlangResult _find(int versionIndex, WinVisualStudioUtil::VersionPath& out // /Fd - followed by name of the pdb file if (options.debugInfoType != DebugInfoType::None) { - StringBuilder builder; - builder << "/Fd" << options.modulePath << ".pdb"; - cmdLine.addArg(builder); + cmdLine.addPrefixPathArg("/Fd", options.modulePath, ".pdb"); } switch (options.targetType) @@ -392,28 +390,20 @@ static SlangResult _find(int versionIndex, WinVisualStudioUtil::VersionPath& out cmdLine.addArg("/LD"); } - StringBuilder builder; - builder << "/Fe" << options.modulePath << ".dll"; - cmdLine.addArg(builder); + cmdLine.addPrefixPathArg("/Fe", options.modulePath, ".dll"); break; } case TargetType::Executable: { - StringBuilder builder; - builder << "/Fe" << options.modulePath << ".exe"; - cmdLine.addArg(builder); + cmdLine.addPrefixPathArg("/Fe", options.modulePath, ".exe"); break; } default: break; } // Object file specify it's location - needed if we are out - { - StringBuilder builder; - builder << "/Fo" << options.modulePath << ".obj"; - cmdLine.addArg(builder); - } - + cmdLine.addPrefixPathArg("/Fo", options.modulePath, ".obj"); + // Add defines for (const auto& define : options.defines) { @@ -434,7 +424,6 @@ static SlangResult _find(int versionIndex, WinVisualStudioUtil::VersionPath& out cmdLine.addArg(include); } - // https://docs.microsoft.com/en-us/cpp/build/reference/eh-exception-handling-model?view=vs-2019 // /Eha - Specifies the model of exception handling. (a, s, c, r are options) @@ -450,10 +439,7 @@ static SlangResult _find(int versionIndex, WinVisualStudioUtil::VersionPath& out for (const auto& libPath : options.libraryPaths) { // Note that any escaping of the path is handled in the ProcessUtil:: - StringBuilder builder; - builder << "/LIBPATH:" << libPath; - - cmdLine.addArg(builder); + cmdLine.addPrefixPathArg("/LIBPATH:", libPath); } } -- cgit v1.2.3