Author: Charles Zablit Date: 2026-07-10T16:10:13+01:00 New Revision: ef3ac7a77a06442ab2ac57f5417cfa9ded7a3b46
URL: https://github.com/llvm/llvm-project/commit/ef3ac7a77a06442ab2ac57f5417cfa9ded7a3b46 DIFF: https://github.com/llvm/llvm-project/commit/ef3ac7a77a06442ab2ac57f5417cfa9ded7a3b46.diff LOG: [lldb][Windows] Support modules with long paths (#206099) `PlatformWindows::DoLoadImage` injected a 261-byte buffer for the loaded module path. Longer paths would be truncated and fail to load. This patch adds a growing buffer (up to the NT limit) which is used only when `GetModuleFileNameA` reports truncation, re-querying without taking an extra reference on the module. This patch also adds 3 tests for lldb, lldb-driver and the SBAPI. They are really regression tests which break if long path support regresses. Requires: - https://github.com/llvm/llvm-project/pull/206046 - https://github.com/llvm/llvm-project/pull/206060 Added: lldb/test/API/driver/longpath/Makefile lldb/test/API/driver/longpath/TestLongPathDriver.py lldb/test/API/driver/longpath/main.c lldb/test/API/functionalities/longpath/Makefile lldb/test/API/functionalities/longpath/TestLongPath.py lldb/test/API/functionalities/longpath/main.c lldb/test/API/tools/lldb-dap/longpath/Makefile lldb/test/API/tools/lldb-dap/longpath/TestDAP_launch_longPath.py lldb/test/API/tools/lldb-dap/longpath/main.c Modified: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp index 22387e04921df..e7c590312b090 100644 --- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp +++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp @@ -12,7 +12,11 @@ #include <optional> #if defined(_WIN32) #include "lldb/Host/windows/windows.h" +#include <pathcch.h> #include <winsock2.h> +#else +#define MAX_PATH 260 +#define PATHCCH_MAX_CCH 0x8000 #endif #include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h" @@ -262,15 +266,14 @@ uint32_t PlatformWindows::DoLoadImage(Process *process, } /* Inject wszModulePath into inferior */ - // FIXME(compnerd) should do something better for the length? - // GetModuleFileNameA is likely limited to PATH_MAX rather than the NT path - // limit. - unsigned injected_length = 261; - - lldb::addr_t injected_module_path = - process->AllocateMemory(injected_length + 1, - ePermissionsReadable | ePermissionsWritable, - status); + // Start with a MAX_PATH-sized buffer (enough for the vast majority of module + // paths) and grow it on demand if GetModuleFileNameW reports truncation (see + // the loop after the helper runs). + unsigned injected_length = MAX_PATH; + + lldb::addr_t injected_module_path = process->AllocateMemory( + (injected_length + 1) * sizeof(llvm::UTF16), + ePermissionsReadable | ePermissionsWritable, status); if (injected_module_path == LLDB_INVALID_ADDRESS) { error = Status::FromErrorStringWithFormat( "LoadLibrary error: unable to allocate memory for module location: %s", @@ -300,6 +303,20 @@ uint32_t PlatformWindows::DoLoadImage(Process *process, process->DeallocateMemory(injected_result); }); + std::vector<lldb::addr_t> grown_path_buffers; + llvm::scope_exit grown_path_cleanup([&]() { + for (lldb::addr_t buffer : grown_path_buffers) + process->DeallocateMemory(buffer); + }); + + process->WritePointerToMemory(injected_result, 0, status); + if (status.Fail()) { + error = Status::FromErrorStringWithFormat( + "LoadLibrary error: could not initialize result: %s", + status.AsCString()); + return LLDB_INVALID_IMAGE_TOKEN; + } + process->WritePointerToMemory(injected_result + word_size, injected_module_path, status); if (status.Fail()) { @@ -404,8 +421,41 @@ uint32_t PlatformWindows::DoLoadImage(Process *process, return LLDB_INVALID_IMAGE_TOKEN; } - std::string module_path; - process->ReadCStringFromMemory(injected_module_path, module_path, status); + lldb::addr_t module_path_addr = injected_module_path; + unsigned capacity = injected_length; + uint32_t path_length = process->ReadUnsignedIntegerFromMemory( + injected_result + 2 * word_size, sizeof(unsigned), 0, status); + while (status.Success() && path_length >= capacity && + capacity < PATHCCH_MAX_CCH) { + capacity = std::min<unsigned>(capacity * 2, PATHCCH_MAX_CCH); + lldb::addr_t buffer = process->AllocateMemory( + (capacity + 1) * sizeof(llvm::UTF16), + ePermissionsReadable | ePermissionsWritable, status); + if (buffer == LLDB_INVALID_ADDRESS || status.Fail()) + break; + grown_path_buffers.push_back(buffer); + + process->WritePointerToMemory(injected_result + word_size, buffer, status); + if (status.Fail()) + break; + process->WriteScalarToMemory(injected_result + 2 * word_size, + Scalar{capacity}, sizeof(unsigned), status); + if (status.Fail()) + break; + + diagnostics.Clear(); + if (invocation->ExecuteFunction(context, &injected_parameters, options, + diagnostics, value) != eExpressionCompleted) + break; + module_path_addr = buffer; + path_length = process->ReadUnsignedIntegerFromMemory( + injected_result + 2 * word_size, sizeof(unsigned), 0, status); + } + + llvm::SmallVector<llvm::UTF16, MAX_PATH> wide_path(path_length); + if (path_length) + process->ReadMemory(module_path_addr, wide_path.data(), + path_length * sizeof(llvm::UTF16), status); if (status.Fail()) { error = Status::FromErrorStringWithFormat( "LoadLibrary error: could not read module path: %s", @@ -413,6 +463,15 @@ uint32_t PlatformWindows::DoLoadImage(Process *process, return LLDB_INVALID_IMAGE_TOKEN; } + std::string module_path; + if (!llvm::convertUTF16ToUTF8String( + llvm::ArrayRef<llvm::UTF16>(wide_path.data(), wide_path.size()), + module_path)) { + error = Status::FromErrorString( + "LoadLibrary error: could not convert module path to UTF-8"); + return LLDB_INVALID_IMAGE_TOKEN; + } + if (loaded_image) loaded_image->SetFile(module_path, llvm::sys::path::Style::native); return process->AddImageToken(token); @@ -648,8 +707,8 @@ extern "C" { // WINBASEAPI BOOL WINAPI FreeModule(HMODULE); /* __declspec(dllimport) */ int __stdcall FreeModule(void *hLibModule); -// WINBASEAPI DWORD WINAPI GetModuleFileNameA(HMODULE hModule, LPSTR lpFilename, DWORD nSize); -/* __declspec(dllimport) */ uint32_t GetModuleFileNameA(void *, char *, uint32_t); +// WINBASEAPI DWORD WINAPI GetModuleFileNameW(HMODULE hModule, LPWSTR lpFilename, DWORD nSize); +/* __declspec(dllimport) */ uint32_t GetModuleFileNameW(void *, wchar_t *, uint32_t); // WINBASEAPI HMODULE WINAPI LoadLibraryExW(LPCWSTR, HANDLE, DWORD); /* __declspec(dllimport) */ void * __stdcall LoadLibraryExW(const wchar_t *, void *, uint32_t); @@ -663,7 +722,7 @@ extern "C" { struct __lldb_LoadLibraryResult { void *ImageBase; - char *ModulePath; + wchar_t *ModulePath; unsigned Length; unsigned ErrorCode; }; @@ -673,43 +732,48 @@ _Static_assert(sizeof(struct __lldb_LoadLibraryResult) <= 3 * sizeof(void *), void * __lldb_LoadLibraryHelper(const wchar_t *name, const wchar_t *paths, __lldb_LoadLibraryResult *result) { - for (const wchar_t *path = paths; path && *path; ) { - (void)AddDllDirectory(path); - path += wcslen(path) + 1; - } - - result->ImageBase = LoadLibraryExW(name, nullptr, - LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); - - // Fallback: if the AddDllDirectory + LOAD_LIBRARY_SEARCH_DEFAULT_DIRS path - // failed to find the library, iterate the search paths ourselves and - // load by absolute path using LOAD_WITH_ALTERED_SEARCH_PATH, which makes - // Windows use the loaded DLL's own directory to resolve its sibling imports. + // When the caller presets ImageBase the module is already loaded and we are + // only re-querying its path with a larger buffer. Skip LoadLibrary in that + // case so we do not take an extra reference on the module. if (result->ImageBase == nullptr) { - wchar_t full[4096]; - for (const wchar_t *path = paths; path && *path; path += wcslen(path) + 1) { - size_t plen = wcslen(path); - size_t nlen = wcslen(name); - // Need room for: path + '\\' + name + '\0' - if (plen + 1 + nlen + 1 > 4096) - continue; - wchar_t *p = full; - for (size_t i = 0; i < plen; ++i) - *p++ = path[i]; - *p++ = L'\\'; - for (size_t i = 0; i <= nlen; ++i) // Copy name including trailing '\0'. - *p++ = name[i]; - result->ImageBase = LoadLibraryExW(full, nullptr, - LOAD_WITH_ALTERED_SEARCH_PATH); - if (result->ImageBase != nullptr) - break; + for (const wchar_t *path = paths; path && *path; ) { + (void)AddDllDirectory(path); + path += wcslen(path) + 1; + } + + result->ImageBase = LoadLibraryExW(name, nullptr, + LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); + + // Fallback: if the AddDllDirectory + LOAD_LIBRARY_SEARCH_DEFAULT_DIRS path + // failed to find the library, iterate the search paths ourselves and + // load by absolute path using LOAD_WITH_ALTERED_SEARCH_PATH, which makes + // Windows use the loaded DLL's own directory to resolve its sibling imports. + if (result->ImageBase == nullptr) { + wchar_t full[4096]; + for (const wchar_t *path = paths; path && *path; path += wcslen(path) + 1) { + size_t plen = wcslen(path); + size_t nlen = wcslen(name); + // Need room for: path + '\\' + name + '\0' + if (plen + 1 + nlen + 1 > 4096) + continue; + wchar_t *p = full; + for (size_t i = 0; i < plen; ++i) + *p++ = path[i]; + *p++ = L'\\'; + for (size_t i = 0; i <= nlen; ++i) // Copy name including trailing '\0'. + *p++ = name[i]; + result->ImageBase = LoadLibraryExW(full, nullptr, + LOAD_WITH_ALTERED_SEARCH_PATH); + if (result->ImageBase != nullptr) + break; + } } } if (result->ImageBase == nullptr) result->ErrorCode = GetLastError(); else - result->Length = GetModuleFileNameA(result->ImageBase, result->ModulePath, + result->Length = GetModuleFileNameW(result->ImageBase, result->ModulePath, result->Length); return result->ImageBase; @@ -789,8 +853,8 @@ extern "C" { // WINBASEAPI BOOL WINAPI FreeModule(HMODULE); /* __declspec(dllimport) */ int __stdcall FreeModule(void *); -// WINBASEAPI DWORD WINAPI GetModuleFileNameA(HMODULE, LPSTR, DWORD); -/* __declspec(dllimport) */ uint32_t GetModuleFileNameA(void *, char *, uint32_t); +// WINBASEAPI DWORD WINAPI GetModuleFileNameW(HMODULE, LPWSTR, DWORD); +/* __declspec(dllimport) */ uint32_t GetModuleFileNameW(void *, wchar_t *, uint32_t); // WINBASEAPI HMODULE WINAPI LoadLibraryExW(LPCWSTR, HANDLE, DWORD); /* __declspec(dllimport) */ void * __stdcall LoadLibraryExW(const wchar_t *, void *, uint32_t); diff --git a/lldb/test/API/driver/longpath/Makefile b/lldb/test/API/driver/longpath/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/driver/longpath/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/driver/longpath/TestLongPathDriver.py b/lldb/test/API/driver/longpath/TestLongPathDriver.py new file mode 100644 index 0000000000000..280180391790f --- /dev/null +++ b/lldb/test/API/driver/longpath/TestLongPathDriver.py @@ -0,0 +1,75 @@ +""" +Test that the lldb driver can target and run an executable whose path exceeds +the Windows MAX_PATH limit (260 characters). +""" + +import os +import shutil +import subprocess + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * + +MAX_PATH = 260 + + +@skipUnlessWindows +class DriverLongPathTestCase(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def _long_path(self, path): + return "\\\\?\\" + os.path.abspath(path) + + def _make_long_dir(self): + components = [self.getBuildArtifact("deep")] + ["d" * 80] * 3 + target_dir = os.path.join(*components) + try: + os.makedirs(self._long_path(target_dir), exist_ok=True) + except OSError: + return None + return target_dir + + def test_driver_runs_long_path_target(self): + self.build() + src_exe = self.getBuildArtifact("a.out") + + long_dir = self._make_long_dir() + if long_dir is None: + self.skipTest("OS cannot create paths longer than MAX_PATH") + + long_exe = os.path.join(long_dir, os.path.basename(src_exe)) + shutil.copyfile(src_exe, self._long_path(long_exe)) + self.assertGreater(len(os.path.abspath(long_exe)), MAX_PATH) + + # Drive the real lldb executable in batch mode: set a breakpoint, run to + # it, and list the modules. This both launches an executable past + # MAX_PATH and prints its full path back. + proc = subprocess.run( + [ + lldbtest_config.lldbExec, + "--batch", + "--no-lldbinit", + "-o", + "breakpoint set --name main", + "-o", + "run", + "-o", + "image list", + long_exe, + ], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + timeout=300, + ) + output = proc.stdout.decode("utf-8", errors="replace") + + self.assertIn( + "stop reason = breakpoint", + output, + "the driver should launch the long-path target and hit main:\n" + output, + ) + # The long directory component must appear untruncated in the output + self.assertIn( + "d" * 80, output, "the full long path must be reported:\n" + output + ) diff --git a/lldb/test/API/driver/longpath/main.c b/lldb/test/API/driver/longpath/main.c new file mode 100644 index 0000000000000..9ba28de2ae01c --- /dev/null +++ b/lldb/test/API/driver/longpath/main.c @@ -0,0 +1,3 @@ +int main(int argc, char **argv) { + return 0; // break here +} diff --git a/lldb/test/API/functionalities/longpath/Makefile b/lldb/test/API/functionalities/longpath/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/functionalities/longpath/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/functionalities/longpath/TestLongPath.py b/lldb/test/API/functionalities/longpath/TestLongPath.py new file mode 100644 index 0000000000000..590d17f82945c --- /dev/null +++ b/lldb/test/API/functionalities/longpath/TestLongPath.py @@ -0,0 +1,109 @@ +""" +Test that lldb can target and debug an executable whose path is longer than the +Windows MAX_PATH limit (260 characters). +""" + +import os +import shutil + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +MAX_PATH = 260 + + +@skipUnlessWindows +class LongPathTargetTestCase(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def _long_path(self, path): + return "\\\\?\\" + os.path.abspath(path) + + def _normalize(self, path): + if path.startswith("\\\\?\\"): + path = path[4:] + return os.path.normcase(os.path.normpath(path)) + + def _make_long_dir(self): + """Create (and return) a directory whose absolute path comfortably + exceeds MAX_PATH, or None if the OS refuses to create it.""" + components = [self.getBuildArtifact("deep")] + ["d" * 80] * 3 + target_dir = os.path.join(*components) + try: + os.makedirs(self._long_path(target_dir), exist_ok=True) + except OSError: + return None + return target_dir + + def _find_module(self, target, long_exe): + """Return the module in `target` whose path matches `long_exe`, or an + invalid module if none does.""" + wanted = self._normalize(long_exe) + for i in range(target.GetNumModules()): + mod = target.GetModuleAtIndex(i) + if self._normalize(mod.GetFileSpec().fullpath) == wanted: + return mod + return lldb.SBModule() + + def test_target_with_long_path(self): + """CreateTarget, launch and break in an executable located past + MAX_PATH, and verify the full path is preserved (not truncated).""" + self.build() + src_exe = self.getBuildArtifact("a.out") + + long_dir = self._make_long_dir() + if long_dir is None: + self.skipTest("OS cannot create paths longer than MAX_PATH") + + exe_basename = os.path.basename(src_exe) + long_exe = os.path.join(long_dir, exe_basename) + shutil.copyfile(src_exe, self._long_path(long_exe)) + + long_exe_abs = os.path.abspath(long_exe) + self.assertGreater( + len(long_exe_abs), + MAX_PATH, + "the test executable path must exceed MAX_PATH to be meaningful", + ) + + # Creating the target has to open and parse the file at the long path. + target = self.dbg.CreateTarget(long_exe) + self.assertTrue(target.IsValid(), VALID_TARGET) + + # The main executable module must report its full, untruncated path. + module = self._find_module(target, long_exe) + self.assertTrue( + module.IsValid(), + "the executable module should be found by its full long path", + ) + self.assertGreater( + len(module.GetFileSpec().fullpath), MAX_PATH, "module path truncated" + ) + + bp = target.BreakpointCreateByName("main", exe_basename) + self.assertGreater(bp.GetNumLocations(), 0, "main breakpoint has a location") + + process = target.LaunchSimple(None, None, self.get_process_working_directory()) + self.assertTrue(process.IsValid(), PROCESS_IS_VALID) + self.assertState( + process.GetState(), lldb.eStateStopped, "process stopped at main" + ) + + thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint) + self.assertIsNotNone(thread, "stopped at the main breakpoint") + self.assertEqual(thread.GetFrameAtIndex(0).GetFunctionName(), "main") + + # After launch the loaded executable module still carries the full path. + live_module = self._find_module(process.GetTarget(), long_exe) + self.assertTrue(live_module.IsValid(), "loaded module found by long path") + self.assertGreater( + len(live_module.GetFileSpec().fullpath), + MAX_PATH, + "loaded module path truncated", + ) + + process.Continue() + self.assertState(process.GetState(), lldb.eStateExited) + self.assertEqual(process.GetExitStatus(), 0) diff --git a/lldb/test/API/functionalities/longpath/main.c b/lldb/test/API/functionalities/longpath/main.c new file mode 100644 index 0000000000000..9ba28de2ae01c --- /dev/null +++ b/lldb/test/API/functionalities/longpath/main.c @@ -0,0 +1,3 @@ +int main(int argc, char **argv) { + return 0; // break here +} diff --git a/lldb/test/API/tools/lldb-dap/longpath/Makefile b/lldb/test/API/tools/lldb-dap/longpath/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/longpath/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/tools/lldb-dap/longpath/TestDAP_launch_longPath.py b/lldb/test/API/tools/lldb-dap/longpath/TestDAP_launch_longPath.py new file mode 100644 index 0000000000000..ebd0afe0f3483 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/longpath/TestDAP_launch_longPath.py @@ -0,0 +1,60 @@ +""" +Test that lldb-dap reports the full executable path in the "process" event when +the program lives at a path longer than the Windows MAX_PATH limit (260). +""" + +import os +import shutil + +import lldbdap_testcase +from lldbsuite.test.decorators import * + +MAX_PATH = 260 + + +@skipUnlessWindows +class TestDAP_launch_longPath(lldbdap_testcase.DAPTestCaseBase): + def _long_path(self, path): + return "\\\\?\\" + os.path.abspath(path) + + def _normalize(self, path): + if path.startswith("\\\\?\\"): + path = path[4:] + return os.path.normcase(os.path.normpath(path)) + + def _make_long_dir(self): + components = [self.getBuildArtifact("deep")] + ["d" * 80] * 3 + target_dir = os.path.join(*components) + try: + os.makedirs(self._long_path(target_dir), exist_ok=True) + except OSError: + return None + return target_dir + + def test_process_event_long_path(self): + self.build() + program = self.getBuildArtifact("a.out") + + long_dir = self._make_long_dir() + if long_dir is None: + self.skipTest("OS cannot create paths longer than MAX_PATH") + + long_program = os.path.join(long_dir, os.path.basename(program)) + shutil.copyfile(program, self._long_path(long_program)) + self.assertGreater(len(os.path.abspath(long_program)), MAX_PATH) + + self.create_debug_adapter() + self.launch_and_configurationDone(long_program) + + process_event = self.dap_server.wait_for_event(["process"]) + self.assertIsNotNone(process_event, "lldb-dap sent a process event") + name = process_event["body"]["name"] + self.assertGreater( + len(name), MAX_PATH, "process event name must not be truncated" + ) + self.assertEqual( + self._normalize(name), + self._normalize(long_program), + ) + + self.dap_server.wait_for_event(["terminated", "exited"]) diff --git a/lldb/test/API/tools/lldb-dap/longpath/main.c b/lldb/test/API/tools/lldb-dap/longpath/main.c new file mode 100644 index 0000000000000..9ba28de2ae01c --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/longpath/main.c @@ -0,0 +1,3 @@ +int main(int argc, char **argv) { + return 0; // break here +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
