https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/207365
This fixes a bug I found where copying a file from a remote Linux platform using the API would claim that it succeeded, but it had not. The local file was read only so it was not updated. The host Linux platform's copy did not have this bug. There are two tests, one which will run if the testing platform is not already a remote. This one spawns a "remote" platform, which is actually still on the host, but it's enough to take the remote code path in the POSIX platform implementation. The second uses the testing platform. If that's a remote, use that, if it's host then we use the host platform to check local copying behaviour. This test is not as specific as I'd like it to be, but I wanted something that could run on many platforms. Even if the copy fails for a reason other than permissions, at least we are checking that the failure is propagated. >From 36717d52d2c9417383077e092d4edf2744c91f67 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Fri, 19 Jun 2026 15:20:59 +0000 Subject: [PATCH] [lldb][POSIX] Correctly report error copying from remote platform This fixes a bug I found where copying a file from a remote Linux platform using the API would claim that it succeeded, but it had not. The local file was read only so it was not updated. This change fixes that bug and adds tests for platform get file. There are two tests, one which will run if the testing platform is not already a remote. This one spawns a "remote" platform, which is actually still on the host, but it's enough to take the remote code path in the POSIX platform implementation. The second uses the testing platform. If that's a remote, fine, if it's host, then we check the local behaviour. (host Linux platform uses a different copy method which does not have this bug) This test is not as specific as I'd like it to be, but I wanted something that could run on many platforms. Even if the copy fails for a reason other than permissions, at least we are checking that the failure is propagated. --- .../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 10 ++-- .../python_api/sbplatform/TestSBPlatform.py | 48 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp index b301890d2b698..1a62145083f8f 100644 --- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp @@ -255,10 +255,12 @@ lldb_private::Status PlatformPOSIX::GetFile( offset += n_read; } } - // Ignore the close error of src. - if (fd_src != UINT64_MAX) - CloseFile(fd_src, error); - // And close the dst file descriptot. + if (fd_src != UINT64_MAX) { + // Ignore the close error of src. + Status close_error; + CloseFile(fd_src, close_error); + } + // And close the dst file descriptor. if (fd_dst != UINT64_MAX && !FileCache::GetInstance().CloseFile(fd_dst, error)) { if (!error.Fail()) diff --git a/lldb/test/API/python_api/sbplatform/TestSBPlatform.py b/lldb/test/API/python_api/sbplatform/TestSBPlatform.py index 3c3e268af35eb..8a5f534be4605 100644 --- a/lldb/test/API/python_api/sbplatform/TestSBPlatform.py +++ b/lldb/test/API/python_api/sbplatform/TestSBPlatform.py @@ -1,7 +1,13 @@ """Test the SBPlatform APIs.""" +import os +import socket +from pathlib import Path +import lldbgdbserverutils + from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil @skipIfWasm # no remote platform file/process APIs @@ -52,3 +58,45 @@ def test_SetCurrentPlatform_associated(self): dbg_platform = self.dbg.GetSelectedPlatform() self.assertEqual(dbg_platform.GetName(), "remote-netbsd") self.assertEqual(dbg_platform.GetWorkingDirectory(), self.getBuildDir()) + + def do_copy_test(self, platform): + source_name = "source-file" + source_path = self.getBuildArtifact(source_name) + Path(source_path).touch() + + remote_path = lldbutil.append_to_process_working_directory(self, source_name) + put_error = platform.Put( + lldb.SBFileSpec(source_path, True), lldb.SBFileSpec(remote_path, False) + ) + self.assertSuccess(put_error) + + destination = self.getBuildArtifact("destination-file") + # Make the file read only. + Path(destination).touch(mode=0o400) + self.addTearDownHook(lambda: os.remove(destination)) + + get_error = platform.Get( + lldb.SBFileSpec(remote_path, False), lldb.SBFileSpec(destination, True) + ) + # Ideally we would check that we failed due to permissions, but as this + # runs on many platforms, we do not know the exact form of the error message. + self.assertFailure(get_error) + + @skipIfDarwin # lldb-server not found correctly + @add_test_categories(["lldb-server"]) + # If we're already remote, no need to spawn a new remote platform. + @skipIfRemote + def test_get_reports_write_failure_remote(self): + if lldb.remote_platform: + platform = lldb.remote_platform + else: + platform = lldbutil.connect_to_new_remote_platform( + self, lldbgdbserverutils.get_lldb_server_exe() + ) + + self.do_copy_test(platform) + + @skipIfDarwin # lldb-server not found correctly + @add_test_categories(["lldb-server"]) + def test_get_reports_write_failure(self): + self.do_copy_test(self.dbg.GetSelectedPlatform()) _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
