[9.2.0] Fix launching executables whose path can't be shortened on Windows (https://github.com/bazelbuild/bazel/pull/29921) (#29984)
### Description
`AsExecutablePathForCreateProcess` shortens an executable's path with
`GetShortPathNameW` so it fits `CreateProcessW`'s `MAX_PATH`-bound
command line, and currently fails outright when shortening can't bring
it under `MAX_PATH`.
This change adds a fallback: it launches the executable through
`CreateProcessW`'s _lpApplicationName_, which is not subject to the
`MAX_PATH` limit, in extended-length (`\\?\`) form.
Supplying _lpApplicationName_ also lifts the `MAX_PATH` limit from the
executable part of _lpCommandLine_, so the original path is kept there
as `argv[0]`.
Shortening is still attempted first, so the change is a pure superset of
the current behavior: paths that shorten today are launched unchanged,
and only the currently-failing ones take the fallback.
The fallback is limited to absolute, normalized paths to plain
executables:
- a batch file is not a valid _lpApplicationName_ as it must be run
through a command interpreter,
- neither is a non-normalized path because its `.`/`..` components would
survive unresolved in the extended-length path.
The fix relies on `\\?\` bypassing `MAX_PATH` at the path-parsing layer,
**independently** of the _LongPathsEnabled_ registry value and the
_longPathAware_ manifest.
### Motivation
Fixes #19710.
Avoid errors like:
```
==================== Test output for //some/configuration/issues/invalidconfig:invalidconfig_test:
ERROR(tools/test/windows/tw.cc:1307) ERROR: src/main/native/windows/process.cc(82): WaitableProcess::Create(C:\user\execroot\_main\bazel-out\x64_windows-fastbuild-ST-000000000001\bin\some\configuration\issues\invalidconfig\invalidconfig_test_\invalidconfig_test.exe.runfiles\_main\some\configuration\issues\invalidconfig\invalidconfig_test_\invalidconfig_test.exe): ERROR: src/main/native/windows/util.cc(292): AsExecutablePathForCreateProcess(C:\user\execroot\_main\bazel-out\x64_windows-fastbuild-ST-000000000001\bin\some\configuration\issues\invalidconfig\invalidconfig_test_\invalidconfig_test.exe.runfiles\_main\some\configuration\issues\invalidconfig\invalidconfig_test_\invalidconfig_test.exe): ERROR: src/main/native/windows/util.cc(262): GetShortPathNameW(\\?\C:\user\execroot\_main\bazel-out\x64_windows-fastbuild-ST-000000000001\bin\some\configuration\issues\invalidconfig\invalidconfig_test_\invalidconfig_test.exe.runfiles\_main\some\configuration\issues\invalidconfig\invalidconfig_test_\invalidconfig_test.exe): cannot shorten the path enough
ERROR(tools/test/windows/tw.cc:1517) Failed to start test process (arg: C:\user\execroot\_main\bazel-out\x64_windows-fastbuild-ST-000000000001\bin\some\configuration\issues\invalidconfig\invalidconfig_test_\invalidconfig_test.exe.runfiles\_main\some\configuration\issues\invalidconfig\invalidconfig_test_\invalidconfig_test.exe)
================================================================================
```
Shortening fails in two situations, both handled here:
- 8dot3 short-name creation is disabled on the volume (common in
containers, microsoft/Windows-Containers#507), so `GetShortPathNameW` is
a no-op and a long runfiles executable path stays over `MAX_PATH`,
- the path is nested so deeply that even fully 8dot3-shortened it still
overflows `MAX_PATH`.
Prior art: Rust's `std::process::Command` passes a verbatim `\\?\` path
as _lpApplicationName_ for a >`MAX_PATH` executable and special-cases
batch files the same way:
- rust-lang/rust#87704,
- rust-lang/rust#92519.
### Build API Changes
No
### Checklist
- [x] I have added tests for the new use cases (if any).
- [ ] I have updated the documentation (if applicable).
### Release Notes
RELNOTES: Fixed Bazel being unable to run a Windows executable whose
path is too long to shorten under `MAX_PATH`, e.g. when 8dot3 short
names are disabled (microsoft/Windows-Containers#507).
Closes #29921.
PiperOrigin-RevId: 937426905
Change-Id: Ia48d41e000622fc122d1b258ad15daa5018be75e
Commit
https://github.com/bazelbuild/bazel/commit/4a80d43e1640d8d1eedcef4a733be3d1b13be6c7
Co-authored-by: Rgis Desgroppes <rdesgroppes@gmail.com>
diff --git a/src/main/native/windows/process.cc b/src/main/native/windows/process.cc
index 4646a65..c8f0a69 100644
--- a/src/main/native/windows/process.cc
+++ b/src/main/native/windows/process.cc
@@ -77,7 +77,9 @@
}
std::wstring argv0short;
- error_msg = AsExecutablePathForCreateProcess(argv0, &argv0short);
+ std::wstring extended_path;
+ error_msg =
+ AsExecutablePathForCreateProcess(argv0, &argv0short, &extended_path);
if (!error_msg.empty()) {
*error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
L"WaitableProcess::Create", argv0, error_msg);
@@ -159,7 +161,8 @@
STARTUPINFOEXW info;
attr_list->InitStartupInfoExW(&info);
if (!CreateProcessW(
- /* lpApplicationName */ nullptr,
+ /* lpApplicationName */ extended_path.empty() ? nullptr
+ : extended_path.c_str(),
/* lpCommandLine */ mutable_commandline.get(),
/* lpProcessAttributes */ nullptr,
/* lpThreadAttributes */ nullptr,
diff --git a/src/main/native/windows/util.cc b/src/main/native/windows/util.cc
index 615d099..872fb10 100644
--- a/src/main/native/windows/util.cc
+++ b/src/main/native/windows/util.cc
@@ -194,6 +194,15 @@
return s.find(substr) != wstring::npos;
}
+static bool IsBatchFile(const wstring& path) {
+ if (path.size() < 4) {
+ return false;
+ }
+ const wchar_t* ext = path.c_str() + (path.size() - 4);
+ return CompareStringOrdinal(ext, 4, L".bat", 4, TRUE) == CSTR_EQUAL ||
+ CompareStringOrdinal(ext, 4, L".cmd", 4, TRUE) == CSTR_EQUAL;
+}
+
wstring AsShortPath(wstring path, wstring* result) {
// Using `MAX_PATH` - 4 (256) instead of `MAX_PATH` to fix
// https://github.com/bazelbuild/bazel/issues/12310
@@ -267,7 +276,8 @@
return L"";
}
-wstring AsExecutablePathForCreateProcess(wstring path, wstring* result) {
+wstring AsExecutablePathForCreateProcess(wstring path, wstring* quoted_path,
+ wstring* extended_path) {
if (path.empty()) {
return MakeErrorMessage(WSTR(__FILE__), __LINE__,
L"AsExecutablePathForCreateProcess", path,
@@ -287,16 +297,34 @@
}
path = cwd + L"\\" + path;
}
- wstring error = AsShortPath(path, result);
- if (!error.empty()) {
- return MakeErrorMessage(WSTR(__FILE__), __LINE__,
- L"AsExecutablePathForCreateProcess", path, error);
+ wstring error = AsShortPath(path, quoted_path);
+ if (error.empty()) {
+ // Quote the path in case it's something like "c:\foo\app name.exe".
+ // Do this unconditionally, there's no harm in quoting. Quotes are not
+ // allowed inside paths so we don't need to escape quotes.
+ QuotePath(*quoted_path, quoted_path);
+ extended_path->clear();
+ return L"";
}
- // Quote the path in case it's something like "c:\foo\app name.exe".
- // Do this unconditionally, there's no harm in quoting. Quotes are not
- // allowed inside paths so we don't need to escape quotes.
- QuotePath(*result, result);
- return L"";
+ // Shortening might fail (https://github.com/bazelbuild/bazel/issues/19710),
+ // typically when 8.3 aliases are disabled on the volume, which is common in
+ // containers (https://github.com/microsoft/Windows-Containers/issues/507).
+ // As a fallback, set `quoted_path` as above and `extended_path` to the
+ // extended-length form of `path`, suitable for CreateProcessW's
+ // lpApplicationName: it is not subject to MAX_PATH, and providing it lifts
+ // that limit from the executable part of CreateProcessW's lpCommandLine too.
+ // This works only for a plain executable with an absolute, normalized path.
+ if (!IsBatchFile(path)) {
+ wstring native_path = path;
+ std::replace(native_path.begin(), native_path.end(), L'/', L'\\');
+ if (IsAbsoluteNormalizedWindowsPath(native_path)) {
+ QuotePath(native_path, quoted_path);
+ *extended_path = wstring(L"\\\\?\\") + native_path;
+ return L"";
+ }
+ }
+ return MakeErrorMessage(WSTR(__FILE__), __LINE__,
+ L"AsExecutablePathForCreateProcess", path, error);
}
} // namespace windows
diff --git a/src/main/native/windows/util.h b/src/main/native/windows/util.h
index a98e7f8..1a107fd 100644
--- a/src/main/native/windows/util.h
+++ b/src/main/native/windows/util.h
@@ -136,12 +136,15 @@
// Same as `AsExecutablePathForCreateProcess` except it won't quote the result.
wstring AsShortPath(wstring path, wstring* result);
-// Computes a path suitable as the executable part in CreateProcessA's cmdline.
+// Computes a path suitable as the executable part in CreateProcessW's
+// lpCommandLine, plus an extended-length form suitable for CreateProcessW's
+// lpApplicationName when applicable.
//
-// The null-terminated executable path for CreateProcessA has to fit into
-// MAX_PATH, therefore the limit for the executable's path is MAX_PATH - 1
-// (not including null terminator). This method attempts to convert the input
-// `path` to a short format to fit it into the MAX_PATH - 1 limit.
+// Unless CreateProcessW's lpApplicationName is populated, the null-terminated
+// executable path in CreateProcessW's lpCommandLine has to fit into MAX_PATH,
+// therefore the limit for the executable's path is MAX_PATH - 1 (not including
+// null terminator). This method attempts to convert the input `path` to a short
+// format to fit it into the MAX_PATH - 1 limit.
//
// `path` must be either an absolute, normalized, Windows-style path with drive
// letter (e.g. "c:\foo\bar.exe", but no "\foo\bar.exe"), or must be just a file
@@ -149,17 +152,23 @@
// In both cases, `path` must be unquoted.
//
// If this function succeeds, it returns an empty string (indicating no error),
-// and sets `result` to the resulting path, which is always quoted, and is
+// and sets `quoted_path` to the resulting path, which is always quoted, and is
// always at most MAX_PATH + 1 long (MAX_PATH - 1 without null terminator, plus
-// two quotes). If there's any error, this function returns the error message.
+// two quotes), unless the `extended_path` fallback below applies. If there's
+// any error, this function returns the error message.
//
// If `path` is at most MAX_PATH - 1 long (not including null terminator), the
// result will be that (plus quotes).
// Otherwise this method attempts to compute an 8dot3 style short name for
// `path`, and if that succeeds and the result is at most MAX_PATH - 1 long (not
// including null terminator), then that will be the result (plus quotes).
+// Otherwise, if `path` is an absolute, normalized path to a plain executable
+// (not a batch file), `extended_path` is set to its extended-length form (the
+// "\\?\" prefix plus `path`) for use as CreateProcessW's lpApplicationName, and
+// `quoted_path` to the quoted `path`.
// Otherwise this function fails and returns an error message.
-wstring AsExecutablePathForCreateProcess(wstring path, wstring* result);
+wstring AsExecutablePathForCreateProcess(wstring path, wstring* quoted_path,
+ wstring* extended_path);
} // namespace windows
} // namespace bazel
diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java
index f01c236..a28ad58 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java
@@ -328,6 +328,24 @@
}
@Test
+ public void testExecutableWithExtendedLengthPath() throws Exception {
+ StringBuilder dir = new StringBuilder(System.getenv("TEST_TMPDIR"));
+ for (char c = 'a'; c <= 'z'; c++) {
+ dir.append('\\').repeat(c, 8).append('.').repeat(c, 3);
+ }
+ String exe = dir + "\\cmd.exe";
+ assertThat(exe.length()).isGreaterThan(260); // Windows MAX_PATH
+ Files.createDirectories(Paths.get("\\\\?\\" + dir));
+ Files.copy(
+ Paths.get(System.getenv("SystemRoot"), "System32", "cmd.exe"), Paths.get("\\\\?\\" + exe));
+
+ process = WindowsProcesses.createProcess(exe, "/c exit 42", null, null, null, null);
+ assertNoProcessError();
+ assertThat(WindowsProcesses.waitFor(process, -1)).isEqualTo(0);
+ assertThat(WindowsProcesses.getExitCode(process)).isEqualTo(42);
+ }
+
+ @Test
public void testReadingAndWritingAfterTermination() throws Exception {
process =
WindowsProcesses.createProcess(mockBinary, mockArgs("X42"), null, null, null, null);
diff --git a/src/test/native/windows/util_test.cc b/src/test/native/windows/util_test.cc
index 2d4cd54..09cbde1 100644
--- a/src/test/native/windows/util_test.cc
+++ b/src/test/native/windows/util_test.cc
@@ -236,21 +236,40 @@
}
// This is a macro so the assertions will have the correct line number.
-#define ASSERT_SHORTENING_FAILS(/* const WCHAR* */ input, \
- /* const WCHAR* */ error_msg) \
- { \
- wstring actual; \
- wstring result(AsExecutablePathForCreateProcess(input, &actual)); \
- ASSERT_CONTAINS(result, error_msg); \
+#define ASSERT_SHORTENING_FAILS(/* const WCHAR* */ input, \
+ /* const WCHAR* */ error_msg) \
+ { \
+ wstring actual; \
+ wstring extended_path; \
+ wstring result( \
+ AsExecutablePathForCreateProcess(input, &actual, &extended_path)); \
+ ASSERT_CONTAINS(result, error_msg); \
+ ASSERT_EQ(extended_path, L""); \
}
// This is a macro so the assertions will have the correct line number.
-#define ASSERT_SHORTENING_SUCCEEDS(/* const WCHAR* */ input, \
- /* const wstring& */ expected_result) \
- { \
- wstring actual; \
- ASSERT_EQ(AsExecutablePathForCreateProcess(input, &actual), L""); \
- ASSERT_EQ(actual, expected_result); \
+#define ASSERT_SHORTENING_SUCCEEDS(/* const WCHAR* */ input, \
+ /* const wstring& */ expected_result) \
+ { \
+ wstring actual; \
+ wstring extended_path = L"stale"; \
+ ASSERT_EQ( \
+ AsExecutablePathForCreateProcess(input, &actual, &extended_path), \
+ L""); \
+ ASSERT_EQ(actual, expected_result); \
+ ASSERT_EQ(extended_path, L""); \
+ }
+
+// This is a macro so the assertions will have the correct line number.
+#define ASSERT_FALLBACK_SUCCEEDS(/* const WCHAR* */ input) \
+ { \
+ wstring actual; \
+ wstring extended_path; \
+ ASSERT_EQ( \
+ AsExecutablePathForCreateProcess(input, &actual, &extended_path), \
+ L""); \
+ ASSERT_EQ(extended_path, wstring(L"\\\\?\\") + input); \
+ ASSERT_EQ(actual, wstring(L"\"") + input + L"\""); \
}
TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessBadInputs) {
@@ -271,7 +290,9 @@
// Relative paths are fine, they are absolutized.
std::wstring rel(L"foo\\bar.exe");
std::wstring actual;
- EXPECT_EQ(AsExecutablePathForCreateProcess(rel, &actual), L"");
+ std::wstring extended_path;
+ EXPECT_EQ(AsExecutablePathForCreateProcess(rel, &actual, &extended_path),
+ L"");
EXPECT_GT(actual.size(), rel.size());
EXPECT_EQ(actual.rfind(rel), actual.size() - rel.size() - 1);
}
@@ -298,7 +319,7 @@
// When i=0 then `wfilename` is `kMaxPath` - 1 long, so
// `AsExecutablePathForCreateProcess` will not attempt to shorten it, and
// so it also won't notice that the file doesn't exist. If however we pass
- // a non-existent path to CreateProcessA, then it'll fail, so we'll find out
+ // a non-existent path to CreateProcessW, then it'll fail, so we'll find out
// about this error in production code.
// When i>0 then `wfilename` is at least `kMaxPath` long, so
// `AsExecutablePathForCreateProcess` will attempt to shorten it, but
@@ -349,5 +370,23 @@
DeleteDirsUnder(tmpdir, short_root);
}
+TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessFallback) {
+ wstring dir = L"c:\\" + wstring(kMaxPath, L'a');
+
+ // A deep, absolute, normalized executable path that can't be shortened (here
+ // because it doesn't exist) takes the fallback: `extended_path` gets the
+ // extended-length form and `quoted_path` the quoted path.
+ ASSERT_FALLBACK_SUCCEEDS(dir + L"\\foo.exe");
+
+ // A batch file is denied the fallback even with an absolute, normalized path:
+ // CreateProcessW's lpApplicationName can only take a plain executable.
+ ASSERT_SHORTENING_FAILS(dir + L"\\foo.bat", L"GetShortPathNameW");
+
+ // A non-normalized path is denied the fallback because the "\\?\" prefix
+ // disables path normalization, so "." and ".." would reach the filesystem
+ // verbatim instead of being resolved.
+ ASSERT_SHORTENING_FAILS(dir + L"\\..\\foo.exe", L"path is not normalized");
+}
+
} // namespace windows
} // namespace bazel
