Perform wildcard expansion in Process::GetArgumentVector on Windows (PR17098)

[hans]

Summary

On Windows, wildcard expansion isn't performed by the shell, but left to the program itself. The common way to do this is to link with setargv.obj, which performs the expansion on argc/argv before main is entered. However, we don't use argv in Clang on Windows, but instead call GetCommandLineW so we can handle unicode arguments. This means we have to do wildcard expansion ourselves.

Here is a stab at making GetArgumentVector perform wildcard expansion. I'm very open for ideas on how to test this, and also if this is the right place for the code to live.

[majnemer]

The code looks fine but it needs tests.

lib/Support/Windows/Process.inc

224–225 ↗

(On Diff #11476)

Can we use FindFirstFile?

[hans]

lib/Support/Windows/Process.inc

224–225 ↗

(On Diff #11476)

Yes, it seems I'm not using any of the Ex capabilities.

238 ↗

(On Diff #11476)

As per your offline comment, I'll see if I can just reuse Dir and avoid this copying.

[hans]

Addressing David's comments.

[hans]

The code looks fine but it needs tests.

We don't have anything exercising this code in LLVM, but I can test it from Clang with the lit test below. What do you think?

Clang does wildcard expansion on Windows. On other OSs, it's done by the shell.
REQUIRES: system-windows

RUN: %clang -c -### %S/Inputs/wildcard*.c 2>&1 | FileCheck %s
CHECK: wildcard1.c
// CHECK: wildcard2.c

[majnemer]

SGTM

[hans]

Closed by commit rL213114 (authored by @hans).

llvm/trunk/lib/Support/Windows/Process.inc

if (windows::UTF16ToUTF8(Buf.data(), Size, Res))
     return None;
   return std::string(Res.data());
 }
 
 static std::error_code windows_error(DWORD E) {
   return mapWindowsError(E);
 }
 
+static void AllocateAndPush(const SmallVectorImpl<char> &S,
+                            SmallVectorImpl<const char *> &Vector,
+                            SpecificBumpPtrAllocator<char> &Allocator) {
+  char *Buffer = Allocator.Allocate(S.size() + 1);
+  ::memcpy(Buffer, S.data(), S.size());
+  Buffer[S.size()] = '\0';
+  Vector.push_back(Buffer);
+}
+
+/// Convert Arg from UTF-16 to UTF-8 and push it onto Args.
+static std::error_code
+ConvertAndPushArg(const wchar_t *Arg, SmallVectorImpl<const char *> &Args,
+                  SpecificBumpPtrAllocator<char> &Allocator) {
+  SmallVector<char, MAX_PATH> ArgString;
+  if (std::error_code ec = windows::UTF16ToUTF8(Arg, wcslen(Arg), ArgString))
+    return ec;
+  AllocateAndPush(ArgString, Args, Allocator);
+  return std::error_code();
+}
+
+/// \brief Perform wildcard expansion of Arg, or just push it into Args if it
+/// doesn't have wildcards or doesn't match any files.
+static std::error_code
+WildcardExpand(const wchar_t *Arg, SmallVectorImpl<const char *> &Args,
+               SpecificBumpPtrAllocator<char> &Allocator) {
+  if (!wcspbrk(Arg, L"*?")) {
+    // Arg does not contain any wildcard characters. This is the common case.
+    return ConvertAndPushArg(Arg, Args, Allocator);
+  }
+
+  // Extract any directory part of the argument.
+  SmallVector<char, MAX_PATH> Dir;
+  if (std::error_code ec = windows::UTF16ToUTF8(Arg, wcslen(Arg), Dir))
+    return ec;
+  sys::path::remove_filename(Dir);
+  const int DirSize = Dir.size();
+
+  // Search for matching files.
+  WIN32_FIND_DATAW FileData;
+  HANDLE FindHandle = FindFirstFileW(Arg, &FileData);
+  if (FindHandle == INVALID_HANDLE_VALUE) {
+    return ConvertAndPushArg(Arg, Args, Allocator);
+  }
+
+  std::error_code ec;
+  do {
+    SmallVector<char, MAX_PATH> FileName;
+    ec = windows::UTF16ToUTF8(FileData.cFileName, wcslen(FileData.cFileName),
+                              FileName);
+    if (ec)
+      break;
+
+    // Push the filename onto Dir, and remove it afterwards.
+    llvm::sys::path::append(Dir, StringRef(FileName.data(), FileName.size()));
+    AllocateAndPush(Dir, Args, Allocator);
+    Dir.resize(DirSize);
+  } while (FindNextFileW(FindHandle, &FileData));
+
+  FindClose(FindHandle);
+  return ec;
+}
+
 std::error_code
 Process::GetArgumentVector(SmallVectorImpl<const char *> &Args,
                            ArrayRef<const char *>,
                            SpecificBumpPtrAllocator<char> &ArgAllocator) {
-  int NewArgCount;
-  std::error_code ec;
-
-  wchar_t **UnicodeCommandLine = CommandLineToArgvW(GetCommandLineW(),
-                                                    &NewArgCount);
+  int ArgCount;
+  wchar_t **UnicodeCommandLine =
+      CommandLineToArgvW(GetCommandLineW(), &ArgCount);
   if (!UnicodeCommandLine)
     return windows_error(::GetLastError());
 
-  Args.reserve(NewArgCount);
+  Args.reserve(ArgCount);
+  std::error_code ec;
 
-  for (int i = 0; i < NewArgCount; ++i) {
-    SmallVector<char, MAX_PATH> NewArgString;
-    ec = windows::UTF16ToUTF8(UnicodeCommandLine[i],
-                              wcslen(UnicodeCommandLine[i]),
-                              NewArgString);
+  for (int i = 0; i < ArgCount; ++i) {
+    ec = WildcardExpand(UnicodeCommandLine[i], Args, ArgAllocator);
     if (ec)
       break;
-
-    char *Buffer = ArgAllocator.Allocate(NewArgString.size() + 1);
-    ::memcpy(Buffer, NewArgString.data(), NewArgString.size() + 1);
-    Args.push_back(Buffer);
   }
+
   LocalFree(UnicodeCommandLine);
-  if (ec)
-    return ec;
-
-  return std::error_code();
 }
 
 bool Process::StandardInIsUserInput() {
   return FileDescriptorIsDisplayed(0);
 }
 
 bool Process::StandardOutIsDisplayed() {
   return FileDescriptorIsDisplayed(1);